[syntax-errors] Duplicate attributes in match class pattern #17186
Conversation
This file contains 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
Summary -- Fixes #17181. The cases being tested with multiple *keys* being equal are actually a slightly different error, more like the error for `MatchMapping` than like the other multiple assignment errors: ```pycon >>> match x: ... case Class(x=x, x=x): ... ... File "<python-input-249>", line 2 case Class(x=x, x=x): ... ^ SyntaxError: attribute name repeated in class pattern: x >>> match x: ... case {"x": 1, "x": 2}: ... ... File "<python-input-251>", line 2 case {"x": 1, "x": 2}: ... ^^^^^^^^^^^^^^^^ SyntaxError: mapping pattern checks duplicate key ('x') >>> match x: ... case [x, x]: ... ... File "<python-input-252>", line 2 case [x, x]: ... ^ SyntaxError: multiple assignments to name 'x' in pattern ``` This PR just stops the false positive reported in the issue, but I will quickly follow it up with a new rule (or possibly combining it with the mapping rule) catching the repeated attributes separately. Test Plan -- New inline `test_ok`, as well as removing cases that are (temporarily) not errors.
Summary
--
Detects duplicate attributes in a `match` class pattern:
```python
match x:
case Class(x=1, x=2): ...
```
which are more analogous to the similar check for mapping patterns than to the
multiple assignments rule.
I also realized that both this and the mapping check would only work on
top-level patterns, despite the possibility that they can be nested inside other
patterns:
```python
match x:
case [{"x": 1, "x": 2}]: ... # false negative in the old version
```
and moved these checks into the recursive pattern visitor instead.
I also tidied up some of the names like the `multiple_case_assignment` function
and the `MultipleCaseAssignmentVisitor`, which are now doing more than checking
for multiple assignments.
Test Plan
--
New inline tests for both classes and mappings.
ntBre
added
rule
|
dhruvmanila
approved these changes
Apr 3, 2025
|
Sure, I threw in the list case as a minimal example, but I'm happy to expand that a bit. Thanks for the reviews! |
maxmynter
pushed a commit
to maxmynter/ruff
that referenced
this pull request
Apr 3, 2025
…h#17186) Summary -- Detects duplicate attributes in a `match` class pattern: ```python match x: case Class(x=1, x=2): ... ``` which are more analogous to the similar check for mapping patterns than to the multiple assignments rule. I also realized that both this and the mapping check would only work on top-level patterns, despite the possibility that they can be nested inside other patterns: ```python match x: case [{"x": 1, "x": 2}]: ... # false negative in the old version ``` and moved these checks into the recursive pattern visitor instead. I also tidied up some of the names like the `multiple_case_assignment` function and the `MultipleCaseAssignmentVisitor`, which are now doing more than checking for multiple assignments. Test Plan -- New inline tests for both classes and mappings.
dcreager
added a commit
that referenced
this pull request
Apr 4, 2025
* main: [red-knot] Empty tuple is always-falsy (#17213) Run fuzzer with `--preview` (#17210) Bump 0.11.4 (#17212) [syntax-errors] Allow `yield` in base classes and annotations (#17206) Don't skip visiting non-tuple slice in `typing.Annotated` subscripts (#17201) [red-knot] mypy_primer: do not specify Python version (#17200) [red-knot] Add `Type.definition` method (#17153) Implement `Invalid rule provided` as rule RUF102 with `--fix` (#17138) [red-knot] Add basic on-hover to playground and LSP (#17057) [red-knot] don't remove negations when simplifying constrained typevars (#17189) [minor] Fix extra semicolon for clippy (#17188) [syntax-errors] Invalid syntax in annotations (#17101) [syntax-errors] Duplicate attributes in match class pattern (#17186) [syntax-errors] Fix multiple assignment for class keyword argument (#17184) use astral-sh/cargo-dist instead (#17187) Enable overindented docs lint (#17182)
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.
Summary
Detects duplicate attributes in a
matchclass pattern:which are more analogous to the similar check for mapping patterns than to the
multiple assignments rule.
I also realized that both this and the mapping check would only work on
top-level patterns, despite the possibility that they can be nested inside other
patterns:
and moved these checks into the recursive pattern visitor instead.
I also tidied up some of the names like the
multiple_case_assignmentfunctionand the
MultipleCaseAssignmentVisitor, which are now doing more than checkingfor multiple assignments.
Test Plan
New inline tests for both classes and mappings.
This is currently stacked on #17184.