Skip to content

Commit 6089e2e

Browse files
committed
Remove unused loops in deferred checks
Summary -- While reviewing #23473, I noticed that the `while` loops in our deferred checks for `for` loops and lambdas would only be used if we pushed additional deferred scopes to the `Checker` while running lint rule code. This should actually be impossible given that the rules only receive a `&Checker` (as shown by the immutable re-borrow), and none of our tests fail when removing the loops. I guess it doesn't hurt to have the loops either since they only run once, but it did serve to confuse me today. I assume, but didn't fully confirm, that this used to be interleaved with the visiting phase, at which point additional deferred scopes could have still been added in the middle of this. Test Plan -- Existing tests
1 parent cb2035a commit 6089e2e

2 files changed

Lines changed: 50 additions & 50 deletions

File tree

crates/ruff_linter/src/checkers/ast/analyze/deferred_for_loops.rs

Lines changed: 34 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -6,41 +6,41 @@ use crate::rules::{flake8_bugbear, flake8_simplify, perflint, pylint, pyupgrade,
66

77
/// Run lint rules over all deferred for-loops in the [`SemanticModel`].
88
pub(crate) fn deferred_for_loops(checker: &mut Checker) {
9-
while !checker.analyze.for_loops.is_empty() {
10-
let for_loops = std::mem::take(&mut checker.analyze.for_loops);
11-
for snapshot in for_loops {
12-
checker.semantic.restore(snapshot);
9+
let for_loops = std::mem::take(&mut checker.analyze.for_loops);
10+
for snapshot in for_loops {
11+
checker.semantic.restore(snapshot);
1312

14-
let Stmt::For(stmt_for) = checker.semantic.current_statement() else {
15-
unreachable!("Expected Stmt::For");
16-
};
17-
if checker.is_rule_enabled(Rule::UnusedLoopControlVariable) {
18-
flake8_bugbear::rules::unused_loop_control_variable(checker, stmt_for);
19-
}
20-
if checker.is_rule_enabled(Rule::IncorrectDictIterator) {
21-
perflint::rules::incorrect_dict_iterator(checker, stmt_for);
22-
}
23-
if checker.is_rule_enabled(Rule::YieldInForLoop) {
24-
pyupgrade::rules::yield_in_for_loop(checker, stmt_for);
25-
}
26-
if checker.is_rule_enabled(Rule::UnnecessaryEnumerate) {
27-
refurb::rules::unnecessary_enumerate(checker, stmt_for);
28-
}
29-
if checker.is_rule_enabled(Rule::EnumerateForLoop) {
30-
flake8_simplify::rules::enumerate_for_loop(checker, stmt_for);
31-
}
32-
if checker.is_rule_enabled(Rule::LoopIteratorMutation) {
33-
flake8_bugbear::rules::loop_iterator_mutation(checker, stmt_for);
34-
}
35-
if checker.is_rule_enabled(Rule::DictIndexMissingItems) {
36-
pylint::rules::dict_index_missing_items(checker, stmt_for);
37-
}
38-
if checker.is_rule_enabled(Rule::ManualDictComprehension) {
39-
perflint::rules::manual_dict_comprehension(checker, stmt_for);
40-
}
41-
if checker.is_rule_enabled(Rule::ManualListComprehension) {
42-
perflint::rules::manual_list_comprehension(checker, stmt_for);
43-
}
13+
let checker = &*checker;
14+
15+
let Stmt::For(stmt_for) = checker.semantic.current_statement() else {
16+
unreachable!("Expected Stmt::For");
17+
};
18+
if checker.is_rule_enabled(Rule::UnusedLoopControlVariable) {
19+
flake8_bugbear::rules::unused_loop_control_variable(checker, stmt_for);
20+
}
21+
if checker.is_rule_enabled(Rule::IncorrectDictIterator) {
22+
perflint::rules::incorrect_dict_iterator(checker, stmt_for);
23+
}
24+
if checker.is_rule_enabled(Rule::YieldInForLoop) {
25+
pyupgrade::rules::yield_in_for_loop(checker, stmt_for);
26+
}
27+
if checker.is_rule_enabled(Rule::UnnecessaryEnumerate) {
28+
refurb::rules::unnecessary_enumerate(checker, stmt_for);
29+
}
30+
if checker.is_rule_enabled(Rule::EnumerateForLoop) {
31+
flake8_simplify::rules::enumerate_for_loop(checker, stmt_for);
32+
}
33+
if checker.is_rule_enabled(Rule::LoopIteratorMutation) {
34+
flake8_bugbear::rules::loop_iterator_mutation(checker, stmt_for);
35+
}
36+
if checker.is_rule_enabled(Rule::DictIndexMissingItems) {
37+
pylint::rules::dict_index_missing_items(checker, stmt_for);
38+
}
39+
if checker.is_rule_enabled(Rule::ManualDictComprehension) {
40+
perflint::rules::manual_dict_comprehension(checker, stmt_for);
41+
}
42+
if checker.is_rule_enabled(Rule::ManualListComprehension) {
43+
perflint::rules::manual_list_comprehension(checker, stmt_for);
4444
}
4545
}
4646
}

crates/ruff_linter/src/checkers/ast/analyze/deferred_lambdas.rs

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,24 +6,24 @@ use crate::rules::{flake8_builtins, flake8_pie, pylint};
66

77
/// Run lint rules over all deferred lambdas in the [`SemanticModel`].
88
pub(crate) fn deferred_lambdas(checker: &mut Checker) {
9-
while !checker.analyze.lambdas.is_empty() {
10-
let lambdas = std::mem::take(&mut checker.analyze.lambdas);
11-
for snapshot in lambdas {
12-
checker.semantic.restore(snapshot);
9+
let lambdas = std::mem::take(&mut checker.analyze.lambdas);
10+
for snapshot in lambdas {
11+
checker.semantic.restore(snapshot);
1312

14-
let Some(Expr::Lambda(lambda)) = checker.semantic.current_expression() else {
15-
unreachable!("Expected Expr::Lambda");
16-
};
13+
let checker = &*checker;
1714

18-
if checker.is_rule_enabled(Rule::UnnecessaryLambda) {
19-
pylint::rules::unnecessary_lambda(checker, lambda);
20-
}
21-
if checker.is_rule_enabled(Rule::ReimplementedContainerBuiltin) {
22-
flake8_pie::rules::reimplemented_container_builtin(checker, lambda);
23-
}
24-
if checker.is_rule_enabled(Rule::BuiltinLambdaArgumentShadowing) {
25-
flake8_builtins::rules::builtin_lambda_argument_shadowing(checker, lambda);
26-
}
15+
let Some(Expr::Lambda(lambda)) = checker.semantic.current_expression() else {
16+
unreachable!("Expected Expr::Lambda");
17+
};
18+
19+
if checker.is_rule_enabled(Rule::UnnecessaryLambda) {
20+
pylint::rules::unnecessary_lambda(checker, lambda);
21+
}
22+
if checker.is_rule_enabled(Rule::ReimplementedContainerBuiltin) {
23+
flake8_pie::rules::reimplemented_container_builtin(checker, lambda);
24+
}
25+
if checker.is_rule_enabled(Rule::BuiltinLambdaArgumentShadowing) {
26+
flake8_builtins::rules::builtin_lambda_argument_shadowing(checker, lambda);
2727
}
2828
}
2929
}

0 commit comments

Comments
 (0)