From 4f3b18c044999683417172bb3d97bf4e30d630da Mon Sep 17 00:00:00 2001 From: AlexBieg Date: Wed, 15 Nov 2023 14:20:41 -0800 Subject: [PATCH 1/7] Add inital implementation of E1132 --- .../src/checkers/ast/analyze/expression.rs | 3 + crates/ruff_linter/src/codes.rs | 1 + .../ruff_linter/src/rules/pylint/rules/mod.rs | 2 + .../rules/pylint/rules/repeated_keywords.rs | 75 +++++++++++++++++++ 4 files changed, 81 insertions(+) create mode 100644 crates/ruff_linter/src/rules/pylint/rules/repeated_keywords.rs diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 106cbecd47909..da5c2fab19d3d 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -776,6 +776,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::NestedMinMax) { pylint::rules::nested_min_max(checker, expr, func, args, keywords); } + if checker.enabled(Rule::RepeatedKeywords) { + pylint::rules::repeated_keywords(checker, keywords); + } if checker.enabled(Rule::PytestPatchWithLambda) { if let Some(diagnostic) = flake8_pytest_style::rules::patch_with_lambda(call) { checker.diagnostics.push(diagnostic); diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 8efb7b5e2c460..5a3551364e39d 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -228,6 +228,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pylint, "E0604") => (RuleGroup::Stable, rules::pylint::rules::InvalidAllObject), (Pylint, "E0605") => (RuleGroup::Stable, rules::pylint::rules::InvalidAllFormat), (Pylint, "E0704") => (RuleGroup::Preview, rules::pylint::rules::MisplacedBareRaise), + (Pylint, "E1132") => (RuleGroup::Preview, rules::pylint::rules::RepeatedKeywords), (Pylint, "E1142") => (RuleGroup::Stable, rules::pylint::rules::AwaitOutsideAsync), (Pylint, "E1205") => (RuleGroup::Stable, rules::pylint::rules::LoggingTooManyArgs), (Pylint, "E1206") => (RuleGroup::Stable, rules::pylint::rules::LoggingTooFewArgs), diff --git a/crates/ruff_linter/src/rules/pylint/rules/mod.rs b/crates/ruff_linter/src/rules/pylint/rules/mod.rs index 978a4bd15e73d..d9da82493d66a 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/mod.rs @@ -44,6 +44,7 @@ pub(crate) use redefined_argument_from_local::*; pub(crate) use redefined_loop_name::*; pub(crate) use repeated_equality_comparison::*; pub(crate) use repeated_isinstance_calls::*; +pub(crate) use repeated_keywords::*; pub(crate) use return_in_init::*; pub(crate) use self_assigning_variable::*; pub(crate) use single_string_slots::*; @@ -116,6 +117,7 @@ mod redefined_argument_from_local; mod redefined_loop_name; mod repeated_equality_comparison; mod repeated_isinstance_calls; +mod repeated_keywords; mod return_in_init; mod self_assigning_variable; mod single_string_slots; diff --git a/crates/ruff_linter/src/rules/pylint/rules/repeated_keywords.rs b/crates/ruff_linter/src/rules/pylint/rules/repeated_keywords.rs new file mode 100644 index 0000000000000..ff2e66a03880c --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/rules/repeated_keywords.rs @@ -0,0 +1,75 @@ +use std::collections::HashSet; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{Expr, ExprDict, ExprStringLiteral, Keyword}; +use ruff_text_size::{Ranged, TextRange}; + +use crate::checkers::ast::Checker; + +/// TODO(Alex): do the docs +#[violation] +pub struct RepeatedKeywords { + duplicate_keyword: String, +} + +impl Violation for RepeatedKeywords { + #[derive_message_formats] + fn message(&self) -> String { + let dupe = &self.duplicate_keyword; + format!("Duplicate keyword argument: `{dupe}`") + } +} + +type KeywordRecordFn<'a> = Box; + +fn generate_record_func<'a>(checker: &'a mut Checker) -> KeywordRecordFn<'a> { + // init some hash sets to be captured by the closure + let mut seen = HashSet::::new(); + let mut dupes = HashSet::::new(); + + let inner = move |keyword: &str, range| { + // Add an error the first time we see the duplicate + if seen.contains(keyword) && !dupes.contains(keyword) { + dupes.insert(String::from(keyword)); + checker.diagnostics.push(Diagnostic::new( + RepeatedKeywords { + duplicate_keyword: keyword.into(), + }, + range, + )); + } else { + seen.insert(String::from(keyword)); + } + }; + + Box::new(inner) +} + +pub(crate) fn repeated_keywords(checker: &mut Checker, keywords: &Vec) { + let mut record_keyword = generate_record_func(checker); + + for keyword in keywords { + if let Some(id) = &keyword.arg { + record_keyword(id.as_str(), keyword.range()); + } else if let Expr::Dict(ExprDict { + // We only want to check dict keys if there is NO arg associated with them + keys, + range: _, + values: _, + }) = &keyword.value + { + for key in keys.iter().flatten() { + if let Expr::StringLiteral(ExprStringLiteral { + value, + range: _, + unicode: _, + implicit_concatenated: _, + }) = key + { + record_keyword(value, key.range()); + } + } + } + } +} From 93f8f0853473a4f1c03d0c5b27322d12dff8db5e Mon Sep 17 00:00:00 2001 From: AlexBieg Date: Wed, 15 Nov 2023 14:30:33 -0800 Subject: [PATCH 2/7] Add docs --- .../rules/pylint/rules/repeated_keywords.rs | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/ruff_linter/src/rules/pylint/rules/repeated_keywords.rs b/crates/ruff_linter/src/rules/pylint/rules/repeated_keywords.rs index ff2e66a03880c..0cb7341c0123d 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/repeated_keywords.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/repeated_keywords.rs @@ -7,7 +7,25 @@ use ruff_text_size::{Ranged, TextRange}; use crate::checkers::ast::Checker; -/// TODO(Alex): do the docs +/// ## What it does +/// Checks for duplicated keyword arguments passed to a function call +/// +/// ## Why is this bad? +/// Python does not allow for multiple values to be assigned to the same +/// keyword argument in a single function call. +/// +/// ## Example +/// ```python +/// func(1, 2, c=3, **{"c": 4}) +/// ``` +/// +/// Use instead: +/// ```python +/// func(1, 2, **{"c": 4}) +/// ``` +/// +/// ## References +/// - [Python documentation: Argument](https://docs.python.org/3/glossary.html#term-argument) #[violation] pub struct RepeatedKeywords { duplicate_keyword: String, From 27949d07c546a8662e9a2586e96d6f7752c8726f Mon Sep 17 00:00:00 2001 From: AlexBieg Date: Wed, 15 Nov 2023 14:51:18 -0800 Subject: [PATCH 3/7] Add tests and update schema file --- .../test/fixtures/pylint/repeated_keywords.py | 20 +++++ crates/ruff_linter/src/rules/pylint/mod.rs | 1 + .../rules/pylint/rules/repeated_keywords.rs | 4 +- ...__tests__PLE1132_repeated_keywords.py.snap | 80 +++++++++++++++++++ ruff.schema.json | 2 + 5 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/pylint/repeated_keywords.py create mode 100644 crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1132_repeated_keywords.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/repeated_keywords.py b/crates/ruff_linter/resources/test/fixtures/pylint/repeated_keywords.py new file mode 100644 index 0000000000000..a437e621d399d --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pylint/repeated_keywords.py @@ -0,0 +1,20 @@ +def func(a=10, b=20, c=30): + pass + +# Valid +func(a=11, b=21, c=31) +func(b=11, a=21, c=31) +func(c=11, a=21, b=31) +func(a=11, b=31, **{"c": 41}) +func(a=11, **{"b": 21}, **{"c": 41}) +func(a=11, **{"b": 21, "c": 41}) +func(**{"b": 21, "c": 41}) +func(**{"a": 11}, **{"b": 21}, **{"c": 41}) +func(**{"a": 11, "b": 21, "c": 41}) + +# Invalid +func(a=11, c=31, **{"c": 41}) +func(a=11, c=31, **{"c": 41, "a": 51}) +func(a=11, b=21, c=31, **{"b": 22, "c": 41, "a": 51}) +func(a=11, b=21, **{"c": 31}, **{"c": 32}) +func(a=11, b=21, **{"c": 31, "c": 32}) \ No newline at end of file diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs index 589bdbc0544b4..b1b3d36bded44 100644 --- a/crates/ruff_linter/src/rules/pylint/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/mod.rs @@ -149,6 +149,7 @@ mod tests { #[test_case(Rule::UnnecessaryLambda, Path::new("unnecessary_lambda.py"))] #[test_case(Rule::NonAsciiImportName, Path::new("non_ascii_module_import.py"))] #[test_case(Rule::NonAsciiName, Path::new("non_ascii_name.py"))] + #[test_case(Rule::RepeatedKeywords, Path::new("repeated_keywords.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/pylint/rules/repeated_keywords.rs b/crates/ruff_linter/src/rules/pylint/rules/repeated_keywords.rs index 0cb7341c0123d..bff01404f6874 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/repeated_keywords.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/repeated_keywords.rs @@ -8,7 +8,7 @@ use ruff_text_size::{Ranged, TextRange}; use crate::checkers::ast::Checker; /// ## What it does -/// Checks for duplicated keyword arguments passed to a function call +/// Checks for repeated keyword arguments passed to a function call /// /// ## Why is this bad? /// Python does not allow for multiple values to be assigned to the same @@ -35,7 +35,7 @@ impl Violation for RepeatedKeywords { #[derive_message_formats] fn message(&self) -> String { let dupe = &self.duplicate_keyword; - format!("Duplicate keyword argument: `{dupe}`") + format!("Repeated keyword argument: `{dupe}`") } } diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1132_repeated_keywords.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1132_repeated_keywords.py.snap new file mode 100644 index 0000000000000..11c310bd7bc11 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1132_repeated_keywords.py.snap @@ -0,0 +1,80 @@ +--- +source: crates/ruff_linter/src/rules/pylint/mod.rs +--- +repeated_keywords.py:16:21: PLE1132 Repeated keyword argument: `c` + | +15 | # Invalid +16 | func(a=11, c=31, **{"c": 41}) + | ^^^ PLE1132 +17 | func(a=11, c=31, **{"c": 41, "a": 51}) +18 | func(a=11, b=21, c=31, **{"b": 22, "c": 41, "a": 51}) + | + +repeated_keywords.py:17:21: PLE1132 Repeated keyword argument: `c` + | +15 | # Invalid +16 | func(a=11, c=31, **{"c": 41}) +17 | func(a=11, c=31, **{"c": 41, "a": 51}) + | ^^^ PLE1132 +18 | func(a=11, b=21, c=31, **{"b": 22, "c": 41, "a": 51}) +19 | func(a=11, b=21, **{"c": 31}, **{"c": 32}) + | + +repeated_keywords.py:17:30: PLE1132 Repeated keyword argument: `a` + | +15 | # Invalid +16 | func(a=11, c=31, **{"c": 41}) +17 | func(a=11, c=31, **{"c": 41, "a": 51}) + | ^^^ PLE1132 +18 | func(a=11, b=21, c=31, **{"b": 22, "c": 41, "a": 51}) +19 | func(a=11, b=21, **{"c": 31}, **{"c": 32}) + | + +repeated_keywords.py:18:27: PLE1132 Repeated keyword argument: `b` + | +16 | func(a=11, c=31, **{"c": 41}) +17 | func(a=11, c=31, **{"c": 41, "a": 51}) +18 | func(a=11, b=21, c=31, **{"b": 22, "c": 41, "a": 51}) + | ^^^ PLE1132 +19 | func(a=11, b=21, **{"c": 31}, **{"c": 32}) +20 | func(a=11, b=21, **{"c": 31, "c": 32}) + | + +repeated_keywords.py:18:36: PLE1132 Repeated keyword argument: `c` + | +16 | func(a=11, c=31, **{"c": 41}) +17 | func(a=11, c=31, **{"c": 41, "a": 51}) +18 | func(a=11, b=21, c=31, **{"b": 22, "c": 41, "a": 51}) + | ^^^ PLE1132 +19 | func(a=11, b=21, **{"c": 31}, **{"c": 32}) +20 | func(a=11, b=21, **{"c": 31, "c": 32}) + | + +repeated_keywords.py:18:45: PLE1132 Repeated keyword argument: `a` + | +16 | func(a=11, c=31, **{"c": 41}) +17 | func(a=11, c=31, **{"c": 41, "a": 51}) +18 | func(a=11, b=21, c=31, **{"b": 22, "c": 41, "a": 51}) + | ^^^ PLE1132 +19 | func(a=11, b=21, **{"c": 31}, **{"c": 32}) +20 | func(a=11, b=21, **{"c": 31, "c": 32}) + | + +repeated_keywords.py:19:34: PLE1132 Repeated keyword argument: `c` + | +17 | func(a=11, c=31, **{"c": 41, "a": 51}) +18 | func(a=11, b=21, c=31, **{"b": 22, "c": 41, "a": 51}) +19 | func(a=11, b=21, **{"c": 31}, **{"c": 32}) + | ^^^ PLE1132 +20 | func(a=11, b=21, **{"c": 31, "c": 32}) + | + +repeated_keywords.py:20:30: PLE1132 Repeated keyword argument: `c` + | +18 | func(a=11, b=21, c=31, **{"b": 22, "c": 41, "a": 51}) +19 | func(a=11, b=21, **{"c": 31}, **{"c": 32}) +20 | func(a=11, b=21, **{"c": 31, "c": 32}) + | ^^^ PLE1132 + | + + diff --git a/ruff.schema.json b/ruff.schema.json index 989b21078fcab..0c65527184029 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3020,6 +3020,8 @@ "PLE0704", "PLE1", "PLE11", + "PLE113", + "PLE1132", "PLE114", "PLE1142", "PLE12", From f5f529206e1d94b1d1a75746a83f56666688abbc Mon Sep 17 00:00:00 2001 From: AlexBieg Date: Wed, 15 Nov 2023 14:56:36 -0800 Subject: [PATCH 4/7] Add newline to test file --- .../resources/test/fixtures/pylint/repeated_keywords.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/repeated_keywords.py b/crates/ruff_linter/resources/test/fixtures/pylint/repeated_keywords.py index a437e621d399d..b7bb0d7e54d34 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/repeated_keywords.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/repeated_keywords.py @@ -17,4 +17,4 @@ def func(a=10, b=20, c=30): func(a=11, c=31, **{"c": 41, "a": 51}) func(a=11, b=21, c=31, **{"b": 22, "c": 41, "a": 51}) func(a=11, b=21, **{"c": 31}, **{"c": 32}) -func(a=11, b=21, **{"c": 31, "c": 32}) \ No newline at end of file +func(a=11, b=21, **{"c": 31, "c": 32}) From d7115f7b3f6e9164269b786680095368a4346517 Mon Sep 17 00:00:00 2001 From: AlexBieg Date: Sat, 18 Nov 2023 12:21:09 -0800 Subject: [PATCH 5/7] Address PR comments --- .../src/checkers/ast/analyze/expression.rs | 2 +- .../rules/pylint/rules/repeated_keywords.rs | 79 ++++++++----------- 2 files changed, 33 insertions(+), 48 deletions(-) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index da5c2fab19d3d..45e01658ac0d0 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -777,7 +777,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { pylint::rules::nested_min_max(checker, expr, func, args, keywords); } if checker.enabled(Rule::RepeatedKeywords) { - pylint::rules::repeated_keywords(checker, keywords); + pylint::rules::repeated_keywords(checker, call); } if checker.enabled(Rule::PytestPatchWithLambda) { if let Some(diagnostic) = flake8_pytest_style::rules::patch_with_lambda(call) { diff --git a/crates/ruff_linter/src/rules/pylint/rules/repeated_keywords.rs b/crates/ruff_linter/src/rules/pylint/rules/repeated_keywords.rs index bff01404f6874..1a654eb9dab09 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/repeated_keywords.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/repeated_keywords.rs @@ -1,9 +1,10 @@ -use std::collections::HashSet; +use std::hash::BuildHasherDefault; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::{Expr, ExprDict, ExprStringLiteral, Keyword}; -use ruff_text_size::{Ranged, TextRange}; +use ruff_python_ast::{Arguments, Expr, ExprCall, ExprDict, ExprStringLiteral}; +use ruff_text_size::Ranged; +use rustc_hash::FxHashSet; use crate::checkers::ast::Checker; @@ -34,58 +35,42 @@ pub struct RepeatedKeywords { impl Violation for RepeatedKeywords { #[derive_message_formats] fn message(&self) -> String { - let dupe = &self.duplicate_keyword; - format!("Repeated keyword argument: `{dupe}`") + let Self { duplicate_keyword } = self; + format!("Repeated keyword argument: `{duplicate_keyword}`") } } -type KeywordRecordFn<'a> = Box; +pub(crate) fn repeated_keywords(checker: &mut Checker, call: &ExprCall) { + let ExprCall { + arguments: Arguments { keywords, .. }, + .. + } = call; -fn generate_record_func<'a>(checker: &'a mut Checker) -> KeywordRecordFn<'a> { - // init some hash sets to be captured by the closure - let mut seen = HashSet::::new(); - let mut dupes = HashSet::::new(); - - let inner = move |keyword: &str, range| { - // Add an error the first time we see the duplicate - if seen.contains(keyword) && !dupes.contains(keyword) { - dupes.insert(String::from(keyword)); - checker.diagnostics.push(Diagnostic::new( - RepeatedKeywords { - duplicate_keyword: keyword.into(), - }, - range, - )); - } else { - seen.insert(String::from(keyword)); - } - }; - - Box::new(inner) -} - -pub(crate) fn repeated_keywords(checker: &mut Checker, keywords: &Vec) { - let mut record_keyword = generate_record_func(checker); + let mut seen = + FxHashSet::with_capacity_and_hasher(keywords.len(), BuildHasherDefault::default()); for keyword in keywords { if let Some(id) = &keyword.arg { - record_keyword(id.as_str(), keyword.range()); - } else if let Expr::Dict(ExprDict { - // We only want to check dict keys if there is NO arg associated with them - keys, - range: _, - values: _, - }) = &keyword.value - { + if !seen.insert(id.as_str()) { + checker.diagnostics.push(Diagnostic::new( + RepeatedKeywords { + duplicate_keyword: id.to_string(), + }, + keyword.range(), + )); + } + // We only want to check dict keys if there is NO arg associated with them + } else if let Expr::Dict(ExprDict { keys, .. }) = &keyword.value { for key in keys.iter().flatten() { - if let Expr::StringLiteral(ExprStringLiteral { - value, - range: _, - unicode: _, - implicit_concatenated: _, - }) = key - { - record_keyword(value, key.range()); + if let Expr::StringLiteral(ExprStringLiteral { value, .. }) = key { + if !seen.insert(value) { + checker.diagnostics.push(Diagnostic::new( + RepeatedKeywords { + duplicate_keyword: value.to_string(), + }, + key.range(), + )); + } } } } From 35ef0303684b2df35c0aa2d35e95820fe3a8da67 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 18 Nov 2023 19:20:31 -0500 Subject: [PATCH 6/7] Tweak docs --- .../src/rules/pylint/rules/repeated_keywords.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/crates/ruff_linter/src/rules/pylint/rules/repeated_keywords.rs b/crates/ruff_linter/src/rules/pylint/rules/repeated_keywords.rs index 1a654eb9dab09..b92446a1e04cb 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/repeated_keywords.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/repeated_keywords.rs @@ -9,22 +9,18 @@ use rustc_hash::FxHashSet; use crate::checkers::ast::Checker; /// ## What it does -/// Checks for repeated keyword arguments passed to a function call +/// Checks for repeated keyword arguments in function calls. /// /// ## Why is this bad? -/// Python does not allow for multiple values to be assigned to the same -/// keyword argument in a single function call. +/// Python does not allow repeated keyword arguments in function calls. If a +/// function is called with the same keyword argument multiple times, the +/// interpreter will raise an exception. /// /// ## Example /// ```python /// func(1, 2, c=3, **{"c": 4}) /// ``` /// -/// Use instead: -/// ```python -/// func(1, 2, **{"c": 4}) -/// ``` -/// /// ## References /// - [Python documentation: Argument](https://docs.python.org/3/glossary.html#term-argument) #[violation] @@ -51,6 +47,7 @@ pub(crate) fn repeated_keywords(checker: &mut Checker, call: &ExprCall) { for keyword in keywords { if let Some(id) = &keyword.arg { + // Ex) `func(a=1, a=2)` if !seen.insert(id.as_str()) { checker.diagnostics.push(Diagnostic::new( RepeatedKeywords { @@ -59,8 +56,8 @@ pub(crate) fn repeated_keywords(checker: &mut Checker, call: &ExprCall) { keyword.range(), )); } - // We only want to check dict keys if there is NO arg associated with them } else if let Expr::Dict(ExprDict { keys, .. }) = &keyword.value { + // Ex) `func(**{"a": 1, "a": 2})` for key in keys.iter().flatten() { if let Expr::StringLiteral(ExprStringLiteral { value, .. }) = key { if !seen.insert(value) { From 529d459cb9638fb91746b68a4e6e960a396f93d5 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 18 Nov 2023 19:21:55 -0500 Subject: [PATCH 7/7] Rename rule --- ..._keywords.py => repeated_keyword_argument.py} | 0 .../src/checkers/ast/analyze/expression.rs | 4 ++-- crates/ruff_linter/src/codes.rs | 2 +- crates/ruff_linter/src/rules/pylint/mod.rs | 5 ++++- crates/ruff_linter/src/rules/pylint/rules/mod.rs | 4 ++-- ..._keywords.rs => repeated_keyword_argument.rs} | 10 +++++----- ...s__PLE1132_repeated_keyword_argument.py.snap} | 16 ++++++++-------- 7 files changed, 22 insertions(+), 19 deletions(-) rename crates/ruff_linter/resources/test/fixtures/pylint/{repeated_keywords.py => repeated_keyword_argument.py} (100%) rename crates/ruff_linter/src/rules/pylint/rules/{repeated_keywords.rs => repeated_keyword_argument.rs} (89%) rename crates/ruff_linter/src/rules/pylint/snapshots/{ruff_linter__rules__pylint__tests__PLE1132_repeated_keywords.py.snap => ruff_linter__rules__pylint__tests__PLE1132_repeated_keyword_argument.py.snap} (77%) diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/repeated_keywords.py b/crates/ruff_linter/resources/test/fixtures/pylint/repeated_keyword_argument.py similarity index 100% rename from crates/ruff_linter/resources/test/fixtures/pylint/repeated_keywords.py rename to crates/ruff_linter/resources/test/fixtures/pylint/repeated_keyword_argument.py diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 45e01658ac0d0..36b7082999923 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -776,8 +776,8 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::NestedMinMax) { pylint::rules::nested_min_max(checker, expr, func, args, keywords); } - if checker.enabled(Rule::RepeatedKeywords) { - pylint::rules::repeated_keywords(checker, call); + if checker.enabled(Rule::RepeatedKeywordArgument) { + pylint::rules::repeated_keyword_argument(checker, call); } if checker.enabled(Rule::PytestPatchWithLambda) { if let Some(diagnostic) = flake8_pytest_style::rules::patch_with_lambda(call) { diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 5a3551364e39d..8111186243291 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -228,7 +228,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pylint, "E0604") => (RuleGroup::Stable, rules::pylint::rules::InvalidAllObject), (Pylint, "E0605") => (RuleGroup::Stable, rules::pylint::rules::InvalidAllFormat), (Pylint, "E0704") => (RuleGroup::Preview, rules::pylint::rules::MisplacedBareRaise), - (Pylint, "E1132") => (RuleGroup::Preview, rules::pylint::rules::RepeatedKeywords), + (Pylint, "E1132") => (RuleGroup::Preview, rules::pylint::rules::RepeatedKeywordArgument), (Pylint, "E1142") => (RuleGroup::Stable, rules::pylint::rules::AwaitOutsideAsync), (Pylint, "E1205") => (RuleGroup::Stable, rules::pylint::rules::LoggingTooManyArgs), (Pylint, "E1206") => (RuleGroup::Stable, rules::pylint::rules::LoggingTooFewArgs), diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs index b1b3d36bded44..2c4e28ca363e6 100644 --- a/crates/ruff_linter/src/rules/pylint/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/mod.rs @@ -149,7 +149,10 @@ mod tests { #[test_case(Rule::UnnecessaryLambda, Path::new("unnecessary_lambda.py"))] #[test_case(Rule::NonAsciiImportName, Path::new("non_ascii_module_import.py"))] #[test_case(Rule::NonAsciiName, Path::new("non_ascii_name.py"))] - #[test_case(Rule::RepeatedKeywords, Path::new("repeated_keywords.py"))] + #[test_case( + Rule::RepeatedKeywordArgument, + Path::new("repeated_keyword_argument.py") + )] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/pylint/rules/mod.rs b/crates/ruff_linter/src/rules/pylint/rules/mod.rs index d9da82493d66a..b130d36260182 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/mod.rs @@ -44,7 +44,7 @@ pub(crate) use redefined_argument_from_local::*; pub(crate) use redefined_loop_name::*; pub(crate) use repeated_equality_comparison::*; pub(crate) use repeated_isinstance_calls::*; -pub(crate) use repeated_keywords::*; +pub(crate) use repeated_keyword_argument::*; pub(crate) use return_in_init::*; pub(crate) use self_assigning_variable::*; pub(crate) use single_string_slots::*; @@ -117,7 +117,7 @@ mod redefined_argument_from_local; mod redefined_loop_name; mod repeated_equality_comparison; mod repeated_isinstance_calls; -mod repeated_keywords; +mod repeated_keyword_argument; mod return_in_init; mod self_assigning_variable; mod single_string_slots; diff --git a/crates/ruff_linter/src/rules/pylint/rules/repeated_keywords.rs b/crates/ruff_linter/src/rules/pylint/rules/repeated_keyword_argument.rs similarity index 89% rename from crates/ruff_linter/src/rules/pylint/rules/repeated_keywords.rs rename to crates/ruff_linter/src/rules/pylint/rules/repeated_keyword_argument.rs index b92446a1e04cb..42e896054028b 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/repeated_keywords.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/repeated_keyword_argument.rs @@ -24,11 +24,11 @@ use crate::checkers::ast::Checker; /// ## References /// - [Python documentation: Argument](https://docs.python.org/3/glossary.html#term-argument) #[violation] -pub struct RepeatedKeywords { +pub struct RepeatedKeywordArgument { duplicate_keyword: String, } -impl Violation for RepeatedKeywords { +impl Violation for RepeatedKeywordArgument { #[derive_message_formats] fn message(&self) -> String { let Self { duplicate_keyword } = self; @@ -36,7 +36,7 @@ impl Violation for RepeatedKeywords { } } -pub(crate) fn repeated_keywords(checker: &mut Checker, call: &ExprCall) { +pub(crate) fn repeated_keyword_argument(checker: &mut Checker, call: &ExprCall) { let ExprCall { arguments: Arguments { keywords, .. }, .. @@ -50,7 +50,7 @@ pub(crate) fn repeated_keywords(checker: &mut Checker, call: &ExprCall) { // Ex) `func(a=1, a=2)` if !seen.insert(id.as_str()) { checker.diagnostics.push(Diagnostic::new( - RepeatedKeywords { + RepeatedKeywordArgument { duplicate_keyword: id.to_string(), }, keyword.range(), @@ -62,7 +62,7 @@ pub(crate) fn repeated_keywords(checker: &mut Checker, call: &ExprCall) { if let Expr::StringLiteral(ExprStringLiteral { value, .. }) = key { if !seen.insert(value) { checker.diagnostics.push(Diagnostic::new( - RepeatedKeywords { + RepeatedKeywordArgument { duplicate_keyword: value.to_string(), }, key.range(), diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1132_repeated_keywords.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1132_repeated_keyword_argument.py.snap similarity index 77% rename from crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1132_repeated_keywords.py.snap rename to crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1132_repeated_keyword_argument.py.snap index 11c310bd7bc11..2e669058dccff 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1132_repeated_keywords.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1132_repeated_keyword_argument.py.snap @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/pylint/mod.rs --- -repeated_keywords.py:16:21: PLE1132 Repeated keyword argument: `c` +repeated_keyword_argument.py:16:21: PLE1132 Repeated keyword argument: `c` | 15 | # Invalid 16 | func(a=11, c=31, **{"c": 41}) @@ -10,7 +10,7 @@ repeated_keywords.py:16:21: PLE1132 Repeated keyword argument: `c` 18 | func(a=11, b=21, c=31, **{"b": 22, "c": 41, "a": 51}) | -repeated_keywords.py:17:21: PLE1132 Repeated keyword argument: `c` +repeated_keyword_argument.py:17:21: PLE1132 Repeated keyword argument: `c` | 15 | # Invalid 16 | func(a=11, c=31, **{"c": 41}) @@ -20,7 +20,7 @@ repeated_keywords.py:17:21: PLE1132 Repeated keyword argument: `c` 19 | func(a=11, b=21, **{"c": 31}, **{"c": 32}) | -repeated_keywords.py:17:30: PLE1132 Repeated keyword argument: `a` +repeated_keyword_argument.py:17:30: PLE1132 Repeated keyword argument: `a` | 15 | # Invalid 16 | func(a=11, c=31, **{"c": 41}) @@ -30,7 +30,7 @@ repeated_keywords.py:17:30: PLE1132 Repeated keyword argument: `a` 19 | func(a=11, b=21, **{"c": 31}, **{"c": 32}) | -repeated_keywords.py:18:27: PLE1132 Repeated keyword argument: `b` +repeated_keyword_argument.py:18:27: PLE1132 Repeated keyword argument: `b` | 16 | func(a=11, c=31, **{"c": 41}) 17 | func(a=11, c=31, **{"c": 41, "a": 51}) @@ -40,7 +40,7 @@ repeated_keywords.py:18:27: PLE1132 Repeated keyword argument: `b` 20 | func(a=11, b=21, **{"c": 31, "c": 32}) | -repeated_keywords.py:18:36: PLE1132 Repeated keyword argument: `c` +repeated_keyword_argument.py:18:36: PLE1132 Repeated keyword argument: `c` | 16 | func(a=11, c=31, **{"c": 41}) 17 | func(a=11, c=31, **{"c": 41, "a": 51}) @@ -50,7 +50,7 @@ repeated_keywords.py:18:36: PLE1132 Repeated keyword argument: `c` 20 | func(a=11, b=21, **{"c": 31, "c": 32}) | -repeated_keywords.py:18:45: PLE1132 Repeated keyword argument: `a` +repeated_keyword_argument.py:18:45: PLE1132 Repeated keyword argument: `a` | 16 | func(a=11, c=31, **{"c": 41}) 17 | func(a=11, c=31, **{"c": 41, "a": 51}) @@ -60,7 +60,7 @@ repeated_keywords.py:18:45: PLE1132 Repeated keyword argument: `a` 20 | func(a=11, b=21, **{"c": 31, "c": 32}) | -repeated_keywords.py:19:34: PLE1132 Repeated keyword argument: `c` +repeated_keyword_argument.py:19:34: PLE1132 Repeated keyword argument: `c` | 17 | func(a=11, c=31, **{"c": 41, "a": 51}) 18 | func(a=11, b=21, c=31, **{"b": 22, "c": 41, "a": 51}) @@ -69,7 +69,7 @@ repeated_keywords.py:19:34: PLE1132 Repeated keyword argument: `c` 20 | func(a=11, b=21, **{"c": 31, "c": 32}) | -repeated_keywords.py:20:30: PLE1132 Repeated keyword argument: `c` +repeated_keyword_argument.py:20:30: PLE1132 Repeated keyword argument: `c` | 18 | func(a=11, b=21, c=31, **{"b": 22, "c": 41, "a": 51}) 19 | func(a=11, b=21, **{"c": 31}, **{"c": 32})