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

## Summary

Implement
[`no-ignored-enumerate-items`](https://github.com/dosisod/refurb/blob/master/refurb/checks/builtin/no_ignored_enumerate.py)
as `unnecessary-enumerate` (`FURB148`).

The auto-fix considers if a `start` argument is passed to the
`enumerate()` function. If only the index is used, then the suggested
fix is to pass the `start` value to the `range()` function. So,

```python
for i, _ in enumerate(xs, 1):
    ...
```

becomes

```python
for i in range(1, len(xs)):
    ...
```

If the index is ignored and only the value is ignored, and if a start
value greater than zero is passed to `enumerate()`, the rule doesn't
produce a suggestion. I couldn't find a unanimously accepted best way to
iterate over a collection whilst skipping the first n elements. The rule
still triggers, however.

Related to #1348.

## Test Plan

`cargo test`

---------

Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
This commit is contained in:
Tom Kuson 2023-09-19 21:19:28 +01:00 committed by GitHub
parent 4c4eceee36
commit 511cc25fc4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 695 additions and 40 deletions

View file

@ -1107,9 +1107,38 @@ impl<'a> SemanticModel<'a> {
.all(|(left, right)| left == right)
}
/// Returns `true` if the given [`BindingId`] is used.
pub fn is_used(&self, binding_id: BindingId) -> bool {
self.bindings[binding_id].is_used()
/// Returns `true` if the given expression is an unused variable, or consists solely of
/// references to other unused variables. This method is conservative in that it considers a
/// variable to be "used" if it's shadowed by another variable with usages.
pub fn is_unused(&self, expr: &Expr) -> bool {
match expr {
Expr::Tuple(ast::ExprTuple { elts, .. }) => {
elts.iter().all(|expr| self.is_unused(expr))
}
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)
// ```
self.current_scope()
.get_all(id)
.map(|binding_id| self.binding(binding_id))
.filter(|binding| binding.start() >= expr.start())
.all(|binding| !binding.is_used())
}
_ => false,
}
}
/// Add a reference to the given [`BindingId`] in the local scope.