From 1cf3b5676f072c281093d1b7425bed1082723e53 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 1 Oct 2023 10:53:54 -0400 Subject: [PATCH] Perform insertions before replacements (#7739) ## Summary If we have, e.g.: ```python sum(( factor.dims for factor in bases ), []) ``` We generate three edits: two insertions (for the `operator` and `functools` imports), and then one replacement (for the `sum` call itself). We need to ensure that the insertions come before the replacement; otherwise, the edits will appear overlapping and out-of-order. Closes https://github.com/astral-sh/ruff/issues/7718. --- crates/ruff_diagnostics/src/fix.rs | 6 +++--- .../fixtures/ruff/{RUF017.py => RUF017_0.py} | 0 .../resources/test/fixtures/ruff/RUF017_1.py | 1 + crates/ruff_linter/src/rules/ruff/mod.rs | 3 ++- ...rules__ruff__tests__RUF017_RUF017_0.py.snap} | 14 +++++++------- ..._rules__ruff__tests__RUF017_RUF017_1.py.snap | 17 +++++++++++++++++ 6 files changed, 30 insertions(+), 11 deletions(-) rename crates/ruff_linter/resources/test/fixtures/ruff/{RUF017.py => RUF017_0.py} (100%) create mode 100644 crates/ruff_linter/resources/test/fixtures/ruff/RUF017_1.py rename crates/ruff_linter/src/rules/ruff/snapshots/{ruff_linter__rules__ruff__tests__RUF017_RUF017.py.snap => ruff_linter__rules__ruff__tests__RUF017_RUF017_0.py.snap} (91%) create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF017_RUF017_1.py.snap diff --git a/crates/ruff_diagnostics/src/fix.rs b/crates/ruff_diagnostics/src/fix.rs index d553e83d6696f..7456e82b7e4e3 100644 --- a/crates/ruff_diagnostics/src/fix.rs +++ b/crates/ruff_diagnostics/src/fix.rs @@ -88,7 +88,7 @@ impl Fix { /// Create a new [`Fix`] with [automatic applicability](Applicability::Automatic) from multiple [`Edit`] elements. pub fn automatic_edits(edit: Edit, rest: impl IntoIterator) -> Self { let mut edits: Vec = std::iter::once(edit).chain(rest).collect(); - edits.sort_by_key(Ranged::start); + edits.sort_by_key(|edit| (edit.start(), edit.end())); Self { edits, applicability: Applicability::Automatic, @@ -108,7 +108,7 @@ impl Fix { /// Create a new [`Fix`] with [suggested applicability](Applicability::Suggested) from multiple [`Edit`] elements. pub fn suggested_edits(edit: Edit, rest: impl IntoIterator) -> Self { let mut edits: Vec = std::iter::once(edit).chain(rest).collect(); - edits.sort_by_key(Ranged::start); + edits.sort_by_key(|edit| (edit.start(), edit.end())); Self { edits, applicability: Applicability::Suggested, @@ -128,7 +128,7 @@ impl Fix { /// Create a new [`Fix`] with [manual applicability](Applicability::Manual) from multiple [`Edit`] elements. pub fn manual_edits(edit: Edit, rest: impl IntoIterator) -> Self { let mut edits: Vec = std::iter::once(edit).chain(rest).collect(); - edits.sort_by_key(Ranged::start); + edits.sort_by_key(|edit| (edit.start(), edit.end())); Self { edits, applicability: Applicability::Manual, diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF017.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF017_0.py similarity index 100% rename from crates/ruff_linter/resources/test/fixtures/ruff/RUF017.py rename to crates/ruff_linter/resources/test/fixtures/ruff/RUF017_0.py diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF017_1.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF017_1.py new file mode 100644 index 0000000000000..13a1e0be940c8 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF017_1.py @@ -0,0 +1 @@ +sum((factor.dims for factor in bases), []) diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index 722caa0bf72c5..cdda430bdefaf 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -40,7 +40,8 @@ mod tests { feature = "unreachable-code", test_case(Rule::UnreachableCode, Path::new("RUF014.py")) )] - #[test_case(Rule::QuadraticListSummation, Path::new("RUF017.py"))] + #[test_case(Rule::QuadraticListSummation, Path::new("RUF017_1.py"))] + #[test_case(Rule::QuadraticListSummation, Path::new("RUF017_0.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF017_RUF017.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF017_RUF017_0.py.snap similarity index 91% rename from crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF017_RUF017.py.snap rename to crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF017_RUF017_0.py.snap index e7556a19c8f82..3d30ebbcbbbe6 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF017_RUF017.py.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF017_RUF017_0.py.snap @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/ruff/mod.rs --- -RUF017.py:5:1: RUF017 [*] Avoid quadratic list summation +RUF017_0.py:5:1: RUF017 [*] Avoid quadratic list summation | 4 | # RUF017 5 | sum([x, y], start=[]) @@ -24,7 +24,7 @@ RUF017.py:5:1: RUF017 [*] Avoid quadratic list summation 7 9 | sum([[1, 2, 3], [4, 5, 6]], start=[]) 8 10 | sum([[1, 2, 3], [4, 5, 6]], []) -RUF017.py:6:1: RUF017 [*] Avoid quadratic list summation +RUF017_0.py:6:1: RUF017 [*] Avoid quadratic list summation | 4 | # RUF017 5 | sum([x, y], start=[]) @@ -49,7 +49,7 @@ RUF017.py:6:1: RUF017 [*] Avoid quadratic list summation 8 10 | sum([[1, 2, 3], [4, 5, 6]], []) 9 11 | sum([[1, 2, 3], [4, 5, 6]], -RUF017.py:7:1: RUF017 [*] Avoid quadratic list summation +RUF017_0.py:7:1: RUF017 [*] Avoid quadratic list summation | 5 | sum([x, y], start=[]) 6 | sum([x, y], []) @@ -75,7 +75,7 @@ RUF017.py:7:1: RUF017 [*] Avoid quadratic list summation 9 11 | sum([[1, 2, 3], [4, 5, 6]], 10 12 | []) -RUF017.py:8:1: RUF017 [*] Avoid quadratic list summation +RUF017_0.py:8:1: RUF017 [*] Avoid quadratic list summation | 6 | sum([x, y], []) 7 | sum([[1, 2, 3], [4, 5, 6]], start=[]) @@ -102,7 +102,7 @@ RUF017.py:8:1: RUF017 [*] Avoid quadratic list summation 10 12 | []) 11 13 | -RUF017.py:9:1: RUF017 [*] Avoid quadratic list summation +RUF017_0.py:9:1: RUF017 [*] Avoid quadratic list summation | 7 | sum([[1, 2, 3], [4, 5, 6]], start=[]) 8 | sum([[1, 2, 3], [4, 5, 6]], []) @@ -131,7 +131,7 @@ RUF017.py:9:1: RUF017 [*] Avoid quadratic list summation 12 13 | # OK 13 14 | sum([x, y]) -RUF017.py:21:5: RUF017 [*] Avoid quadratic list summation +RUF017_0.py:21:5: RUF017 [*] Avoid quadratic list summation | 19 | import functools, operator 20 | @@ -150,7 +150,7 @@ RUF017.py:21:5: RUF017 [*] Avoid quadratic list summation 23 23 | 24 24 | # Regression test for: https://github.com/astral-sh/ruff/issues/7718 -RUF017.py:26:5: RUF017 [*] Avoid quadratic list summation +RUF017_0.py:26:5: RUF017 [*] Avoid quadratic list summation | 24 | # Regression test for: https://github.com/astral-sh/ruff/issues/7718 25 | def func(): diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF017_RUF017_1.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF017_RUF017_1.py.snap new file mode 100644 index 0000000000000..65bb00fdeb0d0 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF017_RUF017_1.py.snap @@ -0,0 +1,17 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +RUF017_1.py:1:1: RUF017 [*] Avoid quadratic list summation + | +1 | sum((factor.dims for factor in bases), []) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF017 + | + = help: Replace with `functools.reduce` + +ℹ Suggested fix +1 |-sum((factor.dims for factor in bases), []) + 1 |+import functools + 2 |+import operator + 3 |+functools.reduce(operator.iadd, (factor.dims for factor in bases), []) + +