Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flake8-comprehension] Strip parentheses around generators in C400 #11607

Merged
merged 1 commit into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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):
Loading