Consider the new f-string tokens for flake8-commas
#8582
Conversation
|
Current dependencies on/for this PR: This stack of pull requests is managed by Graphite. |
dhruvmanila
force-pushed
the
dhruv/fstring-comma
branch
from
781aef7 to
0c12b82
Compare
| struct Token<'tok> { | ||
| type_: TokenType, | ||
| // Underlying token. | ||
| spanned: Option<&'tok Spanned>, | ||
| struct Token { | ||
| r#type: TokenType, | ||
| range: TextRange, |
There was a problem hiding this comment.
The raw token was never used, so let's simplify this a bit.
I've also renamed type_ to r#type. This is just a personal preference and I can revert if the former is better.
| .filter_map(|spanned @ (tok, tok_range)| match tok { | ||
| // Completely ignore comments -- they just interfere with the logic. | ||
| Tok::Comment(_) => None, | ||
| // F-strings are handled as `String` token type with the complete range | ||
| // of the outermost f-string. This means that the expression inside the | ||
| // f-string is not checked for trailing commas. | ||
| Tok::FStringStart => { | ||
| fstrings = fstrings.saturating_add(1); | ||
| None | ||
| } | ||
| Tok::FStringEnd => { | ||
| fstrings = fstrings.saturating_sub(1); | ||
| if fstrings == 0 { | ||
| indexer | ||
| .fstring_ranges() | ||
| .outermost(tok_range.start()) | ||
| .map(|range| Token::new(TokenType::String, range)) | ||
| } else { | ||
| None | ||
| } | ||
| } | ||
| _ => { | ||
| if fstrings == 0 { | ||
| Some(Token::from(spanned)) | ||
| } else { | ||
| None | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
This is the main change.
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| COM819 | 1 | 0 | 1 | 0 | 0 |
Linter (preview)
ℹ️ ecosystem check detected linter changes. (+0 -1 violations, +0 -0 fixes in 41 projects)
sphinx-doc/sphinx (+0 -1 violations, +0 -0 fixes)
ruff check --no-cache --exit-zero --preview
- sphinx/writers/texinfo.py:433:68: COM819 [*] Trailing comma prohibited
Changes by rule (1 rules affected)
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| COM819 | 1 | 0 | 1 | 0 | 0 |
| indexer | ||
| .fstring_ranges() | ||
| .outermost(tok_range.start()) | ||
| .map(|range| Token::new(TokenType::String, range)) |
There was a problem hiding this comment.
We're getting the range on FStringEnd, so the None case isn't possible.
|
The ecosystem change is the fact that we've stopped checking for trailing comma within the expression part of f-strings. |
Summary
This fixes the bug where the
flake8-commasrules weren't taking the new f-string tokens into account.Test Plan
Add new test cases around f-strings for all of
flake8-commas's rules.fixes: #8556