From bb979e05acc1b8d0e02541c536c8e0c61de01ee1 Mon Sep 17 00:00:00 2001 From: InSync Date: Fri, 7 Feb 2025 14:52:10 +0700 Subject: [PATCH] [`flake8-pie`] Remove following comma correctly when the unpacked dictionary is empty (`PIE800`) (#16008) ## Summary Resolves #15997. Ruff used to introduce syntax errors while fixing these cases, but no longer will: ```python {"a": [], **{},} # ^^^^ Removed, leaving two contiguous commas {"a": [], **({})} # ^^^^^ Removed, leaving a stray closing parentheses ``` Previously, the function would take a shortcut if the unpacked dictionary is empty; now, both cases are handled using the same logic introduced in #15394. This change slightly modifies that logic to also remove the first comma following the dictionary, if and only if it is empty. ## Test Plan `cargo nextest run` and `cargo insta test`. --- .../test/fixtures/flake8_pie/PIE800.py | 26 +++ .../flake8_pie/rules/unnecessary_spread.rs | 119 +++++++------ ...__flake8_pie__tests__PIE800_PIE800.py.snap | 166 +++++++++++++++++- 3 files changed, 254 insertions(+), 57 deletions(-) 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 9ef34dd9ea..f294b6cdf2 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE800.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE800.py @@ -70,6 +70,32 @@ foo({**foo, **{"bar": True}}) # PIE800 , }) +{ + "data": [], + ** # Foo + ( # Comment + { "a": b, + # Comment + } + ) , + c: 9, +} + + +# https://github.com/astral-sh/ruff/issues/15997 +{"a": [], **{},} +{"a": [], **({}),} + +{"a": [], **{}, 6: 3} +{"a": [], **({}), 6: 3} + +{"a": [], **{ + # Comment +}, 6: 3} +{"a": [], **({ + # Comment +}), 6: 3} + {**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 efbcada3bd..f5d867ecfc 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,7 +2,7 @@ use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast::{self as ast, Expr}; use ruff_python_parser::{TokenKind, Tokens}; -use ruff_text_size::{Ranged, TextSize}; +use ruff_text_size::{Ranged, TextLen, TextSize}; use crate::checkers::ast::Checker; @@ -75,66 +75,73 @@ fn unnecessary_spread_fix( .iter() .find(|tok| matches!(tok.kind(), TokenKind::DoubleStar))?; - if let Some(last) = dict.iter_values().last() { - // Ex) `**{a: 1, b: 2}` - let mut edits = vec![]; - let mut open_parens: u32 = 0; + let (empty, last_value_end) = if let Some(last) = dict.iter_values().last() { + (false, last.end()) + } else { + (true, dict.start() + "{".text_len()) + }; - for tok in tokens.after(doublestar.end()) { - match tok.kind() { - kind if kind.is_trivia() => {} - TokenKind::Lpar => { - edits.push(Edit::range_deletion(tok.range())); - open_parens += 1; - } - TokenKind::Lbrace => { - edits.push(Edit::range_deletion(tok.range())); - break; - } - _ => { - // Unexpected token, bail - return None; - } + let mut edits = vec![]; + let mut open_parens: u32 = 0; + + for tok in tokens.after(doublestar.end()) { + match tok.kind() { + kind if kind.is_trivia() => {} + TokenKind::Lpar => { + edits.push(Edit::range_deletion(tok.range())); + open_parens += 1; } - } - - let mut found_r_curly = false; - for tok in tokens.after(last.end()) { - if found_r_curly && open_parens == 0 { + TokenKind::Lbrace => { + edits.push(Edit::range_deletion(tok.range())); break; } - - match tok.kind() { - kind if kind.is_trivia() => {} - TokenKind::Comma => { - edits.push(Edit::range_deletion(tok.range())); - } - TokenKind::Rpar => { - if found_r_curly { - edits.push(Edit::range_deletion(tok.range())); - open_parens -= 1; - } - } - TokenKind::Rbrace => { - edits.push(Edit::range_deletion(tok.range())); - found_r_curly = true; - } - _ => { - // Unexpected token, bail - return None; - } + _ => { + // Unexpected token, bail + return None; } } - - Some(Fix::safe_edits( - Edit::range_deletion(doublestar.range()), - edits, - )) - } else { - // Ex) `**{}` - Some(Fix::safe_edit(Edit::deletion( - doublestar.start(), - dict.end(), - ))) } + + let mut found_r_curly = false; + let mut found_dict_comma = false; + + for tok in tokens.after(last_value_end) { + if found_r_curly && open_parens == 0 && (!empty || found_dict_comma) { + break; + } + + match tok.kind() { + kind if kind.is_trivia() => {} + TokenKind::Comma => { + edits.push(Edit::range_deletion(tok.range())); + + if found_r_curly { + found_dict_comma = true; + } + } + TokenKind::Rpar => { + if found_r_curly { + edits.push(Edit::range_deletion(tok.range())); + open_parens -= 1; + } + } + TokenKind::Rbrace => { + if found_r_curly { + break; + } + + edits.push(Edit::range_deletion(tok.range())); + found_r_curly = true; + } + _ => { + // Unexpected token, bail + return None; + } + } + } + + Some(Fix::safe_edits( + Edit::range_deletion(doublestar.range()), + edits, + )) } 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 393e7d05f1..6eb78b21f6 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 @@ -295,4 +295,168 @@ PIE800.py:65:1: PIE800 [*] Unnecessary spread `**` 69 |+ # Comment 70 70 | , 71 71 | }) -72 72 | +72 72 | + +PIE800.py:77:9: PIE800 [*] Unnecessary spread `**` + | +75 | ** # Foo +76 | ( # Comment +77 | / { "a": b, +78 | | # Comment +79 | | } + | |___________^ PIE800 +80 | ) , +81 | c: 9, + | + = help: Remove unnecessary dict + +ℹ Safe fix +72 72 | +73 73 | { +74 74 | "data": [], +75 |- ** # Foo +76 |- ( # Comment +77 |- { "a": b, + 75 |+ # Foo + 76 |+ # Comment + 77 |+ "a": b +78 78 | # Comment +79 |- } +80 |- ) , + 79 |+ + 80 |+ , +81 81 | c: 9, +82 82 | } +83 83 | + +PIE800.py:86:13: PIE800 [*] Unnecessary spread `**` + | +85 | # https://github.com/astral-sh/ruff/issues/15997 +86 | {"a": [], **{},} + | ^^ PIE800 +87 | {"a": [], **({}),} + | + = help: Remove unnecessary dict + +ℹ Safe fix +83 83 | +84 84 | +85 85 | # https://github.com/astral-sh/ruff/issues/15997 +86 |-{"a": [], **{},} + 86 |+{"a": [], } +87 87 | {"a": [], **({}),} +88 88 | +89 89 | {"a": [], **{}, 6: 3} + +PIE800.py:87:14: PIE800 [*] Unnecessary spread `**` + | +85 | # https://github.com/astral-sh/ruff/issues/15997 +86 | {"a": [], **{},} +87 | {"a": [], **({}),} + | ^^ PIE800 +88 | +89 | {"a": [], **{}, 6: 3} + | + = help: Remove unnecessary dict + +ℹ Safe fix +84 84 | +85 85 | # https://github.com/astral-sh/ruff/issues/15997 +86 86 | {"a": [], **{},} +87 |-{"a": [], **({}),} + 87 |+{"a": [], } +88 88 | +89 89 | {"a": [], **{}, 6: 3} +90 90 | {"a": [], **({}), 6: 3} + +PIE800.py:89:13: PIE800 [*] Unnecessary spread `**` + | +87 | {"a": [], **({}),} +88 | +89 | {"a": [], **{}, 6: 3} + | ^^ PIE800 +90 | {"a": [], **({}), 6: 3} + | + = help: Remove unnecessary dict + +ℹ Safe fix +86 86 | {"a": [], **{},} +87 87 | {"a": [], **({}),} +88 88 | +89 |-{"a": [], **{}, 6: 3} + 89 |+{"a": [], 6: 3} +90 90 | {"a": [], **({}), 6: 3} +91 91 | +92 92 | {"a": [], **{ + +PIE800.py:90:14: PIE800 [*] Unnecessary spread `**` + | +89 | {"a": [], **{}, 6: 3} +90 | {"a": [], **({}), 6: 3} + | ^^ PIE800 +91 | +92 | {"a": [], **{ + | + = help: Remove unnecessary dict + +ℹ Safe fix +87 87 | {"a": [], **({}),} +88 88 | +89 89 | {"a": [], **{}, 6: 3} +90 |-{"a": [], **({}), 6: 3} + 90 |+{"a": [], 6: 3} +91 91 | +92 92 | {"a": [], **{ +93 93 | # Comment + +PIE800.py:92:13: PIE800 [*] Unnecessary spread `**` + | +90 | {"a": [], **({}), 6: 3} +91 | +92 | {"a": [], **{ + | _____________^ +93 | | # Comment +94 | | }, 6: 3} + | |_^ PIE800 +95 | {"a": [], **({ +96 | # Comment + | + = help: Remove unnecessary dict + +ℹ Safe fix +89 89 | {"a": [], **{}, 6: 3} +90 90 | {"a": [], **({}), 6: 3} +91 91 | +92 |-{"a": [], **{ + 92 |+{"a": [], +93 93 | # Comment +94 |-}, 6: 3} + 94 |+ 6: 3} +95 95 | {"a": [], **({ +96 96 | # Comment +97 97 | }), 6: 3} + +PIE800.py:95:14: PIE800 [*] Unnecessary spread `**` + | +93 | # Comment +94 | }, 6: 3} +95 | {"a": [], **({ + | ______________^ +96 | | # Comment +97 | | }), 6: 3} + | |_^ PIE800 + | + = help: Remove unnecessary dict + +ℹ Safe fix +92 92 | {"a": [], **{ +93 93 | # Comment +94 94 | }, 6: 3} +95 |-{"a": [], **({ + 95 |+{"a": [], +96 96 | # Comment +97 |-}), 6: 3} + 97 |+ 6: 3} +98 98 | +99 99 | +100 100 | {**foo, "bar": True } # OK