[flake8-simplify] Implement enumerate-for-loop (SIM113) (#7777)

Implements SIM113 from #998

Added tests
Limitations 
   - No fix yet
   - Only flag cases where index variable immediately precede `for` loop

@charliermarsh please review and let me know any improvements

---------

Co-authored-by: Charlie Marsh <[email protected]>

crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM113.py

@@ -0,0 +1,168 @@
+def func():
+    # SIM113
+    idx = 0
+    for x in range(5):
+        g(x, idx)
+        idx += 1
+        h(x)
+
+
+def func():
+    # SIM113
+    sum = 0
+    idx = 0
+    for x in range(5):
+        if g(x):
+            break
+        idx += 1
+        sum += h(x, idx)
+
+
+def func():
+    # SIM113
+    idx = 0
+    for x, y in zip(range(5), range(5)):
+        g(x)
+        h(x, y)
+        idx += 1
+
+
+def func():
+    # SIM113
+    idx = 0
+    sum = 0
+    for x in range(5):
+        sum += h(x, idx)
+        idx += 1
+
+
+def func():
+    # SIM113
+    sum, idx = 0, 0
+    for x in range(5):
+        g(x, idx)
+        idx += 1
+        h(x)
+
+
+def func():
+    # OK (index doesn't start at 0
+    idx = 10
+    for x in range(5):
+        g(x, idx)
+        idx += 1
+        h(x)
+
+
+def func():
+    # OK (incremented by more than 1)
+    idx = 0
+    for x in range(5):
+        g(x, idx)
+        idx += 2
+        h(x)
+
+
+def func():
+    # OK (incremented in `if`)
+    idx = 0
+    for x in range(5):
+        if g(x):
+            idx += 1
+        h(x)
+
+
+def func():
+    # OK (`continue` in match-case)
+    idx = 0
+    for x in range(5):
+        match g(x):
+            case 1:
+                h(x)
+            case 2:
+                continue
+            case _:
+                h(idx)
+        idx += 1
+
+
+def func():
+    # OK (`continue` inside `with` clause)
+    idx = 0
+    for x in range(5):
+        with context as c:
+            if g(x):
+                continue
+            h(x, idx, c)
+        idx += 1
+
+
+def func():
+    # OK (incremented in `try` block)
+    idx = 0
+    for x in range(5):
+        try:
+            g(x, idx)
+        except:
+            h(x)
+            continue
+        idx += 1
+
+
+def func(idx: 0):
+    # OK (index is an argument)
+    for x in range(5):
+        g(x, idx)
+        idx += 1
+
+
+def func(x: int):
+    # OK (index _may_ be non-zero)
+    if x > 0:
+        idx = 1
+    else:
+        idx = 0
+    for x in range(5):
+        g(x, idx)
+        idx += 1
+        h(x)
+
+
+def func():
+    # OK
+    idx = 0
+    for x in range(5):
+        g(x, idx)
+        idx += 1
+        idx += 1
+        h(x)
+
+
+def func():
+    # OK
+    idx = 0
+    for x in range(5):
+        g(x, idx)
+        idx += 1
+        if idx > 10:
+            idx *= 2
+        h(x)
+
+
+def func():
+    # OK (index used after loop)
+    idx = 0
+    for x in range(5):
+        g(x, idx)
+        idx += 1
+        h(x)
+    print(idx)
+
+
+def func():
+    # OK (index within nested loop)
+    idx = 0
+    for x in range(5):
+        for y in range(5):
+            g(x, idx)
+            idx += 1

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

@@ -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, refurb};
+use crate::rules::{flake8_bugbear, flake8_simplify, perflint, pyupgrade, refurb};
 
 /// Run lint rules over all deferred for-loops in the [`SemanticModel`].
 pub(crate) fn deferred_for_loops(checker: &mut Checker) {
@@ -27,6 +27,9 @@ pub(crate) fn deferred_for_loops(checker: &mut Checker) {
             if checker.enabled(Rule::UnnecessaryEnumerate) {
                 refurb::rules::unnecessary_enumerate(checker, stmt_for);
             }
+            if checker.enabled(Rule::EnumerateForLoop) {
+                flake8_simplify::rules::enumerate_for_loop(checker, stmt_for);
+            }
         }
     }
 }

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

@@ -1258,9 +1258,10 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
             },
         ) => {
             if checker.any_enabled(&[
-                Rule::UnusedLoopControlVariable,
+                Rule::EnumerateForLoop,
                 Rule::IncorrectDictIterator,
                 Rule::UnnecessaryEnumerate,
+                Rule::UnusedLoopControlVariable,
                 Rule::YieldInForLoop,
             ]) {
                 checker.deferred.for_loops.push(checker.semantic.snapshot());

crates/ruff_linter/src/codes.rs

@@ -454,6 +454,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Flake8Simplify, "109") => (RuleGroup::Stable, rules::flake8_simplify::rules::CompareWithTuple),
         (Flake8Simplify, "110") => (RuleGroup::Stable, rules::flake8_simplify::rules::ReimplementedBuiltin),
         (Flake8Simplify, "112") => (RuleGroup::Stable, rules::flake8_simplify::rules::UncapitalizedEnvironmentVariables),
+        (Flake8Simplify, "113") => (RuleGroup::Preview, rules::flake8_simplify::rules::EnumerateForLoop),
         (Flake8Simplify, "114") => (RuleGroup::Stable, rules::flake8_simplify::rules::IfWithSameArms),
         (Flake8Simplify, "115") => (RuleGroup::Stable, rules::flake8_simplify::rules::OpenFileWithContextHandler),
         (Flake8Simplify, "116") => (RuleGroup::Stable, rules::flake8_simplify::rules::IfElseBlockInsteadOfDictLookup),

crates/ruff_linter/src/rules/flake8_simplify/mod.rs

@@ -27,6 +27,7 @@ mod tests {
     #[test_case(Rule::ReimplementedBuiltin, Path::new("SIM110.py"))]
     #[test_case(Rule::ReimplementedBuiltin, Path::new("SIM111.py"))]
     #[test_case(Rule::UncapitalizedEnvironmentVariables, Path::new("SIM112.py"))]
+    #[test_case(Rule::EnumerateForLoop, Path::new("SIM113.py"))]
     #[test_case(Rule::OpenFileWithContextHandler, Path::new("SIM115.py"))]
     #[test_case(Rule::MultipleWithStatements, Path::new("SIM117.py"))]
     #[test_case(Rule::InDictKeys, Path::new("SIM118.py"))]

crates/ruff_linter/src/rules/flake8_simplify/rules/ast_bool_op.rs

@@ -287,7 +287,7 @@ impl AlwaysFixableViolation for ExprAndFalse {
 }
 
 /// Return `true` if two `Expr` instances are equivalent names.
-fn is_same_expr<'a>(a: &'a Expr, b: &'a Expr) -> Option<&'a str> {
+pub(crate) fn is_same_expr<'a>(a: &'a Expr, b: &'a Expr) -> Option<&'a str> {
     if let (Expr::Name(ast::ExprName { id: a, .. }), Expr::Name(ast::ExprName { id: b, .. })) =
         (&a, &b)
     {