Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Pylint rule C0208 (use-sequence-for-iteration) as PLC0208 (iteration-over-set) #4706

Merged
merged 11 commits into from Jun 1, 2023
Merged
43 changes: 43 additions & 0 deletions crates/ruff/resources/test/fixtures/pylint/iterate_over_set.py
@@ -0,0 +1,43 @@
"""Test iterate-over-set rule.

Should trigger seven times.
"""

# True positives.

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

# True negatives.

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
18 changes: 18 additions & 0 deletions crates/ruff/src/checkers/ast/mod.rs
Expand Up @@ -1554,6 +1554,9 @@ where
if self.enabled(Rule::RedefinedLoopName) {
pylint::rules::redefined_loop_name(self, &Node::Stmt(stmt));
}
if self.enabled(Rule::IterateOverSet) {
pylint::rules::iterate_over_set(self, iter);
}
if matches!(stmt, Stmt::For(_)) {
if self.enabled(Rule::ReimplementedBuiltin) {
flake8_simplify::rules::convert_for_loop_to_any_all(
Expand Down Expand Up @@ -3502,6 +3505,11 @@ where
);
}
}
if self.enabled(Rule::IterateOverSet) {
for generator in generators {
pylint::rules::iterate_over_set(self, &generator.iter);
}
}
}
Expr::DictComp(ast::ExprDictComp {
key,
Expand All @@ -3526,6 +3534,11 @@ where
);
}
}
if self.enabled(Rule::IterateOverSet) {
for generator in generators {
pylint::rules::iterate_over_set(self, &generator.iter);
}
}
}
Expr::GeneratorExp(ast::ExprGeneratorExp {
generators,
Expand All @@ -3544,6 +3557,11 @@ where
);
}
}
if self.enabled(Rule::IterateOverSet) {
for generator in generators {
pylint::rules::iterate_over_set(self, &generator.iter);
}
}
}
Expr::BoolOp(ast::ExprBoolOp {
op,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Expand Up @@ -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::IterateOverSet),
(Pylint, "E0100") => (RuleGroup::Unspecified, Rule::YieldInInit),
(Pylint, "E0101") => (RuleGroup::Unspecified, Rule::ReturnInInit),
(Pylint, "E0116") => (RuleGroup::Unspecified, Rule::ContinueInFinally),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/registry.rs
Expand Up @@ -163,6 +163,7 @@ ruff_macros::register_rules!(
rules::pylint::rules::DuplicateValue,
rules::pylint::rules::DuplicateBases,
rules::pylint::rules::NamedExprWithoutContext,
rules::pylint::rules::IterateOverSet,
// flake8-async
rules::flake8_async::rules::BlockingHttpCallInAsyncFunction,
rules::flake8_async::rules::OpenSleepOrSubprocessInAsyncFunction,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/rules/pylint/mod.rs
Expand Up @@ -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::IterateOverSet, Path::new("iterate_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"))]
Expand Down
68 changes: 68 additions & 0 deletions crates/ruff/src/rules/pylint/rules/iterate_over_set.rs
@@ -0,0 +1,68 @@
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 code that iterates over an in-place `set`.
///
/// ## Why is this bad?
/// Iterating over a `set` is slower than iterating over a sequence type (such
/// as `list` or `tuple`) because `set` access is performed using a hash
/// function, whereas sequenced items are accessed using an index directly.
///
/// ## Example
/// ```python
/// for number in {1, 2, 3}:
/// ...
/// ```
///
/// Use instead:
/// ```python
/// for number in (1, 2, 3):
/// ...
/// ```
///
/// ## References
/// - [Python documentation](https://docs.python.org/3/library/stdtypes.html#set)
/// - [Python documentation](https://docs.python.org/3/library/stdtypes.html?highlight=list#sequence-types-list-tuple-range)
#[violation]
pub struct IterateOverSet;

impl Violation for IterateOverSet {
#[derive_message_formats]
fn message(&self) -> String {
format!("Use a sequence type instead of a set when iterating over values")
}
}

