Change iteration-over-set to flag set literals only (#4907)

astral-sh · Jun 7, 2023 · 8e386a0 · 8e386a0
1 parent f81a3a4
commit 8e386a0
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 67 deletions.
diff --git a/crates/ruff/resources/test/fixtures/pylint/iteration_over_set.py b/crates/ruff/resources/test/fixtures/pylint/iteration_over_set.py
@@ -3,12 +3,6 @@
 for item in {"apples", "lemons", "water"}:  # flags in-line set literals
     print(f"I like {item}.")
 
-for item in set(("apples", "lemons", "water")):  # flags set() calls
-    print(f"I like {item}.")
-
-for number in {i for i in range(10)}:  # flags set comprehensions
-    print(number)
-
 numbers_list = [i for i in {1, 2, 3}]  # flags sets in list comprehensions
 
 numbers_set = {i for i in {1, 2, 3}}  # flags sets in set comprehensions
@@ -36,3 +30,9 @@
 numbers_dict = {str(i): i for i in [1, 2, 3]}  # lists in dict comprehensions are fine
 
 numbers_gen = (i for i in (1, 2, 3))  # tuples in generator expressions are fine
+
+for item in set(("apples", "lemons", "water")):  # set constructor is fine
+    print(f"I like {item}.")
+
+for number in {i for i in range(10)}:  # set comprehensions are fine
+    print(number)
diff --git a/crates/ruff/src/rules/pylint/rules/iteration_over_set.rs b/crates/ruff/src/rules/pylint/rules/iteration_over_set.rs
@@ -1,12 +1,12 @@
-use rustpython_parser::ast::{Expr, ExprName, Ranged};
+use rustpython_parser::ast::{Expr, Ranged};
 
 use ruff_diagnostics::{Diagnostic, Violation};
 use ruff_macros::{derive_message_formats, violation};
 
 use crate::checkers::ast::Checker;
 
 /// ## What it does
-/// Checks for iterations over `set` literals and comprehensions.
+/// Checks for iterations over `set` literals.
 ///
 /// ## Why is this bad?
 /// Iterating over a `set` is less efficient than iterating over a sequence
@@ -38,23 +38,7 @@ impl Violation for IterationOverSet {
 
 /// PLC0208
 pub(crate) fn iteration_over_set(checker: &mut Checker, expr: &Expr) {
-    let is_set = match expr {
-        // Ex) `for i in {1, 2, 3}`
-        Expr::Set(_) => true,
-        // Ex)` for i in {n for n in range(1, 4)}`
-        Expr::SetComp(_) => true,
-        // Ex) `for i in set(1, 2, 3)`
-        Expr::Call(call) => {
-            if let Expr::Name(ExprName { id, .. }) = call.func.as_ref() {
-                id.as_str() == "set" && checker.semantic_model().is_builtin("set")
-            } else {
-                false
-            }
-        }
-        _ => false,
-    };
-
-    if is_set {
+    if expr.is_set_expr() {
         checker
             .diagnostics
             .push(Diagnostic::new(IterationOverSet, expr.range()));

diff --git a/...src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLC0208_iteration_over_set.py.snap b/...src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLC0208_iteration_over_set.py.snap
@@ -10,62 +10,44 @@ iteration_over_set.py:3:13: PLC0208 Use a sequence type instead of a `set` when
 6 |     print(f"I like {item}.")
   |
 
-iteration_over_set.py:6:13: PLC0208 Use a sequence type instead of a `set` when iterating over values
-  |
-6 |     print(f"I like {item}.")
-7 | 
-8 | for item in set(("apples", "lemons", "water")):  # flags set() calls
-  |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLC0208
-9 |     print(f"I like {item}.")
-  |
-
-iteration_over_set.py:9:15: PLC0208 Use a sequence type instead of a `set` when iterating over values
-   |
- 9 |     print(f"I like {item}.")
-10 | 
-11 | for number in {i for i in range(10)}:  # flags set comprehensions
-   |               ^^^^^^^^^^^^^^^^^^^^^^ PLC0208
-12 |     print(number)
-   |
-
-iteration_over_set.py:12:28: PLC0208 Use a sequence type instead of a `set` when iterating over values
+iteration_over_set.py:6:28: PLC0208 Use a sequence type instead of a `set` when iterating over values
    |
-12 |     print(number)
-13 | 
-14 | numbers_list = [i for i in {1, 2, 3}]  # flags sets in list comprehensions
+ 6 |     print(f"I like {item}.")
+ 7 | 
+ 8 | numbers_list = [i for i in {1, 2, 3}]  # flags sets in list comprehensions
    |                            ^^^^^^^^^ PLC0208
-15 | 
-16 | numbers_set = {i for i in {1, 2, 3}}  # flags sets in set comprehensions
+ 9 | 
+10 | numbers_set = {i for i in {1, 2, 3}}  # flags sets in set comprehensions
    |
 
-iteration_over_set.py:14:27: PLC0208 Use a sequence type instead of a `set` when iterating over values
+iteration_over_set.py:8:27: PLC0208 Use a sequence type instead of a `set` when iterating over values
    |
-14 | numbers_list = [i for i in {1, 2, 3}]  # flags sets in list comprehensions
-15 | 
-16 | numbers_set = {i for i in {1, 2, 3}}  # flags sets in set comprehensions
+ 8 | numbers_list = [i for i in {1, 2, 3}]  # flags sets in list comprehensions
+ 9 | 
+10 | numbers_set = {i for i in {1, 2, 3}}  # flags sets in set comprehensions
    |                           ^^^^^^^^^ PLC0208
-17 | 
-18 | numbers_dict = {str(i): i for i in {1, 2, 3}}  # flags sets in dict comprehensions
+11 | 
+12 | numbers_dict = {str(i): i for i in {1, 2, 3}}  # flags sets in dict comprehensions
    |
 
-iteration_over_set.py:16:36: PLC0208 Use a sequence type instead of a `set` when iterating over values
+iteration_over_set.py:10:36: PLC0208 Use a sequence type instead of a `set` when iterating over values
    |
-16 | numbers_set = {i for i in {1, 2, 3}}  # flags sets in set comprehensions
-17 | 
-18 | numbers_dict = {str(i): i for i in {1, 2, 3}}  # flags sets in dict comprehensions
+10 | numbers_set = {i for i in {1, 2, 3}}  # flags sets in set comprehensions
+11 | 
+12 | numbers_dict = {str(i): i for i in {1, 2, 3}}  # flags sets in dict comprehensions
    |                                    ^^^^^^^^^ PLC0208
-19 | 
-20 | numbers_gen = (i for i in {1, 2, 3})  # flags sets in generator expressions
+13 | 
+14 | numbers_gen = (i for i in {1, 2, 3})  # flags sets in generator expressions
    |
 
-iteration_over_set.py:18:27: PLC0208 Use a sequence type instead of a `set` when iterating over values
+iteration_over_set.py:12:27: PLC0208 Use a sequence type instead of a `set` when iterating over values
    |
-18 | numbers_dict = {str(i): i for i in {1, 2, 3}}  # flags sets in dict comprehensions
-19 | 
-20 | numbers_gen = (i for i in {1, 2, 3})  # flags sets in generator expressions
+12 | numbers_dict = {str(i): i for i in {1, 2, 3}}  # flags sets in dict comprehensions
+13 | 
+14 | numbers_gen = (i for i in {1, 2, 3})  # flags sets in generator expressions
    |                           ^^^^^^^^^ PLC0208
-21 | 
-22 | # Non-errors
+15 | 
+16 | # Non-errors
    |