Skip to content

Commit

Permalink
Perform insertions before replacements (#7739)
Browse files Browse the repository at this point in the history
## 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 #7718.
  • Loading branch information
charliermarsh authored Oct 1, 2023
1 parent e91ffe3 commit 1cf3b56
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 11 deletions.
6 changes: 3 additions & 3 deletions crates/ruff_diagnostics/src/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Item = Edit>) -> Self {
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 {
edits,
applicability: Applicability::Automatic,
Expand All @@ -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<Item = Edit>) -> Self {
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 {
edits,
applicability: Applicability::Suggested,
Expand All @@ -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<Item = Edit>) -> Self {
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 {
edits,
applicability: Applicability::Manual,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
sum((factor.dims for factor in bases), [])
3 changes: 2 additions & 1 deletion crates/ruff_linter/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
@@ -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=[])
Expand All @@ -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=[])
Expand All @@ -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], [])
Expand All @@ -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=[])
Expand All @@ -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]], [])
Expand Down Expand Up @@ -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 |
Expand All @@ -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():
Expand Down
Original file line number Diff line number Diff line change
@@ -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), [])


0 comments on commit 1cf3b56

Please sign in to comment.