diff --git a/crates/ruff_linter/resources/test/fixtures/perflint/PERF101.py b/crates/ruff_linter/resources/test/fixtures/perflint/PERF101.py index b9ef94dbc3..706eae62f4 100644 --- a/crates/ruff_linter/resources/test/fixtures/perflint/PERF101.py +++ b/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) diff --git a/crates/ruff_linter/src/rules/perflint/rules/unnecessary_list_cast.rs b/crates/ruff_linter/src/rules/perflint/rules/unnecessary_list_cast.rs index 08d3ccacce..8949c0997d 100644 --- a/crates/ruff_linter/src/rules/perflint/rules/unnecessary_list_cast.rs +++ b/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 { - Fix::safe_edits( - Edit::deletion(list_range.start(), iterable_range.start()), - [Edit::deletion(iterable_range.end(), list_range.end())], +fn remove_cast(checker: &Checker, list_range: TextRange, iterable_range: TextRange) -> Fix { + let content = edits::pad( + checker.locator().slice(iterable_range).to_string(), + 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 + }, ) } diff --git a/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF101_PERF101.py.snap b/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF101_PERF101.py.snap index fd545e1364..25fc58e80c 100644 --- a/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF101_PERF101.py.snap +++ b/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)