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

[refurb] Implement unnecessary-enumerate (FURB148) #7454

Merged
merged 14 commits into from
Sep 19, 2023
Merged
66 changes: 66 additions & 0 deletions crates/ruff/resources/test/fixtures/refurb/FURB148.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
books = ["Dune", "Foundation", "Neuromancer"]

# Errors
for index, _ in enumerate(books):
print(index)

for index, _ in enumerate(books, start=0):
print(index)

for index, _ in enumerate(books, 0):
print(index)

for index, _ in enumerate(books, start=1):
print(index)

for index, _ in enumerate(books, 1):
print(index)

for index, _ in enumerate(books, start=x):
print(book)

for index, _ in enumerate(books, x):
print(book)

for _, book in enumerate(books):
print(book)

for _, book in enumerate(books, start=0):
print(book)

for _, book in enumerate(books, 0):
print(book)

for _, book in enumerate(books, start=1):
print(book)

for _, book in enumerate(books, 1):
print(book)

for _, book in enumerate(books, start=x):
print(book)

for _, book in enumerate(books, x):
print(book)

for index, (_, _) in enumerate(books):
print(index)

for (_, _), book in enumerate(books):
print(book)

for(index, _)in enumerate(books):
print(index)

for(index), _ in enumerate(books):
print(index)

# OK
for index, book in enumerate(books):
print(index, book)

for index in range(len(books)):
print(index)

for book in books:
print(book)
5 changes: 4 additions & 1 deletion crates/ruff/src/checkers/ast/analyze/deferred_for_loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use ruff_python_ast::Stmt;

use crate::checkers::ast::Checker;
use crate::codes::Rule;
use crate::rules::{flake8_bugbear, perflint, pyupgrade};
use crate::rules::{flake8_bugbear, perflint, pyupgrade, refurb};

/// Run lint rules over all deferred for-loops in the [`SemanticModel`].
pub(crate) fn deferred_for_loops(checker: &mut Checker) {
Expand All @@ -24,6 +24,9 @@ pub(crate) fn deferred_for_loops(checker: &mut Checker) {
if checker.enabled(Rule::YieldInForLoop) {
pyupgrade::rules::yield_in_for_loop(checker, stmt_for);
}
if checker.enabled(Rule::UnnecessaryEnumerate) {
refurb::rules::unnecessary_enumerate(checker, stmt_for);
}
}
}
}
1 change: 1 addition & 0 deletions crates/ruff/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1190,6 +1190,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.any_enabled(&[
Rule::UnusedLoopControlVariable,
Rule::IncorrectDictIterator,
Rule::UnnecessaryEnumerate,
Rule::YieldInForLoop,
]) {
checker.deferred.for_loops.push(checker.semantic.snapshot());
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -921,6 +921,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Refurb, "132") => (RuleGroup::Nursery, rules::refurb::rules::CheckAndRemoveFromSet),
(Refurb, "140") => (RuleGroup::Preview, rules::refurb::rules::ReimplementedStarmap),
(Refurb, "145") => (RuleGroup::Preview, rules::refurb::rules::SliceCopy),
(Refurb, "148") => (RuleGroup::Preview, rules::refurb::rules::UnnecessaryEnumerate),

// flake8-logging
(Flake8Logging, "001") => (RuleGroup::Preview, rules::flake8_logging::rules::DirectLoggerInstantiation),
Expand Down
38 changes: 2 additions & 36 deletions crates/ruff/src/rules/perflint/rules/incorrect_dict_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast as ast;
use ruff_python_ast::{Arguments, Expr};
use ruff_python_semantic::SemanticModel;
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -83,8 +82,8 @@ pub(crate) fn incorrect_dict_iterator(checker: &mut Checker, stmt_for: &ast::Stm
}

match (
is_unused(key, checker.semantic()),
is_unused(value, checker.semantic()),
checker.semantic().is_unused(key),
checker.semantic().is_unused(value),
) {
(true, true) => {
// Both the key and the value are unused.
Expand Down Expand Up @@ -145,36 +144,3 @@ impl fmt::Display for DictSubset {
}
}
}

/// Returns `true` if the given expression is either an unused value or a tuple of unused values.
fn is_unused(expr: &Expr, semantic: &SemanticModel) -> bool {
match expr {
Expr::Tuple(ast::ExprTuple { elts, .. }) => {
elts.iter().all(|expr| is_unused(expr, semantic))
}
Expr::Name(ast::ExprName { id, .. }) => {
// Treat a variable as used if it has any usages, _or_ it's shadowed by another variable
// with usages.
//
// If we don't respect shadowing, we'll incorrectly flag `bar` as unused in:
// ```python
// from random import random
//
// for bar in range(10):
// if random() > 0.5:
// break
// else:
// bar = 1
//
// print(bar)
// ```
let scope = semantic.current_scope();
scope
.get_all(id)
.map(|binding_id| semantic.binding(binding_id))
.filter(|binding| binding.start() >= expr.start())
.all(|binding| !binding.is_used())
}
_ => false,
}
}
1 change: 1 addition & 0 deletions crates/ruff/src/rules/refurb/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ mod tests {
#[test_case(Rule::CheckAndRemoveFromSet, Path::new("FURB132.py"))]
#[test_case(Rule::ReimplementedStarmap, Path::new("FURB140.py"))]
#[test_case(Rule::SliceCopy, Path::new("FURB145.py"))]
#[test_case(Rule::UnnecessaryEnumerate, Path::new("FURB148.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/refurb/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ pub(crate) use delete_full_slice::*;
pub(crate) use reimplemented_starmap::*;
pub(crate) use repeated_append::*;
pub(crate) use slice_copy::*;
pub(crate) use unnecessary_enumerate::*;

mod check_and_remove_from_set;
mod delete_full_slice;
mod reimplemented_starmap;
mod repeated_append;
mod slice_copy;
mod unnecessary_enumerate;