From bd34850f9856eb770c969dc88029e80076a09c3b Mon Sep 17 00:00:00 2001 From: Harutaka Kawamura Date: Fri, 7 Oct 2022 21:53:18 +0900 Subject: [PATCH] Implement C404 (#338) --- README.md | 1 + resources/test/fixtures/C404.py | 1 + src/ast/checks.rs | 49 ++++++++++++++++---- src/check_ast.rs | 12 ++++- src/checks.rs | 13 +++++- src/linter.rs | 12 +++++ src/snapshots/ruff__linter__tests__c404.snap | 13 ++++++ 7 files changed, 91 insertions(+), 10 deletions(-) create mode 100644 resources/test/fixtures/C404.py create mode 100644 src/snapshots/ruff__linter__tests__c404.snap diff --git a/README.md b/README.md index 4d0f681c06257..e070a4df8b6e7 100644 --- a/README.md +++ b/README.md @@ -276,6 +276,7 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com | A002 | BuiltinArgumentShadowing | Argument `...` is shadowing a python builtin | | | | A003 | BuiltinAttributeShadowing | Class attribute `...` is shadowing a python builtin | | | | C403 | UnnecessaryListComprehensionSet | Unnecessary list comprehension - rewrite as a set comprehension | | | +| C404 | UnnecessaryListComprehensionDict | Unnecessary list comprehension - rewrite as a dict comprehension | | | | SPR001 | SuperCallWithParameters | Use `super()` instead of `super(__class__, self)` | | 🛠 | | T201 | PrintFound | `print` found | | 🛠 | | T203 | PPrintFound | `pprint` found | | 🛠 | diff --git a/resources/test/fixtures/C404.py b/resources/test/fixtures/C404.py new file mode 100644 index 0000000000000..5aefac5b1b9c3 --- /dev/null +++ b/resources/test/fixtures/C404.py @@ -0,0 +1 @@ +d = dict([(i, i) for i in range(3)]) diff --git a/src/ast/checks.rs b/src/ast/checks.rs index 4286cdb834117..c4f398f649c19 100644 --- a/src/ast/checks.rs +++ b/src/ast/checks.rs @@ -714,14 +714,47 @@ pub fn is_super_call_with_arguments(func: &Expr, args: &Vec) -> bool { } // flakes8-comprehensions -pub fn unnecessary_list_comprehension(expr: &Expr, func: &Expr, args: &Vec) -> Option { - if let ExprKind::Name { id, .. } = &func.node { - if id == "set" && args.len() == 1 { - if let ExprKind::ListComp { .. } = &args[0].node { - return Some(Check::new( - CheckKind::UnnecessaryListComprehensionSet, - Range::from_located(expr), - )); +/// Check `set([...])` compliance. +pub fn unnecessary_list_comprehension_set( + expr: &Expr, + func: &Expr, + args: &Vec, +) -> Option { + if args.len() == 1 { + if let ExprKind::Name { id, .. } = &func.node { + if id == "set" { + if let ExprKind::ListComp { .. } = &args[0].node { + return Some(Check::new( + CheckKind::UnnecessaryListComprehensionSet, + Range::from_located(expr), + )); + } + } + } + } + None +} + +/// Check `dict([...])` compliance. +pub fn unnecessary_list_comprehension_dict( + expr: &Expr, + func: &Expr, + args: &Vec, +) -> Option { + if args.len() == 1 { + if let ExprKind::Name { id, .. } = &func.node { + if id == "dict" { + if let ExprKind::ListComp { elt, .. } = &args[0].node { + match &elt.node { + ExprKind::Tuple { elts, .. } if elts.len() == 2 => { + return Some(Check::new( + CheckKind::UnnecessaryListComprehensionDict, + Range::from_located(expr), + )); + } + _ => {} + } + } } } } diff --git a/src/check_ast.rs b/src/check_ast.rs index c6b6da2933f87..308ab8c4148fb 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -767,7 +767,17 @@ where // flake8-comprehensions if self.settings.enabled.contains(&CheckCode::C403) { - if let Some(check) = checks::unnecessary_list_comprehension(expr, func, args) { + if let Some(check) = + checks::unnecessary_list_comprehension_set(expr, func, args) + { + self.checks.push(check); + }; + } + + if self.settings.enabled.contains(&CheckCode::C404) { + if let Some(check) = + checks::unnecessary_list_comprehension_dict(expr, func, args) + { self.checks.push(check); }; } diff --git a/src/checks.rs b/src/checks.rs index 14aae4f1adf11..2002353def1d3 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -53,7 +53,7 @@ pub const DEFAULT_CHECK_CODES: [CheckCode; 42] = [ CheckCode::F901, ]; -pub const ALL_CHECK_CODES: [CheckCode; 54] = [ +pub const ALL_CHECK_CODES: [CheckCode; 55] = [ // pycodestyle CheckCode::E402, CheckCode::E501, @@ -104,6 +104,7 @@ pub const ALL_CHECK_CODES: [CheckCode; 54] = [ CheckCode::A003, // flake8-comprehensions CheckCode::C403, + CheckCode::C404, // flake8-super CheckCode::SPR001, // flake8-print @@ -171,6 +172,7 @@ pub enum CheckCode { A003, // flake8-comprehensions C403, + C404, // flake8-super SPR001, // flake8-print @@ -241,6 +243,7 @@ impl FromStr for CheckCode { "A003" => Ok(CheckCode::A003), // flake8-comprehensions "C403" => Ok(CheckCode::C403), + "C404" => Ok(CheckCode::C404), // flake8-super "SPR001" => Ok(CheckCode::SPR001), // flake8-print @@ -312,6 +315,7 @@ impl CheckCode { CheckCode::A003 => "A003", // flake8-comprehensions CheckCode::C403 => "C403", + CheckCode::C404 => "C404", // flake8-super CheckCode::SPR001 => "SPR001", // flake8-print @@ -392,6 +396,7 @@ impl CheckCode { CheckCode::A003 => CheckKind::BuiltinAttributeShadowing("...".to_string()), // flake8-comprehensions CheckCode::C403 => CheckKind::UnnecessaryListComprehensionSet, + CheckCode::C404 => CheckKind::UnnecessaryListComprehensionDict, // flake8-super CheckCode::SPR001 => CheckKind::SuperCallWithParameters, // flake8-print @@ -472,6 +477,7 @@ pub enum CheckKind { BuiltinAttributeShadowing(String), // flakes8-comprehensions UnnecessaryListComprehensionSet, + UnnecessaryListComprehensionDict, // flake8-super SuperCallWithParameters, // flake8-print @@ -539,6 +545,7 @@ impl CheckKind { CheckKind::BuiltinAttributeShadowing(_) => "BuiltinAttributeShadowing", // flake8-comprehensions CheckKind::UnnecessaryListComprehensionSet => "UnnecessaryListComprehensionSet", + CheckKind::UnnecessaryListComprehensionDict => "UnnecessaryListComprehensionDict", // flake8-super CheckKind::SuperCallWithParameters => "SuperCallWithParameters", // flake8-print @@ -606,6 +613,7 @@ impl CheckKind { CheckKind::BuiltinAttributeShadowing(_) => &CheckCode::A003, // flake8-comprehensions CheckKind::UnnecessaryListComprehensionSet => &CheckCode::C403, + CheckKind::UnnecessaryListComprehensionDict => &CheckCode::C404, // flake8-super CheckKind::SuperCallWithParameters => &CheckCode::SPR001, // flake8-print @@ -764,6 +772,9 @@ impl CheckKind { CheckKind::UnnecessaryListComprehensionSet => { "Unnecessary list comprehension - rewrite as a set comprehension".to_string() } + CheckKind::UnnecessaryListComprehensionDict => { + "Unnecessary list comprehension - rewrite as a dict comprehension".to_string() + } // flake8-super CheckKind::SuperCallWithParameters => { "Use `super()` instead of `super(__class__, self)`".to_string() diff --git a/src/linter.rs b/src/linter.rs index 81331c99c17c5..30643f69e81be 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -798,6 +798,18 @@ mod tests { Ok(()) } + #[test] + fn c404() -> Result<()> { + let mut checks = check_path( + Path::new("./resources/test/fixtures/C404.py"), + &settings::Settings::for_rule(CheckCode::C404), + &fixer::Mode::Generate, + )?; + checks.sort_by_key(|check| check.location); + insta::assert_yaml_snapshot!(checks); + Ok(()) + } + #[test] fn spr001() -> Result<()> { let mut checks = check_path( diff --git a/src/snapshots/ruff__linter__tests__c404.snap b/src/snapshots/ruff__linter__tests__c404.snap new file mode 100644 index 0000000000000..b2735948d5ec5 --- /dev/null +++ b/src/snapshots/ruff__linter__tests__c404.snap @@ -0,0 +1,13 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: UnnecessaryListComprehensionDict + location: + row: 1 + column: 5 + end_location: + row: 1 + column: 37 + fix: ~ +