From 38c30905e6c27c36aa6c240152383c5a9f51feac Mon Sep 17 00:00:00 2001 From: Harutaka Kawamura Date: Mon, 10 Oct 2022 11:34:52 +0900 Subject: [PATCH] Implement C409 (#381) --- README.md | 1 + resources/test/fixtures/C409.py | 3 ++ src/ast/checks.rs | 30 ++++++++++++++++++ src/check_ast.rs | 8 +++++ src/checks.rs | 17 +++++++++++ src/linter.rs | 12 ++++++++ src/snapshots/ruff__linter__tests__c409.snap | 32 ++++++++++++++++++++ 7 files changed, 103 insertions(+) create mode 100644 resources/test/fixtures/C409.py create mode 100644 src/snapshots/ruff__linter__tests__c409.snap diff --git a/README.md b/README.md index 002e1269cc..8706dcc88c 100644 --- a/README.md +++ b/README.md @@ -286,6 +286,7 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com | C405 | UnnecessaryLiteralSet | Unnecessary literal - rewrite as a set literal | | | | 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() | | | | C415 | UnnecessarySubscriptReversal | Unnecessary subscript reversal of iterable within () | | | | T201 | PrintFound | `print` found | | 🛠 | | T203 | PPrintFound | `pprint` found | | 🛠 | diff --git a/resources/test/fixtures/C409.py b/resources/test/fixtures/C409.py new file mode 100644 index 0000000000..492569742e --- /dev/null +++ b/resources/test/fixtures/C409.py @@ -0,0 +1,3 @@ +t1 = tuple([1, 2]) +t2 = tuple((1, 2)) +t3 = tuple([]) diff --git a/src/ast/checks.rs b/src/ast/checks.rs index 424967c1fc..fdd09729a2 100644 --- a/src/ast/checks.rs +++ b/src/ast/checks.rs @@ -969,6 +969,36 @@ pub fn unnecessary_collection_call( None } +pub fn unnecessary_literal_within_tuple_call( + expr: &Expr, + func: &Expr, + args: &[Expr], +) -> Option { + if let ExprKind::Name { id, .. } = &func.node { + if id == "tuple" { + if let Some(arg) = args.first() { + match &arg.node { + ExprKind::Tuple { .. } => { + return Some(Check::new( + CheckKind::UnnecessaryLiteralWithinTupleCall("tuple".to_string()), + Range::from_located(expr), + )); + } + ExprKind::List { .. } => { + return Some(Check::new( + CheckKind::UnnecessaryLiteralWithinTupleCall("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 2e58939c2c..02db85b42c 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -796,6 +796,14 @@ where }; } + if self.settings.enabled.contains(&CheckCode::C409) { + if let Some(check) = + checks::unnecessary_literal_within_tuple_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 7ee06e386b..4f2ca9a7b9 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -129,6 +129,7 @@ pub enum CheckCode { C405, C406, C408, + C409, C415, // flake8-print T201, @@ -219,6 +220,7 @@ pub enum CheckKind { UnnecessaryLiteralSet(String), UnnecessaryLiteralDict(String), UnnecessaryCollectionCall(String), + UnnecessaryLiteralWithinTupleCall(String), UnnecessarySubscriptReversal(String), // flake8-print PrintFound, @@ -312,6 +314,9 @@ impl CheckCode { CheckCode::C408 => { CheckKind::UnnecessaryCollectionCall("".to_string()) } + CheckCode::C409 => { + CheckKind::UnnecessaryLiteralWithinTupleCall("".to_string()) + } CheckCode::C415 => { CheckKind::UnnecessarySubscriptReversal("".to_string()) } @@ -398,6 +403,7 @@ impl CheckKind { CheckKind::UnnecessaryLiteralSet(_) => &CheckCode::C405, CheckKind::UnnecessaryLiteralDict(_) => &CheckCode::C406, CheckKind::UnnecessaryCollectionCall(_) => &CheckCode::C408, + CheckKind::UnnecessaryLiteralWithinTupleCall(..) => &CheckCode::C409, CheckKind::UnnecessarySubscriptReversal(_) => &CheckCode::C415, // flake8-print CheckKind::PrintFound => &CheckCode::T201, @@ -584,6 +590,17 @@ impl CheckKind { CheckKind::UnnecessaryCollectionCall(obj_type) => { format!("Unnecessary {obj_type} call - rewrite as a literal") } + CheckKind::UnnecessaryLiteralWithinTupleCall(literal) => { + if literal == "list" { + format!( + "Unnecessary {literal} literal passed to tuple() - rewrite as a tuple literal" + ) + } else { + format!( + "Unnecessary {literal} literal passed to tuple() - remove the outer call to tuple()" + ) + } + } CheckKind::UnnecessarySubscriptReversal(func) => { format!("Unnecessary subscript reversal of iterable within {func}()") } diff --git a/src/linter.rs b/src/linter.rs index bd6dd11cc2..ef6e27f71c 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -942,6 +942,18 @@ mod tests { Ok(()) } + #[test] + fn c409() -> Result<()> { + let mut checks = check_path( + Path::new("./resources/test/fixtures/C409.py"), + &settings::Settings::for_rule(CheckCode::C409), + &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__c409.snap b/src/snapshots/ruff__linter__tests__c409.snap new file mode 100644 index 0000000000..d554172aa5 --- /dev/null +++ b/src/snapshots/ruff__linter__tests__c409.snap @@ -0,0 +1,32 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: + UnnecessaryLiteralWithinTupleCall: list + location: + row: 1 + column: 6 + end_location: + row: 1 + column: 19 + fix: ~ +- kind: + UnnecessaryLiteralWithinTupleCall: tuple + location: + row: 2 + column: 6 + end_location: + row: 2 + column: 19 + fix: ~ +- kind: + UnnecessaryLiteralWithinTupleCall: list + location: + row: 3 + column: 6 + end_location: + row: 3 + column: 15 + fix: ~ +