/// PLC0208
pub(crate) fn iterate_over_set(checker: &mut Checker, iter: &Expr) {
match iter {
// Check if set literal; e.g., `{1, 2, 3}`.
Expr::Set(_) => {
checker
.diagnostics
.push(Diagnostic::new(IterateOverSet, iter.range()));
}
// Check if call to set constructor; e.g., `set(1, 2, 3)`.
Expr::Call(call) => {
if let Expr::Name(ExprName { id, .. }) = &*call.func {
if id.as_str() == "set" {
checker
.diagnostics
.push(Diagnostic::new(IterateOverSet, iter.range()));
}
}
}
// Check if set comprehension; e.g., `{n for n in range(1, 4)}`.
Expr::SetComp(_) => {
checker
.diagnostics
.push(Diagnostic::new(IterateOverSet, iter.range()));
}
_ => {}
}
}
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/pylint/rules/mod.rs
Expand Up @@ -21,6 +21,7 @@ pub(crate) use invalid_string_characters::{
invalid_string_characters, InvalidCharacterBackspace, InvalidCharacterEsc, InvalidCharacterNul,
InvalidCharacterSub, InvalidCharacterZeroWidthSpace,
};
pub(crate) use iterate_over_set::{iterate_over_set, IterateOverSet};
pub(crate) use load_before_global_declaration::{
load_before_global_declaration, LoadBeforeGlobalDeclaration,
};
Expand Down Expand Up @@ -73,6 +74,7 @@ mod invalid_all_object;
mod invalid_envvar_default;
mod invalid_envvar_value;
mod invalid_string_characters;
mod iterate_over_set;
mod load_before_global_declaration;
mod logging;
mod magic_value_comparison;
Expand Down
@@ -0,0 +1,71 @@
---
source: crates/ruff/src/rules/pylint/mod.rs
---
iterate_over_set.py:8:13: PLC0208 Use a sequence type instead of a set when iterating over values
|
8 | # True positives.
9 |
10 | for item in {"apples", "lemons", "water"}: # flags in-line set literals
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLC0208
11 | print(f"I like {item}.")
|

iterate_over_set.py:11:13: PLC0208 Use a sequence type instead of a set when iterating over values
|
11 | print(f"I like {item}.")
12 |
13 | for item in set("apples", "lemons", "water"): # flags set() calls
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLC0208
14 | print(f"I like {item}.")
|

iterate_over_set.py:14:15: PLC0208 Use a sequence type instead of a set when iterating over values
|
14 | print(f"I like {item}.")
15 |
16 | for number in {i for i in range(10)}: # flags set comprehensions
| ^^^^^^^^^^^^^^^^^^^^^^ PLC0208
17 | print(number)
|

iterate_over_set.py:17:28: PLC0208 Use a sequence type instead of a set when iterating over values
|
17 | print(number)
18 |
19 | numbers_list = [i for i in {1, 2, 3}] # flags sets in list comprehensions
| ^^^^^^^^^ PLC0208
20 |
21 | numbers_set = {i for i in {1, 2, 3}} # flags sets in set comprehensions
|

iterate_over_set.py:19:27: PLC0208 Use a sequence type instead of a set when iterating over values
|
19 | numbers_list = [i for i in {1, 2, 3}] # flags sets in list comprehensions
20 |
21 | numbers_set = {i for i in {1, 2, 3}} # flags sets in set comprehensions
| ^^^^^^^^^ PLC0208
22 |
23 | numbers_dict = {str(i): i for i in {1, 2, 3}} # flags sets in dict comprehensions
|

iterate_over_set.py:21:36: PLC0208 Use a sequence type instead of a set when iterating over values
|
21 | numbers_set = {i for i in {1, 2, 3}} # flags sets in set comprehensions
22 |
23 | numbers_dict = {str(i): i for i in {1, 2, 3}} # flags sets in dict comprehensions
| ^^^^^^^^^ PLC0208
24 |
25 | numbers_gen = (i for i in {1, 2, 3}) # flags sets in generator expressions
|

iterate_over_set.py:23:27: PLC0208 Use a sequence type instead of a set when iterating over values
|
23 | numbers_dict = {str(i): i for i in {1, 2, 3}} # flags sets in dict comprehensions
24 |
25 | numbers_gen = (i for i in {1, 2, 3}) # flags sets in generator expressions
| ^^^^^^^^^ PLC0208
26 |
27 | # True negatives.
|


3 changes: 3 additions & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.