diff --git a/crates/ruff/resources/test/fixtures/pyflakes/F601.py b/crates/ruff/resources/test/fixtures/pyflakes/F601.py index 3a42484852..e8e9e6a89f 100644 --- a/crates/ruff/resources/test/fixtures/pyflakes/F601.py +++ b/crates/ruff/resources/test/fixtures/pyflakes/F601.py @@ -48,3 +48,8 @@ x = { x = {"a": 1, "a": 1} x = {"a": 1, "b": 2, "a": 1} + +x = { + ('a', 'b'): 'asdf', + ('a', 'b'): 'qwer', +} diff --git a/crates/ruff/src/rules/pyflakes/rules/repeated_keys.rs b/crates/ruff/src/rules/pyflakes/rules/repeated_keys.rs index 13c4ceaf9a..ec4412042c 100644 --- a/crates/ruff/src/rules/pyflakes/rules/repeated_keys.rs +++ b/crates/ruff/src/rules/pyflakes/rules/repeated_keys.rs @@ -1,11 +1,11 @@ -use std::hash::{BuildHasherDefault, Hash}; +use std::hash::BuildHasherDefault; use rustc_hash::{FxHashMap, FxHashSet}; -use rustpython_parser::ast::{self, Expr, Ranged}; +use rustpython_parser::ast::{Expr, Ranged}; use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::comparable::{ComparableConstant, ComparableExpr}; +use ruff_python_ast::comparable::ComparableExpr; use crate::checkers::ast::Checker; use crate::registry::{AsRule, Rule}; @@ -43,7 +43,6 @@ use crate::registry::{AsRule, Rule}; #[violation] pub struct MultiValueRepeatedKeyLiteral { name: String, - repeated_value: bool, } impl Violation for MultiValueRepeatedKeyLiteral { @@ -51,20 +50,13 @@ impl Violation for MultiValueRepeatedKeyLiteral { #[derive_message_formats] fn message(&self) -> String { - let MultiValueRepeatedKeyLiteral { name, .. } = self; + let MultiValueRepeatedKeyLiteral { name } = self; format!("Dictionary key literal `{name}` repeated") } fn autofix_title(&self) -> Option { - let MultiValueRepeatedKeyLiteral { - repeated_value, - name, - } = self; - if *repeated_value { - Some(format!("Remove repeated key literal `{name}`")) - } else { - None - } + let MultiValueRepeatedKeyLiteral { name } = self; + Some(format!("Remove repeated key literal `{name}`")) } } @@ -100,7 +92,6 @@ impl Violation for MultiValueRepeatedKeyLiteral { #[violation] pub struct MultiValueRepeatedKeyVariable { name: String, - repeated_value: bool, } impl Violation for MultiValueRepeatedKeyVariable { @@ -108,43 +99,20 @@ impl Violation for MultiValueRepeatedKeyVariable { #[derive_message_formats] fn message(&self) -> String { - let MultiValueRepeatedKeyVariable { name, .. } = self; + let MultiValueRepeatedKeyVariable { name } = self; format!("Dictionary key `{name}` repeated") } fn autofix_title(&self) -> Option { - let MultiValueRepeatedKeyVariable { - repeated_value, - name, - } = self; - if *repeated_value { - Some(format!("Remove repeated key `{name}`")) - } else { - None - } - } -} - -#[derive(Debug, Eq, PartialEq, Hash)] -enum DictionaryKey<'a> { - Constant(ComparableConstant<'a>), - Variable(&'a str), -} - -fn into_dictionary_key(expr: &Expr) -> Option { - match expr { - Expr::Constant(ast::ExprConstant { value, .. }) => { - Some(DictionaryKey::Constant(value.into())) - } - Expr::Name(ast::ExprName { id, .. }) => Some(DictionaryKey::Variable(id)), - _ => None, + let MultiValueRepeatedKeyVariable { name } = self; + Some(format!("Remove repeated key `{name}`")) } } /// F601, F602 pub(crate) fn repeated_keys(checker: &mut Checker, keys: &[Option], values: &[Expr]) { // Generate a map from key to (index, value). - let mut seen: FxHashMap> = + let mut seen: FxHashMap> = FxHashMap::with_capacity_and_hasher(keys.len(), BuildHasherDefault::default()); // Detect duplicate keys. @@ -152,61 +120,56 @@ pub(crate) fn repeated_keys(checker: &mut Checker, keys: &[Option], values let Some(key) = key else { continue; }; - if let Some(dict_key) = into_dictionary_key(key) { - if let Some(seen_values) = seen.get_mut(&dict_key) { - match dict_key { - DictionaryKey::Constant(..) => { - if checker.enabled(Rule::MultiValueRepeatedKeyLiteral) { - let comparable_value: ComparableExpr = (&values[i]).into(); - let is_duplicate_value = seen_values.contains(&comparable_value); - let mut diagnostic = Diagnostic::new( - MultiValueRepeatedKeyLiteral { - name: checker.generator().expr(key), - repeated_value: is_duplicate_value, - }, - key.range(), - ); - if is_duplicate_value { - if checker.patch(diagnostic.kind.rule()) { - diagnostic.set_fix(Fix::suggested(Edit::deletion( - values[i - 1].end(), - values[i].end(), - ))); - } - } else { - seen_values.insert(comparable_value); - } - checker.diagnostics.push(diagnostic); - } - } - DictionaryKey::Variable(dict_key) => { - if checker.enabled(Rule::MultiValueRepeatedKeyVariable) { - let comparable_value: ComparableExpr = (&values[i]).into(); - let is_duplicate_value = seen_values.contains(&comparable_value); - let mut diagnostic = Diagnostic::new( - MultiValueRepeatedKeyVariable { - name: dict_key.to_string(), - repeated_value: is_duplicate_value, - }, - key.range(), - ); - if is_duplicate_value { - if checker.patch(diagnostic.kind.rule()) { - diagnostic.set_fix(Fix::suggested(Edit::deletion( - values[i - 1].end(), - values[i].end(), - ))); - } - } else { - seen_values.insert(comparable_value); - } - checker.diagnostics.push(diagnostic); + + let comparable_key = ComparableExpr::from(key); + let comparable_value = ComparableExpr::from(&values[i]); + + let Some(seen_values) = seen.get_mut(&comparable_key) else { + seen.insert(comparable_key, FxHashSet::from_iter([comparable_value])); + continue; + }; + + match key { + Expr::Constant(_) | Expr::Tuple(_) | Expr::JoinedStr(_) => { + if checker.enabled(Rule::MultiValueRepeatedKeyLiteral) { + let mut diagnostic = Diagnostic::new( + MultiValueRepeatedKeyLiteral { + name: checker.locator.slice(key.range()).to_string(), + }, + key.range(), + ); + if checker.patch(diagnostic.kind.rule()) { + if !seen_values.insert(comparable_value) { + diagnostic.set_fix(Fix::suggested(Edit::deletion( + values[i - 1].end(), + values[i].end(), + ))); } } + checker.diagnostics.push(diagnostic); } - } else { - seen.insert(dict_key, FxHashSet::from_iter([(&values[i]).into()])); } + Expr::Name(_) => { + if checker.enabled(Rule::MultiValueRepeatedKeyVariable) { + let mut diagnostic = Diagnostic::new( + MultiValueRepeatedKeyVariable { + name: checker.locator.slice(key.range()).to_string(), + }, + key.range(), + ); + if checker.patch(diagnostic.kind.rule()) { + let comparable_value: ComparableExpr = (&values[i]).into(); + if !seen_values.insert(comparable_value) { + diagnostic.set_fix(Fix::suggested(Edit::deletion( + values[i - 1].end(), + values[i].end(), + ))); + } + } + checker.diagnostics.push(diagnostic); + } + } + _ => {} } } } diff --git a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F601_F601.py.snap b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F601_F601.py.snap index f6fa5fdd65..1a5b94a092 100644 --- a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F601_F601.py.snap +++ b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F601_F601.py.snap @@ -10,6 +10,18 @@ F601.py:3:5: F601 Dictionary key literal `"a"` repeated 4 | "b": 3, 5 | ("a", "b"): 3, | + = help: Remove repeated key literal `"a"` + +F601.py:6:5: F601 Dictionary key literal `("a", "b")` repeated + | +4 | "b": 3, +5 | ("a", "b"): 3, +6 | ("a", "b"): 4, + | ^^^^^^^^^^ F601 +7 | 1.0: 2, +8 | 1: 0, + | + = help: Remove repeated key literal `("a", "b")` F601.py:9:5: F601 Dictionary key literal `1` repeated | @@ -20,6 +32,7 @@ F601.py:9:5: F601 Dictionary key literal `1` repeated 10 | b"123": 1, 11 | b"123": 4, | + = help: Remove repeated key literal `1` F601.py:11:5: F601 Dictionary key literal `b"123"` repeated | @@ -29,6 +42,7 @@ F601.py:11:5: F601 Dictionary key literal `b"123"` repeated | ^^^^^^ F601 12 | } | + = help: Remove repeated key literal `b"123"` F601.py:16:5: F601 Dictionary key literal `"a"` repeated | @@ -39,6 +53,7 @@ F601.py:16:5: F601 Dictionary key literal `"a"` repeated 17 | "a": 3, 18 | "a": 3, | + = help: Remove repeated key literal `"a"` F601.py:17:5: F601 Dictionary key literal `"a"` repeated | @@ -49,6 +64,7 @@ F601.py:17:5: F601 Dictionary key literal `"a"` repeated 18 | "a": 3, 19 | } | + = help: Remove repeated key literal `"a"` F601.py:18:5: F601 [*] Dictionary key literal `"a"` repeated | @@ -78,6 +94,7 @@ F601.py:23:5: F601 Dictionary key literal `"a"` repeated 24 | "a": 3, 25 | "a": 3, | + = help: Remove repeated key literal `"a"` F601.py:24:5: F601 Dictionary key literal `"a"` repeated | @@ -88,6 +105,7 @@ F601.py:24:5: F601 Dictionary key literal `"a"` repeated 25 | "a": 3, 26 | "a": 4, | + = help: Remove repeated key literal `"a"` F601.py:25:5: F601 [*] Dictionary key literal `"a"` repeated | @@ -117,6 +135,7 @@ F601.py:26:5: F601 Dictionary key literal `"a"` repeated | ^^^ F601 27 | } | + = help: Remove repeated key literal `"a"` F601.py:31:5: F601 [*] Dictionary key literal `"a"` repeated | @@ -147,6 +166,7 @@ F601.py:32:5: F601 Dictionary key literal `"a"` repeated 33 | "a": 3, 34 | "a": 4, | + = help: Remove repeated key literal `"a"` F601.py:33:5: F601 Dictionary key literal `"a"` repeated | @@ -157,6 +177,7 @@ F601.py:33:5: F601 Dictionary key literal `"a"` repeated 34 | "a": 4, 35 | } | + = help: Remove repeated key literal `"a"` F601.py:34:5: F601 Dictionary key literal `"a"` repeated | @@ -166,6 +187,7 @@ F601.py:34:5: F601 Dictionary key literal `"a"` repeated | ^^^ F601 35 | } | + = help: Remove repeated key literal `"a"` F601.py:41:5: F601 Dictionary key literal `"a"` repeated | @@ -176,6 +198,7 @@ F601.py:41:5: F601 Dictionary key literal `"a"` repeated 42 | a: 2, 43 | "a": 3, | + = help: Remove repeated key literal `"a"` F601.py:43:5: F601 Dictionary key literal `"a"` repeated | @@ -186,6 +209,7 @@ F601.py:43:5: F601 Dictionary key literal `"a"` repeated 44 | a: 3, 45 | "a": 3, | + = help: Remove repeated key literal `"a"` F601.py:45:5: F601 [*] Dictionary key literal `"a"` repeated | @@ -224,12 +248,16 @@ F601.py:49:14: F601 [*] Dictionary key literal `"a"` repeated 49 |-x = {"a": 1, "a": 1} 49 |+x = {"a": 1} 50 50 | x = {"a": 1, "b": 2, "a": 1} +51 51 | +52 52 | x = { F601.py:50:22: F601 [*] Dictionary key literal `"a"` repeated | 49 | x = {"a": 1, "a": 1} 50 | x = {"a": 1, "b": 2, "a": 1} | ^^^ F601 +51 | +52 | x = { | = help: Remove repeated key literal `"a"` @@ -239,5 +267,18 @@ F601.py:50:22: F601 [*] Dictionary key literal `"a"` repeated 49 49 | x = {"a": 1, "a": 1} 50 |-x = {"a": 1, "b": 2, "a": 1} 50 |+x = {"a": 1, "b": 2} +51 51 | +52 52 | x = { +53 53 | ('a', 'b'): 'asdf', + +F601.py:54:5: F601 Dictionary key literal `('a', 'b')` repeated + | +52 | x = { +53 | ('a', 'b'): 'asdf', +54 | ('a', 'b'): 'qwer', + | ^^^^^^^^^^ F601 +55 | } + | + = help: Remove repeated key literal `('a', 'b')` diff --git a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F602_F602.py.snap b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F602_F602.py.snap index cdbc8c3290..c29e85b114 100644 --- a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F602_F602.py.snap +++ b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F602_F602.py.snap @@ -10,6 +10,7 @@ F602.py:5:5: F602 Dictionary key `a` repeated 6 | b: 3, 7 | } | + = help: Remove repeated key `a` F602.py:11:5: F602 Dictionary key `a` repeated | @@ -20,6 +21,7 @@ F602.py:11:5: F602 Dictionary key `a` repeated 12 | a: 3, 13 | a: 3, | + = help: Remove repeated key `a` F602.py:12:5: F602 Dictionary key `a` repeated | @@ -30,6 +32,7 @@ F602.py:12:5: F602 Dictionary key `a` repeated 13 | a: 3, 14 | } | + = help: Remove repeated key `a` F602.py:13:5: F602 [*] Dictionary key `a` repeated | @@ -59,6 +62,7 @@ F602.py:18:5: F602 Dictionary key `a` repeated 19 | a: 3, 20 | a: 3, | + = help: Remove repeated key `a` F602.py:19:5: F602 Dictionary key `a` repeated | @@ -69,6 +73,7 @@ F602.py:19:5: F602 Dictionary key `a` repeated 20 | a: 3, 21 | a: 4, | + = help: Remove repeated key `a` F602.py:20:5: F602 [*] Dictionary key `a` repeated | @@ -98,6 +103,7 @@ F602.py:21:5: F602 Dictionary key `a` repeated | ^ F602 22 | } | + = help: Remove repeated key `a` F602.py:26:5: F602 [*] Dictionary key `a` repeated | @@ -128,6 +134,7 @@ F602.py:27:5: F602 Dictionary key `a` repeated 28 | a: 3, 29 | a: 4, | + = help: Remove repeated key `a` F602.py:28:5: F602 Dictionary key `a` repeated | @@ -138,6 +145,7 @@ F602.py:28:5: F602 Dictionary key `a` repeated 29 | a: 4, 30 | } | + = help: Remove repeated key `a` F602.py:29:5: F602 Dictionary key `a` repeated | @@ -147,6 +155,7 @@ F602.py:29:5: F602 Dictionary key `a` repeated | ^ F602 30 | } | + = help: Remove repeated key `a` F602.py:35:5: F602 [*] Dictionary key `a` repeated | @@ -177,6 +186,7 @@ F602.py:37:5: F602 Dictionary key `a` repeated 38 | "a": 3, 39 | a: 3, | + = help: Remove repeated key `a` F602.py:39:5: F602 Dictionary key `a` repeated | @@ -187,6 +197,7 @@ F602.py:39:5: F602 Dictionary key `a` repeated 40 | "a": 3, 41 | a: 4, | + = help: Remove repeated key `a` F602.py:41:5: F602 Dictionary key `a` repeated | @@ -196,6 +207,7 @@ F602.py:41:5: F602 Dictionary key `a` repeated | ^ F602 42 | } | + = help: Remove repeated key `a` F602.py:44:12: F602 [*] Dictionary key `a` repeated |