diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE804.py b/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE804.py index a8233d2917f92..b6229ed24ca1f 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE804.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE804.py @@ -28,3 +28,14 @@ # Some values need to be parenthesized. abc(foo=1, **{'bar': (bar := 1)}) # PIE804 abc(foo=1, **{'bar': (yield 1)}) # PIE804 + +# https://github.com/astral-sh/ruff/issues/18036 +# The autofix for this is unsafe due to the comments inside the dictionary. +foo( + **{ + # Comment 1 + "x": 1.0, + # Comment 2 + "y": 2.0, + } +) diff --git a/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_dict_kwargs.rs b/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_dict_kwargs.rs index f0f9ba7c598c9..b06744c58eee2 100644 --- a/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_dict_kwargs.rs +++ b/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_dict_kwargs.rs @@ -1,7 +1,7 @@ use itertools::Itertools; use rustc_hash::{FxBuildHasher, FxHashSet}; -use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; +use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast::parenthesize::parenthesized_range; use ruff_python_ast::{self as ast, Expr}; @@ -19,6 +19,7 @@ use crate::fix::edits::{remove_argument, Parentheses}; /// arguments directly. /// /// ## Example +/// /// ```python /// def foo(bar): /// return bar + 1 @@ -28,6 +29,7 @@ use crate::fix::edits::{remove_argument, Parentheses}; /// ``` /// /// Use instead: +/// /// ```python /// def foo(bar): /// return bar + 1 @@ -36,6 +38,26 @@ use crate::fix::edits::{remove_argument, Parentheses}; /// print(foo(bar=2)) # prints 3 /// ``` /// +/// ## Fix safety +/// +/// This rule's fix is marked as unsafe for dictionaries with comments interleaved between +/// the items, as comments may be removed. +/// +/// For example, the fix would be marked as unsafe in the following case: +/// +/// ```python +/// foo( +/// **{ +/// # comment +/// "x": 1.0, +/// # comment +/// "y": 2.0, +/// } +/// ) +/// ``` +/// +/// as this is converted to `foo(x=1.0, y=2.0)` without any of the comments. +/// /// ## References /// - [Python documentation: Dictionary displays](https://docs.python.org/3/reference/expressions.html#dictionary-displays) /// - [Python documentation: Calls](https://docs.python.org/3/reference/expressions.html#calls) @@ -113,7 +135,7 @@ pub(crate) fn unnecessary_dict_kwargs(checker: &Checker, call: &ast::ExprCall) { .iter() .all(|kwarg| !duplicate_keywords.contains(kwarg)) { - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( + let edit = Edit::range_replacement( kwargs .iter() .zip(dict.iter_values()) @@ -134,7 +156,15 @@ pub(crate) fn unnecessary_dict_kwargs(checker: &Checker, call: &ast::ExprCall) { }) .join(", "), keyword.range(), - ))); + ); + diagnostic.set_fix(Fix::applicable_edit( + edit, + if checker.comment_ranges().intersects(dict.range()) { + Applicability::Unsafe + } else { + Applicability::Safe + }, + )); } } } diff --git a/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE804_PIE804.py.snap b/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE804_PIE804.py.snap index b26502fe8ccf7..68e4f648e62bf 100644 --- a/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE804_PIE804.py.snap +++ b/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE804_PIE804.py.snap @@ -208,6 +208,8 @@ PIE804.py:29:12: PIE804 [*] Unnecessary `dict` kwargs 29 |-abc(foo=1, **{'bar': (bar := 1)}) # PIE804 29 |+abc(foo=1, bar=(bar := 1)) # PIE804 30 30 | abc(foo=1, **{'bar': (yield 1)}) # PIE804 +31 31 | +32 32 | # https://github.com/astral-sh/ruff/issues/18036 PIE804.py:30:12: PIE804 [*] Unnecessary `dict` kwargs | @@ -215,6 +217,8 @@ PIE804.py:30:12: PIE804 [*] Unnecessary `dict` kwargs 29 | abc(foo=1, **{'bar': (bar := 1)}) # PIE804 30 | abc(foo=1, **{'bar': (yield 1)}) # PIE804 | ^^^^^^^^^^^^^^^^^^^^ PIE804 +31 | +32 | # https://github.com/astral-sh/ruff/issues/18036 | = help: Remove unnecessary kwargs @@ -224,3 +228,34 @@ PIE804.py:30:12: PIE804 [*] Unnecessary `dict` kwargs 29 29 | abc(foo=1, **{'bar': (bar := 1)}) # PIE804 30 |-abc(foo=1, **{'bar': (yield 1)}) # PIE804 30 |+abc(foo=1, bar=(yield 1)) # PIE804 +31 31 | +32 32 | # https://github.com/astral-sh/ruff/issues/18036 +33 33 | # The autofix for this is unsafe due to the comments inside the dictionary. + +PIE804.py:35:5: PIE804 [*] Unnecessary `dict` kwargs + | +33 | # The autofix for this is unsafe due to the comments inside the dictionary. +34 | foo( +35 | / **{ +36 | | # Comment 1 +37 | | "x": 1.0, +38 | | # Comment 2 +39 | | "y": 2.0, +40 | | } + | |_____^ PIE804 +41 | ) + | + = help: Remove unnecessary kwargs + +ℹ Unsafe fix +32 32 | # https://github.com/astral-sh/ruff/issues/18036 +33 33 | # The autofix for this is unsafe due to the comments inside the dictionary. +34 34 | foo( +35 |- **{ +36 |- # Comment 1 +37 |- "x": 1.0, +38 |- # Comment 2 +39 |- "y": 2.0, +40 |- } + 35 |+ x=1.0, y=2.0 +41 36 | )