Implement C403 (#335)

astral-sh · Oct 6, 2022 · 5141285 · 5141285
1 parent 82cc139
commit 5141285
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 1 deletion.
diff --git a/README.md b/README.md
@@ -274,6 +274,7 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com
 | A001 | BuiltinVariableShadowing | Variable `...` is shadowing a python builtin |  |  |
 | 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 |  |  |
 | SPR001 | SuperCallWithParameters | Use `super()` instead of `super(__class__, self)` |  | 🛠 |
 | T201 | PrintFound | `print` found |  | 🛠 |
 | T203 | PPrintFound | `pprint` found |  | 🛠 |

diff --git a/resources/test/fixtures/C403.py b/resources/test/fixtures/C403.py
@@ -0,0 +1 @@
+s = set([x for x in range(3)])
diff --git a/src/ast/checks.rs b/src/ast/checks.rs
@@ -692,6 +692,21 @@ pub fn is_super_call_with_arguments(func: &Expr, args: &Vec<Expr>) -> bool {
     }
 }
 
+// flakes8-comprehensions
+pub fn unnecessary_list_comprehension(expr: &Expr, func: &Expr, args: &Vec<Expr>) -> Option<Check> {
+    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),
+                ));
+            }
+        }
+    }
+    None
+}
+
 // flake8-super
 /// Check that `super()` has no args
 pub fn check_super_args(

diff --git a/src/check_ast.rs b/src/check_ast.rs
@@ -765,6 +765,13 @@ where
                     plugins::print_call(self, expr, func);
                 }
 
+                // flake8-comprehensions
+                if self.settings.enabled.contains(&CheckCode::C403) {
+                    if let Some(check) = checks::unnecessary_list_comprehension(expr, func, args) {
+                        self.checks.push(check);
+                    };
+                }
+
                 if let ExprKind::Name { id, ctx } = &func.node {
                     if id == "locals" && matches!(ctx, ExprContext::Load) {
                         let scope = &mut self.scopes[*(self

diff --git 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; 52] = [
+pub const ALL_CHECK_CODES: [CheckCode; 53] = [
     // pycodestyle
     CheckCode::E402,
     CheckCode::E501,
@@ -102,6 +102,8 @@ pub const ALL_CHECK_CODES: [CheckCode; 52] = [
     CheckCode::A001,
     CheckCode::A002,
     CheckCode::A003,
+    // flake8-comprehensions
+    CheckCode::C403,
     // flake8-super
     CheckCode::SPR001,
     // flake8-print
@@ -166,6 +168,8 @@ pub enum CheckCode {
     A001,
     A002,
     A003,
+    // flake8-comprehensions
+    C403,
     // flake8-super
     SPR001,
     // flake8-print
@@ -233,6 +237,8 @@ impl FromStr for CheckCode {
             "A001" => Ok(CheckCode::A001),
             "A002" => Ok(CheckCode::A002),
             "A003" => Ok(CheckCode::A003),
+            // flake8-comprehensions
+            "C403" => Ok(CheckCode::C403),
             // flake8-super
             "SPR001" => Ok(CheckCode::SPR001),
             // flake8-print
@@ -301,6 +307,8 @@ impl CheckCode {
             CheckCode::A001 => "A001",
             CheckCode::A002 => "A002",
             CheckCode::A003 => "A003",
+            // flake8-comprehensions
+            CheckCode::C403 => "C403",
             // flake8-super
             CheckCode::SPR001 => "SPR001",
             // flake8-print
@@ -378,6 +386,8 @@ impl CheckCode {
             CheckCode::A001 => CheckKind::BuiltinVariableShadowing("...".to_string()),
             CheckCode::A002 => CheckKind::BuiltinArgumentShadowing("...".to_string()),
             CheckCode::A003 => CheckKind::BuiltinAttributeShadowing("...".to_string()),
+            // flake8-comprehensions
+            CheckCode::C403 => CheckKind::UnnecessaryListComprehensionSet,
             // flake8-super
             CheckCode::SPR001 => CheckKind::SuperCallWithParameters,
             // flake8-print
@@ -459,6 +469,8 @@ pub enum CheckKind {
     BuiltinVariableShadowing(String),
     BuiltinArgumentShadowing(String),
     BuiltinAttributeShadowing(String),
+    // flakes8-comprehensions
+    UnnecessaryListComprehensionSet,
     // flake8-super
     SuperCallWithParameters,
     // flake8-print
@@ -516,6 +528,8 @@ impl CheckKind {
             CheckKind::BuiltinVariableShadowing(_) => "BuiltinVariableShadowing",
             CheckKind::BuiltinArgumentShadowing(_) => "BuiltinArgumentShadowing",
             CheckKind::BuiltinAttributeShadowing(_) => "BuiltinAttributeShadowing",
+            // flake8-comprehensions
+            CheckKind::UnnecessaryListComprehensionSet => "UnnecessaryListComprehensionSet",
             // flake8-super
             CheckKind::SuperCallWithParameters => "SuperCallWithParameters",
             // flake8-print
@@ -580,6 +594,8 @@ impl CheckKind {
             CheckKind::BuiltinVariableShadowing(_) => &CheckCode::A001,
             CheckKind::BuiltinArgumentShadowing(_) => &CheckCode::A002,
             CheckKind::BuiltinAttributeShadowing(_) => &CheckCode::A003,
+            // flake8-comprehensions
+            CheckKind::UnnecessaryListComprehensionSet => &CheckCode::C403,
             // flake8-super
             CheckKind::SuperCallWithParameters => &CheckCode::SPR001,
             // flake8-print
@@ -733,6 +749,10 @@ impl CheckKind {
             CheckKind::BuiltinAttributeShadowing(name) => {
                 format!("Class attribute `{name}` is shadowing a python builtin")
             }
+            // flake8-comprehensions
+            CheckKind::UnnecessaryListComprehensionSet => {
+                "Unnecessary list comprehension - rewrite as a set 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
@@ -786,6 +786,18 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn c403() -> Result<()> {
+        let mut checks = check_path(
+            Path::new("./resources/test/fixtures/C403.py"),
+            &settings::Settings::for_rule(CheckCode::C403),
+            &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__c403.snap b/src/snapshots/ruff__linter__tests__c403.snap
@@ -0,0 +1,13 @@
+---
+source: src/linter.rs
+expression: checks
+---
+- kind: UnnecessaryListComprehensionSet
+  location:
+    row: 1
+    column: 5
+  end_location:
+    row: 1
+    column: 31
+  fix: ~
+