Skip to content

Commit df45a9d

Browse files
ayushbawejadylwil3
andauthored
[flake8-comprehensions]: Handle trailing comma in C403 fix (#16110)
## Summary Resolves [#16099 ](#16099) based on [#15929 ](#15929) ## Test Plan Added test case `s = set([x for x in range(3)],)` and updated snapshot. --------- Co-authored-by: dylwil3 <[email protected]>
1 parent 3c69b68 commit df45a9d

File tree

5 files changed

+51
-14
lines changed

5 files changed

+51
-14
lines changed

crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C403.py

+3
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,6 @@ def f(x):
3232
[ # comprehension comment
3333
x for x in range(3)]
3434
))))
35+
36+
# Test trailing comma case
37+
s = set([x for x in range(3)],)

crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_generator_list.rs

+8-6
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use ruff_python_ast as ast;
44
use ruff_python_ast::comparable::ComparableExpr;
55
use ruff_python_ast::parenthesize::parenthesized_range;
66
use ruff_python_ast::ExprGenerator;
7-
use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
7+
use ruff_python_parser::TokenKind;
88
use ruff_text_size::{Ranged, TextRange, TextSize};
99

1010
use crate::checkers::ast::Checker;
@@ -125,11 +125,13 @@ pub(crate) fn unnecessary_generator_list(checker: &Checker, call: &ast::ExprCall
125125

126126
// Replace `)` with `]`.
127127
// Place `]` at argument's end or at trailing comma if present
128-
let mut tokenizer =
129-
SimpleTokenizer::new(checker.source(), TextRange::new(argument.end(), call.end()));
130-
let right_bracket_loc = tokenizer
131-
.find(|token| token.kind == SimpleTokenKind::Comma)
132-
.map_or(call.arguments.end(), |comma| comma.end())
128+
let after_arg_tokens = checker
129+
.tokens()
130+
.in_range(TextRange::new(argument.end(), call.end()));
131+
let right_bracket_loc = after_arg_tokens
132+
.iter()
133+
.find(|token| token.kind() == TokenKind::Comma)
134+
.map_or(call.arguments.end(), Ranged::end)
133135
- TextSize::from(1);
134136
let call_end = Edit::replacement("]".to_string(), right_bracket_loc, call.end());
135137

crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_generator_set.rs

+8-6
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use ruff_python_ast as ast;
44
use ruff_python_ast::comparable::ComparableExpr;
55
use ruff_python_ast::parenthesize::parenthesized_range;
66
use ruff_python_ast::ExprGenerator;
7-
use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
7+
use ruff_python_parser::TokenKind;
88
use ruff_text_size::{Ranged, TextRange, TextSize};
99

1010
use crate::checkers::ast::Checker;
@@ -128,11 +128,13 @@ pub(crate) fn unnecessary_generator_set(checker: &Checker, call: &ast::ExprCall)
128128

129129
// Replace `)` with `}`.
130130
// Place `}` at argument's end or at trailing comma if present
131-
let mut tokenizer =
132-
SimpleTokenizer::new(checker.source(), TextRange::new(argument.end(), call.end()));
133-
let right_brace_loc = tokenizer
134-
.find(|token| token.kind == SimpleTokenKind::Comma)
135-
.map_or(call.arguments.end(), |comma| comma.end())
131+
let after_arg_tokens = checker
132+
.tokens()
133+
.in_range(TextRange::new(argument.end(), call.end()));
134+
let right_brace_loc = after_arg_tokens
135+
.iter()
136+
.find(|token| token.kind() == TokenKind::Comma)
137+
.map_or(call.arguments.end(), Ranged::end)
136138
- TextSize::from(1);
137139
let call_end = Edit::replacement(
138140
pad_end("}", call.range(), checker.locator(), checker.semantic()),

crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_list_comprehension_set.rs

+12-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
22
use ruff_macros::{derive_message_formats, ViolationMetadata};
33
use ruff_python_ast as ast;
44
use ruff_python_ast::parenthesize::parenthesized_range;
5-
use ruff_text_size::{Ranged, TextSize};
5+
use ruff_python_parser::TokenKind;
6+
use ruff_text_size::{Ranged, TextRange, TextSize};
67

78
use crate::checkers::ast::Checker;
89
use crate::rules::flake8_comprehensions::fixes::{pad_end, pad_start};
@@ -70,9 +71,18 @@ pub(crate) fn unnecessary_list_comprehension_set(checker: &Checker, call: &ast::
7071
);
7172

7273
// Replace `)` with `}`.
74+
// Place `}` at argument's end or at trailing comma if present
75+
let after_arg_tokens = checker
76+
.tokens()
77+
.in_range(TextRange::new(argument.end(), call.end()));
78+
let right_brace_loc = after_arg_tokens
79+
.iter()
80+
.find(|token| token.kind() == TokenKind::Comma)
81+
.map_or(call.arguments.end() - one, |comma| comma.end() - one);
82+
7383
let call_end = Edit::replacement(
7484
pad_end("}", call.range(), checker.locator(), checker.semantic()),
75-
call.arguments.end() - one,
85+
right_brace_loc,
7686
call.end(),
7787
);
7888

crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C403_C403.py.snap

+20
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,8 @@ C403.py:29:5: C403 [*] Unnecessary list comprehension (rewrite as a set comprehe
292292
33 | | x for x in range(3)]
293293
34 | | ))))
294294
| |_____^ C403
295+
35 |
296+
36 | # Test trailing comma case
295297
|
296298
= help: Rewrite as a set comprehension
297299

@@ -308,3 +310,21 @@ C403.py:29:5: C403 [*] Unnecessary list comprehension (rewrite as a set comprehe
308310
29 |+s = { # outer set comment
309311
30 |+ # comprehension comment
310312
31 |+ x for x in range(3)}
313+
35 32 |
314+
36 33 | # Test trailing comma case
315+
37 34 | s = set([x for x in range(3)],)
316+
317+
C403.py:37:5: C403 [*] Unnecessary list comprehension (rewrite as a set comprehension)
318+
|
319+
36 | # Test trailing comma case
320+
37 | s = set([x for x in range(3)],)
321+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ C403
322+
|
323+
= help: Rewrite as a set comprehension
324+
325+
Unsafe fix
326+
34 34 | ))))
327+
35 35 |
328+
36 36 | # Test trailing comma case
329+
37 |-s = set([x for x in range(3)],)
330+
37 |+s = {x for x in range(3)}

0 commit comments

Comments
 (0)