add autofix for PIE804 (#7884)

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, Constant, Expr, Keyword};
 
-use ruff_diagnostics::Diagnostic;
-use ruff_diagnostics::Violation;
 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::registry::AsRule;
 
 /// ## What it does
 /// Checks for unnecessary `dict` kwargs.
@@ -40,41 +41,83 @@ use crate::checkers::ast::Checker;
 #[violation]
 pub struct UnnecessaryDictKwargs;
 
-impl Violation for UnnecessaryDictKwargs {
+impl AlwaysFixableViolation for UnnecessaryDictKwargs {
     #[derive_message_formats]
     fn message(&self) -> String {
         format!("Unnecessary `dict` kwargs")
     }
+
+    fn fix_title(&self) -> String {
+        format!("Remove unnecessary kwargs")
+    }
 }
 
 /// PIE804
 pub(crate) fn unnecessary_dict_kwargs(checker: &mut Checker, expr: &Expr, kwargs: &[Keyword]) {
     for kw in kwargs {
         // keyword is a spread operator (indicated by None)
-        if kw.arg.is_none() {
-            if let Expr::Dict(ast::ExprDict { keys, .. }) = &kw.value {
-                // ensure foo(**{"bar-bar": 1}) doesn't error
-                if keys.iter().all(|expr| expr.as_ref().is_some_and( is_valid_kwarg_name)) ||
-                    // handle case of foo(**{**bar})
-                    (keys.len() == 1 && keys[0].is_none())
-                {
-                    let diagnostic = Diagnostic::new(UnnecessaryDictKwargs, expr.range());
-                    checker.diagnostics.push(diagnostic);
-                }
+        if kw.arg.is_some() {
+            continue;
+        }
+
+        let Expr::Dict(ast::ExprDict { keys, values, .. }) = &kw.value else {
+            continue;
+        };
+
+        // Ex) `foo(**{**bar})`
+        if matches!(keys.as_slice(), [None]) {
+            let mut diagnostic = Diagnostic::new(UnnecessaryDictKwargs, expr.range());
+
+            if checker.patch(diagnostic.kind.rule()) {
+                diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
+                    format!("**{}", checker.locator().slice(values[0].range())),
+                    kw.range(),
+                )));
             }
+
+            checker.diagnostics.push(diagnostic);
+            continue;
         }
+
+        // Ensure that every keyword is a valid keyword argument (e.g., avoid errors for cases like
+        // `foo(**{"bar-bar": 1})`).
+        let kwargs = keys
+            .iter()
+            .filter_map(|key| key.as_ref().and_then(as_kwarg))
+            .collect::<Vec<_>>();
+        if kwargs.len() != keys.len() {
+            continue;
+        }
+
+        let mut diagnostic = Diagnostic::new(UnnecessaryDictKwargs, expr.range());
+
+        if checker.patch(diagnostic.kind.rule()) {
+            diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
+                kwargs
+                    .iter()
+                    .zip(values.iter())
+                    .map(|(kwarg, value)| {
+                        format!("{}={}", kwarg.value, checker.locator().slice(value.range()))
+                    })
+                    .join(", "),
+                kw.range(),
+            )));
+        }
+
+        checker.diagnostics.push(diagnostic);
     }
 }
 
