Implement C416 (#415)

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 <list/tuple> literal passed to list() - rewrite as a list literal |  |  |
 | C414 | UnnecessaryDoubleCastOrProcess | Unnecessary <list/reversed/set/sorted/tuple> call within <list/set/sorted/tuple>(). |  |  |
 | C415 | UnnecessarySubscriptReversal | Unnecessary subscript reversal of iterable within <reversed/set/sorted>() |  |  |
+| C416 | UnnecessaryComprehension |  Unnecessary <list/set> comprehension - rewrite using <list/set>() |  |  |
 | T201 | PrintFound | `print` found |  | 🛠 |
 | T203 | PPrintFound | `pprint` found |  | 🛠 |
 | U001 | UselessMetaclassType | `__metaclass__ = type` is implied |  | 🛠 |

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]

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<ExprKind>,
+    generators: &Vec<Comprehension>,
+) -> Option<Check> {
+    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(

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))
+            }
             _ => {}
         };
 

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("<reversed/set/sorted>".to_string())
             }
+            CheckCode::C416 => CheckKind::UnnecessaryComprehension("<list/set>".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(),

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(

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: ~
+