mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-07 20:28:56 +00:00
[syntax-errors] Duplicate attributes in match class pattern (#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.
This commit is contained in:
Brent Westbrook
GitHub
parent
6a07dd227d
commit
24b1b1d52c
6 changed files with 1257 additions and 69 deletions
|
|
@ -65,8 +65,13 @@ impl SemanticSyntaxChecker {
|
|||
}
|
||||
Stmt::Match(match_stmt) => {
|
||||
Self::irrefutable_match_case(match_stmt, ctx);
|
||||
Self::multiple_case_assignment(match_stmt, ctx);
|
||||
Self::duplicate_match_mapping_keys(match_stmt, ctx);
|
||||
for case in &match_stmt.cases {
|
||||
let mut visitor = MatchPatternVisitor {
|
||||
names: FxHashSet::default(),
|
||||
ctx,
|
||||
};
|
||||
visitor.visit_pattern(&case.pattern);
|
||||
}
|
||||
}
|
||||
Stmt::FunctionDef(ast::StmtFunctionDef { type_params, .. })
|
||||
| Stmt::ClassDef(ast::StmtClassDef { type_params, .. })
|
||||
|
|
@ -262,68 +267,6 @@ impl SemanticSyntaxChecker {
|
|||
}
|
||||
}
|
||||
|
||||
fn multiple_case_assignment<Ctx: SemanticSyntaxContext>(stmt: &ast::StmtMatch, ctx: &Ctx) {
|
||||
for case in &stmt.cases {
|
||||
let mut visitor = MultipleCaseAssignmentVisitor {
|
||||
names: FxHashSet::default(),
|
||||
ctx,
|
||||
};
|
||||
visitor.visit_pattern(&case.pattern);
|
||||
}
|
||||
}
|
||||
|
||||
fn duplicate_match_mapping_keys<Ctx: SemanticSyntaxContext>(stmt: &ast::StmtMatch, ctx: &Ctx) {
|
||||
for mapping in stmt
|
||||
.cases
|
||||
.iter()
|
||||
.filter_map(|case| case.pattern.as_match_mapping())
|
||||
{
|
||||
let mut seen = FxHashSet::default();
|
||||
for key in mapping
|
||||
.keys
|
||||
.iter()
|
||||
// complex numbers (`1 + 2j`) are allowed as keys but are not literals
|
||||
// because they are represented as a `BinOp::Add` between a real number and
|
||||
// an imaginary number
|
||||
.filter(|key| key.is_literal_expr() || key.is_bin_op_expr())
|
||||
{
|
||||
if !seen.insert(ComparableExpr::from(key)) {
|
||||
let key_range = key.range();
|
||||
let duplicate_key = ctx.source()[key_range].to_string();
|
||||
// test_ok duplicate_match_key_attr
|
||||
// match x:
|
||||
// case {x.a: 1, x.a: 2}: ...
|
||||
|
||||
// test_err duplicate_match_key
|
||||
// match x:
|
||||
// case {"x": 1, "x": 2}: ...
|
||||
// case {b"x": 1, b"x": 2}: ...
|
||||
// case {0: 1, 0: 2}: ...
|
||||
// case {1.0: 1, 1.0: 2}: ...
|
||||
// case {1.0 + 2j: 1, 1.0 + 2j: 2}: ...
|
||||
// case {True: 1, True: 2}: ...
|
||||
// case {None: 1, None: 2}: ...
|
||||
// case {
|
||||
// """x
|
||||
// y
|
||||
// z
|
||||
// """: 1,
|
||||
// """x
|
||||
// y
|
||||
// z
|
||||
// """: 2}: ...
|
||||
// case {"x": 1, "x": 2, "x": 3}: ...
|
||||
// case {0: 1, "x": 1, 0: 2, "x": 2}: ...
|
||||
Self::add_error(
|
||||
ctx,
|
||||
SemanticSyntaxErrorKind::DuplicateMatchKey(duplicate_key),
|
||||
key_range,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn irrefutable_match_case<Ctx: SemanticSyntaxContext>(stmt: &ast::StmtMatch, ctx: &Ctx) {
|
||||
// test_ok irrefutable_case_pattern_at_end
|
||||
// match x:
|
||||
|
|
@ -575,6 +518,9 @@ impl Display for SemanticSyntaxError {
|
|||
EscapeDefault(key)
|
||||
)
|
||||
}
|
||||
SemanticSyntaxErrorKind::DuplicateMatchClassAttribute(name) => {
|
||||
write!(f, "attribute name `{name}` repeated in class pattern",)
|
||||
}
|
||||
SemanticSyntaxErrorKind::LoadBeforeGlobalDeclaration { name, start: _ } => {
|
||||
write!(f, "name `{name}` is used prior to global declaration")
|
||||
}
|
||||
|
|
@ -730,6 +676,16 @@ pub enum SemanticSyntaxErrorKind {
|
|||
/// [CPython grammar]: https://docs.python.org/3/reference/grammar.html
|
||||
DuplicateMatchKey(String),
|
||||
|
||||
/// Represents a duplicate attribute name in a `match` class pattern.
|
||||
///
|
||||
/// ## Examples
|
||||
///
|
||||
/// ```python
|
||||
/// match x:
|
||||
/// case Class(x=1, x=2): ...
|
||||
/// ```
|
||||
DuplicateMatchClassAttribute(ast::name::Name),
|
||||
|
||||
/// Represents the use of a `global` variable before its `global` declaration.
|
||||
///
|
||||
/// ## Examples
|
||||
|
|
@ -787,12 +743,12 @@ impl Visitor<'_> for ReboundComprehensionVisitor<'_> {
|
|||
}
|
||||
}
|
||||
|
||||
struct MultipleCaseAssignmentVisitor<'a, Ctx> {
|
||||
struct MatchPatternVisitor<'a, Ctx> {
|
||||
names: FxHashSet<&'a ast::name::Name>,
|
||||
ctx: &'a Ctx,
|
||||
}
|
||||
|
||||
impl<'a, Ctx: SemanticSyntaxContext> MultipleCaseAssignmentVisitor<'a, Ctx> {
|
||||
impl<'a, Ctx: SemanticSyntaxContext> MatchPatternVisitor<'a, Ctx> {
|
||||
fn visit_pattern(&mut self, pattern: &'a Pattern) {
|
||||
// test_ok class_keyword_in_case_pattern
|
||||
// match 2:
|
||||
|
|
@ -821,19 +777,87 @@ impl<'a, Ctx: SemanticSyntaxContext> MultipleCaseAssignmentVisitor<'a, Ctx> {
|
|||
self.visit_pattern(pattern);
|
||||
}
|
||||
}
|
||||
Pattern::MatchMapping(ast::PatternMatchMapping { patterns, rest, .. }) => {
|
||||
Pattern::MatchMapping(ast::PatternMatchMapping {
|
||||
keys,
|
||||
patterns,
|
||||
rest,
|
||||
..
|
||||
}) => {
|
||||
for pattern in patterns {
|
||||
self.visit_pattern(pattern);
|
||||
}
|
||||
if let Some(rest) = rest {
|
||||
self.insert(rest);
|
||||
}
|
||||
|
||||
let mut seen = FxHashSet::default();
|
||||
for key in keys
|
||||
.iter()
|
||||
// complex numbers (`1 + 2j`) are allowed as keys but are not literals
|
||||
// because they are represented as a `BinOp::Add` between a real number and
|
||||
// an imaginary number
|
||||
.filter(|key| key.is_literal_expr() || key.is_bin_op_expr())
|
||||
{
|
||||
if !seen.insert(ComparableExpr::from(key)) {
|
||||
let key_range = key.range();
|
||||
let duplicate_key = self.ctx.source()[key_range].to_string();
|
||||
// test_ok duplicate_match_key_attr
|
||||
// match x:
|
||||
// case {x.a: 1, x.a: 2}: ...
|
||||
|
||||
// test_err duplicate_match_key
|
||||
// match x:
|
||||
// case {"x": 1, "x": 2}: ...
|
||||
// case {b"x": 1, b"x": 2}: ...
|
||||
// case {0: 1, 0: 2}: ...
|
||||
// case {1.0: 1, 1.0: 2}: ...
|
||||
// case {1.0 + 2j: 1, 1.0 + 2j: 2}: ...
|
||||
// case {True: 1, True: 2}: ...
|
||||
// case {None: 1, None: 2}: ...
|
||||
// case {
|
||||
// """x
|
||||
// y
|
||||
// z
|
||||
// """: 1,
|
||||
// """x
|
||||
// y
|
||||
// z
|
||||
// """: 2}: ...
|
||||
// case {"x": 1, "x": 2, "x": 3}: ...
|
||||
// case {0: 1, "x": 1, 0: 2, "x": 2}: ...
|
||||
// case [{"x": 1, "x": 2}]: ...
|
||||
// case Foo(x=1, y={"x": 1, "x": 2}): ...
|
||||
// case [Foo(x=1), Foo(x=1, y={"x": 1, "x": 2})]: ...
|
||||
SemanticSyntaxChecker::add_error(
|
||||
self.ctx,
|
||||
SemanticSyntaxErrorKind::DuplicateMatchKey(duplicate_key),
|
||||
key_range,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
Pattern::MatchClass(ast::PatternMatchClass { arguments, .. }) => {
|
||||
for pattern in &arguments.patterns {
|
||||
self.visit_pattern(pattern);
|
||||
}
|
||||
let mut seen = FxHashSet::default();
|
||||
for keyword in &arguments.keywords {
|
||||
if !seen.insert(&keyword.attr.id) {
|
||||
// test_err duplicate_match_class_attr
|
||||
// match x:
|
||||
// case Class(x=1, x=2): ...
|
||||
// case [Class(x=1, x=2)]: ...
|
||||
// case {"x": x, "y": Foo(x=1, x=2)}: ...
|
||||
// case [{}, {"x": x, "y": Foo(x=1, x=2)}]: ...
|
||||
// case Class(x=1, d={"x": 1, "x": 2}, other=Class(x=1, x=2)): ...
|
||||
SemanticSyntaxChecker::add_error(
|
||||
self.ctx,
|
||||
SemanticSyntaxErrorKind::DuplicateMatchClassAttribute(
|
||||
keyword.attr.id.clone(),
|
||||
),
|
||||
keyword.attr.range,
|
||||
);
|
||||
}
|
||||
self.visit_pattern(&keyword.pattern);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue