[perflint] Implement fix for manual-dict-comprehension (PERF403)

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

@@ -18,7 +18,9 @@ def foo():
     result = {}
     for idx, name in enumerate(fruit):
         if idx % 2:
-            result[idx] = name  # Ok (false negative: edge case where `else` is same as `if`)
+            result[idx] = (
+                name  # Ok (false negative: edge case where `else` is same as `if`)
+            )
         else:
             result[idx] = name
 
@@ -85,7 +87,67 @@ def foo():
 
 def foo():
     from builtins import dict as SneakyDict
+
     fruit = ["apple", "pear", "orange"]
     result = SneakyDict()
     for idx, name in enumerate(fruit):
         result[name] = idx  # PERF403
+
+
+def foo():
+    fruit = ["apple", "pear", "orange"]
+    result: dict[str, int] = {
+        # comment 1
+    }
+    for idx, name in enumerate(
+        fruit  # comment 2
+    ):
+        # comment 3
+        result[
+            name  # comment 4
+        ] = idx  # PERF403
+
+
+def foo():
+    fruit = ["apple", "pear", "orange"]
+    a = 1; result = {}; b = 2
+    for idx, name in enumerate(fruit):
+        result[name] = idx  # PERF403
+
+
+def foo():
+    fruit = ["apple", "pear", "orange"]
+    result = {"kiwi": 3}
+    for idx, name in enumerate(fruit):
+        result[name] = idx  # PERF403
+
+
+def foo():
+    fruit = ["apple", "pear", "orange"]
+    (_, result) = (None, {"kiwi": 3})
+    for idx, name in enumerate(fruit):
+        result[name] = idx  # PERF403
+
+
+def foo():
+    fruit = ["apple", "pear", "orange"]
+    result = {}
+    print(len(result))
+    for idx, name in enumerate(fruit):
+        result[name] = idx  # PERF403
+
+
+def foo():
+    fruit = ["apple", "pear", "orange"]
+    result = {}
+    for idx, name in enumerate(fruit):
+        if last_idx := idx % 3:
+            result[name] = idx  # PERF403
+
+
+def foo():
+    fruit = ["apple", "pear", "orange"]
+    indices = [0, 1, 2]
+    result = {}
+    for idx, name in indices, fruit:
+        result[name] = idx  # PERF403

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

