Skip to content

Commit e63dfa3

Browse files
danparizherntBre
andauthored
[flake8-commas] Add support for trailing comma checks in type parameter lists (COM812,COM819) (#19390)
## Summary Fixes #18844 I'm not too sure if the solution is as simple as the way I implemented it, but I'm curious to see if we are covering all cases correctly here. --------- Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com> Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
1 parent 6d0f3ef commit e63dfa3

File tree

7 files changed

+1185
-2
lines changed

7 files changed

+1185
-2
lines changed

crates/ruff_linter/resources/test/fixtures/flake8_commas/COM81.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -650,3 +650,17 @@ def foo(
650650
if True else
651651
"Don't add a trailing comma here ->"
652652
}"""
653+
654+
type X[
655+
T
656+
] = T
657+
def f[
658+
T
659+
](): pass
660+
class C[
661+
T
662+
]: pass
663+
664+
type X[T,] = T
665+
def f[T,](): pass
666+
class C[T,]: pass

crates/ruff_linter/src/checkers/tokens.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ pub(crate) fn check_tokens(
118118
Rule::TrailingCommaOnBareTuple,
119119
Rule::ProhibitedTrailingComma,
120120
]) {
121-
flake8_commas::rules::trailing_commas(context, tokens, locator, indexer);
121+
flake8_commas::rules::trailing_commas(context, tokens, locator, indexer, settings);
122122
}
123123

124124
if context.is_rule_enabled(Rule::ExtraneousParentheses) {

crates/ruff_linter/src/preview.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,3 +225,8 @@ pub(crate) const fn is_assert_raises_exception_call_enabled(settings: &LinterSet
225225
pub(crate) const fn is_add_future_annotations_imports_enabled(settings: &LinterSettings) -> bool {
226226
settings.preview.is_enabled()
227227
}
228+
229+
// https://github.com/astral-sh/ruff/pull/19390
230+
pub(crate) const fn is_trailing_comma_type_params_enabled(settings: &LinterSettings) -> bool {
231+
settings.preview.is_enabled()
232+
}

crates/ruff_linter/src/rules/flake8_commas/mod.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,23 @@ mod tests {
2727
assert_diagnostics!(snapshot, diagnostics);
2828
Ok(())
2929
}
30+
31+
#[test_case(Path::new("COM81.py"))]
32+
#[test_case(Path::new("COM81_syntax_error.py"))]
33+
fn preview_rules(path: &Path) -> Result<()> {
34+
let snapshot = format!("preview__{}", path.to_string_lossy());
35+
let diagnostics = test_path(
36+
Path::new("flake8_commas").join(path).as_path(),
37+
&settings::LinterSettings {
38+
preview: crate::settings::types::PreviewMode::Enabled,
39+
..settings::LinterSettings::for_rules(vec![
40+
Rule::MissingTrailingComma,
41+
Rule::TrailingCommaOnBareTuple,
42+
Rule::ProhibitedTrailingComma,
43+
])
44+
},
45+
)?;
46+
assert_diagnostics!(snapshot, diagnostics);
47+
Ok(())
48+
}
3049
}

crates/ruff_linter/src/rules/flake8_commas/rules/trailing_commas.rs

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ use ruff_text_size::{Ranged, TextRange};
55

66
use crate::Locator;
77
use crate::checkers::ast::LintContext;
8+
use crate::preview::is_trailing_comma_type_params_enabled;
9+
use crate::settings::LinterSettings;
810
use crate::{AlwaysFixableViolation, Violation};
911
use crate::{Edit, Fix};
1012

@@ -24,6 +26,8 @@ enum TokenType {
2426
Def,
2527
For,
2628
Lambda,
29+
Class,
30+
Type,
2731
Irrelevant,
2832
}
2933

@@ -69,6 +73,8 @@ impl From<(TokenKind, TextRange)> for SimpleToken {
6973
TokenKind::Lbrace => TokenType::OpeningCurlyBracket,
7074
TokenKind::Rbrace => TokenType::ClosingBracket,
7175
TokenKind::Def => TokenType::Def,
76+
TokenKind::Class => TokenType::Class,
77+
TokenKind::Type => TokenType::Type,
7278
TokenKind::For => TokenType::For,
7379
TokenKind::Lambda => TokenType::Lambda,
7480
// Import treated like a function.
@@ -98,6 +104,8 @@ enum ContextType {
98104
Dict,
99105
/// Lambda parameter list, e.g. `lambda a, b`.
100106
LambdaParameters,
107+
/// Type parameter list, e.g. `def foo[T, U](): ...`
108+
TypeParameters,
101109
}
102110

103111
/// Comma context - described a comma-delimited "situation".
@@ -243,6 +251,7 @@ pub(crate) fn trailing_commas(
243251
tokens: &Tokens,
244252
locator: &Locator,
245253
indexer: &Indexer,
254+
settings: &LinterSettings,
246255
) {
247256
let mut fstrings = 0u32;
248257
let simple_tokens = tokens.iter().filter_map(|token| {
@@ -290,7 +299,7 @@ pub(crate) fn trailing_commas(
290299
}
291300

292301
// Update the comma context stack.
293-
let context = update_context(token, prev, prev_prev, &mut stack);
302+
let context = update_context(token, prev, prev_prev, &mut stack, settings);
294303

295304
check_token(token, prev, prev_prev, context, locator, lint_context);
296305

@@ -326,6 +335,7 @@ fn check_token(
326335
ContextType::No => false,
327336
ContextType::FunctionParameters => true,
328337
ContextType::CallArguments => true,
338+
ContextType::TypeParameters => true,
329339
// `(1)` is not equivalent to `(1,)`.
330340
ContextType::Tuple => context.num_commas != 0,
331341
// `x[1]` is not equivalent to `x[1,]`.
@@ -408,6 +418,7 @@ fn update_context(
408418
prev: SimpleToken,
409419
prev_prev: SimpleToken,
410420
stack: &mut Vec<Context>,
421+
settings: &LinterSettings,
411422
) -> Context {
412423
let new_context = match token.ty {
413424
TokenType::OpeningBracket => match (prev.ty, prev_prev.ty) {
@@ -417,6 +428,17 @@ fn update_context(
417428
}
418429
_ => Context::new(ContextType::Tuple),
419430
},
431+
TokenType::OpeningSquareBracket if is_trailing_comma_type_params_enabled(settings) => {
432+
match (prev.ty, prev_prev.ty) {
433+
(TokenType::Named, TokenType::Def | TokenType::Class | TokenType::Type) => {
434+
Context::new(ContextType::TypeParameters)
435+
}
436+
(TokenType::ClosingBracket | TokenType::Named | TokenType::String, _) => {
437+
Context::new(ContextType::Subscript)
438+
}
439+
_ => Context::new(ContextType::List),
440+
}
441+
}
420442
TokenType::OpeningSquareBracket => match prev.ty {
421443
TokenType::ClosingBracket | TokenType::Named | TokenType::String => {
422444
Context::new(ContextType::Subscript)

0 commit comments

Comments
 (0)