Skip to content

Commit

Permalink
[flake8-comprehension] Strip parentheses around generators in C400 (#…
Browse files Browse the repository at this point in the history
…11607)

## Summary

Closes #11603.
  • Loading branch information
charliermarsh committed May 30, 2024
1 parent e35deee commit a8d1328
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@
x for x in range(3)
)

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

# Not built-in list.
def list(*args, **kwargs):
return None
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, violation};
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 @@ -26,12 +27,14 @@ use super::helpers;
/// ```python
/// list(f(x) for x in foo)
/// list(x for x in foo)
/// list((x for x in foo))
/// ```
///
/// Use instead:
/// ```python
/// [f(x) for x in foo]
/// list(foo)
/// list(foo)
/// ```
///
/// ## Fix safety
Expand Down Expand Up @@ -76,7 +79,10 @@ pub(crate) fn unnecessary_generator_list(checker: &mut Checker, call: &ast::Expr
}

let Some(ExprGenerator {
elt, generators, ..
elt,
generators,
parenthesized,
..
}) = argument.as_generator_expr()
else {
return;
Expand Down Expand Up @@ -125,7 +131,28 @@ pub(crate) fn unnecessary_generator_list(checker: &mut Checker, call: &ast::Expr
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.indexer().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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ C400.py:10:5: C400 [*] Unnecessary generator (rewrite using `list()`)
12 | | )
| |_^ C400
13 |
14 | # Not built-in list.
14 | # Strip parentheses from inner generators.
|
= help: Rewrite using `list()`

Expand All @@ -86,5 +86,66 @@ C400.py:10:5: C400 [*] Unnecessary generator (rewrite using `list()`)
12 |-)
10 |+x = list(range(3))
13 11 |
14 12 | # Not built-in list.
15 13 | def list(*args, **kwargs):
14 12 | # Strip parentheses from inner generators.
15 13 | list((2 * x for x in range(3)))

C400.py:15:1: C400 [*] Unnecessary generator (rewrite as a `list` comprehension)
|
14 | # Strip parentheses from inner generators.
15 | list((2 * x for x in range(3)))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C400
16 | list(((2 * x for x in range(3))))
17 | list((((2 * x for x in range(3)))))
|
= help: Rewrite as a `list` comprehension

Unsafe fix
12 12 | )
13 13 |
14 14 | # Strip parentheses from inner generators.
15 |-list((2 * x for x in range(3)))
15 |+[2 * x for x in range(3)]
16 16 | list(((2 * x for x in range(3))))
17 17 | list((((2 * x for x in range(3)))))
18 18 |

C400.py:16:1: C400 [*] Unnecessary generator (rewrite as a `list` comprehension)
|
14 | # Strip parentheses from inner generators.
15 | list((2 * x for x in range(3)))
16 | list(((2 * x for x in range(3))))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C400
17 | list((((2 * x for x in range(3)))))
|
= help: Rewrite as a `list` comprehension

Unsafe fix
13 13 |
14 14 | # Strip parentheses from inner generators.
15 15 | list((2 * x for x in range(3)))
16 |-list(((2 * x for x in range(3))))
16 |+[2 * x for x in range(3)]
17 17 | list((((2 * x for x in range(3)))))
18 18 |
19 19 | # Not built-in list.

C400.py:17:1: C400 [*] Unnecessary generator (rewrite as a `list` comprehension)
|
15 | list((2 * x for x in range(3)))
16 | list(((2 * x for x in range(3))))
17 | list((((2 * x for x in range(3)))))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C400
18 |
19 | # Not built-in list.
|
= help: Rewrite as a `list` comprehension

Unsafe fix
14 14 | # Strip parentheses from inner generators.
15 15 | list((2 * x for x in range(3)))
16 16 | list(((2 * x for x in range(3))))
17 |-list((((2 * x for x in range(3)))))
17 |+[2 * x for x in range(3)]
18 18 |
19 19 | # Not built-in list.
20 20 | def list(*args, **kwargs):

0 comments on commit a8d1328

Please sign in to comment.