Skip to content

fix: do not flag block argument shadowing across mutually exclusive branches in Lint/ShadowingOuterLocalVar#824

Merged
Sija merged 2 commits into
masterfrom
fix-shadowing-outer-local-var-mutually-exclusive-branches
May 11, 2026
Merged

fix: do not flag block argument shadowing across mutually exclusive branches in Lint/ShadowingOuterLocalVar#824
Sija merged 2 commits into
masterfrom
fix-shadowing-outer-local-var-mutually-exclusive-branches

Conversation

@veelenga
Copy link
Copy Markdown
Member

@veelenga veelenga commented May 9, 2026

Closes #819

Added a localized AST-walking check (mutually_exclusive_branches?) that records the branch path of the block argument and each assignment of the candidate variable. When every assignment diverges from the argument's path at a common branching ancestor, the outer assignment cannot reach the block argument and the warning is suppressed.

No Branch / Branchable infrastructure was reintroduced (recently removed in #795) — the check is contained in the rule.

…ranches in `Lint/ShadowingOuterLocalVar`

The rule used a purely positional check (`declared_before?`) to decide
whether a block or proc argument shadowed an outer local variable. As a
result it produced false positives whenever the outer assignment lived
in a control-flow branch that could not reach the block argument:

```cr
if cond
  x = 1
else
  foo { |x| } # incorrectly flagged
end
```

Add a localized AST-walking check that records the branch path of the
block argument and each assignment of the candidate variable. When every
assignment diverges from the argument's path at a common branching
ancestor, the outer assignment cannot reach the block argument and the
warning is suppressed.

Handles `if`/`unless`, `case`/`when`/`else`, `select`/`when`/`else`, and
`begin`/`rescue`/`else` (body and `ensure` always run, so they remain
non-exclusive). The visitor is cached per outer scope across candidates
within a single rule invocation to avoid re-walking the same AST.
@veelenga veelenga self-assigned this May 9, 2026
@Sija Sija added this to the 1.7.0 milestone May 9, 2026
@veelenga veelenga marked this pull request as ready for review May 11, 2026 10:02
@veelenga veelenga requested a review from Sija May 11, 2026 10:02
Comment thread spec/ameba/rule/lint/shadowing_outer_local_var_spec.cr Outdated
Comment thread spec/ameba/rule/lint/shadowing_outer_local_var_spec.cr Outdated
Comment thread spec/ameba/rule/lint/shadowing_outer_local_var_spec.cr Outdated
Comment thread src/ameba/rule/lint/shadowing_outer_local_var.cr Outdated
Comment thread src/ameba/rule/lint/shadowing_outer_local_var.cr Outdated
Copy link
Copy Markdown
Member

@Sija Sija left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Sija Sija merged commit 33b823d into master May 11, 2026
13 checks passed
@Sija Sija deleted the fix-shadowing-outer-local-var-mutually-exclusive-branches branch May 11, 2026 13:00
@Sija Sija changed the title fix: do not flag block argument shadowing across mutually exclusive branches fix: do not flag block argument shadowing across mutually exclusive branches in Lint/ShadowingOuterLocalVar May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Lint/ShadowingOuterLocalVar consideres local variables from other code branch

2 participants