From afa74337c3d1a06a3003d20b4ce29655d698e4d7 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 5 Dec 2023 15:31:33 -0500 Subject: [PATCH] Respect trailing comma in unnecessary-dict-kwargs --- .../test/fixtures/flake8_pie/PIE804.py | 3 ++- .../src/checkers/ast/analyze/expression.rs | 2 +- .../rules/unnecessary_dict_kwargs.rs | 21 +++++++++++++----- ...__flake8_pie__tests__PIE804_PIE804.py.snap | 22 +++++++++++++++++-- 4 files changed, 38 insertions(+), 10 deletions(-) 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 84274c853a8aa..03d88e0ef8e22 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE804.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE804.py @@ -10,7 +10,6 @@ foo(**{}) - foo(**{**data, "foo": "buzz"}) foo(**buzz) foo(**{"bar-foo": True}) @@ -20,3 +19,5 @@ foo(**{"": True}) foo(**{f"buzz__{bar}": True}) abc(**{"for": 3}) + +foo(**{},) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 87a354b9641f3..381a53adbd1dd 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -543,7 +543,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { flake8_bugbear::rules::no_explicit_stacklevel(checker, call); } if checker.enabled(Rule::UnnecessaryDictKwargs) { - flake8_pie::rules::unnecessary_dict_kwargs(checker, expr, keywords); + flake8_pie::rules::unnecessary_dict_kwargs(checker, call); } if checker.enabled(Rule::UnnecessaryRangeStart) { flake8_pie::rules::unnecessary_range_start(checker, call); 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 c4625444f7bde..ab4c7ceb9ddcc 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,6 +1,6 @@ use itertools::Itertools; use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; -use ruff_python_ast::{self as ast, Expr, Keyword}; +use ruff_python_ast::{self as ast, Expr}; use ruff_macros::{derive_message_formats, violation}; use ruff_text_size::Ranged; @@ -8,6 +8,7 @@ use ruff_text_size::Ranged; use ruff_python_stdlib::identifiers::is_identifier; use crate::checkers::ast::Checker; +use crate::fix::edits::{remove_argument, Parentheses}; /// ## What it does /// Checks for unnecessary `dict` kwargs. @@ -52,8 +53,8 @@ impl AlwaysFixableViolation for UnnecessaryDictKwargs { } /// PIE804 -pub(crate) fn unnecessary_dict_kwargs(checker: &mut Checker, expr: &Expr, kwargs: &[Keyword]) { - for kw in kwargs { +pub(crate) fn unnecessary_dict_kwargs(checker: &mut Checker, call: &ast::ExprCall) { + for kw in &call.arguments.keywords { // keyword is a spread operator (indicated by None) if kw.arg.is_some() { continue; @@ -65,7 +66,7 @@ pub(crate) fn unnecessary_dict_kwargs(checker: &mut Checker, expr: &Expr, kwargs // Ex) `foo(**{**bar})` if matches!(keys.as_slice(), [None]) { - let mut diagnostic = Diagnostic::new(UnnecessaryDictKwargs, expr.range()); + let mut diagnostic = Diagnostic::new(UnnecessaryDictKwargs, call.range()); diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( format!("**{}", checker.locator().slice(values[0].range())), @@ -86,10 +87,18 @@ pub(crate) fn unnecessary_dict_kwargs(checker: &mut Checker, expr: &Expr, kwargs continue; } - let mut diagnostic = Diagnostic::new(UnnecessaryDictKwargs, expr.range()); + let mut diagnostic = Diagnostic::new(UnnecessaryDictKwargs, call.range()); if values.is_empty() { - diagnostic.set_fix(Fix::safe_edit(Edit::deletion(kw.start(), kw.end()))); + diagnostic.try_set_fix(|| { + remove_argument( + kw, + &call.arguments, + Parentheses::Preserve, + checker.locator().contents(), + ) + .map(Fix::safe_edit) + }); } else { diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( kwargs 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 450ed048e6e4a..0f608fbedb040 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 @@ -106,6 +106,8 @@ PIE804.py:11:1: PIE804 [*] Unnecessary `dict` kwargs 10 | 11 | foo(**{}) | ^^^^^^^^^ PIE804 +12 | +13 | foo(**{**data, "foo": "buzz"}) | = help: Remove unnecessary kwargs @@ -116,7 +118,23 @@ PIE804.py:11:1: PIE804 [*] Unnecessary `dict` kwargs 11 |-foo(**{}) 11 |+foo() 12 12 | -13 13 | -14 14 | foo(**{**data, "foo": "buzz"}) +13 13 | foo(**{**data, "foo": "buzz"}) +14 14 | foo(**buzz) + +PIE804.py:23:1: PIE804 [*] Unnecessary `dict` kwargs + | +21 | abc(**{"for": 3}) +22 | +23 | foo(**{},) + | ^^^^^^^^^^ PIE804 + | + = help: Remove unnecessary kwargs + +ℹ Safe fix +20 20 | foo(**{f"buzz__{bar}": True}) +21 21 | abc(**{"for": 3}) +22 22 | +23 |-foo(**{},) + 23 |+foo()