From c2a36ebd1eab7abc00fa0e1d9b3047dc61d68250 Mon Sep 17 00:00:00 2001 From: Harutaka Kawamura Date: Mon, 10 Oct 2022 12:33:55 +0900 Subject: [PATCH] Implement C410 (#382) --- README.md | 1 + resources/test/fixtures/C410.py | 4 ++ src/ast/checks.rs | 30 ++++++++++++++ src/check_ast.rs | 7 ++++ src/checks.rs | 17 ++++++++ src/linter.rs | 12 ++++++ src/snapshots/ruff__linter__tests__c410.snap | 41 ++++++++++++++++++++ 7 files changed, 112 insertions(+) create mode 100644 resources/test/fixtures/C410.py create mode 100644 src/snapshots/ruff__linter__tests__c410.snap diff --git a/README.md b/README.md index 8706dcc88c..f16cfc4cc0 100644 --- a/README.md +++ b/README.md @@ -287,6 +287,7 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com | C406 | UnnecessaryLiteralDict | Unnecessary literal - rewrite as a dict literal | | | | C408 | UnnecessaryCollectionCall | Unnecessary call - rewrite as a literal | | | | C409 | UnnecessaryLiteralWithinTupleCall | Unnecessary literal passed to tuple() - remove the outer call to tuple() | | | +| C410 | UnnecessaryLiteralWithinListCall | Unnecessary literal passed to list() - rewrite as a list literal | | | | C415 | UnnecessarySubscriptReversal | Unnecessary subscript reversal of iterable within () | | | | T201 | PrintFound | `print` found | | 🛠 | | T203 | PPrintFound | `pprint` found | | 🛠 | diff --git a/resources/test/fixtures/C410.py b/resources/test/fixtures/C410.py new file mode 100644 index 0000000000..4fbed5b5ce --- /dev/null +++ b/resources/test/fixtures/C410.py @@ -0,0 +1,4 @@ +l1 = list([1, 2]) +l2 = list((1, 2)) +l3 = list([]) +l4 = list(()) diff --git a/src/ast/checks.rs b/src/ast/checks.rs index 0076ef378b..628b5b0401 100644 --- a/src/ast/checks.rs +++ b/src/ast/checks.rs @@ -999,6 +999,36 @@ pub fn unnecessary_literal_within_tuple_call( None } +pub fn unnecessary_literal_within_list_call( + expr: &Expr, + func: &Expr, + args: &[Expr], +) -> Option { + if let ExprKind::Name { id, .. } = &func.node { + if id == "list" { + if let Some(arg) = args.first() { + match &arg.node { + ExprKind::Tuple { .. } => { + return Some(Check::new( + CheckKind::UnnecessaryLiteralWithinListCall("tuple".to_string()), + Range::from_located(expr), + )); + } + ExprKind::List { .. } => { + return Some(Check::new( + CheckKind::UnnecessaryLiteralWithinListCall("list".to_string()), + Range::from_located(expr), + )); + } + _ => {} + } + } + } + } + + None +} + pub fn unnecessary_subscript_reversal(expr: &Expr, func: &Expr, args: &[Expr]) -> Option { if let Some(first_arg) = args.first() { if let ExprKind::Name { id, .. } = &func.node { diff --git a/src/check_ast.rs b/src/check_ast.rs index 02db85b42c..8518a793f7 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -804,6 +804,13 @@ where }; } + if self.settings.enabled.contains(&CheckCode::C410) { + if let Some(check) = + checks::unnecessary_literal_within_list_call(expr, func, args) + { + self.checks.push(check); + }; + } if self.settings.enabled.contains(&CheckCode::C415) { if let Some(check) = checks::unnecessary_subscript_reversal(expr, func, args) { self.checks.push(check); diff --git a/src/checks.rs b/src/checks.rs index 77a66ef67d..e19bb3c306 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -130,6 +130,7 @@ pub enum CheckCode { C406, C408, C409, + C410, C415, // flake8-print T201, @@ -221,6 +222,7 @@ pub enum CheckKind { UnnecessaryLiteralDict(String), UnnecessaryCollectionCall(String), UnnecessaryLiteralWithinTupleCall(String), + UnnecessaryLiteralWithinListCall(String), UnnecessarySubscriptReversal(String), // flake8-print PrintFound, @@ -317,6 +319,9 @@ impl CheckCode { CheckCode::C409 => { CheckKind::UnnecessaryLiteralWithinTupleCall("".to_string()) } + CheckCode::C410 => { + CheckKind::UnnecessaryLiteralWithinListCall("".to_string()) + } CheckCode::C415 => { CheckKind::UnnecessarySubscriptReversal("".to_string()) } @@ -404,6 +409,7 @@ impl CheckKind { CheckKind::UnnecessaryLiteralDict(_) => &CheckCode::C406, CheckKind::UnnecessaryCollectionCall(_) => &CheckCode::C408, CheckKind::UnnecessaryLiteralWithinTupleCall(..) => &CheckCode::C409, + CheckKind::UnnecessaryLiteralWithinListCall(..) => &CheckCode::C410, CheckKind::UnnecessarySubscriptReversal(_) => &CheckCode::C415, // flake8-print CheckKind::PrintFound => &CheckCode::T201, @@ -601,6 +607,17 @@ impl CheckKind { ) } } + CheckKind::UnnecessaryLiteralWithinListCall(literal) => { + if literal == "list" { + format!( + "Unnecessary {literal} literal passed to list() - remove the outer call to list()" + ) + } else { + format!( + "Unnecessary {literal} literal passed to list() - rewrite as a list literal" + ) + } + } CheckKind::UnnecessarySubscriptReversal(func) => { format!("Unnecessary subscript reversal of iterable within {func}()") } diff --git a/src/linter.rs b/src/linter.rs index ef6e27f71c..2b1922b72a 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -954,6 +954,18 @@ mod tests { Ok(()) } + #[test] + fn c410() -> Result<()> { + let mut checks = check_path( + Path::new("./resources/test/fixtures/C410.py"), + &settings::Settings::for_rule(CheckCode::C410), + &fixer::Mode::Generate, + )?; + checks.sort_by_key(|check| check.location); + insta::assert_yaml_snapshot!(checks); + Ok(()) + } + #[test] fn c415() -> Result<()> { let mut checks = check_path( diff --git a/src/snapshots/ruff__linter__tests__c410.snap b/src/snapshots/ruff__linter__tests__c410.snap new file mode 100644 index 0000000000..d15a0fde18 --- /dev/null +++ b/src/snapshots/ruff__linter__tests__c410.snap @@ -0,0 +1,41 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: + UnnecessaryLiteralWithinListCall: list + location: + row: 1 + column: 6 + end_location: + row: 1 + column: 18 + fix: ~ +- kind: + UnnecessaryLiteralWithinListCall: tuple + location: + row: 2 + column: 6 + end_location: + row: 2 + column: 18 + fix: ~ +- kind: + UnnecessaryLiteralWithinListCall: list + location: + row: 3 + column: 6 + end_location: + row: 3 + column: 14 + fix: ~ +- kind: + UnnecessaryLiteralWithinListCall: tuple + location: + row: 4 + column: 6 + end_location: + row: 4 + column: 14 + fix: ~ +