diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE800.py b/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE800.py index 3584d2c03f32a..56a74de9c0e58 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE800.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE800.py @@ -1,9 +1,21 @@ {"foo": 1, **{"bar": 1}} # PIE800 +{**{"bar": 10}, "a": "b"} # PIE800 + foo({**foo, **{"bar": True}}) # PIE800 {**foo, **{"bar": 10}} # PIE800 +{ # PIE800 + "a": "b", + # Preserve + **{ + # all + "bar": 10, # the + # comments + }, +} + {**foo, **buzz, **{bar: 10}} # PIE800 {**foo, "bar": True } # OK diff --git a/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_spread.rs b/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_spread.rs index f27934ec4f712..d3d47988d4f17 100644 --- a/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_spread.rs +++ b/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_spread.rs @@ -2,6 +2,7 @@ use ruff_python_ast::Expr; use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_trivia::{BackwardsTokenizer, SimpleTokenKind}; use ruff_text_size::{Ranged, TextSize}; use crate::checkers::ast::Checker; @@ -49,22 +50,56 @@ pub(crate) fn unnecessary_spread(checker: &mut Checker, keys: &[Option], v if let (None, value) = item { // We only care about when the key is None which indicates a spread `**` // inside a dict. - if let Expr::Dict(_) = value { + if let Expr::Dict(dict) = value { let mut diagnostic = Diagnostic::new(UnnecessarySpread, value.range()); if checker.settings.preview.is_enabled() { - let range = value.range(); - // unwrap -- `item.0 == None` iff this is a spread operator - // which means there *must* be a `**` here - let doublestar = checker.locator().up_to(range.start()).rfind("**").unwrap(); - diagnostic.set_fix(Fix::safe_edits( - // delete the `**{` - Edit::deletion( - TextSize::from(doublestar as u32), - range.start() + TextSize::from(1), - ), - // delete the `}` - [Edit::deletion(range.end() - TextSize::from(1), range.end())], - )); + // Delete the `**{` + let tokenizer = BackwardsTokenizer::up_to( + dict.range.start(), + checker.locator().contents(), + &[], + ); + let mut start = None; + for tok in tokenizer { + if let SimpleTokenKind::DoubleStar = tok.kind() { + start = Some(tok.range.start()); + break; + } + } + // unwrap is ok, b/c item.0 can't be None without a DoubleStar + let first = + Edit::deletion(start.unwrap(), dict.range.start() + TextSize::from(1)); + + // Delete the `}` (and possibly a trailing comma) but preserve comments + let mut edits = Vec::with_capacity(1); + let mut end = dict.range.end(); + + let tokenizer = BackwardsTokenizer::up_to( + dict.range.end() - TextSize::from(1), + checker.locator().contents(), + &[], + ); + for tok in tokenizer { + match tok.kind() { + SimpleTokenKind::Comment => { + if tok.range.end() != end { + edits.push(Edit::deletion(tok.range.end(), end)); + } + end = tok.range.start(); + } + SimpleTokenKind::Comma + | SimpleTokenKind::Whitespace + | SimpleTokenKind::Newline + | SimpleTokenKind::Continuation => {} + _ => { + if tok.range.end() != end { + edits.push(Edit::deletion(tok.range.end(), end)); + } + break; + } + } + } + diagnostic.set_fix(Fix::safe_edits(first, edits)); } checker.diagnostics.push(diagnostic); } diff --git a/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE800_PIE800.py.snap b/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE800_PIE800.py.snap index 7825ad433c6e9..ad6c9e293eead 100644 --- a/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE800_PIE800.py.snap +++ b/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE800_PIE800.py.snap @@ -6,41 +6,67 @@ PIE800.py:1:14: PIE800 Unnecessary spread `**` 1 | {"foo": 1, **{"bar": 1}} # PIE800 | ^^^^^^^^^^ PIE800 2 | -3 | foo({**foo, **{"bar": True}}) # PIE800 +3 | {**{"bar": 10}, "a": "b"} # PIE800 | = help: Remove unnecessary dict -PIE800.py:3:15: PIE800 Unnecessary spread `**` +PIE800.py:3:4: PIE800 Unnecessary spread `**` | 1 | {"foo": 1, **{"bar": 1}} # PIE800 2 | -3 | foo({**foo, **{"bar": True}}) # PIE800 - | ^^^^^^^^^^^^^ PIE800 +3 | {**{"bar": 10}, "a": "b"} # PIE800 + | ^^^^^^^^^^^ PIE800 4 | -5 | {**foo, **{"bar": 10}} # PIE800 +5 | foo({**foo, **{"bar": True}}) # PIE800 | = help: Remove unnecessary dict -PIE800.py:5:11: PIE800 Unnecessary spread `**` +PIE800.py:5:15: PIE800 Unnecessary spread `**` | -3 | foo({**foo, **{"bar": True}}) # PIE800 +3 | {**{"bar": 10}, "a": "b"} # PIE800 4 | -5 | {**foo, **{"bar": 10}} # PIE800 - | ^^^^^^^^^^^ PIE800 +5 | foo({**foo, **{"bar": True}}) # PIE800 + | ^^^^^^^^^^^^^ PIE800 6 | -7 | {**foo, **buzz, **{bar: 10}} # PIE800 +7 | {**foo, **{"bar": 10}} # PIE800 | = help: Remove unnecessary dict -PIE800.py:7:19: PIE800 Unnecessary spread `**` +PIE800.py:7:11: PIE800 Unnecessary spread `**` | -5 | {**foo, **{"bar": 10}} # PIE800 +5 | foo({**foo, **{"bar": True}}) # PIE800 6 | -7 | {**foo, **buzz, **{bar: 10}} # PIE800 - | ^^^^^^^^^ PIE800 +7 | {**foo, **{"bar": 10}} # PIE800 + | ^^^^^^^^^^^ PIE800 8 | -9 | {**foo, "bar": True } # OK +9 | { # PIE800 | = help: Remove unnecessary dict +PIE800.py:12:7: PIE800 Unnecessary spread `**` + | +10 | "a": "b", +11 | # Preserve +12 | **{ + | _______^ +13 | | # all +14 | | "bar": 10, # the +15 | | # comments +16 | | }, + | |_____^ PIE800 +17 | } + | + = help: Remove unnecessary dict + +PIE800.py:19:19: PIE800 Unnecessary spread `**` + | +17 | } +18 | +19 | {**foo, **buzz, **{bar: 10}} # PIE800 + | ^^^^^^^^^ PIE800 +20 | +21 | {**foo, "bar": True } # OK + | + = help: Remove unnecessary dict + diff --git a/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__preview__PIE800_PIE800.py.snap b/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__preview__PIE800_PIE800.py.snap index 0fe76288763ba..d6fea2db4e700 100644 --- a/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__preview__PIE800_PIE800.py.snap +++ b/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__preview__PIE800_PIE800.py.snap @@ -6,7 +6,7 @@ PIE800.py:1:14: PIE800 [*] Unnecessary spread `**` 1 | {"foo": 1, **{"bar": 1}} # PIE800 | ^^^^^^^^^^ PIE800 2 | -3 | foo({**foo, **{"bar": True}}) # PIE800 +3 | {**{"bar": 10}, "a": "b"} # PIE800 | = help: Remove unnecessary dict @@ -14,69 +14,120 @@ PIE800.py:1:14: PIE800 [*] Unnecessary spread `**` 1 |-{"foo": 1, **{"bar": 1}} # PIE800 1 |+{"foo": 1, "bar": 1} # PIE800 2 2 | -3 3 | foo({**foo, **{"bar": True}}) # PIE800 +3 3 | {**{"bar": 10}, "a": "b"} # PIE800 4 4 | -PIE800.py:3:15: PIE800 [*] Unnecessary spread `**` +PIE800.py:3:4: PIE800 [*] Unnecessary spread `**` | 1 | {"foo": 1, **{"bar": 1}} # PIE800 2 | -3 | foo({**foo, **{"bar": True}}) # PIE800 - | ^^^^^^^^^^^^^ PIE800 +3 | {**{"bar": 10}, "a": "b"} # PIE800 + | ^^^^^^^^^^^ PIE800 4 | -5 | {**foo, **{"bar": 10}} # PIE800 +5 | foo({**foo, **{"bar": True}}) # PIE800 | = help: Remove unnecessary dict ℹ Safe fix 1 1 | {"foo": 1, **{"bar": 1}} # PIE800 2 2 | -3 |-foo({**foo, **{"bar": True}}) # PIE800 - 3 |+foo({**foo, "bar": True}) # PIE800 +3 |-{**{"bar": 10}, "a": "b"} # PIE800 + 3 |+{"bar": 10, "a": "b"} # PIE800 4 4 | -5 5 | {**foo, **{"bar": 10}} # PIE800 +5 5 | foo({**foo, **{"bar": True}}) # PIE800 6 6 | -PIE800.py:5:11: PIE800 [*] Unnecessary spread `**` +PIE800.py:5:15: PIE800 [*] Unnecessary spread `**` | -3 | foo({**foo, **{"bar": True}}) # PIE800 +3 | {**{"bar": 10}, "a": "b"} # PIE800 4 | -5 | {**foo, **{"bar": 10}} # PIE800 - | ^^^^^^^^^^^ PIE800 +5 | foo({**foo, **{"bar": True}}) # PIE800 + | ^^^^^^^^^^^^^ PIE800 6 | -7 | {**foo, **buzz, **{bar: 10}} # PIE800 +7 | {**foo, **{"bar": 10}} # PIE800 | = help: Remove unnecessary dict ℹ Safe fix 2 2 | -3 3 | foo({**foo, **{"bar": True}}) # PIE800 +3 3 | {**{"bar": 10}, "a": "b"} # PIE800 4 4 | -5 |-{**foo, **{"bar": 10}} # PIE800 - 5 |+{**foo, "bar": 10} # PIE800 +5 |-foo({**foo, **{"bar": True}}) # PIE800 + 5 |+foo({**foo, "bar": True}) # PIE800 6 6 | -7 7 | {**foo, **buzz, **{bar: 10}} # PIE800 +7 7 | {**foo, **{"bar": 10}} # PIE800 8 8 | -PIE800.py:7:19: PIE800 [*] Unnecessary spread `**` +PIE800.py:7:11: PIE800 [*] Unnecessary spread `**` | -5 | {**foo, **{"bar": 10}} # PIE800 +5 | foo({**foo, **{"bar": True}}) # PIE800 6 | -7 | {**foo, **buzz, **{bar: 10}} # PIE800 - | ^^^^^^^^^ PIE800 +7 | {**foo, **{"bar": 10}} # PIE800 + | ^^^^^^^^^^^ PIE800 8 | -9 | {**foo, "bar": True } # OK +9 | { # PIE800 | = help: Remove unnecessary dict ℹ Safe fix 4 4 | -5 5 | {**foo, **{"bar": 10}} # PIE800 +5 5 | foo({**foo, **{"bar": True}}) # PIE800 6 6 | -7 |-{**foo, **buzz, **{bar: 10}} # PIE800 - 7 |+{**foo, **buzz, bar: 10} # PIE800 +7 |-{**foo, **{"bar": 10}} # PIE800 + 7 |+{**foo, "bar": 10} # PIE800 8 8 | -9 9 | {**foo, "bar": True } # OK -10 10 | +9 9 | { # PIE800 +10 10 | "a": "b", + +PIE800.py:12:7: PIE800 [*] Unnecessary spread `**` + | +10 | "a": "b", +11 | # Preserve +12 | **{ + | _______^ +13 | | # all +14 | | "bar": 10, # the +15 | | # comments +16 | | }, + | |_____^ PIE800 +17 | } + | + = help: Remove unnecessary dict + +ℹ Safe fix +9 9 | { # PIE800 +10 10 | "a": "b", +11 11 | # Preserve +12 |- **{ + 12 |+ +13 13 | # all +14 14 | "bar": 10, # the +15 |- # comments +16 |- }, + 15 |+ # comments, +17 16 | } +18 17 | +19 18 | {**foo, **buzz, **{bar: 10}} # PIE800 + +PIE800.py:19:19: PIE800 [*] Unnecessary spread `**` + | +17 | } +18 | +19 | {**foo, **buzz, **{bar: 10}} # PIE800 + | ^^^^^^^^^ PIE800 +20 | +21 | {**foo, "bar": True } # OK + | + = help: Remove unnecessary dict + +ℹ Safe fix +16 16 | }, +17 17 | } +18 18 | +19 |-{**foo, **buzz, **{bar: 10}} # PIE800 + 19 |+{**foo, **buzz, bar: 10} # PIE800 +20 20 | +21 21 | {**foo, "bar": True } # OK +22 22 |