A binding reaches where Jinja says it reaches, and no further - #461
Merged
Conversation
Both validators answered "is this name the template's own?" with one
template-wide set of every bound name. That is right for a name used only
where it is bound and wrong everywhere else, because binding a name anywhere
silenced it everywhere:
{{ ghost.done }} <- undefined; reported nothing
{% for ghost in rows %}
{{ ghost.name }} <- the binding that silenced it
{% endfor %}
The reference outside the loop is exactly the typo a validator exists to
catch, and adding an unrelated loop to the file made it vanish. The trade was
backwards: the false positive this replaced was loud, a false negative is
silent.
core/template_scope.py tracks scope as Jinja does. A binding reaches the body
of the construct that introduces it and nothing outside it; {% set %} reaches
the statements that follow it in the same block. {% if %} opens no scope,
because Jinja gives it none.
Identity, not spelling: shadowed_name_nodes returns the id() of each Name
node that refers to a local binding, so one template can hold both a shadowed
and an unshadowed use of the same word.
Both validators ask it, so they cannot disagree about what a template defines.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`jinja2.meta` was how references used to be found. It reports only top-level undeclared names, which is why the chain and subscript forms had to be recovered from the raw text afterwards -- the text-chopping #458 removed. Walking the AST replaced both, and these three imports have had no caller since. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first version of this guard passed under the very mutation it existed to catch. Asserting that validation fails proves nothing here: `rows` is undeclared, so the pipeline is refused either way, and the template validator finds undeclared names with jinja2.meta -- which is scope-aware already and reports 'ghost' on its own. The data-flow validator was the blind one. It has its own phrasing, so that is what the assertion looks for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 3, 2026
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
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.
Fixes the false negative in reference extraction reported against #458.
The defect
Both validators answered "is this name the template's own?" with one template-wide set of every bound name. That is right for a name used only where it is bound, and wrong everywhere else — binding a name anywhere silenced it everywhere:
{{ ghost.done }} <- undefined; reported nothing {% for ghost in rows %} {{ ghost.name }} <- the binding that silenced it {% endfor %}The reference outside the loop is exactly the typo a validator exists to catch, and adding an unrelated loop elsewhere in the file made it vanish. The trade was backwards: the false positive this replaced was loud, a false negative is silent.
The fix
core/template_scope.pytracks scope the way Jinja does. A binding reaches the body of the construct that introduces it and nothing outside it;{% set %}reaches the statements that follow it in the same block.{% if %}opens no scope, because Jinja gives it none — treating its{% set %}as reaching the rest of the block can only suppress a report, never invent one.Identity, not spelling:
shadowed_name_nodesreturns theid()of eachNamenode that refers to a local binding, so one template can hold both a shadowed and an unshadowed use of the same word.Both validators ask it, so they cannot disagree about what a template defines.
All seven cases from the review are covered, plus: the iterable evaluated outside its own loop,
{% for %}'s filter test, the{% else %}branch,loopitself, tuple targets,{% set %}reading its own prior value,{% call %}arguments, and macro defaults.Verification
Two things worth flagging
The end-to-end guard initially passed under the very mutation it existed to catch. Two reasons: the probe pipeline's
rowsis undeclared, so it is refused either way; and the template validator finds undeclared names withjinja2.meta, which is scope-aware already and reportedghoston its own. Only the data-flow validator was blind. The assertion now targets its specific phrasing, and fails under the mutation as it should.Three dead imports removed —
jinja2.meta,TemplateSyntaxErrorandUnionindata_flow_validator.py.metareports only top-level undeclared names, which is why the chain and subscript forms had to be recovered from raw text; walking the AST replaced both, and these have had no caller since #458.🤖 Generated with Claude Code