[flake8-comprehensions] Preserve trailing commas for single-element lists (C409)#19571
[flake8-comprehensions] Preserve trailing commas for single-element lists (C409)#19571ntBre merged 6 commits intoastral-sh:mainfrom
flake8-comprehensions] Preserve trailing commas for single-element lists (C409)#19571Conversation
|
| let has_trailing_comma = { | ||
| // Look for a comma after the argument in the call | ||
| let after_argument = checker | ||
| .locator() | ||
| .slice(TextRange::new(argument.end(), call.end())); | ||
| after_argument.trim_start().starts_with(',') |
There was a problem hiding this comment.
These two checks, needs_trailing_comma and has_trailing_comma aren't mutually exclusive. We may need to either add a new comma (needs_trailing_comma) or preserve it (has_trailing_comma). You can see that deleting needs_trailing_comma was wrong because the t6 test case below has now lost its comma.
I'm wondering if we could reuse the token-based check for has_trailing_comma too, but the main issue first is restoring the old functionality.
ntBre
added
bug
ntBre
changed the title
flake8_comprehensions] Fix C409 to preserve trailing commas in tuple callsflake8-comprehensions] Fix C409 to preserve trailing commas in tuple calls
|
Okay I looked at this much more closely today, and I believe the only diff we needed was this: diff --git a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_literal_within_tuple_call.rs b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_literal_within_tuple_call.rs
index b2d3af263f..30a250e7ce 100644
--- a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_literal_within_tuple_call.rs
+++ b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_literal_within_tuple_call.rs
@@ -124,7 +124,7 @@ pub(crate) fn unnecessary_literal_within_tuple_call(
let needs_trailing_comma = if let [item] = elts.as_slice() {
SimpleTokenizer::new(
checker.locator().contents(),
- TextRange::new(item.end(), call.end()),
+ TextRange::new(item.end(), argument.end()),
)
.all(|token| token.kind != SimpleTokenKind::Comma)
} else {This preserves the comma in We shouldn't check the tokens all the way to the end of the call because those are not preserved. |
| // Check if the original call had a trailing comma after the argument | ||
| let has_trailing_comma = { | ||
| // Look for a comma after the argument in the call | ||
| let after_argument = checker | ||
| .locator() | ||
| .slice(TextRange::new(argument.end(), call.end())); | ||
| after_argument.trim_start().starts_with(',') | ||
| }; | ||
|
|
There was a problem hiding this comment.
You should be able to revert all of this with the change I suggested.
|
@danparizher do you plan to come back to this or should we close this PR? |
ntBre
changed the title
flake8-comprehensions] Fix C409 to preserve trailing commas in tuple callsflake8-comprehensions] Preserve trailing commas for single-element lists (C409)
3 participants
Summary
Fixes #19568