Check sequence type before triggering unnecessary-enumerate (FURB148) len suggestion

crates/ruff_linter/resources/test/fixtures/refurb/FURB148.py

@@ -1,5 +1,15 @@
 books = ["Dune", "Foundation", "Neuromancer"]
 
+books_and_authors = {
+    "Dune": "Frank Herbert",
+    "Foundation": "Isaac Asimov",
+    "Neuromancer": "William Gibson",
+}
+
+books_set = {"Dune", "Foundation", "Neuromancer"}
+
+books_tuple = ("Dune", "Foundation", "Neuromancer")
+
 # Errors
 for index, _ in enumerate(books):
     print(index)
@@ -55,6 +65,24 @@
 for(index), _ in enumerate(books):
     print(index)
 
+for index, _ in enumerate(books_and_authors):
+    print(index)
+
+for _, book in enumerate(books_and_authors):
+    print(book)
+
+for index, _ in enumerate(books_set):
+    print(index)
+
+for _, book in enumerate(books_set):
+    print(book)
+
+for index, _ in enumerate(books_tuple):
+    print(index)
+
+for _, book in enumerate(books_tuple):
+    print(book)
+
 # OK
 for index, book in enumerate(books):
     print(index, book)
@@ -64,3 +92,9 @@
 
 for book in books:
     print(book)
+
+# Generators don't support the len() function.
+# https://github.com/astral-sh/ruff/issues/7656
+a = (b for b in range(1, 100))
+for i, _ in enumerate(a):
+    print(i)

crates/ruff_linter/src/rules/refurb/rules/unnecessary_enumerate.rs

@@ -5,6 +5,8 @@ use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast as ast;
 use ruff_python_ast::{Arguments, Constant, Expr, Int};
 use ruff_python_codegen::Generator;
+use ruff_python_semantic::analyze::typing::{is_dict, is_list, is_set, is_tuple};
+use ruff_python_semantic::Binding;
 use ruff_text_size::{Ranged, TextRange};
 
 use crate::checkers::ast::Checker;
@@ -23,6 +25,16 @@ use crate::registry::AsRule;
 /// `range(len(...))` or the sequence itself, respectively, instead. This is
 /// more efficient and communicates the intent of the code more clearly.
 ///
+/// ## Known problems
+/// This rule is prone to false negatives due to type inference limitations;
+/// namely, it will only suggest a fix using the `len` builtin function if the
+/// sequence passed to `enumerate` is an instantiated as a list, set, dict, or
+/// tuple literal, or annotated as such with a type annotation.
+///
+/// The `len` builtin function is not defined for all object types (such as
+/// generators), and so refactoring to use `len` over `enumerate` is not always
+/// safe.
+///
 /// ## Example
 /// ```python
 /// for index, _ in enumerate(sequence):
@@ -143,6 +155,26 @@ pub(crate) fn unnecessary_enumerate(checker: &mut Checker, stmt_for: &ast::StmtF
             checker.diagnostics.push(diagnostic);
         }
         (false, true) => {
+            // Ensure the sequence object works with `len`. If it doesn't, the
+            // fix is unclear.
+            let scope = checker.semantic().current_scope();
+            let bindings: Vec<&Binding> = scope
+                .get_all(sequence)
+                .map(|binding_id| checker.semantic().binding(binding_id))
+                .collect();
+            let [binding] = bindings.as_slice() else {
+                return;
+            };
+            // This will lead to a lot of false negatives, but it is the best
+            // we can do with the current type inference.
+            if !is_list(binding, checker.semantic())
+                && !is_dict(binding, checker.semantic())
+                && !is_set(binding, checker.semantic())
+                && !is_tuple(binding, checker.semantic())
+            {
+                return;
+            }
+
             // The value is unused, so replace with `for index in range(len(sequence))`.
             let mut diagnostic = Diagnostic::new(
                 UnnecessaryEnumerate {