From a71c4dfabbed166cdddbe61d275f26e73a4100d3 Mon Sep 17 00:00:00 2001 From: Steve C Date: Wed, 11 Oct 2023 15:05:49 -0400 Subject: [PATCH] fix edge case with PIE804 (#7922) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary `foo(**{})` was an overlooked edge case for `PIE804` which introduced a crash within the Fix, introduced in #7884. I've made it so that `foo(**{})` turns into `foo()` when applied with `--fix`, but is that desired/expected? 🤔 Should we just ignore instead? ## Test Plan `cargo test` --- .../test/fixtures/flake8_pie/PIE804.py | 2 + .../rules/unnecessary_dict_kwargs.rs | 24 +++++++----- ...__flake8_pie__tests__PIE804_PIE804.py.snap | 39 ++++++++++++++----- 3 files changed, 46 insertions(+), 19 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 de112a074e..84274c853a 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE804.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE804.py @@ -8,6 +8,8 @@ Foo.objects.create(**{"_id": some_id}) # PIE804 Foo.objects.create(**{**bar}) # PIE804 +foo(**{}) + foo(**{**data, "foo": "buzz"}) foo(**buzz) 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 3fa3a60cd4..b28000c41b 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 @@ -88,16 +88,20 @@ pub(crate) fn unnecessary_dict_kwargs(checker: &mut Checker, expr: &Expr, kwargs let mut diagnostic = Diagnostic::new(UnnecessaryDictKwargs, expr.range()); - 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(), - ))); + if values.is_empty() { + diagnostic.set_fix(Fix::safe_edit(Edit::deletion(kw.start(), kw.end()))); + } else { + 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); } 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 67f8e3831c..fee4c78e95 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 @@ -80,13 +80,15 @@ PIE804.py:7:1: PIE804 [*] Unnecessary `dict` kwargs 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 + | + 7 | Foo.objects.create(**{"_id": some_id}) # PIE804 + 8 | + 9 | Foo.objects.create(**{**bar}) # PIE804 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PIE804 +10 | +11 | foo(**{}) + | + = help: Remove unnecessary kwargs ℹ Fix 6 6 | @@ -95,7 +97,26 @@ PIE804.py:9:1: PIE804 [*] Unnecessary `dict` kwargs 9 |-Foo.objects.create(**{**bar}) # PIE804 9 |+Foo.objects.create(**bar) # PIE804 10 10 | -11 11 | -12 12 | foo(**{**data, "foo": "buzz"}) +11 11 | foo(**{}) +12 12 | + +PIE804.py:11:1: PIE804 [*] Unnecessary `dict` kwargs + | + 9 | Foo.objects.create(**{**bar}) # PIE804 +10 | +11 | foo(**{}) + | ^^^^^^^^^ PIE804 + | + = help: Remove unnecessary kwargs + +ℹ Fix +8 8 | +9 9 | Foo.objects.create(**{**bar}) # PIE804 +10 10 | +11 |-foo(**{}) + 11 |+foo() +12 12 | +13 13 | +14 14 | foo(**{**data, "foo": "buzz"})