-/// Return `true` if a key is a valid keyword argument name.
-fn is_valid_kwarg_name(key: &Expr) -> bool {
+/// Return `Some` if a key is a valid keyword argument name, or `None` otherwise.
+fn as_kwarg(key: &Expr) -> Option<&ast::StringConstant> {
     if let Expr::Constant(ast::ExprConstant {
         value: Constant::Str(value),
         ..
     }) = key
     {
-        is_identifier(value)
-    } else {
-        false
+        if is_identifier(value) {
+            return Some(value);
+        }
     }
+    None
 }

crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE804_PIE804.py.snap

@@ -1,15 +1,23 @@
 ---
 source: crates/ruff_linter/src/rules/flake8_pie/mod.rs
 ---
-PIE804.py:1:1: PIE804 Unnecessary `dict` kwargs
+PIE804.py:1:1: PIE804 [*] Unnecessary `dict` kwargs
   |
 1 | foo(**{"bar": True})  # PIE804
   | ^^^^^^^^^^^^^^^^^^^^ PIE804
 2 | 
 3 | foo(**{"r2d2": True})  # PIE804
   |
+  = help: Remove unnecessary kwargs
 
-PIE804.py:3:1: PIE804 Unnecessary `dict` kwargs
+ℹ Fix
+1   |-foo(**{"bar": True})  # PIE804
+  1 |+foo(bar=True)  # PIE804
+2 2 | 
+3 3 | foo(**{"r2d2": True})  # PIE804
+4 4 | 
+
+PIE804.py:3:1: PIE804 [*] Unnecessary `dict` kwargs
   |
 1 | foo(**{"bar": True})  # PIE804
 2 | 
@@ -18,8 +26,18 @@ PIE804.py:3:1: PIE804 Unnecessary `dict` kwargs
 4 | 
 5 | Foo.objects.create(**{"bar": True})  # PIE804
   |
+  = help: Remove unnecessary kwargs
+
+ℹ Fix
+1 1 | foo(**{"bar": True})  # PIE804
+2 2 | 
+3   |-foo(**{"r2d2": True})  # PIE804
+  3 |+foo(r2d2=True)  # PIE804
+4 4 | 
+5 5 | Foo.objects.create(**{"bar": True})  # PIE804
+6 6 | 
 
-PIE804.py:5:1: PIE804 Unnecessary `dict` kwargs
+PIE804.py:5:1: PIE804 [*] Unnecessary `dict` kwargs
   |
 3 | foo(**{"r2d2": True})  # PIE804
 4 | 
@@ -28,8 +46,19 @@ PIE804.py:5:1: PIE804 Unnecessary `dict` kwargs
 6 | 
 7 | Foo.objects.create(**{"_id": some_id})  # PIE804
   |
+  = help: Remove unnecessary kwargs
+
+ℹ Fix
+2 2 | 
+3 3 | foo(**{"r2d2": True})  # PIE804
+4 4 | 
+5   |-Foo.objects.create(**{"bar": True})  # PIE804
+  5 |+Foo.objects.create(bar=True)  # PIE804
+6 6 | 
+7 7 | Foo.objects.create(**{"_id": some_id})  # PIE804
+8 8 | 
 
-PIE804.py:7:1: PIE804 Unnecessary `dict` kwargs
+PIE804.py:7:1: PIE804 [*] Unnecessary `dict` kwargs
   |
 5 | Foo.objects.create(**{"bar": True})  # PIE804
 6 | 
@@ -38,13 +67,35 @@ PIE804.py:7:1: PIE804 Unnecessary `dict` kwargs
 8 | 
 9 | Foo.objects.create(**{**bar})  # PIE804
   |
+  = help: Remove unnecessary kwargs
 
-PIE804.py:9:1: PIE804 Unnecessary `dict` kwargs
+ℹ Fix
+4 4 | 
+5 5 | Foo.objects.create(**{"bar": True})  # PIE804
+6 6 | 
+7   |-Foo.objects.create(**{"_id": some_id})  # PIE804
+  7 |+Foo.objects.create(_id=some_id)  # PIE804
+8 8 | 
+9 9 | Foo.objects.create(**{**bar})  # PIE804
+10 10 | 
+
+PIE804.py:9:1: PIE804 [*] Unnecessary `dict` kwargs
   |
 7 | Foo.objects.create(**{"_id": some_id})  # PIE804
 8 | 
 9 | Foo.objects.create(**{**bar})  # PIE804
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PIE804
   |
+  = help: Remove unnecessary kwargs
+
+ℹ Fix
+6  6  | 
+7  7  | Foo.objects.create(**{"_id": some_id})  # PIE804
+8  8  | 
+9     |-Foo.objects.create(**{**bar})  # PIE804
+   9  |+Foo.objects.create(**bar)  # PIE804
+10 10 | 
+11 11 | 
+12 12 | foo(**{**data, "foo": "buzz"})