Tweak rule; tweak tests

crates/ruff/resources/test/fixtures/perflint/PERF403.py

@@ -14,13 +14,11 @@ def foo():
 
 
 def foo():
-    result = {}
+    result = []
     fruit = ["apple", "pear", "orange"]
     for idx, name in enumerate(fruit):
         if idx % 2:
-            result[idx] = name  # Ok because if/elif/else not replaceable by dict comprehension
-        elif idx % 3:
-            result[idx] = name
+            result[idx] = name  # OK because result is not a dictionary
         else:
             result[idx] = name
 
@@ -30,27 +28,28 @@ def foo():
     fruit = ["apple", "pear", "orange"]
     for idx, name in enumerate(fruit):
         if idx % 2:
-            result[idx] = name  # PERF403
+            result[idx] = name  # OK (if/elif/else isn't replaceable)
+        elif idx % 3:
+            result[idx] = name
         else:
             result[idx] = name
 
 
 def foo():
-    result = {1: "banana"}
+    result = {}
     fruit = ["apple", "pear", "orange"]
     for idx, name in enumerate(fruit):
         if idx % 2:
-            result[idx] = name  # Ok because dict was not empty before loop
+            result[idx] = name  # OK (false negative)
         else:
             result[idx] = name
 
 
 def foo():
-    result = []
+    result = {1: "banana"}
     fruit = ["apple", "pear", "orange"]
     for idx, name in enumerate(fruit):
         if idx % 2:
-            result[idx] = name  # Ok because result is not a dictionary
+            result[idx] = name  # PERF403 (false positive)
         else:
             result[idx] = name
-

crates/ruff/src/rules/perflint/rules/manual_dict_comprehension.rs

@@ -47,91 +47,98 @@ impl Violation for ManualDictComprehension {
 
 /// PERF403
 pub(crate) fn manual_dict_comprehension(checker: &mut Checker, target: &Expr, body: &[Stmt]) {
-    let [stmt] = body else {
-        return;
-    };
-
     // For a dictionary comprehension to be appropriate, the loop needs both an index
     // and a value, so the target must be a tuple.
     let Expr::Tuple(ast::ExprTuple { elts, .. }) = target else {
         return;
     };
 
-
-    let names = elts
-        .iter()
-        .filter_map(|elt| {
-            if let Expr::Name(ast::ExprName { id, .. }) = elt {
-                Some(id.as_str())
-            } else {
-                None
-            }
-        })
-        .collect::<Vec<_>>();
-
-    if names.is_empty() {
+    let [Expr::Name(ast::ExprName { id: target_key, .. }), Expr::Name(ast::ExprName {
+        id: target_value, ..
+    })] = elts.as_slice()
+    else {
         return;
-    }
+    };
 
-    if let Stmt::If(ast::StmtIf { body: if_body, elif_else_clauses, .. }) = stmt {
-        // Key-value assignment within an if-statement (e.g., `if condition: result[key] = value`).
-        // Exclude if/elif/else statements as they cannot easily be replaced by a dict comprehension
-        if elif_else_clauses.len() > 1 {
-            return;
-        }
-        for stmt in if_body {
-            check_for_slow_dict_creation(checker, &names, stmt);
+    let stmt = match body {
+        // ```python
+        // for idx, name in enumerate(names):
+        //     if idx % 2 == 0:
+        //         result[name] = idx
+        // ```
+        [Stmt::If(ast::StmtIf {
+            body,
+            elif_else_clauses,
+            ..
+        })] => {
+            if !elif_else_clauses.is_empty() {
+                return;
+            }
+            let [stmt] = body.as_slice() else {
+                return;
+            };
+            stmt
         }
-    } else {
-        // Direct key-value assignment (e.g., `result[key] = value`).
-        check_for_slow_dict_creation(checker, &names, stmt);
-    }
-}
+        // ```python
+        // for idx, name in enumerate(names):
+        //     result[name] = idx
+        // ```
+        [stmt] => stmt,
+        _ => return,
+    };
 
-fn check_for_slow_dict_creation(checker: &mut Checker, names: &[&str], stmt: &Stmt) {
     let Stmt::Assign(ast::StmtAssign {
         targets,
         value,
         range,
-        ..
     }) = stmt
     else {
         return;
     };
 
-    let [Expr::Subscript(ast::ExprSubscript { value: subscript_value, slice, .. })] = targets.as_slice() else {
+    let [Expr::Subscript(ast::ExprSubscript {
+        value: subscript_value,
+        slice,
+        ..
+    })] = targets.as_slice()
+    else {
         return;
     };
 
-    // Exclude non dictionary values from the check
-    let Expr::Name(ast::ExprName { id: subscript_name, .. }) = subscript_value.as_ref() else {
+    let Expr::Name(ast::ExprName { id: key, .. }) = slice.as_ref() else {
         return;
     };
-    let scope = checker.semantic().current_scope();
-    let bindings: Vec<&Binding> = scope
-        .get_all(subscript_name)
-        .map(|binding_id| checker.semantic().binding(binding_id))
-        .collect();
 
-    let [binding] = bindings.as_slice() else {
+    let Expr::Name(ast::ExprName { id: value, .. }) = value.as_ref() else {
         return;
     };
 
-    if !is_dict(binding, checker.semantic()) {
-        return
+    if key != target_key || value != target_value {
+        return;
     }
 
-    let Expr::Name(ast::ExprName { id: key, .. }) = slice.as_ref() else {
+    // Exclude non-dictionary value.
+    let Expr::Name(ast::ExprName {
+        id: subscript_name, ..
+    }) = subscript_value.as_ref()
+    else {
         return;
     };
+    let scope = checker.semantic().current_scope();
+    let bindings: Vec<&Binding> = scope
+        .get_all(subscript_name)
+        .map(|binding_id| checker.semantic().binding(binding_id))
+        .collect();
 
-    let Expr::Name(ast::ExprName { id: value, .. }) = value.as_ref() else {
+    let [binding] = bindings.as_slice() else {
         return;
     };
 
-    if names.contains(&value.as_str()) && names.contains(&key.as_str()) {
-        checker
-            .diagnostics
-            .push(Diagnostic::new(ManualDictComprehension, *range));
+    if !is_dict(binding, checker.semantic()) {
+        return;
     }
+
+    checker
+        .diagnostics
+        .push(Diagnostic::new(ManualDictComprehension, *range));
 }

crates/ruff/src/rules/perflint/snapshots/ruff__rules__perflint__tests__PERF403_PERF403.py.snap

@@ -17,14 +17,4 @@ PERF403.py:13:13: PERF403 Use a dictionary comprehension instead of a for-loop
    |             ^^^^^^^^^^^^^^^^^^ PERF403
    |
 
-PERF403.py:33:13: PERF403 Use a dictionary comprehension instead of a for-loop
-   |
-31 |     for idx, name in enumerate(fruit):
-32 |         if idx % 2:
-33 |             result[idx] = name  # PERF403
-   |             ^^^^^^^^^^^^^^^^^^ PERF403
-34 |         else:
-35 |             result[idx] = name
-   |
-