Skip to content

Commit

Permalink
[flake8-comprehensions] strip parentheses around generators in `unn…
Browse files Browse the repository at this point in the history
…ecessary-generator-set` (`C401`) (astral-sh#15553)

<!--
Thank you for contributing to Ruff! To help us out with reviewing,
please consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary

Fixes parentheses not being stripped in C401. Pretty much the same as
astral-sh#11607 which fixed it for C400.

## Test Plan
`cargo nextest run`
  • Loading branch information
wooly18 authored Jan 17, 2025
1 parent 5cdac25 commit 1ba8e61
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 13 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Cannot conbime with C416. Should use set comprehension here.
# Cannot combine with C416. Should use set comprehension here.
even_nums = set(2 * x for x in range(3))
odd_nums = set(
2 * x + 1 for x in range(3)
Expand All @@ -21,6 +21,10 @@ def f(x):
print(f"{set(a for a in 'abc') - set(a for a in 'ab')}")
print(f"{ set(a for a in 'abc') - set(a for a in 'ab') }")

# Strip parentheses from inner generators.
set((2 * x for x in range(3)))
set(((2 * x for x in range(3))))
set((((2 * x for x in range(3)))))

# Not built-in set.
def set(*args, **kwargs):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, ViolationMetadata};
use ruff_python_ast as ast;
use ruff_python_ast::comparable::ComparableExpr;
use ruff_python_ast::parenthesize::parenthesized_range;
use ruff_python_ast::ExprGenerator;
use ruff_text_size::{Ranged, TextSize};

Expand All @@ -27,12 +28,14 @@ use super::helpers;
/// ```python
/// set(f(x) for x in foo)
/// set(x for x in foo)
/// set((x for x in foo))
/// ```
///
/// Use instead:
/// ```python
/// {f(x) for x in foo}
/// set(foo)
/// set(foo)
/// ```
///
/// ## Fix safety
Expand Down Expand Up @@ -74,7 +77,10 @@ pub(crate) fn unnecessary_generator_set(checker: &mut Checker, call: &ast::ExprC
};

let ast::Expr::Generator(ExprGenerator {
elt, generators, ..
elt,
generators,
parenthesized,
..
}) = argument
else {
return;
Expand Down Expand Up @@ -126,7 +132,28 @@ pub(crate) fn unnecessary_generator_set(checker: &mut Checker, call: &ast::ExprC
call.end(),
);

Fix::unsafe_edits(call_start, [call_end])
// Remove the inner parentheses, if the expression is a generator. The easiest way to do
// this reliably is to use the printer.
if *parenthesized {
// The generator's range will include the innermost parentheses, but it could be
// surrounded by additional parentheses.
let range = parenthesized_range(
argument.into(),
(&call.arguments).into(),
checker.comment_ranges(),
checker.locator().contents(),
)
.unwrap_or(argument.range());

// The generator always parenthesizes the expression; trim the parentheses.
let generator = checker.generator().expr(argument);
let generator = generator[1..generator.len() - 1].to_string();

let replacement = Edit::range_replacement(generator, range);
Fix::unsafe_edits(call_start, [call_end, replacement])
} else {
Fix::unsafe_edits(call_start, [call_end])
}
};
checker.diagnostics.push(diagnostic.with_fix(fix));
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ source: crates/ruff_linter/src/rules/flake8_comprehensions/mod.rs
---
C401.py:2:13: C401 [*] Unnecessary generator (rewrite as a set comprehension)
|
1 | # Cannot conbime with C416. Should use set comprehension here.
1 | # Cannot combine with C416. Should use set comprehension here.
2 | even_nums = set(2 * x for x in range(3))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C401
3 | odd_nums = set(
Expand All @@ -12,7 +12,7 @@ C401.py:2:13: C401 [*] Unnecessary generator (rewrite as a set comprehension)
= help: Rewrite as a set comprehension

Unsafe fix
1 1 | # Cannot conbime with C416. Should use set comprehension here.
1 1 | # Cannot combine with C416. Should use set comprehension here.
2 |-even_nums = set(2 * x for x in range(3))
2 |+even_nums = {2 * x for x in range(3)}
3 3 | odd_nums = set(
Expand All @@ -21,7 +21,7 @@ C401.py:2:13: C401 [*] Unnecessary generator (rewrite as a set comprehension)

C401.py:3:12: C401 [*] Unnecessary generator (rewrite as a set comprehension)
|
1 | # Cannot conbime with C416. Should use set comprehension here.
1 | # Cannot combine with C416. Should use set comprehension here.
2 | even_nums = set(2 * x for x in range(3))
3 | odd_nums = set(
| ____________^
Expand All @@ -33,7 +33,7 @@ C401.py:3:12: C401 [*] Unnecessary generator (rewrite as a set comprehension)
= help: Rewrite as a set comprehension

Unsafe fix
1 1 | # Cannot conbime with C416. Should use set comprehension here.
1 1 | # Cannot combine with C416. Should use set comprehension here.
2 2 | even_nums = set(2 * x for x in range(3))
3 |-odd_nums = set(
3 |+odd_nums = {
Expand Down Expand Up @@ -188,7 +188,7 @@ C401.py:21:10: C401 [*] Unnecessary generator (rewrite using `set()`)
21 |+print(f"{set('abc') - set(a for a in 'ab')}")
22 22 | print(f"{ set(a for a in 'abc') - set(a for a in 'ab') }")
23 23 |
24 24 |
24 24 | # Strip parentheses from inner generators.

C401.py:21:34: C401 [*] Unnecessary generator (rewrite using `set()`)
|
Expand All @@ -208,14 +208,16 @@ C401.py:21:34: C401 [*] Unnecessary generator (rewrite using `set()`)
21 |+print(f"{set(a for a in 'abc') - set('ab')}")
22 22 | print(f"{ set(a for a in 'abc') - set(a for a in 'ab') }")
23 23 |
24 24 |
24 24 | # Strip parentheses from inner generators.

C401.py:22:11: C401 [*] Unnecessary generator (rewrite using `set()`)
|
20 | print(f"Hello {set(a for a in range(3))} World")
21 | print(f"{set(a for a in 'abc') - set(a for a in 'ab')}")
22 | print(f"{ set(a for a in 'abc') - set(a for a in 'ab') }")
| ^^^^^^^^^^^^^^^^^^^^^ C401
23 |
24 | # Strip parentheses from inner generators.
|
= help: Rewrite using `set()`

Expand All @@ -226,15 +228,17 @@ C401.py:22:11: C401 [*] Unnecessary generator (rewrite using `set()`)
22 |-print(f"{ set(a for a in 'abc') - set(a for a in 'ab') }")
22 |+print(f"{ set('abc') - set(a for a in 'ab') }")
23 23 |
24 24 |
25 25 | # Not built-in set.
24 24 | # Strip parentheses from inner generators.
25 25 | set((2 * x for x in range(3)))

C401.py:22:35: C401 [*] Unnecessary generator (rewrite using `set()`)
|
20 | print(f"Hello {set(a for a in range(3))} World")
21 | print(f"{set(a for a in 'abc') - set(a for a in 'ab')}")
22 | print(f"{ set(a for a in 'abc') - set(a for a in 'ab') }")
| ^^^^^^^^^^^^^^^^^^^^ C401
23 |
24 | # Strip parentheses from inner generators.
|
= help: Rewrite using `set()`

Expand All @@ -245,5 +249,66 @@ C401.py:22:35: C401 [*] Unnecessary generator (rewrite using `set()`)
22 |-print(f"{ set(a for a in 'abc') - set(a for a in 'ab') }")
22 |+print(f"{ set(a for a in 'abc') - set('ab') }")
23 23 |
24 24 |
25 25 | # Not built-in set.
24 24 | # Strip parentheses from inner generators.
25 25 | set((2 * x for x in range(3)))

C401.py:25:1: C401 [*] Unnecessary generator (rewrite as a set comprehension)
|
24 | # Strip parentheses from inner generators.
25 | set((2 * x for x in range(3)))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C401
26 | set(((2 * x for x in range(3))))
27 | set((((2 * x for x in range(3)))))
|
= help: Rewrite as a set comprehension

Unsafe fix
22 22 | print(f"{ set(a for a in 'abc') - set(a for a in 'ab') }")
23 23 |
24 24 | # Strip parentheses from inner generators.
25 |-set((2 * x for x in range(3)))
25 |+{2 * x for x in range(3)}
26 26 | set(((2 * x for x in range(3))))
27 27 | set((((2 * x for x in range(3)))))
28 28 |

C401.py:26:1: C401 [*] Unnecessary generator (rewrite as a set comprehension)
|
24 | # Strip parentheses from inner generators.
25 | set((2 * x for x in range(3)))
26 | set(((2 * x for x in range(3))))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C401
27 | set((((2 * x for x in range(3)))))
|
= help: Rewrite as a set comprehension

Unsafe fix
23 23 |
24 24 | # Strip parentheses from inner generators.
25 25 | set((2 * x for x in range(3)))
26 |-set(((2 * x for x in range(3))))
26 |+{2 * x for x in range(3)}
27 27 | set((((2 * x for x in range(3)))))
28 28 |
29 29 | # Not built-in set.

C401.py:27:1: C401 [*] Unnecessary generator (rewrite as a set comprehension)
|
25 | set((2 * x for x in range(3)))
26 | set(((2 * x for x in range(3))))
27 | set((((2 * x for x in range(3)))))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C401
28 |
29 | # Not built-in set.
|
= help: Rewrite as a set comprehension

Unsafe fix
24 24 | # Strip parentheses from inner generators.
25 25 | set((2 * x for x in range(3)))
26 26 | set(((2 * x for x in range(3))))
27 |-set((((2 * x for x in range(3)))))
27 |+{2 * x for x in range(3)}
28 28 |
29 29 | # Not built-in set.
30 30 | def set(*args, **kwargs):

0 comments on commit 1ba8e61

Please sign in to comment.