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.
This commit is contained in:
Charlie Marsh 2023-10-01 10:53:54 -04:00 committed by GitHub
parent e91ffe3e93
commit 1cf3b5676f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 30 additions and 11 deletions

View file

@ -88,7 +88,7 @@ impl Fix {
/// Create a new [`Fix`] with [automatic applicability](Applicability::Automatic) from multiple [`Edit`] elements. /// Create a new [`Fix`] with [automatic applicability](Applicability::Automatic) from multiple [`Edit`] elements.
pub fn automatic_edits(edit: Edit, rest: impl IntoIterator<Item = Edit>) -> Self { pub fn automatic_edits(edit: Edit, rest: impl IntoIterator<Item = Edit>) -> Self {
let mut edits: Vec<Edit> = std::iter::once(edit).chain(rest).collect(); let mut edits: Vec<Edit> = std::iter::once(edit).chain(rest).collect();
edits.sort_by_key(Ranged::start); edits.sort_by_key(|edit| (edit.start(), edit.end()));
Self { Self {
edits, edits,
applicability: Applicability::Automatic, applicability: Applicability::Automatic,
@ -108,7 +108,7 @@ impl Fix {
/// Create a new [`Fix`] with [suggested applicability](Applicability::Suggested) from multiple [`Edit`] elements. /// Create a new [`Fix`] with [suggested applicability](Applicability::Suggested) from multiple [`Edit`] elements.
pub fn suggested_edits(edit: Edit, rest: impl IntoIterator<Item = Edit>) -> Self { pub fn suggested_edits(edit: Edit, rest: impl IntoIterator<Item = Edit>) -> Self {
let mut edits: Vec<Edit> = std::iter::once(edit).chain(rest).collect(); let mut edits: Vec<Edit> = std::iter::once(edit).chain(rest).collect();
edits.sort_by_key(Ranged::start); edits.sort_by_key(|edit| (edit.start(), edit.end()));
Self { Self {
edits, edits,
applicability: Applicability::Suggested, applicability: Applicability::Suggested,
@ -128,7 +128,7 @@ impl Fix {
/// Create a new [`Fix`] with [manual applicability](Applicability::Manual) from multiple [`Edit`] elements. /// Create a new [`Fix`] with [manual applicability](Applicability::Manual) from multiple [`Edit`] elements.
pub fn manual_edits(edit: Edit, rest: impl IntoIterator<Item = Edit>) -> Self { pub fn manual_edits(edit: Edit, rest: impl IntoIterator<Item = Edit>) -> Self {
let mut edits: Vec<Edit> = std::iter::once(edit).chain(rest).collect(); let mut edits: Vec<Edit> = std::iter::once(edit).chain(rest).collect();
edits.sort_by_key(Ranged::start); edits.sort_by_key(|edit| (edit.start(), edit.end()));
Self { Self {
edits, edits,
applicability: Applicability::Manual, applicability: Applicability::Manual,

View file

@ -0,0 +1 @@
sum((factor.dims for factor in bases), [])

View file

@ -40,7 +40,8 @@ mod tests {
feature = "unreachable-code", feature = "unreachable-code",
test_case(Rule::UnreachableCode, Path::new("RUF014.py")) 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<()> { fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path( let diagnostics = test_path(

View file

@ -1,7 +1,7 @@
--- ---
source: crates/ruff_linter/src/rules/ruff/mod.rs 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 4 | # RUF017
5 | sum([x, y], start=[]) 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=[]) 7 9 | sum([[1, 2, 3], [4, 5, 6]], start=[])
8 10 | sum([[1, 2, 3], [4, 5, 6]], []) 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 4 | # RUF017
5 | sum([x, y], start=[]) 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]], []) 8 10 | sum([[1, 2, 3], [4, 5, 6]], [])
9 11 | 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=[]) 5 | sum([x, y], start=[])
6 | sum([x, y], []) 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]], 9 11 | sum([[1, 2, 3], [4, 5, 6]],
10 12 | []) 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], []) 6 | sum([x, y], [])
7 | sum([[1, 2, 3], [4, 5, 6]], start=[]) 7 | sum([[1, 2, 3], [4, 5, 6]], start=[])
@ -102,7 +102,7 @@ RUF017.py:8:1: RUF017 [*] Avoid quadratic list summation
10 12 | []) 10 12 | [])
11 13 | 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=[]) 7 | sum([[1, 2, 3], [4, 5, 6]], start=[])
8 | sum([[1, 2, 3], [4, 5, 6]], []) 8 | sum([[1, 2, 3], [4, 5, 6]], [])
@ -131,7 +131,7 @@ RUF017.py:9:1: RUF017 [*] Avoid quadratic list summation
12 13 | # OK 12 13 | # OK
13 14 | sum([x, y]) 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 19 | import functools, operator
20 | 20 |
@ -150,7 +150,7 @@ RUF017.py:21:5: RUF017 [*] Avoid quadratic list summation
23 23 | 23 23 |
24 24 | # Regression test for: https://github.com/astral-sh/ruff/issues/7718 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 24 | # Regression test for: https://github.com/astral-sh/ruff/issues/7718
25 | def func(): 25 | def func():

View file

@ -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), [])