diff --git a/README.md b/README.md index b1e93819d4..d266951edc 100644 --- a/README.md +++ b/README.md @@ -215,7 +215,7 @@ ruff also implements some of the most popular Flake8 plugins natively, including - [`flake8-builtins`](https://pypi.org/project/flake8-builtins/) - [`flake8-super`](https://pypi.org/project/flake8-super/) - [`flake8-print`](https://pypi.org/project/flake8-print/) -- [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/) (12/16) +- [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/) (13/16) - [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (3/32) - [`flake8-docstrings`](https://pypi.org/project/flake8-docstrings/) (37/48) - [`pyupgrade`](https://pypi.org/project/pyupgrade/) (8/34) @@ -295,6 +295,7 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com | C410 | UnnecessaryLiteralWithinListCall | Unnecessary literal passed to list() - rewrite as a list literal | | | | C414 | UnnecessaryDoubleCastOrProcess | Unnecessary call within (). | | | | C415 | UnnecessarySubscriptReversal | Unnecessary subscript reversal of iterable within () | | | +| C416 | UnnecessaryComprehension | Unnecessary comprehension - rewrite using () | | | | T201 | PrintFound | `print` found | | 🛠 | | T203 | PPrintFound | `pprint` found | | 🛠 | | U001 | UselessMetaclassType | `__metaclass__ = type` is implied | | 🛠 | diff --git a/resources/test/fixtures/C416.py b/resources/test/fixtures/C416.py new file mode 100644 index 0000000000..9d2df37c84 --- /dev/null +++ b/resources/test/fixtures/C416.py @@ -0,0 +1,6 @@ +x = [1, 2, 3] +[i for i in x] +{i for i in x} + +[i for i in x if i > 1] +[i for i in x for j in x] diff --git a/src/ast/checkers.rs b/src/ast/checkers.rs index 03d0176669..644284afba 100644 --- a/src/ast/checkers.rs +++ b/src/ast/checkers.rs @@ -4,8 +4,8 @@ use itertools::izip; use num_bigint::BigInt; use regex::Regex; use rustpython_parser::ast::{ - Arg, ArgData, Arguments, Cmpop, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprKind, - KeywordData, Located, Stmt, StmtKind, Unaryop, + Arg, ArgData, Arguments, Cmpop, Comprehension, Constant, Excepthandler, ExcepthandlerKind, + Expr, ExprKind, KeywordData, Located, Stmt, StmtKind, Unaryop, }; use serde::{Deserialize, Serialize}; @@ -1104,6 +1104,41 @@ pub fn unnecessary_subscript_reversal(expr: &Expr, func: &Expr, args: &[Expr]) - None } +pub fn unnecessary_comprehension( + expr: &Expr, + elt: &Located, + generators: &Vec, +) -> Option { + if generators.len() == 1 { + let generator = &generators[0]; + if generator.ifs.is_empty() && generator.is_async == 0 { + if let ExprKind::Name { id: elt_id, .. } = &elt.node { + if let ExprKind::Name { id: target_id, .. } = &generator.target.node { + if elt_id == target_id { + match &expr.node { + ExprKind::ListComp { .. } => { + return Some(Check::new( + CheckKind::UnnecessaryComprehension("list".to_string()), + Range::from_located(expr), + )) + } + ExprKind::SetComp { .. } => { + return Some(Check::new( + CheckKind::UnnecessaryComprehension("set".to_string()), + Range::from_located(expr), + )) + } + _ => {} + }; + } + } + } + } + } + + None +} + // flake8-super /// Check that `super()` has no args pub fn super_args( diff --git a/src/check_ast.rs b/src/check_ast.rs index 505a20e685..359caf9884 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -1021,10 +1021,20 @@ where self.visit_expr(expr); } } - ExprKind::GeneratorExp { .. } - | ExprKind::ListComp { .. } - | ExprKind::DictComp { .. } - | ExprKind::SetComp { .. } => self.push_scope(Scope::new(ScopeKind::Generator)), + + ExprKind::ListComp { elt, generators } | ExprKind::SetComp { elt, generators } => { + if self.settings.enabled.contains(&CheckCode::C416) { + if let Some(check) = checkers::unnecessary_comprehension(expr, elt, generators) + { + self.checks.push(check); + }; + } + self.push_scope(Scope::new(ScopeKind::Generator)) + } + + ExprKind::GeneratorExp { .. } | ExprKind::DictComp { .. } => { + self.push_scope(Scope::new(ScopeKind::Generator)) + } _ => {} }; diff --git a/src/checks.rs b/src/checks.rs index 2f291f6344..ba74f840c3 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -139,6 +139,7 @@ pub enum CheckCode { C410, C414, C415, + C416, // flake8-print T201, T203, @@ -274,6 +275,7 @@ pub enum CheckKind { UnnecessaryLiteralWithinListCall(String), UnnecessaryDoubleCastOrProcess(String, String), UnnecessarySubscriptReversal(String), + UnnecessaryComprehension(String), // flake8-print PrintFound, PPrintFound, @@ -421,6 +423,7 @@ impl CheckCode { CheckCode::C415 => { CheckKind::UnnecessarySubscriptReversal("".to_string()) } + CheckCode::C416 => CheckKind::UnnecessaryComprehension("".to_string()), // flake8-print CheckCode::T201 => CheckKind::PrintFound, CheckCode::T203 => CheckKind::PPrintFound, @@ -554,6 +557,7 @@ impl CheckKind { CheckKind::UnnecessaryLiteralWithinListCall(..) => &CheckCode::C410, CheckKind::UnnecessaryDoubleCastOrProcess(..) => &CheckCode::C414, CheckKind::UnnecessarySubscriptReversal(_) => &CheckCode::C415, + CheckKind::UnnecessaryComprehension(..) => &CheckCode::C416, // flake8-print CheckKind::PrintFound => &CheckCode::T201, CheckKind::PPrintFound => &CheckCode::T203, @@ -822,6 +826,9 @@ impl CheckKind { CheckKind::UnnecessarySubscriptReversal(func) => { format!("Unnecessary subscript reversal of iterable within {func}()") } + CheckKind::UnnecessaryComprehension(obj_type) => { + format!(" Unnecessary {obj_type} comprehension - rewrite using {obj_type}()") + } // flake8-print CheckKind::PrintFound => "`print` found".to_string(), CheckKind::PPrintFound => "`pprint` found".to_string(), diff --git a/src/linter.rs b/src/linter.rs index b16a8c2ae2..dd470657b6 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -1031,6 +1031,18 @@ mod tests { Ok(()) } + #[test] + fn c416() -> Result<()> { + let mut checks = check_path( + Path::new("./resources/test/fixtures/C416.py"), + &settings::Settings::for_rule(CheckCode::C416), + &fixer::Mode::Generate, + )?; + checks.sort_by_key(|check| check.location); + insta::assert_yaml_snapshot!(checks); + Ok(()) + } + #[test] fn d100() -> Result<()> { let mut checks = check_path( diff --git a/src/snapshots/ruff__linter__tests__c416.snap b/src/snapshots/ruff__linter__tests__c416.snap new file mode 100644 index 0000000000..549a94f6ed --- /dev/null +++ b/src/snapshots/ruff__linter__tests__c416.snap @@ -0,0 +1,23 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: + UnnecessaryComprehension: list + location: + row: 2 + column: 1 + end_location: + row: 2 + column: 15 + fix: ~ +- kind: + UnnecessaryComprehension: set + location: + row: 3 + column: 1 + end_location: + row: 3 + column: 15 + fix: ~ +