Get attribute fixes - #119
Merged
Merged
Conversation
The no-arguments branch subscripted Hash and Array with the attribute
name instead of calling it, which is only ever reached when the key/index
lookup above has already missed — so the subscript could only miss too.
On an Array it raised TypeError ({{ list.any? }}), and on a Hash it
returned nil and silently rendered nothing. Collections now dispatch
methods like any other object, while a real key or index still wins over
a method of the same name.
Also public_send rather than send: the respond_to? guard above already
excludes private and protected methods, but an object with a loose
respond_to_missing? can report true for one, and templates should only
reach the public interface.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The bounds check was only an upper one, so every negative index passed it. Ruby indexes from the end with a negative, so -1 is genuinely the last element, but -99 on a three-element array is as absent as 99 is — and it fell through to `object[attribute] || object[attribute.to_s]`, where the String subscript raised TypeError out of the array. Because the exception came from inside the lookup rather than the strict check, `??` and `default()` couldn't suppress it either. Negative indices within range still resolve from the end; out of range now reports the same "can't find key" as an out-of-range positive, or returns nil when strict_variables is off. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`object[attribute] || object[attribute.to_s]` cannot distinguish a key
that is absent from a key that is present and holds a falsy value, so it
fell through to the other form of the name and returned whatever that
gave — usually nothing. { 'billable' => false } read as h['billable']
came back nil, and so did { billable: false } read as h[:billable]. The
cases that appeared to work only did so by accident: when the name's form
mismatched the key's, the fallback lookup happened to be the real one.
The guard already knew which key matched; it just threw that away and
re-derived it by fetching. subscript_key now returns the matching key (or
a KEY_NOT_FOUND sentinel, since nil is both a valid key and a valid
value) and the caller subscripts once with it. Where both forms of a name
exist, the one the template asked for now wins rather than whichever is
truthy.
The array bounds check moves into the same helper, since it answers the
same question.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The two paths out of get_attribute described the same failure differently. Dot access produced Twig's own wording — Key "x" for sequence/mapping with keys "a, b" does not exist — while bracket access produced "Can't find key x in " followed by the whole object inspected, which for anything the size of a request's parameters buries the one absent key under everything else. It also raised the bare message, so unlike its neighbour it carried no lineno or source. Both now build the message through key_not_found_message and raise with the source attached. Also drops the double space the old wording left when the object has no keys to list, as an array doesn't. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The version was a literal in the gemspec, so it existed in exactly one place that nothing else could see: the library couldn't report its own version at runtime, and neither could anything depending on it. Twig::VERSION now holds it and the gemspec reads that. require_relative rather than requiring the gem, since this needs one constant and evaluating the whole library would pull ActiveSupport in at packaging time. The existing loader globs lib/twig/*.rb, so the constant is there at runtime without anything else being wired up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.