[syntax-errors] Detect duplicate keys in match mapping patterns

crates/ruff_linter/src/checkers/ast/mod.rs

@@ -573,13 +573,18 @@ impl SemanticSyntaxContext for Checker<'_> {
             | SemanticSyntaxErrorKind::IrrefutableCasePattern(_)
             | SemanticSyntaxErrorKind::SingleStarredAssignment
             | SemanticSyntaxErrorKind::WriteToDebug(_)
+            | SemanticSyntaxErrorKind::DuplicateMatchKey(_)
             | SemanticSyntaxErrorKind::InvalidStarExpression => {
                 if self.settings.preview.is_enabled() {
                     self.semantic_errors.borrow_mut().push(error);
                 }
             }
         }
     }
+
+    fn source(&self) -> &str {
+        self.source()
+    }
 }
 
 impl<'a> Visitor<'a> for Checker<'a> {

crates/ruff_python_parser/resources/inline/err/duplicate_match_key.py

@@ -0,0 +1,19 @@
+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}: ...

crates/ruff_python_parser/resources/inline/ok/duplicate_match_key_attr.py

@@ -0,0 +1,2 @@
+match x:
+    case {x.a: 1, x.a: 2}: ...

crates/ruff_python_parser/src/semantic_errors.rs

@@ -8,6 +8,7 @@ use std::fmt::Display;
 
 use ruff_python_ast::{
     self as ast,
+    comparable::ComparableExpr,
     visitor::{walk_expr, Visitor},
     Expr, ExprContext, IrrefutablePatternKind, Pattern, PythonVersion, Stmt, StmtExpr,
     StmtImportFrom,
@@ -65,6 +66,7 @@ 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);
             }
             Stmt::FunctionDef(ast::StmtFunctionDef { type_params, .. })
             | Stmt::ClassDef(ast::StmtClassDef { type_params, .. })
@@ -270,6 +272,58 @@ impl SemanticSyntaxChecker {
         }
     }
 
+    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:
@@ -514,6 +568,13 @@ impl Display for SemanticSyntaxError {
                     write!(f, "cannot delete `__debug__` on Python {python_version} (syntax was removed in 3.9)")
                 }
             },
+            SemanticSyntaxErrorKind::DuplicateMatchKey(key) => {
+                write!(
+                    f,
+                    "mapping pattern checks duplicate key `{}`",
+                    EscapeDefault(key)
+                )
+            }
             SemanticSyntaxErrorKind::LoadBeforeGlobalDeclaration { name, start: _ } => {
                 write!(f, "name `{name}` is used prior to global declaration")
             }
@@ -634,6 +695,41 @@ pub enum SemanticSyntaxErrorKind {
     /// [BPO 45000]: https://github.com/python/cpython/issues/89163
     WriteToDebug(WriteToDebugKind),
 
+    /// Represents a duplicate key in a `match` mapping pattern.
+    ///
+    /// The [CPython grammar] allows keys in mapping patterns to be literals or attribute accesses:
+    ///
+    /// ```text
+    /// key_value_pattern:
+    ///     | (literal_expr | attr) ':' pattern
+    /// ```
+    ///
+    /// But only literals are checked for duplicates:
+    ///
+    /// ```pycon
+    /// >>> match x:
+    /// ...     case {"x": 1, "x": 2}: ...
+    /// ...
+    ///   File "<python-input-160>", line 2
+    ///     case {"x": 1, "x": 2}: ...
+    ///          ^^^^^^^^^^^^^^^^
+    /// SyntaxError: mapping pattern checks duplicate key ('x')
+    /// >>> match x:
+    /// ...     case {x.a: 1, x.a: 2}: ...
+    /// ...
+    /// >>>
+    /// ```
+    ///
+    /// ## Examples
+    ///
+    /// ```python
+    /// match x:
+    ///     case {"x": 1, "x": 2}: ...
+    /// ```
+    ///
+    /// [CPython grammar]: https://docs.python.org/3/reference/grammar.html
+    DuplicateMatchKey(String),
+
     /// Represents the use of a `global` variable before its `global` declaration.
     ///
     /// ## Examples
@@ -789,6 +885,9 @@ pub trait SemanticSyntaxContext {
     /// The target Python version for detecting backwards-incompatible syntax changes.
     fn python_version(&self) -> PythonVersion;
 
+    /// Returns the source text under analysis.
+    fn source(&self) -> &str;
+
     /// Return the [`TextRange`] at which a name is declared as `global` in the current scope.
     fn global(&self, name: &str) -> Option<TextRange>;
 
@@ -828,3 +927,20 @@ where
         ruff_python_ast::visitor::walk_expr(self, expr);
     }
 }
+
+/// Modified version of [`std::str::EscapeDefault`] that does not escape single or double quotes.
+struct EscapeDefault<'a>(&'a str);
+
+impl Display for EscapeDefault<'_> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        use std::fmt::Write;
+
+        for c in self.0.chars() {
+            match c {
+                '\'' | '\"' => f.write_char(c)?,
+                _ => write!(f, "{}", c.escape_default())?,
+            }
+        }
+        Ok(())
+    }
+}

crates/ruff_python_parser/tests/fixtures.rs

@@ -89,7 +89,7 @@ fn test_valid_syntax(input_path: &Path) {
     let parsed = parsed.try_into_module().expect("Parsed with Mode::Module");
 
     let mut visitor = SemanticSyntaxCheckerVisitor::new(
-        TestContext::default().with_python_version(options.target_version()),
+        TestContext::new(&source).with_python_version(options.target_version()),
     );
 
     for stmt in parsed.suite() {
@@ -185,7 +185,7 @@ fn test_invalid_syntax(input_path: &Path) {
     let parsed = parsed.try_into_module().expect("Parsed with Mode::Module");
 
     let mut visitor = SemanticSyntaxCheckerVisitor::new(
-        TestContext::default().with_python_version(options.target_version()),
+        TestContext::new(&source).with_python_version(options.target_version()),
     );
 
     for stmt in parsed.suite() {
@@ -462,21 +462,30 @@ impl<'ast> SourceOrderVisitor<'ast> for ValidateAstVisitor<'ast> {
     }
 }
 
-#[derive(Debug, Default)]
-struct TestContext {
+#[derive(Debug)]
+struct TestContext<'a> {
     diagnostics: RefCell<Vec<SemanticSyntaxError>>,
     python_version: PythonVersion,
+    source: &'a str,
 }
 
-impl TestContext {
+impl<'a> TestContext<'a> {
+    fn new(source: &'a str) -> Self {
+        Self {
+            diagnostics: RefCell::default(),
+            python_version: PythonVersion::default(),
+            source,
+        }
+    }
+
     #[must_use]
     fn with_python_version(mut self, python_version: PythonVersion) -> Self {
         self.python_version = python_version;
         self
     }
 }
 
-impl SemanticSyntaxContext for TestContext {
+impl SemanticSyntaxContext for TestContext<'_> {
     fn seen_docstring_boundary(&self) -> bool {
         false
     }
@@ -489,6 +498,10 @@ impl SemanticSyntaxContext for TestContext {
         self.diagnostics.borrow_mut().push(error);
     }
 
+    fn source(&self) -> &str {
+        self.source
+    }
+
     fn global(&self, _name: &str) -> Option<TextRange> {
         None
     }