[perflint] Fix PERF101 autofix creating a syntax error and mark autofix as unsafe if there are comments in the list call expr (#18803)

Co-authored-by: Micha Reiser <micha@reiser.io>

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

@@ -85,3 +85,19 @@ import builtins
 
 for i in builtins.list(nested_tuple):  # PERF101
     pass
+
+# https://github.com/astral-sh/ruff/issues/18783
+items = (1, 2, 3)
+for i in(list)(items):
+    print(i)
+
+# https://github.com/astral-sh/ruff/issues/18784
+items = (1, 2, 3)
+for i in (  # 1
+    list  # 2
+    # 3
+)(  # 4
+    items  # 5
+    # 6
+):
+    print(i)

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

@@ -1,3 +1,4 @@
+use ruff_diagnostics::Applicability;
 use ruff_macros::{ViolationMetadata, derive_message_formats};
 use ruff_python_ast::statement_visitor::{StatementVisitor, walk_stmt};
 use ruff_python_ast::{self as ast, Arguments, Expr, Stmt};
@@ -5,6 +6,7 @@ use ruff_python_semantic::analyze::typing::find_assigned_value;
 use ruff_text_size::TextRange;
 
 use crate::checkers::ast::Checker;
+use crate::fix::edits;
 use crate::{AlwaysFixableViolation, Edit, Fix};
 
 /// ## What it does
@@ -35,6 +37,19 @@ use crate::{AlwaysFixableViolation, Edit, Fix};
 /// for i in items:
 ///     print(i)
 /// ```
+///
+/// ## Fix safety
+/// This rule's fix is marked as unsafe if there's comments in the
+/// `list()` call, as comments may be removed.
+///
+/// For example, the fix would be marked as unsafe in the following case:
+/// ```python
+/// items = (1, 2, 3)
+/// for i in list(  # comment
+///     items
+/// ):
+///     print(i)
+/// ```
 #[derive(ViolationMetadata)]
 pub(crate) struct UnnecessaryListCast;
 
@@ -89,7 +104,7 @@ pub(crate) fn unnecessary_list_cast(checker: &Checker, iter: &Expr, body: &[Stmt
             ..
         }) => {
             let mut diagnostic = checker.report_diagnostic(UnnecessaryListCast, *list_range);
-            diagnostic.set_fix(remove_cast(*list_range, *iterable_range));
+            diagnostic.set_fix(remove_cast(checker, *list_range, *iterable_range));
         }
         Expr::Name(ast::ExprName {
             id,
@@ -116,7 +131,7 @@ pub(crate) fn unnecessary_list_cast(checker: &Checker, iter: &Expr, body: &[Stmt
                 }
 
                 let mut diagnostic = checker.report_diagnostic(UnnecessaryListCast, *list_range);
-                diagnostic.set_fix(remove_cast(*list_range, *iterable_range));
+                diagnostic.set_fix(remove_cast(checker, *list_range, *iterable_range));
             }
         }
         _ => {}
@@ -124,10 +139,19 @@ pub(crate) fn unnecessary_list_cast(checker: &Checker, iter: &Expr, body: &[Stmt
 }
 
 /// Generate a [`Fix`] to remove a `list` cast from an expression.
-fn remove_cast(list_range: TextRange, iterable_range: TextRange) -> Fix {
+fn remove_cast(checker: &Checker, list_range: TextRange, iterable_range: TextRange) -> Fix {
-    Fix::safe_edits(
+    let content = edits::pad(
-        Edit::deletion(list_range.start(), iterable_range.start()),
+        checker.locator().slice(iterable_range).to_string(),
-        [Edit::deletion(iterable_range.end(), list_range.end())],
+        list_range,
+        checker.locator(),
+    );
+    Fix::applicable_edit(
+        Edit::range_replacement(content, list_range),
+        if checker.comment_ranges().intersects(list_range) {
+            Applicability::Unsafe
+        } else {
+            Applicability::Safe
+        },
     )
 }
 

crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF101_PERF101.py.snap

@@ -168,7 +168,7 @@ PERF101.py:34:10: PERF101 [*] Do not cast an iterable to `list` before iterating
    |
    = help: Remove `list()` cast
 
-ℹ Safe fix
+ℹ Unsafe fix
 31 31 | ):
 32 32 |     pass
 33 33 | 
@@ -238,3 +238,56 @@ PERF101.py:86:10: PERF101 [*] Do not cast an iterable to `list` before iterating
 86    |-for i in builtins.list(nested_tuple):  # PERF101
    86 |+for i in nested_tuple:  # PERF101
 87 87 |     pass
+88 88 | 
+89 89 | # https://github.com/astral-sh/ruff/issues/18783
+
+PERF101.py:91:9: PERF101 [*] Do not cast an iterable to `list` before iterating over it
+   |
+89 | # https://github.com/astral-sh/ruff/issues/18783
+90 | items = (1, 2, 3)
+91 | for i in(list)(items):
+   |         ^^^^^^^^^^^^^ PERF101
+92 |     print(i)
+   |
+   = help: Remove `list()` cast
+
+ℹ Safe fix
+88 88 | 
+89 89 | # https://github.com/astral-sh/ruff/issues/18783
+90 90 | items = (1, 2, 3)
+91    |-for i in(list)(items):
+   91 |+for i in items:
+92 92 |     print(i)
+93 93 | 
+94 94 | # https://github.com/astral-sh/ruff/issues/18784
+
+PERF101.py:96:10: PERF101 [*] Do not cast an iterable to `list` before iterating over it
+    |
+ 94 |   # https://github.com/astral-sh/ruff/issues/18784
+ 95 |   items = (1, 2, 3)
+ 96 |   for i in (  # 1
+    |  __________^
+ 97 | |     list  # 2
+ 98 | |     # 3
+ 99 | | )(  # 4
+100 | |     items  # 5
+101 | |     # 6
+102 | | ):
+    | |_^ PERF101
+103 |       print(i)
+    |
+    = help: Remove `list()` cast
+
+ℹ Unsafe fix
+93  93  | 
+94  94  | # https://github.com/astral-sh/ruff/issues/18784
+95  95  | items = (1, 2, 3)
+96      |-for i in (  # 1
+97      |-    list  # 2
+98      |-    # 3
+99      |-)(  # 4
+100     |-    items  # 5
+101     |-    # 6
+102     |-):
+    96  |+for i in items:
+103 97  |     print(i)