Respect trailing comma in unnecessary-dict-kwargs

astral-sh · Dec 5, 2023 · afa7433 · afa7433
1 parent 268d95e
commit afa7433
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 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
@@ -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
@@ -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
@@ -1,13 +1,14 @@
 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;
 
 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/...c/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE804_PIE804.py.snap b/...c/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()