Add Pylint rule C0208 (use-sequence-for-iteration) as PLC0208 (…

…`iteration-over-set`) (#4706)

crates/ruff/resources/test/fixtures/pylint/iteration_over_set.py

@@ -0,0 +1,38 @@
+# Errors
+
+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
+
+numbers_dict = {str(i): i for i in {1, 2, 3}}  # flags sets in dict comprehensions
+
+numbers_gen = (i for i in {1, 2, 3})  # flags sets in generator expressions
+
+# Non-errors
+
+items = {"apples", "lemons", "water"}
+for item in items:  # only complains about in-line sets (as per Pylint)
+    print(f"I like {item}.")
+
+for item in ["apples", "lemons", "water"]:  # lists are fine
+    print(f"I like {item}.")
+
+for item in ("apples", "lemons", "water"):  # tuples are fine
+    print(f"I like {item}.")
+
+numbers_list = [i for i in [1, 2, 3]]  # lists in comprehensions are fine
+
+numbers_set = {i for i in (1, 2, 3)}  # tuples in comprehensions are fine
+
+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

crates/ruff/src/checkers/ast/mod.rs

@@ -1554,6 +1554,9 @@ where
                 if self.enabled(Rule::RedefinedLoopName) {
                     pylint::rules::redefined_loop_name(self, &Node::Stmt(stmt));
                 }
+                if self.enabled(Rule::IterationOverSet) {
+                    pylint::rules::iteration_over_set(self, iter);
+                }
                 if matches!(stmt, Stmt::For(_)) {
                     if self.enabled(Rule::ReimplementedBuiltin) {
                         flake8_simplify::rules::convert_for_loop_to_any_all(
@@ -3502,6 +3505,11 @@ where
                         );
                     }
                 }
+                if self.enabled(Rule::IterationOverSet) {
+                    for generator in generators {
+                        pylint::rules::iteration_over_set(self, &generator.iter);
+                    }
+                }
             }
             Expr::DictComp(ast::ExprDictComp {
                 key,
@@ -3526,6 +3534,11 @@ where
                         );
                     }
                 }
+                if self.enabled(Rule::IterationOverSet) {
+                    for generator in generators {
+                        pylint::rules::iteration_over_set(self, &generator.iter);
+                    }
+                }
             }
             Expr::GeneratorExp(ast::ExprGeneratorExp {
                 generators,
@@ -3544,6 +3557,11 @@ where
                         );
                     }
                 }
+                if self.enabled(Rule::IterationOverSet) {
+                    for generator in generators {
+                        pylint::rules::iteration_over_set(self, &generator.iter);
+                    }
+                }
             }
             Expr::BoolOp(ast::ExprBoolOp {
                 op,

crates/ruff/src/codes.rs

@@ -150,6 +150,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Pylint, "C0414") => (RuleGroup::Unspecified, Rule::UselessImportAlias),
         (Pylint, "C1901") => (RuleGroup::Unspecified, Rule::CompareToEmptyString),
         (Pylint, "C3002") => (RuleGroup::Unspecified, Rule::UnnecessaryDirectLambdaCall),
+        (Pylint, "C0208") => (RuleGroup::Unspecified, Rule::IterationOverSet),
         (Pylint, "E0100") => (RuleGroup::Unspecified, Rule::YieldInInit),
         (Pylint, "E0101") => (RuleGroup::Unspecified, Rule::ReturnInInit),
         (Pylint, "E0116") => (RuleGroup::Unspecified, Rule::ContinueInFinally),

crates/ruff/src/registry.rs

@@ -163,6 +163,7 @@ ruff_macros::register_rules!(
     rules::pylint::rules::DuplicateValue,
     rules::pylint::rules::DuplicateBases,
     rules::pylint::rules::NamedExprWithoutContext,
+    rules::pylint::rules::IterationOverSet,
     // flake8-async
     rules::flake8_async::rules::BlockingHttpCallInAsyncFunction,
     rules::flake8_async::rules::OpenSleepOrSubprocessInAsyncFunction,

crates/ruff/src/rules/pylint/mod.rs

@@ -64,6 +64,7 @@ mod tests {
     )]
     #[test_case(Rule::InvalidEnvvarDefault, Path::new("invalid_envvar_default.py"))]
     #[test_case(Rule::InvalidEnvvarValue, Path::new("invalid_envvar_value.py"))]
+    #[test_case(Rule::IterationOverSet, Path::new("iteration_over_set.py"))]
     #[test_case(Rule::LoggingTooFewArgs, Path::new("logging_too_few_args.py"))]
     #[test_case(Rule::LoggingTooManyArgs, Path::new("logging_too_many_args.py"))]
     #[test_case(Rule::MagicValueComparison, Path::new("magic_value_comparison.py"))]

crates/ruff/src/rules/pylint/rules/iteration_over_set.rs

@@ -0,0 +1,62 @@
+use rustpython_parser::ast::{Expr, ExprName, 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.
+///
+/// ## Why is this bad?
+/// Iterating over a `set` is less efficient than iterating over a sequence
+/// type, like `list` or `tuple`.
+///
+/// ## Example
+/// ```python
+/// for number in {1, 2, 3}:
+///     ...
+/// ```
+///
+/// Use instead:
+/// ```python
+/// for number in (1, 2, 3):
+///     ...
+/// ```
+///
+/// ## References
+/// - [Python documentation: `set`](https://docs.python.org/3/library/stdtypes.html#set)
+#[violation]
+pub struct IterationOverSet;
+
+impl Violation for IterationOverSet {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Use a sequence type instead of a `set` when iterating over values")
+    }
+}
+
+/// 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 {
+        checker
+            .diagnostics
+            .push(Diagnostic::new(IterationOverSet, expr.range()));
+    }
+}

crates/ruff/src/rules/pylint/rules/mod.rs

@@ -21,6 +21,7 @@ pub(crate) use invalid_string_characters::{
     invalid_string_characters, InvalidCharacterBackspace, InvalidCharacterEsc, InvalidCharacterNul,
     InvalidCharacterSub, InvalidCharacterZeroWidthSpace,
 };
+pub(crate) use iteration_over_set::{iteration_over_set, IterationOverSet};
 pub(crate) use load_before_global_declaration::{
     load_before_global_declaration, LoadBeforeGlobalDeclaration,
 };
@@ -73,6 +74,7 @@ mod invalid_all_object;
 mod invalid_envvar_default;
 mod invalid_envvar_value;
 mod invalid_string_characters;
+mod iteration_over_set;
 mod load_before_global_declaration;
 mod logging;
 mod magic_value_comparison;

crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLC0208_iteration_over_set.py.snap

@@ -0,0 +1,71 @@
+---
+source: crates/ruff/src/rules/pylint/mod.rs
+---
+iteration_over_set.py:3:13: PLC0208 Use a sequence type instead of a `set` when iterating over values
+  |
+3 | # Errors
+4 | 
+5 | for item in {"apples", "lemons", "water"}:  # flags in-line set literals
+  |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLC0208
+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
+   |
+12 |     print(number)
+13 | 
+14 | 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
+   |
+
+iteration_over_set.py:14: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
+   |                           ^^^^^^^^^ PLC0208
+17 | 
+18 | 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
+   |
+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
+   |                                    ^^^^^^^^^ PLC0208
+19 | 
+20 | 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
+   |
+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
+   |                           ^^^^^^^^^ PLC0208
+21 | 
+22 | # Non-errors
+   |
+
+