@@ -35,6 +35,9 @@ pub(crate) fn deferred_for_loops(checker: &mut Checker) {
             if checker.enabled(Rule::DictIndexMissingItems) {
                 pylint::rules::dict_index_missing_items(checker, stmt_for);
             }
+            if checker.enabled(Rule::ManualDictComprehension) {
+                perflint::rules::manual_dict_comprehension(checker, stmt_for);
+            }
             if checker.enabled(Rule::ManualListComprehension) {
                 perflint::rules::manual_list_comprehension(checker, stmt_for);
             }

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

@@ -1334,6 +1334,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
                 Rule::UnnecessaryEnumerate,
                 Rule::UnusedLoopControlVariable,
                 Rule::YieldInForLoop,
+                Rule::ManualDictComprehension,
                 Rule::ManualListComprehension,
             ]) {
                 checker.analyze.for_loops.push(checker.semantic.snapshot());
@@ -1362,9 +1363,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
             if checker.enabled(Rule::ManualListCopy) {
                 perflint::rules::manual_list_copy(checker, for_stmt);
             }
-            if checker.enabled(Rule::ManualDictComprehension) {
-                perflint::rules::manual_dict_comprehension(checker, target, body);
-            }
+
             if checker.enabled(Rule::ModifiedIteratingSet) {
                 pylint::rules::modified_iterating_set(checker, for_stmt);
             }

crates/ruff_linter/src/rules/perflint/helpers.rs

@@ -0,0 +1,98 @@
+use ruff_python_trivia::{
+    BackwardsTokenizer, PythonWhitespace, SimpleToken, SimpleTokenKind, SimpleTokenizer,
+};
+use ruff_source_file::LineRanges;
+use ruff_text_size::{Ranged, TextRange};
+
+use crate::checkers::ast::Checker;
+
+pub(super) fn comment_strings_in_range<'a>(
+    checker: &'a Checker,
+    range: TextRange,
+    ranges_to_ignore: &[TextRange],
+) -> Vec<&'a str> {
+    checker
+        .comment_ranges()
+        .comments_in_range(range)
+        .iter()
+        // Ignore comments inside of the append or iterator, since these are preserved
+        .filter(|comment| {
+            !ranges_to_ignore
+                .iter()
+                .any(|to_ignore| to_ignore.contains_range(**comment))
+        })
+        .map(|range| checker.locator().slice(range).trim_whitespace_start())
+        .collect()
+}
+
+fn semicolon_before_and_after(
+    checker: &Checker,
+    statement: TextRange,
+) -> (Option<SimpleToken>, Option<SimpleToken>) {
+    // determine whether there's a semicolon either before or after the binding statement.
+    // Since it's a binding statement, we can just check whether there's a semicolon immediately
+    // after the whitespace in front of or behind it
+    let mut after_tokenizer =
+        SimpleTokenizer::starts_at(statement.end(), checker.locator().contents()).skip_trivia();
+
+    let after_semicolon = if after_tokenizer
+        .next()
+        .is_some_and(|token| token.kind() == SimpleTokenKind::Semi)
+    {
+        after_tokenizer.next()
+    } else {
+        None
+    };
+
+    let semicolon_before = BackwardsTokenizer::up_to(
+        statement.start(),
+        checker.locator().contents(),
+        checker.comment_ranges(),
+    )
+    .skip_trivia()
+    .next()
+    .filter(|token| token.kind() == SimpleTokenKind::Semi);
+
+    (semicolon_before, after_semicolon)
+}
+
+/// Finds the range necessary to delete a statement (including any semicolons around it).
+/// Returns the range and whether there were multiple statements on the line
+pub(super) fn statement_deletion_range(
+    checker: &Checker,
+    statement_range: TextRange,
+) -> (TextRange, bool) {
+    let locator = checker.locator();
+    // If the binding has multiple statements on its line, the fix would be substantially more complicated
+    let (semicolon_before, after_semicolon) = semicolon_before_and_after(checker, statement_range);
+
+    // If there are multiple binding statements in one line, we don't want to accidentally delete them
+    // Instead, we just delete the binding statement and leave any comments where they are
+
+    match (semicolon_before, after_semicolon) {
+        // ```python
+        // a = []
+        // ```
+        (None, None) => (locator.full_lines_range(statement_range), false),
+
+        // ```python
+        // a = 1; b = []
+        //      ^^^^^^^^
+        // a = 1; b = []; c = 3
+        //      ^^^^^^^^
+        // ```
+        (Some(semicolon_before), Some(_) | None) => (
+            TextRange::new(semicolon_before.start(), statement_range.end()),
+            true,
+        ),
+
+        // ```python
+        // a = []; b = 3
+        // ^^^^^^^
+        // ```
+        (None, Some(after_semicolon)) => (
+            TextRange::new(statement_range.start(), after_semicolon.start()),
+            true,
+        ),
+    }
+}

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

@@ -1,6 +1,6 @@
 //! Rules from [perflint](https://pypi.org/project/perflint/).
+mod helpers;
 pub(crate) mod rules;
-
 #[cfg(test)]
 mod tests {
     use std::path::Path;
@@ -31,7 +31,8 @@ mod tests {
         Ok(())
     }
 
-    // TODO: remove this test case when the fix for `perf401` is stabilized
+    // TODO: remove this test case when the fixes for `perf401` and `perf403` are stabilized
+    #[test_case(Rule::ManualDictComprehension, Path::new("PERF403.py"))]
     #[test_case(Rule::ManualListComprehension, Path::new("PERF401.py"))]
     fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!(