diff --git a/README.md b/README.md index 92f872cfff997..fbcf66a3ea5ea 100644 --- a/README.md +++ b/README.md @@ -551,8 +551,8 @@ For more, see [Pyflakes](https://pypi.org/project/pyflakes/2.5.0/) on PyPI. | F524 | StringDotFormatMissingArguments | '...'.format(...) is missing argument(s) for placeholder(s): ... | | | F525 | StringDotFormatMixingAutomatic | '...'.format(...) mixes automatic and manual numbering | | | F541 | FStringMissingPlaceholders | f-string without any placeholders | 🛠 | -| F601 | MultiValueRepeatedKeyLiteral | Dictionary key literal repeated | | -| F602 | MultiValueRepeatedKeyVariable | Dictionary key `...` repeated | | +| F601 | MultiValueRepeatedKeyLiteral | Dictionary key literal `...` repeated | 🛠 | +| F602 | MultiValueRepeatedKeyVariable | Dictionary key `...` repeated | 🛠 | | F621 | ExpressionsInStarAssignment | Too many expressions in star-unpacking assignment | | | F622 | TwoStarredExpressions | Two starred expressions in assignment | | | F631 | AssertTuple | Assert test is a non-empty tuple, which is always `True` | | diff --git a/resources/test/fixtures/pyflakes/F601.py b/resources/test/fixtures/pyflakes/F601.py index fa43186d275e2..3a42484852334 100644 --- a/resources/test/fixtures/pyflakes/F601.py +++ b/resources/test/fixtures/pyflakes/F601.py @@ -10,3 +10,41 @@ b"123": 1, b"123": 4, } + +x = { + "a": 1, + "a": 2, + "a": 3, + "a": 3, +} + +x = { + "a": 1, + "a": 2, + "a": 3, + "a": 3, + "a": 4, +} + +x = { + "a": 1, + "a": 1, + "a": 2, + "a": 3, + "a": 4, +} + +x = { + a: 1, + "a": 1, + a: 1, + "a": 2, + a: 2, + "a": 3, + a: 3, + "a": 3, + a: 4, +} + +x = {"a": 1, "a": 1} +x = {"a": 1, "b": 2, "a": 1} diff --git a/resources/test/fixtures/pyflakes/F602.py b/resources/test/fixtures/pyflakes/F602.py index f904411da3a63..56bf68f6a276c 100644 --- a/resources/test/fixtures/pyflakes/F602.py +++ b/resources/test/fixtures/pyflakes/F602.py @@ -5,3 +5,41 @@ a: 2, b: 3, } + +x = { + a: 1, + a: 2, + a: 3, + a: 3, +} + +x = { + a: 1, + a: 2, + a: 3, + a: 3, + a: 4, +} + +x = { + a: 1, + a: 1, + a: 2, + a: 3, + a: 4, +} + +x = { + a: 1, + "a": 1, + a: 1, + "a": 2, + a: 2, + "a": 3, + a: 3, + "a": 3, + a: 4, +} + +x = {a: 1, a: 1} +x = {a: 1, b: 2, a: 1} diff --git a/src/ast/cmp.rs b/src/ast/cmp.rs new file mode 100644 index 0000000000000..18afcbf82e703 --- /dev/null +++ b/src/ast/cmp.rs @@ -0,0 +1,394 @@ +//! Compare two AST nodes for equality, ignoring locations. + +use rustpython_ast::{Arg, Arguments, Comprehension, Expr, ExprKind, Keyword}; + +/// Returns `true` if the two `Expr` are equal, ignoring locations. +pub fn expr(a: &Expr, b: &Expr) -> bool { + match (&a.node, &b.node) { + ( + ExprKind::BoolOp { + op: op_a, + values: values_a, + }, + ExprKind::BoolOp { + op: op_b, + values: values_b, + }, + ) => { + op_a == op_b + && values_a.len() == values_b.len() + && values_a.iter().zip(values_b).all(|(a, b)| expr(a, b)) + } + ( + ExprKind::NamedExpr { + target: target_a, + value: value_a, + }, + ExprKind::NamedExpr { + target: target_b, + value: value_b, + }, + ) => expr(target_a, target_b) && expr(value_a, value_b), + ( + ExprKind::BinOp { + left: left_a, + op: op_a, + right: right_a, + }, + ExprKind::BinOp { + left: left_b, + op: op_b, + right: right_b, + }, + ) => op_a == op_b && expr(left_a, left_b) && expr(right_a, right_b), + ( + ExprKind::UnaryOp { + op: op_a, + operand: operand_a, + }, + ExprKind::UnaryOp { + op: op_b, + operand: operand_b, + }, + ) => op_a == op_b && expr(operand_a, operand_b), + ( + ExprKind::Lambda { + args: args_a, + body: body_a, + }, + ExprKind::Lambda { + args: args_b, + body: body_b, + }, + ) => expr(body_a, body_b) && arguments(args_a, args_b), + ( + ExprKind::IfExp { + test: test_a, + body: body_a, + orelse: orelse_a, + }, + ExprKind::IfExp { + test: test_b, + body: body_b, + orelse: orelse_b, + }, + ) => expr(test_a, test_b) && expr(body_a, body_b) && expr(orelse_a, orelse_b), + ( + ExprKind::Dict { + keys: keys_a, + values: values_a, + }, + ExprKind::Dict { + keys: keys_b, + values: values_b, + }, + ) => { + keys_a.len() == keys_b.len() + && values_a.len() == values_b.len() + && keys_a.iter().zip(keys_b).all(|(a, b)| expr(a, b)) + && values_a.iter().zip(values_b).all(|(a, b)| expr(a, b)) + } + (ExprKind::Set { elts: elts_a }, ExprKind::Set { elts: elts_b }) => { + elts_a.len() == elts_b.len() && elts_a.iter().zip(elts_b).all(|(a, b)| expr(a, b)) + } + ( + ExprKind::ListComp { + elt: elt_a, + generators: generators_a, + }, + ExprKind::ListComp { + elt: elt_b, + generators: generators_b, + }, + ) => { + expr(elt_a, elt_b) + && generators_a.len() == generators_b.len() + && generators_a + .iter() + .zip(generators_b) + .all(|(a, b)| comprehension(a, b)) + } + ( + ExprKind::SetComp { + elt: elt_a, + generators: generators_a, + }, + ExprKind::SetComp { + elt: elt_b, + generators: generators_b, + }, + ) => { + expr(elt_a, elt_b) + && generators_a.len() == generators_b.len() + && generators_a + .iter() + .zip(generators_b) + .all(|(a, b)| comprehension(a, b)) + } + ( + ExprKind::DictComp { + key: key_a, + value: value_a, + generators: generators_a, + }, + ExprKind::DictComp { + key: key_b, + value: value_b, + generators: generators_b, + }, + ) => { + expr(key_a, key_b) + && expr(value_a, value_b) + && generators_a.len() == generators_b.len() + && generators_a + .iter() + .zip(generators_b) + .all(|(a, b)| comprehension(a, b)) + } + ( + ExprKind::GeneratorExp { + elt: elt_a, + generators: generators_a, + }, + ExprKind::GeneratorExp { + elt: elt_b, + generators: generators_b, + }, + ) => { + expr(elt_a, elt_b) + && generators_a.len() == generators_b.len() + && generators_a + .iter() + .zip(generators_b) + .all(|(a, b)| comprehension(a, b)) + } + (ExprKind::Await { value: value_a }, ExprKind::Await { value: value_b }) => { + expr(value_a, value_b) + } + (ExprKind::Yield { value: value_a }, ExprKind::Yield { value: value_b }) => { + option_expr(value_a.as_deref(), value_b.as_deref()) + } + (ExprKind::YieldFrom { value: value_a }, ExprKind::YieldFrom { value: value_b }) => { + expr(value_a, value_b) + } + ( + ExprKind::Compare { + left: left_a, + ops: ops_a, + comparators: comparators_a, + }, + ExprKind::Compare { + left: left_b, + ops: ops_b, + comparators: comparators_b, + }, + ) => { + expr(left_a, left_b) + && ops_a == ops_b + && comparators_a.len() == comparators_b.len() + && comparators_a + .iter() + .zip(comparators_b) + .all(|(a, b)| expr(a, b)) + } + ( + ExprKind::Call { + func: func_a, + args: args_a, + keywords: keywords_a, + }, + ExprKind::Call { + func: func_b, + args: args_b, + keywords: keywords_b, + }, + ) => { + expr(func_a, func_b) + && args_a.len() == args_b.len() + && args_a.iter().zip(args_b).all(|(a, b)| expr(a, b)) + && keywords_a.len() == keywords_b.len() + && keywords_a + .iter() + .zip(keywords_b) + .all(|(a, b)| keyword(a, b)) + } + ( + ExprKind::FormattedValue { + value: value_a, + conversion: conversion_a, + format_spec: format_spec_a, + }, + ExprKind::FormattedValue { + value: value_b, + conversion: conversion_b, + format_spec: format_spec_b, + }, + ) => { + expr(value_a, value_b) + && conversion_a == conversion_b + && option_expr(format_spec_a.as_deref(), format_spec_b.as_deref()) + } + (ExprKind::JoinedStr { values: values_a }, ExprKind::JoinedStr { values: values_b }) => { + values_a.len() == values_b.len() + && values_a.iter().zip(values_b).all(|(a, b)| expr(a, b)) + } + ( + ExprKind::Constant { + value: value_a, + kind: kind_a, + }, + ExprKind::Constant { + value: value_b, + kind: kind_b, + }, + ) => value_a == value_b && kind_a == kind_b, + ( + ExprKind::Attribute { + value: value_a, + attr: attr_a, + ctx: ctx_a, + }, + ExprKind::Attribute { + value: value_b, + attr: attr_b, + ctx: ctx_b, + }, + ) => attr_a == attr_b && ctx_a == ctx_b && expr(value_a, value_b), + ( + ExprKind::Subscript { + value: value_a, + slice: slice_a, + ctx: ctx_a, + }, + ExprKind::Subscript { + value: value_b, + slice: slice_b, + ctx: ctx_b, + }, + ) => ctx_a == ctx_b && expr(value_a, value_b) && expr(slice_a, slice_b), + ( + ExprKind::Starred { + value: value_a, + ctx: ctx_a, + }, + ExprKind::Starred { + value: value_b, + ctx: ctx_b, + }, + ) => ctx_a == ctx_b && expr(value_a, value_b), + ( + ExprKind::Name { + id: id_a, + ctx: ctx_a, + }, + ExprKind::Name { + id: id_b, + ctx: ctx_b, + }, + ) => id_a == id_b && ctx_a == ctx_b, + ( + ExprKind::List { + elts: elts_a, + ctx: ctx_a, + }, + ExprKind::List { + elts: elts_b, + ctx: ctx_b, + }, + ) => { + ctx_a == ctx_b + && elts_a.len() == elts_b.len() + && elts_a.iter().zip(elts_b).all(|(a, b)| expr(a, b)) + } + ( + ExprKind::Tuple { + elts: elts_a, + ctx: ctx_a, + }, + ExprKind::Tuple { + elts: elts_b, + ctx: ctx_b, + }, + ) => { + ctx_a == ctx_b + && elts_a.len() == elts_b.len() + && elts_a.iter().zip(elts_b).all(|(a, b)| expr(a, b)) + } + ( + ExprKind::Slice { + lower: lower_a, + upper: upper_a, + step: step_a, + }, + ExprKind::Slice { + lower: lower_b, + upper: upper_b, + step: step_b, + }, + ) => { + option_expr(lower_a.as_deref(), lower_b.as_deref()) + && option_expr(upper_a.as_deref(), upper_b.as_deref()) + && option_expr(step_a.as_deref(), step_b.as_deref()) + } + _ => false, + } +} + +fn option_expr(a: Option<&Expr>, b: Option<&Expr>) -> bool { + match (a, b) { + (Some(a), Some(b)) => expr(a, b), + (None, None) => true, + _ => false, + } +} + +fn arguments(a: &Arguments, b: &Arguments) -> bool { + a.posonlyargs.len() == b.posonlyargs.len() + && a.posonlyargs + .iter() + .zip(b.posonlyargs.iter()) + .all(|(a, b)| arg(a, b)) + && a.args.len() == b.args.len() + && a.args.iter().zip(b.args.iter()).all(|(a, b)| arg(a, b)) + && option_arg(a.vararg.as_deref(), b.vararg.as_deref()) + && a.kwonlyargs.len() == b.kwonlyargs.len() + && a.kwonlyargs + .iter() + .zip(b.kwonlyargs.iter()) + .all(|(a, b)| arg(a, b)) + && a.kw_defaults.len() == b.kw_defaults.len() + && a.kw_defaults + .iter() + .zip(b.kw_defaults.iter()) + .all(|(a, b)| expr(a, b)) + && option_arg(a.kwarg.as_deref(), b.kwarg.as_deref()) + && a.defaults.len() == b.defaults.len() + && a.defaults + .iter() + .zip(b.defaults.iter()) + .all(|(a, b)| expr(a, b)) +} + +fn arg(a: &Arg, b: &Arg) -> bool { + a.node.arg == b.node.arg + && option_expr(a.node.annotation.as_deref(), b.node.annotation.as_deref()) +} + +fn option_arg(a: Option<&Arg>, b: Option<&Arg>) -> bool { + match (a, b) { + (Some(a), Some(b)) => arg(a, b), + (None, None) => true, + _ => false, + } +} + +fn keyword(a: &Keyword, b: &Keyword) -> bool { + a.node.arg == b.node.arg && expr(&a.node.value, &b.node.value) +} + +fn comprehension(a: &Comprehension, b: &Comprehension) -> bool { + expr(&a.iter, &b.iter) + && expr(&a.target, &b.target) + && a.ifs.len() == b.ifs.len() + && a.ifs.iter().zip(b.ifs.iter()).all(|(a, b)| expr(a, b)) +} diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 855f89af71b82..a4e54e0bc244a 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -1,5 +1,6 @@ pub mod branch_detection; pub mod cast; +pub mod cmp; pub mod function_type; pub mod helpers; pub mod operations; diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs index af98c85462000..3be2eedba415a 100644 --- a/src/checkers/ast.rs +++ b/src/checkers/ast.rs @@ -2385,15 +2385,11 @@ where )); } } - ExprKind::Dict { keys, .. } => { - let check_repeated_literals = self.settings.enabled.contains(&CheckCode::F601); - let check_repeated_variables = self.settings.enabled.contains(&CheckCode::F602); - if check_repeated_literals || check_repeated_variables { - self.checks.extend(pyflakes::checks::repeated_keys( - keys, - check_repeated_literals, - check_repeated_variables, - )); + ExprKind::Dict { keys, values } => { + if self.settings.enabled.contains(&CheckCode::F601) + || self.settings.enabled.contains(&CheckCode::F602) + { + pyflakes::plugins::repeated_keys(self, keys, values); } } ExprKind::Yield { .. } => { diff --git a/src/pyflakes/checks.rs b/src/pyflakes/checks.rs index 53788e29c1167..46b9f548e7cba 100644 --- a/src/pyflakes/checks.rs +++ b/src/pyflakes/checks.rs @@ -1,8 +1,6 @@ use std::string::ToString; -use rustpython_parser::ast::{ - Constant, Excepthandler, ExcepthandlerKind, Expr, ExprKind, Stmt, StmtKind, -}; +use rustpython_parser::ast::{Excepthandler, ExcepthandlerKind, Expr, ExprKind, Stmt, StmtKind}; use crate::ast::helpers::except_range; use crate::ast::types::{Binding, Range, Scope, ScopeKind}; @@ -70,59 +68,6 @@ pub fn default_except_not_last( None } -#[derive(Debug, PartialEq)] -enum DictionaryKey<'a> { - Constant(&'a Constant), - Variable(&'a str), -} - -fn convert_to_value(expr: &Expr) -> Option { - match &expr.node { - ExprKind::Constant { value, .. } => Some(DictionaryKey::Constant(value)), - ExprKind::Name { id, .. } => Some(DictionaryKey::Variable(id)), - _ => None, - } -} - -/// F601, F602 -pub fn repeated_keys( - keys: &[Expr], - check_repeated_literals: bool, - check_repeated_variables: bool, -) -> Vec { - let mut checks: Vec = vec![]; - - let num_keys = keys.len(); - for i in 0..num_keys { - let k1 = &keys[i]; - let v1 = convert_to_value(k1); - for k2 in keys.iter().take(num_keys).skip(i + 1) { - let v2 = convert_to_value(k2); - match (&v1, &v2) { - (Some(DictionaryKey::Constant(v1)), Some(DictionaryKey::Constant(v2))) => { - if check_repeated_literals && v1 == v2 { - checks.push(Check::new( - violations::MultiValueRepeatedKeyLiteral, - Range::from_located(k2), - )); - } - } - (Some(DictionaryKey::Variable(v1)), Some(DictionaryKey::Variable(v2))) => { - if check_repeated_variables && v1 == v2 { - checks.push(Check::new( - violations::MultiValueRepeatedKeyVariable((*v2).to_string()), - Range::from_located(k2), - )); - } - } - _ => {} - } - } - } - - checks -} - /// F621, F622 pub fn starred_expressions( elts: &[Expr], diff --git a/src/pyflakes/plugins/mod.rs b/src/pyflakes/plugins/mod.rs index 1b1470702840a..993765daefd8f 100644 --- a/src/pyflakes/plugins/mod.rs +++ b/src/pyflakes/plugins/mod.rs @@ -4,6 +4,7 @@ pub use if_tuple::if_tuple; pub use invalid_literal_comparisons::invalid_literal_comparison; pub use invalid_print_syntax::invalid_print_syntax; pub use raise_not_implemented::raise_not_implemented; +pub use repeated_keys::repeated_keys; pub(crate) use strings::{ percent_format_expected_mapping, percent_format_expected_sequence, percent_format_extra_named_arguments, percent_format_missing_arguments, @@ -21,6 +22,7 @@ mod if_tuple; mod invalid_literal_comparisons; mod invalid_print_syntax; mod raise_not_implemented; +mod repeated_keys; mod strings; mod unused_annotation; mod unused_variable; diff --git a/src/pyflakes/plugins/repeated_keys.rs b/src/pyflakes/plugins/repeated_keys.rs new file mode 100644 index 0000000000000..143f0c7c871d7 --- /dev/null +++ b/src/pyflakes/plugins/repeated_keys.rs @@ -0,0 +1,94 @@ +use std::hash::{BuildHasherDefault, Hash}; + +use rustc_hash::FxHashMap; +use rustpython_ast::{Expr, ExprKind}; + +use crate::ast::cmp; +use crate::ast::helpers::unparse_expr; +use crate::ast::types::Range; +use crate::autofix::Fix; +use crate::checkers::ast::Checker; +use crate::registry::{Check, CheckCode}; +use crate::source_code_style::SourceCodeStyleDetector; +use crate::violations; + +#[derive(Debug, Eq, PartialEq, Hash)] +enum DictionaryKey<'a> { + Constant(String), + Variable(&'a str), +} + +fn into_dictionary_key<'a>( + expr: &'a Expr, + stylist: &SourceCodeStyleDetector, +) -> Option> { + match &expr.node { + ExprKind::Constant { .. } => Some(DictionaryKey::Constant(unparse_expr(expr, stylist))), + ExprKind::Name { id, .. } => Some(DictionaryKey::Variable(id)), + _ => None, + } +} + +/// F601, F602 +pub fn repeated_keys(checker: &mut Checker, keys: &[Expr], values: &[Expr]) { + // Generate a map from key to (index, value). + let mut seen: FxHashMap> = + FxHashMap::with_capacity_and_hasher(keys.len(), BuildHasherDefault::default()); + + // Detect duplicate keys. + for (i, key) in keys.iter().enumerate() { + if let Some(key) = into_dictionary_key(key, checker.style) { + if let Some(seen_values) = seen.get_mut(&key) { + match key { + DictionaryKey::Constant(key) => { + if checker.settings.enabled.contains(&CheckCode::F601) { + let repeated_value = + seen_values.iter().any(|value| cmp::expr(value, &values[i])); + let mut check = Check::new( + violations::MultiValueRepeatedKeyLiteral(key, repeated_value), + Range::from_located(&keys[i]), + ); + if repeated_value { + if checker.patch(&CheckCode::F601) { + check.amend(Fix::deletion( + values[i - 1].end_location.unwrap(), + values[i].end_location.unwrap(), + )); + } + } else { + seen_values.push(&values[i]); + } + checker.checks.push(check); + } + } + DictionaryKey::Variable(key) => { + if checker.settings.enabled.contains(&CheckCode::F602) { + let repeated_value = + seen_values.iter().any(|value| cmp::expr(value, &values[i])); + let mut check = Check::new( + violations::MultiValueRepeatedKeyVariable( + key.to_string(), + repeated_value, + ), + Range::from_located(&keys[i]), + ); + if repeated_value { + if checker.patch(&CheckCode::F602) { + check.amend(Fix::deletion( + values[i - 1].end_location.unwrap(), + values[i].end_location.unwrap(), + )); + } + } else { + seen_values.push(&values[i]); + } + checker.checks.push(check); + } + } + } + } else { + seen.insert(key, vec![&values[i]]); + } + } + } +} diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__F601_F601.py.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__F601_F601.py.snap index 9e28311637d65..448ce23301d86 100644 --- a/src/pyflakes/snapshots/ruff__pyflakes__tests__F601_F601.py.snap +++ b/src/pyflakes/snapshots/ruff__pyflakes__tests__F601_F601.py.snap @@ -3,7 +3,9 @@ source: src/pyflakes/mod.rs expression: checks --- - kind: - MultiValueRepeatedKeyLiteral: ~ + MultiValueRepeatedKeyLiteral: + - "\"a\"" + - false location: row: 3 column: 4 @@ -13,7 +15,9 @@ expression: checks fix: ~ parent: ~ - kind: - MultiValueRepeatedKeyLiteral: ~ + MultiValueRepeatedKeyLiteral: + - "1" + - false location: row: 9 column: 4 @@ -23,7 +27,9 @@ expression: checks fix: ~ parent: ~ - kind: - MultiValueRepeatedKeyLiteral: ~ + MultiValueRepeatedKeyLiteral: + - "b\"123\"" + - false location: row: 11 column: 4 @@ -32,4 +38,238 @@ expression: checks column: 10 fix: ~ parent: ~ +- kind: + MultiValueRepeatedKeyLiteral: + - "\"a\"" + - false + location: + row: 16 + column: 4 + end_location: + row: 16 + column: 7 + fix: ~ + parent: ~ +- kind: + MultiValueRepeatedKeyLiteral: + - "\"a\"" + - false + location: + row: 17 + column: 4 + end_location: + row: 17 + column: 7 + fix: ~ + parent: ~ +- kind: + MultiValueRepeatedKeyLiteral: + - "\"a\"" + - true + location: + row: 18 + column: 4 + end_location: + row: 18 + column: 7 + fix: + content: "" + location: + row: 17 + column: 10 + end_location: + row: 18 + column: 10 + parent: ~ +- kind: + MultiValueRepeatedKeyLiteral: + - "\"a\"" + - false + location: + row: 23 + column: 4 + end_location: + row: 23 + column: 7 + fix: ~ + parent: ~ +- kind: + MultiValueRepeatedKeyLiteral: + - "\"a\"" + - false + location: + row: 24 + column: 4 + end_location: + row: 24 + column: 7 + fix: ~ + parent: ~ +- kind: + MultiValueRepeatedKeyLiteral: + - "\"a\"" + - true + location: + row: 25 + column: 4 + end_location: + row: 25 + column: 7 + fix: + content: "" + location: + row: 24 + column: 10 + end_location: + row: 25 + column: 10 + parent: ~ +- kind: + MultiValueRepeatedKeyLiteral: + - "\"a\"" + - false + location: + row: 26 + column: 4 + end_location: + row: 26 + column: 7 + fix: ~ + parent: ~ +- kind: + MultiValueRepeatedKeyLiteral: + - "\"a\"" + - true + location: + row: 31 + column: 4 + end_location: + row: 31 + column: 7 + fix: + content: "" + location: + row: 30 + column: 10 + end_location: + row: 31 + column: 10 + parent: ~ +- kind: + MultiValueRepeatedKeyLiteral: + - "\"a\"" + - false + location: + row: 32 + column: 4 + end_location: + row: 32 + column: 7 + fix: ~ + parent: ~ +- kind: + MultiValueRepeatedKeyLiteral: + - "\"a\"" + - false + location: + row: 33 + column: 4 + end_location: + row: 33 + column: 7 + fix: ~ + parent: ~ +- kind: + MultiValueRepeatedKeyLiteral: + - "\"a\"" + - false + location: + row: 34 + column: 4 + end_location: + row: 34 + column: 7 + fix: ~ + parent: ~ +- kind: + MultiValueRepeatedKeyLiteral: + - "\"a\"" + - false + location: + row: 41 + column: 4 + end_location: + row: 41 + column: 7 + fix: ~ + parent: ~ +- kind: + MultiValueRepeatedKeyLiteral: + - "\"a\"" + - false + location: + row: 43 + column: 4 + end_location: + row: 43 + column: 7 + fix: ~ + parent: ~ +- kind: + MultiValueRepeatedKeyLiteral: + - "\"a\"" + - true + location: + row: 45 + column: 4 + end_location: + row: 45 + column: 7 + fix: + content: "" + location: + row: 44 + column: 8 + end_location: + row: 45 + column: 10 + parent: ~ +- kind: + MultiValueRepeatedKeyLiteral: + - "\"a\"" + - true + location: + row: 49 + column: 13 + end_location: + row: 49 + column: 16 + fix: + content: "" + location: + row: 49 + column: 11 + end_location: + row: 49 + column: 19 + parent: ~ +- kind: + MultiValueRepeatedKeyLiteral: + - "\"a\"" + - true + location: + row: 50 + column: 21 + end_location: + row: 50 + column: 24 + fix: + content: "" + location: + row: 50 + column: 19 + end_location: + row: 50 + column: 27 + parent: ~ diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__F602_F602.py.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__F602_F602.py.snap index 61afbdb79f4e6..a1de35bfbc3a8 100644 --- a/src/pyflakes/snapshots/ruff__pyflakes__tests__F602_F602.py.snap +++ b/src/pyflakes/snapshots/ruff__pyflakes__tests__F602_F602.py.snap @@ -3,7 +3,9 @@ source: src/pyflakes/mod.rs expression: checks --- - kind: - MultiValueRepeatedKeyVariable: a + MultiValueRepeatedKeyVariable: + - a + - false location: row: 5 column: 4 @@ -12,4 +14,250 @@ expression: checks column: 5 fix: ~ parent: ~ +- kind: + MultiValueRepeatedKeyVariable: + - a + - false + location: + row: 11 + column: 4 + end_location: + row: 11 + column: 5 + fix: ~ + parent: ~ +- kind: + MultiValueRepeatedKeyVariable: + - a + - false + location: + row: 12 + column: 4 + end_location: + row: 12 + column: 5 + fix: ~ + parent: ~ +- kind: + MultiValueRepeatedKeyVariable: + - a + - true + location: + row: 13 + column: 4 + end_location: + row: 13 + column: 5 + fix: + content: "" + location: + row: 12 + column: 8 + end_location: + row: 13 + column: 8 + parent: ~ +- kind: + MultiValueRepeatedKeyVariable: + - a + - false + location: + row: 18 + column: 4 + end_location: + row: 18 + column: 5 + fix: ~ + parent: ~ +- kind: + MultiValueRepeatedKeyVariable: + - a + - false + location: + row: 19 + column: 4 + end_location: + row: 19 + column: 5 + fix: ~ + parent: ~ +- kind: + MultiValueRepeatedKeyVariable: + - a + - true + location: + row: 20 + column: 4 + end_location: + row: 20 + column: 5 + fix: + content: "" + location: + row: 19 + column: 8 + end_location: + row: 20 + column: 8 + parent: ~ +- kind: + MultiValueRepeatedKeyVariable: + - a + - false + location: + row: 21 + column: 4 + end_location: + row: 21 + column: 5 + fix: ~ + parent: ~ +- kind: + MultiValueRepeatedKeyVariable: + - a + - true + location: + row: 26 + column: 4 + end_location: + row: 26 + column: 5 + fix: + content: "" + location: + row: 25 + column: 8 + end_location: + row: 26 + column: 8 + parent: ~ +- kind: + MultiValueRepeatedKeyVariable: + - a + - false + location: + row: 27 + column: 4 + end_location: + row: 27 + column: 5 + fix: ~ + parent: ~ +- kind: + MultiValueRepeatedKeyVariable: + - a + - false + location: + row: 28 + column: 4 + end_location: + row: 28 + column: 5 + fix: ~ + parent: ~ +- kind: + MultiValueRepeatedKeyVariable: + - a + - false + location: + row: 29 + column: 4 + end_location: + row: 29 + column: 5 + fix: ~ + parent: ~ +- kind: + MultiValueRepeatedKeyVariable: + - a + - true + location: + row: 35 + column: 4 + end_location: + row: 35 + column: 5 + fix: + content: "" + location: + row: 34 + column: 10 + end_location: + row: 35 + column: 8 + parent: ~ +- kind: + MultiValueRepeatedKeyVariable: + - a + - false + location: + row: 37 + column: 4 + end_location: + row: 37 + column: 5 + fix: ~ + parent: ~ +- kind: + MultiValueRepeatedKeyVariable: + - a + - false + location: + row: 39 + column: 4 + end_location: + row: 39 + column: 5 + fix: ~ + parent: ~ +- kind: + MultiValueRepeatedKeyVariable: + - a + - false + location: + row: 41 + column: 4 + end_location: + row: 41 + column: 5 + fix: ~ + parent: ~ +- kind: + MultiValueRepeatedKeyVariable: + - a + - true + location: + row: 44 + column: 11 + end_location: + row: 44 + column: 12 + fix: + content: "" + location: + row: 44 + column: 9 + end_location: + row: 44 + column: 15 + parent: ~ +- kind: + MultiValueRepeatedKeyVariable: + - a + - true + location: + row: 45 + column: 17 + end_location: + row: 45 + column: 18 + fix: + content: "" + location: + row: 45 + column: 15 + end_location: + row: 45 + column: 21 + parent: ~ diff --git a/src/violations.rs b/src/violations.rs index d26ec8fb63a2a..5a7fb780dffea 100644 --- a/src/violations.rs +++ b/src/violations.rs @@ -651,29 +651,50 @@ impl AlwaysAutofixableViolation for FStringMissingPlaceholders { } define_violation!( - pub struct MultiValueRepeatedKeyLiteral; + pub struct MultiValueRepeatedKeyLiteral(pub String, pub bool); ); impl Violation for MultiValueRepeatedKeyLiteral { fn message(&self) -> String { - "Dictionary key literal repeated".to_string() + let MultiValueRepeatedKeyLiteral(name, ..) = self; + format!("Dictionary key literal `{name}` repeated") + } + + fn autofix_title_formatter(&self) -> Option String> { + let MultiValueRepeatedKeyLiteral(.., repeated_value) = self; + if *repeated_value { + Some(|MultiValueRepeatedKeyLiteral(name, ..)| { + format!("Remove repeated key literal `{name}`") + }) + } else { + None + } } fn placeholder() -> Self { - MultiValueRepeatedKeyLiteral + MultiValueRepeatedKeyLiteral("...".to_string(), true) } } define_violation!( - pub struct MultiValueRepeatedKeyVariable(pub String); + pub struct MultiValueRepeatedKeyVariable(pub String, pub bool); ); impl Violation for MultiValueRepeatedKeyVariable { fn message(&self) -> String { - let MultiValueRepeatedKeyVariable(name) = self; + let MultiValueRepeatedKeyVariable(name, ..) = self; format!("Dictionary key `{name}` repeated") } + fn autofix_title_formatter(&self) -> Option String> { + let MultiValueRepeatedKeyVariable(.., repeated_value) = self; + if *repeated_value { + Some(|MultiValueRepeatedKeyVariable(name, ..)| format!("Remove repeated key `{name}`")) + } else { + None + } + } + fn placeholder() -> Self { - MultiValueRepeatedKeyVariable("...".to_string()) + MultiValueRepeatedKeyVariable("...".to_string(), true) } }