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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 38 additions & 0 deletions 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
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::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(
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
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::IterationOverSet),
(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::IterationOverSet,
// 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::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"))]
Expand Down
62 changes: 62 additions & 0 deletions 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()));
}
}
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 iteration_over_set::{iteration_over_set, IterationOverSet};
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 iteration_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
---
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
|


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.