Fix syntax error false positive on alternative match patterns
#21362
Conversation
Summary -- Fixes #21360 by using the union of names instead of overwriting them, as Micha suggested originally on #21104. This avoids overwriting the `n` name in the `Subscript` by the empty set of names visited in the nested OR pattern before visiting the other arm of the outer OR pattern. Test Plan -- A new inline test case taken from the issue
|
| break; | ||
| } | ||
| self.names = visitor.names; | ||
| self.names.extend(visitor.names); |
There was a problem hiding this comment.
Can you say a bit more about the fix. My understanding (and why I recommended it) was that using union or intersection would only matter for the error recovery case. But it seems that using union over intersection is important for valid cases too or are we only fixing a symptom here?
There was a problem hiding this comment.
I think the union is correct because there can be existing patterns in self.names before visiting nested alternative patterns. We need to compare all of the names we visit in that arm to all of the names we visit in the next arm, not just the set of names visited in the last, innermost alternative pattern.
That's what's happening in this case because we first visit the outer class pattern, which captures n, then we were overwriting that n with the empty set when visiting the Constant | Slice part of the pattern. Instead we need the union of n and the empty set to compare to the Attribute(n) branch.
match 42:
case ast.Subscript(n, ast.Constant() | ast.Slice())
| ast.Attribute(n): ...
* origin/main: (38 commits) [ty] Make implicit submodule imports only occur in global scope (#21370) [ty] introduce local variables for `from` imports of submodules in `__init__.py(i)` (#21173) [`ruff`] Ignore `str()` when not used for simple conversion (`RUF065`) (#21330) [ty] implement `typing.NewType` by adding `Type::NewTypeInstance` [ty] supress inlay hints for `+1` and `-1` (#21368) [ty] Use type context for inference of generic constructors (#20933) [ty] Improve generic call expression inference (#21210) [ty] supress some trivial expr inlay hints (#21367) [`configuration`] Fix unclear error messages for line-length values exceeding `u16::MAX` (#21329) [ty] Fix incorrect inference of `enum.auto()` for enums with non-`int` mixins, and imprecise inference of `enum.auto()` for single-member enums (#20541) [`refurb`] Detect empty f-strings (`FURB105`) (#21348) [ty] provide `import` completion when in `from <name> <name>` statement (#21291) [ty] elide redundant inlay hints for function args (#21365) Fix syntax error false positive on alternative `match` patterns (#21362) Add a new "Opening a PR" section to the contribution guide (#21298) [`flake8-simplify`] Fix SIM222 false positive for `tuple(generator) or None` (`SIM222`) (#21187) Rebuild ruff binary instead of sharing it across jobs (#21361) [ty] Fix `--exclude` and `src.exclude` merging (#21341) [ty] Add support for properties that return `Self` (#21335) Add upstream linter URL to `ruff linter --output-format=json` (#21316) ...
Summary
Fixes #21360 by using the union of names instead of overwriting them, as Micha suggested originally on #21104.
This avoids overwriting the
nname in theSubscriptby the empty set of names visited in the nested OR pattern before visiting the other arm of the outer OR pattern.Test Plan
A new inline test case taken from the issue