[flake8-comprehensions] Handle trailing comma in fixes for unnecessary-generator-list/set (C400,C401) (#15929)

The unsafe fixes for the rules [unnecessary-generator-list
(C400)](https://docs.astral.sh/ruff/rules/unnecessary-generator-list/#unnecessary-generator-list-c400)
and [unnecessary-generator-set
(C401)](https://docs.astral.sh/ruff/rules/unnecessary-generator-set/#unnecessary-generator-set-c401)
used to introduce syntax errors if the argument to `list` or `set` had a
trailing comma, because the fix would retain the comma after
transforming the function call to a comprehension.

This PR accounts for the trailing comma when replacing the end of the
call with a `]` or `}`.

Closes #15852
This commit is contained in:
Dylan 2025-02-05 07:38:03 -06:00 committed by GitHub
parent 076d35fb93
commit c69b19fe1d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 155 additions and 16 deletions

View file

@ -16,6 +16,17 @@ list((2 * x for x in range(3)))
list(((2 * x for x in range(3))))
list((((2 * x for x in range(3)))))
# Account for trailing comma in fix
# See https://github.com/astral-sh/ruff/issues/15852
list((0 for _ in []),)
list(
(0 for _ in [])
# some comments
,
# some more
)
# Not built-in list.
def list(*args, **kwargs):
return None

View file

@ -26,6 +26,16 @@ set((2 * x for x in range(3)))
set(((2 * x for x in range(3))))
set((((2 * x for x in range(3)))))
# Account for trailing comma in fix
# See https://github.com/astral-sh/ruff/issues/15852
set((0 for _ in []),)
set(
(0 for _ in [])
# some comments
,
# some more
)
# Not built-in set.
def set(*args, **kwargs):
return None

View file

@ -4,7 +4,8 @@ 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};
use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
use ruff_text_size::{Ranged, TextRange, TextSize};
use crate::checkers::ast::Checker;
@ -123,11 +124,14 @@ pub(crate) fn unnecessary_generator_list(checker: &mut Checker, call: &ast::Expr
);
// Replace `)` with `]`.
let call_end = Edit::replacement(
"]".to_string(),
call.arguments.end() - TextSize::from(1),
call.end(),
);
// Place `]` at argument's end or at trailing comma if present
let mut tokenizer =
SimpleTokenizer::new(checker.source(), TextRange::new(argument.end(), call.end()));
let right_bracket_loc = tokenizer
.find(|token| token.kind == SimpleTokenKind::Comma)
.map_or(call.arguments.end(), |comma| comma.end())
- TextSize::from(1);
let call_end = Edit::replacement("]".to_string(), right_bracket_loc, call.end());
// Remove the inner parentheses, if the expression is a generator. The easiest way to do
// this reliably is to use the printer.

View file

@ -4,7 +4,8 @@ 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};
use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
use ruff_text_size::{Ranged, TextRange, TextSize};
use crate::checkers::ast::Checker;
use crate::rules::flake8_comprehensions::fixes::{pad_end, pad_start};
@ -126,9 +127,16 @@ pub(crate) fn unnecessary_generator_set(checker: &mut Checker, call: &ast::ExprC
);
// Replace `)` with `}`.
// Place `}` at argument's end or at trailing comma if present
let mut tokenizer =
SimpleTokenizer::new(checker.source(), TextRange::new(argument.end(), call.end()));
let right_brace_loc = tokenizer
.find(|token| token.kind == SimpleTokenKind::Comma)
.map_or(call.arguments.end(), |comma| comma.end())
- TextSize::from(1);
let call_end = Edit::replacement(
pad_end("}", call.range(), checker.locator(), checker.semantic()),
call.arguments.end() - TextSize::from(1),
right_brace_loc,
call.end(),
);

View file

@ -127,7 +127,7 @@ C400.py:16:1: C400 [*] Unnecessary generator (rewrite as a list comprehension)
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.
19 19 | # Account for trailing comma in fix
C400.py:17:1: C400 [*] Unnecessary generator (rewrite as a list comprehension)
|
@ -136,7 +136,7 @@ C400.py:17:1: C400 [*] Unnecessary generator (rewrite as a list comprehension)
17 | list((((2 * x for x in range(3)))))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C400
18 |
19 | # Not built-in list.
19 | # Account for trailing comma in fix
|
= help: Rewrite as a list comprehension
@ -147,5 +147,57 @@ C400.py:17:1: C400 [*] Unnecessary generator (rewrite as a list comprehension)
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):
19 19 | # Account for trailing comma in fix
20 20 | # See https://github.com/astral-sh/ruff/issues/15852
C400.py:21:1: C400 [*] Unnecessary generator (rewrite as a list comprehension)
|
19 | # Account for trailing comma in fix
20 | # See https://github.com/astral-sh/ruff/issues/15852
21 | list((0 for _ in []),)
| ^^^^^^^^^^^^^^^^^^^^^^ C400
22 | list(
23 | (0 for _ in [])
|
= help: Rewrite as a list comprehension
Unsafe fix
18 18 |
19 19 | # Account for trailing comma in fix
20 20 | # See https://github.com/astral-sh/ruff/issues/15852
21 |-list((0 for _ in []),)
21 |+[0 for _ in []]
22 22 | list(
23 23 | (0 for _ in [])
24 24 | # some comments
C400.py:22:1: C400 [*] Unnecessary generator (rewrite as a list comprehension)
|
20 | # See https://github.com/astral-sh/ruff/issues/15852
21 | list((0 for _ in []),)
22 | / list(
23 | | (0 for _ in [])
24 | | # some comments
25 | | ,
26 | | # some more
27 | | )
| |__^ C400
|
= help: Rewrite as a list comprehension
Unsafe fix
19 19 | # Account for trailing comma in fix
20 20 | # See https://github.com/astral-sh/ruff/issues/15852
21 21 | list((0 for _ in []),)
22 |-list(
23 |- (0 for _ in [])
22 |+[
23 |+ 0 for _ in []
24 24 | # some comments
25 |- ,
26 |- # some more
27 |- )
25 |+ ]
28 26 |
29 27 |
30 28 | # Not built-in list.

View file

@ -290,7 +290,7 @@ C401.py:26:1: C401 [*] Unnecessary generator (rewrite as a set comprehension)
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.
29 29 | # Account for trailing comma in fix
C401.py:27:1: C401 [*] Unnecessary generator (rewrite as a set comprehension)
|
@ -299,7 +299,7 @@ C401.py:27:1: C401 [*] Unnecessary generator (rewrite as a set comprehension)
27 | set((((2 * x for x in range(3)))))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C401
28 |
29 | # Not built-in set.
29 | # Account for trailing comma in fix
|
= help: Rewrite as a set comprehension
@ -310,5 +310,59 @@ C401.py:27:1: C401 [*] Unnecessary generator (rewrite as a set comprehension)
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):
29 29 | # Account for trailing comma in fix
30 30 | # See https://github.com/astral-sh/ruff/issues/15852
C401.py:31:1: C401 [*] Unnecessary generator (rewrite as a set comprehension)
|
29 | # Account for trailing comma in fix
30 | # See https://github.com/astral-sh/ruff/issues/15852
31 | set((0 for _ in []),)
| ^^^^^^^^^^^^^^^^^^^^^ C401
32 | set(
33 | (0 for _ in [])
|
= help: Rewrite as a set comprehension
Unsafe fix
28 28 |
29 29 | # Account for trailing comma in fix
30 30 | # See https://github.com/astral-sh/ruff/issues/15852
31 |-set((0 for _ in []),)
31 |+{0 for _ in []}
32 32 | set(
33 33 | (0 for _ in [])
34 34 | # some comments
C401.py:32:1: C401 [*] Unnecessary generator (rewrite as a set comprehension)
|
30 | # See https://github.com/astral-sh/ruff/issues/15852
31 | set((0 for _ in []),)
32 | / set(
33 | | (0 for _ in [])
34 | | # some comments
35 | | ,
36 | | # some more
37 | | )
| |_^ C401
38 |
39 | # Not built-in set.
|
= help: Rewrite as a set comprehension
Unsafe fix
29 29 | # Account for trailing comma in fix
30 30 | # See https://github.com/astral-sh/ruff/issues/15852
31 31 | set((0 for _ in []),)
32 |-set(
33 |- (0 for _ in [])
32 |+{
33 |+ 0 for _ in []
34 34 | # some comments
35 |- ,
36 |- # some more
37 |-)
35 |+ }
38 36 |
39 37 | # Not built-in set.
40 38 | def set(*args, **kwargs):