From ac7a097f05733da00f1fbc49966f3427df9327fe Mon Sep 17 00:00:00 2001 From: Alan Du Date: Tue, 14 Nov 2023 08:27:13 -0500 Subject: [PATCH] Use BackwardsTokenizer --- .../flake8_pie/rules/unnecessary_spread.rs | 63 +++++++--- ...__flake8_pie__tests__PIE800_PIE800.py.snap | 56 +++++---- ...pie__tests__preview__PIE800_PIE800.py.snap | 117 +++++++++--------- 3 files changed, 134 insertions(+), 102 deletions(-) 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 9587f8b4c4a08d..d3d47988d4f176 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; @@ -53,28 +54,52 @@ pub(crate) fn unnecessary_spread(checker: &mut Checker, keys: &[Option], v let mut diagnostic = Diagnostic::new(UnnecessarySpread, value.range()); if checker.settings.preview.is_enabled() { // Delete the `**{` - let doublestar = checker - .locator() - .up_to(dict.range.start()) - .rfind("**") - .unwrap(); // If item.0 is None, then we *must* have a `**` - let first_pos = match dict.keys.iter().next() { - Some(Some(inner_key)) => inner_key.range().start(), - _ => dict.range.start() + TextSize::from(1) - }; - let first = Edit::deletion( - TextSize::from(doublestar as u32), - first_pos, + 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 `}` - let last_pos = match dict.values.iter().rev().next() { - Some(inner_value) => inner_value.range().end(), - _ => dict.range.end() - TextSize::from(1), - }; - let second = Edit::deletion(last_pos, dict.range.end()); + // Delete the `}` (and possibly a trailing comma) but preserve comments + let mut edits = Vec::with_capacity(1); + let mut end = dict.range.end(); - diagnostic.set_fix(Fix::safe_edits(first, [second])); + 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 1d99832b79a4c3..ad6c9e293eead0 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,64 +6,66 @@ 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 | {**{"bar": 10}, "a": "b"} # PIE800 +7 | {**foo, **{"bar": 10}} # PIE800 | = help: Remove unnecessary dict -PIE800.py:7:4: PIE800 Unnecessary spread `**` +PIE800.py:7:11: PIE800 Unnecessary spread `**` | -5 | {**foo, **{"bar": 10}} # PIE800 +5 | foo({**foo, **{"bar": True}}) # PIE800 6 | -7 | {**{"bar": 10}, "a": "b"} # PIE800 - | ^^^^^^^^^^^ PIE800 +7 | {**foo, **{"bar": 10}} # PIE800 + | ^^^^^^^^^^^ PIE800 8 | 9 | { # PIE800 | = help: Remove unnecessary dict -PIE800.py:11:7: PIE800 Unnecessary spread `**` +PIE800.py:12:7: PIE800 Unnecessary spread `**` | - 9 | { # PIE800 -10 | **foo, -11 | **{ +10 | "a": "b", +11 | # Preserve +12 | **{ | _______^ -12 | | "bar": 10, -13 | | }, +13 | | # all +14 | | "bar": 10, # the +15 | | # comments +16 | | }, | |_____^ PIE800 -14 | } +17 | } | = help: Remove unnecessary dict -PIE800.py:16:19: PIE800 Unnecessary spread `**` +PIE800.py:19:19: PIE800 Unnecessary spread `**` | -14 | } -15 | -16 | {**foo, **buzz, **{bar: 10}} # PIE800 +17 | } +18 | +19 | {**foo, **buzz, **{bar: 10}} # PIE800 | ^^^^^^^^^ PIE800 -17 | -18 | {**foo, "bar": True } # OK +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 cecabadba6587e..d6fea2db4e7003 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,56 +14,56 @@ 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 | {**{"bar": 10}, "a": "b"} # 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 | {**{"bar": 10}, "a": "b"} # PIE800 +7 7 | {**foo, **{"bar": 10}} # PIE800 8 8 | -PIE800.py:7:4: PIE800 [*] Unnecessary spread `**` +PIE800.py:7:11: PIE800 [*] Unnecessary spread `**` | -5 | {**foo, **{"bar": 10}} # PIE800 +5 | foo({**foo, **{"bar": True}}) # PIE800 6 | -7 | {**{"bar": 10}, "a": "b"} # PIE800 - | ^^^^^^^^^^^ PIE800 +7 | {**foo, **{"bar": 10}} # PIE800 + | ^^^^^^^^^^^ PIE800 8 | 9 | { # PIE800 | @@ -71,58 +71,63 @@ PIE800.py:7:4: PIE800 [*] Unnecessary spread `**` ℹ Safe fix 4 4 | -5 5 | {**foo, **{"bar": 10}} # PIE800 +5 5 | foo({**foo, **{"bar": True}}) # PIE800 6 6 | -7 |-{**{"bar": 10}, "a": "b"} # PIE800 - 7 |+{"bar": 10, "a": "b"} # PIE800 +7 |-{**foo, **{"bar": 10}} # PIE800 + 7 |+{**foo, "bar": 10} # PIE800 8 8 | 9 9 | { # PIE800 -10 10 | **foo, +10 10 | "a": "b", -PIE800.py:11:7: PIE800 [*] Unnecessary spread `**` +PIE800.py:12:7: PIE800 [*] Unnecessary spread `**` | - 9 | { # PIE800 -10 | **foo, -11 | **{ +10 | "a": "b", +11 | # Preserve +12 | **{ | _______^ -12 | | "bar": 10, -13 | | }, +13 | | # all +14 | | "bar": 10, # the +15 | | # comments +16 | | }, | |_____^ PIE800 -14 | } +17 | } | = help: Remove unnecessary dict ℹ Safe fix -8 8 | 9 9 | { # PIE800 -10 10 | **foo, -11 |- **{ -12 |- "bar": 10, -13 |- }, - 11 |+ "bar": 10, -14 12 | } -15 13 | -16 14 | {**foo, **buzz, **{bar: 10}} # 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:16:19: PIE800 [*] Unnecessary spread `**` +PIE800.py:19:19: PIE800 [*] Unnecessary spread `**` | -14 | } -15 | -16 | {**foo, **buzz, **{bar: 10}} # PIE800 +17 | } +18 | +19 | {**foo, **buzz, **{bar: 10}} # PIE800 | ^^^^^^^^^ PIE800 -17 | -18 | {**foo, "bar": True } # OK +20 | +21 | {**foo, "bar": True } # OK | = help: Remove unnecessary dict ℹ Safe fix -13 13 | }, -14 14 | } -15 15 | -16 |-{**foo, **buzz, **{bar: 10}} # PIE800 - 16 |+{**foo, **buzz, bar: 10} # PIE800 -17 17 | -18 18 | {**foo, "bar": True } # OK -19 19 | +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 |