mirror of
				https://github.com/astral-sh/ruff.git
				synced 2025-10-30 03:27:07 +00:00 
			
		
		
		
	[flake8-commas] Add support for trailing comma checks in type parameter lists (COM812,COM819) (#19390)
	
		
			
	
		
	
	
		
	
		
			Some checks are pending
		
		
	
	
		
			
				
	
				CI / Determine changes (push) Waiting to run
				
			
		
			
				
	
				CI / cargo fmt (push) Waiting to run
				
			
		
			
				
	
				CI / cargo clippy (push) Blocked by required conditions
				
			
		
			
				
	
				CI / cargo test (linux) (push) Blocked by required conditions
				
			
		
			
				
	
				CI / cargo test (linux, release) (push) Blocked by required conditions
				
			
		
			
				
	
				CI / cargo test (windows) (push) Blocked by required conditions
				
			
		
			
				
	
				CI / cargo test (wasm) (push) Blocked by required conditions
				
			
		
			
				
	
				CI / cargo build (release) (push) Waiting to run
				
			
		
			
				
	
				CI / cargo build (msrv) (push) Blocked by required conditions
				
			
		
			
				
	
				CI / cargo fuzz build (push) Blocked by required conditions
				
			
		
			
				
	
				CI / fuzz parser (push) Blocked by required conditions
				
			
		
			
				
	
				CI / test scripts (push) Blocked by required conditions
				
			
		
			
				
	
				CI / ecosystem (push) Blocked by required conditions
				
			
		
			
				
	
				CI / Fuzz for new ty panics (push) Blocked by required conditions
				
			
		
			
				
	
				CI / cargo shear (push) Blocked by required conditions
				
			
		
			
				
	
				CI / python package (push) Waiting to run
				
			
		
			
				
	
				CI / pre-commit (push) Waiting to run
				
			
		
			
				
	
				CI / mkdocs (push) Waiting to run
				
			
		
			
				
	
				CI / formatter instabilities and black similarity (push) Blocked by required conditions
				
			
		
			
				
	
				CI / test ruff-lsp (push) Blocked by required conditions
				
			
		
			
				
	
				CI / check playground (push) Blocked by required conditions
				
			
		
			
				
	
				CI / benchmarks-instrumented (push) Blocked by required conditions
				
			
		
			
				
	
				CI / benchmarks-walltime (push) Blocked by required conditions
				
			
		
			
				
	
				[ty Playground] Release / publish (push) Waiting to run
				
			
		
		
	
	
				
					
				
			
		
			Some checks are pending
		
		
	
	CI / Determine changes (push) Waiting to run
				
			CI / cargo fmt (push) Waiting to run
				
			CI / cargo clippy (push) Blocked by required conditions
				
			CI / cargo test (linux) (push) Blocked by required conditions
				
			CI / cargo test (linux, release) (push) Blocked by required conditions
				
			CI / cargo test (windows) (push) Blocked by required conditions
				
			CI / cargo test (wasm) (push) Blocked by required conditions
				
			CI / cargo build (release) (push) Waiting to run
				
			CI / cargo build (msrv) (push) Blocked by required conditions
				
			CI / cargo fuzz build (push) Blocked by required conditions
				
			CI / fuzz parser (push) Blocked by required conditions
				
			CI / test scripts (push) Blocked by required conditions
				
			CI / ecosystem (push) Blocked by required conditions
				
			CI / Fuzz for new ty panics (push) Blocked by required conditions
				
			CI / cargo shear (push) Blocked by required conditions
				
			CI / python package (push) Waiting to run
				
			CI / pre-commit (push) Waiting to run
				
			CI / mkdocs (push) Waiting to run
				
			CI / formatter instabilities and black similarity (push) Blocked by required conditions
				
			CI / test ruff-lsp (push) Blocked by required conditions
				
			CI / check playground (push) Blocked by required conditions
				
			CI / benchmarks-instrumented (push) Blocked by required conditions
				
			CI / benchmarks-walltime (push) Blocked by required conditions
				
			[ty Playground] Release / publish (push) Waiting to run
				
			## 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>
This commit is contained in:
		
							parent
							
								
									6d0f3ef3a5
								
							
						
					
					
						commit
						e63dfa3d18
					
				
					 7 changed files with 1185 additions and 2 deletions
				
			
		|  | @ -650,3 +650,17 @@ f"""This is a test. { | ||||||
|     if True else |     if True else | ||||||
|     "Don't add a trailing comma here ->" |     "Don't add a trailing comma here ->" | ||||||
| }""" | }""" | ||||||
|  | 
 | ||||||
|  | type X[ | ||||||
|  |     T | ||||||
|  | ] = T | ||||||
|  | def f[ | ||||||
|  |     T | ||||||
|  | ](): pass | ||||||
|  | class C[ | ||||||
|  |     T | ||||||
|  | ]: pass | ||||||
|  | 
 | ||||||
|  | type X[T,] = T | ||||||
|  | def f[T,](): pass | ||||||
|  | class C[T,]: pass | ||||||
|  | @ -118,7 +118,7 @@ pub(crate) fn check_tokens( | ||||||
|         Rule::TrailingCommaOnBareTuple, |         Rule::TrailingCommaOnBareTuple, | ||||||
|         Rule::ProhibitedTrailingComma, |         Rule::ProhibitedTrailingComma, | ||||||
|     ]) { |     ]) { | ||||||
|         flake8_commas::rules::trailing_commas(context, tokens, locator, indexer); |         flake8_commas::rules::trailing_commas(context, tokens, locator, indexer, settings); | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     if context.is_rule_enabled(Rule::ExtraneousParentheses) { |     if context.is_rule_enabled(Rule::ExtraneousParentheses) { | ||||||
|  |  | ||||||
|  | @ -225,3 +225,8 @@ pub(crate) const fn is_assert_raises_exception_call_enabled(settings: &LinterSet | ||||||
| pub(crate) const fn is_add_future_annotations_imports_enabled(settings: &LinterSettings) -> bool { | pub(crate) const fn is_add_future_annotations_imports_enabled(settings: &LinterSettings) -> bool { | ||||||
|     settings.preview.is_enabled() |     settings.preview.is_enabled() | ||||||
| } | } | ||||||
|  | 
 | ||||||
|  | // https://github.com/astral-sh/ruff/pull/19390
 | ||||||
|  | pub(crate) const fn is_trailing_comma_type_params_enabled(settings: &LinterSettings) -> bool { | ||||||
|  |     settings.preview.is_enabled() | ||||||
|  | } | ||||||
|  |  | ||||||
|  | @ -27,4 +27,23 @@ mod tests { | ||||||
|         assert_diagnostics!(snapshot, diagnostics); |         assert_diagnostics!(snapshot, diagnostics); | ||||||
|         Ok(()) |         Ok(()) | ||||||
|     } |     } | ||||||
|  | 
 | ||||||
|  |     #[test_case(Path::new("COM81.py"))] | ||||||
|  |     #[test_case(Path::new("COM81_syntax_error.py"))] | ||||||
|  |     fn preview_rules(path: &Path) -> Result<()> { | ||||||
|  |         let snapshot = format!("preview__{}", path.to_string_lossy()); | ||||||
|  |         let diagnostics = test_path( | ||||||
|  |             Path::new("flake8_commas").join(path).as_path(), | ||||||
|  |             &settings::LinterSettings { | ||||||
|  |                 preview: crate::settings::types::PreviewMode::Enabled, | ||||||
|  |                 ..settings::LinterSettings::for_rules(vec![ | ||||||
|  |                     Rule::MissingTrailingComma, | ||||||
|  |                     Rule::TrailingCommaOnBareTuple, | ||||||
|  |                     Rule::ProhibitedTrailingComma, | ||||||
|  |                 ]) | ||||||
|  |             }, | ||||||
|  |         )?; | ||||||
|  |         assert_diagnostics!(snapshot, diagnostics); | ||||||
|  |         Ok(()) | ||||||
|  |     } | ||||||
| } | } | ||||||
|  |  | ||||||
|  | @ -5,6 +5,8 @@ use ruff_text_size::{Ranged, TextRange}; | ||||||
| 
 | 
 | ||||||
| use crate::Locator; | use crate::Locator; | ||||||
| use crate::checkers::ast::LintContext; | use crate::checkers::ast::LintContext; | ||||||
|  | use crate::preview::is_trailing_comma_type_params_enabled; | ||||||
|  | use crate::settings::LinterSettings; | ||||||
| use crate::{AlwaysFixableViolation, Violation}; | use crate::{AlwaysFixableViolation, Violation}; | ||||||
| use crate::{Edit, Fix}; | use crate::{Edit, Fix}; | ||||||
| 
 | 
 | ||||||
|  | @ -24,6 +26,8 @@ enum TokenType { | ||||||
|     Def, |     Def, | ||||||
|     For, |     For, | ||||||
|     Lambda, |     Lambda, | ||||||
|  |     Class, | ||||||
|  |     Type, | ||||||
|     Irrelevant, |     Irrelevant, | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | @ -69,6 +73,8 @@ impl From<(TokenKind, TextRange)> for SimpleToken { | ||||||
|             TokenKind::Lbrace => TokenType::OpeningCurlyBracket, |             TokenKind::Lbrace => TokenType::OpeningCurlyBracket, | ||||||
|             TokenKind::Rbrace => TokenType::ClosingBracket, |             TokenKind::Rbrace => TokenType::ClosingBracket, | ||||||
|             TokenKind::Def => TokenType::Def, |             TokenKind::Def => TokenType::Def, | ||||||
|  |             TokenKind::Class => TokenType::Class, | ||||||
|  |             TokenKind::Type => TokenType::Type, | ||||||
|             TokenKind::For => TokenType::For, |             TokenKind::For => TokenType::For, | ||||||
|             TokenKind::Lambda => TokenType::Lambda, |             TokenKind::Lambda => TokenType::Lambda, | ||||||
|             // Import treated like a function.
 |             // Import treated like a function.
 | ||||||
|  | @ -98,6 +104,8 @@ enum ContextType { | ||||||
|     Dict, |     Dict, | ||||||
|     /// Lambda parameter list, e.g. `lambda a, b`.
 |     /// Lambda parameter list, e.g. `lambda a, b`.
 | ||||||
|     LambdaParameters, |     LambdaParameters, | ||||||
|  |     /// Type parameter list, e.g. `def foo[T, U](): ...`
 | ||||||
|  |     TypeParameters, | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| /// Comma context - described a comma-delimited "situation".
 | /// Comma context - described a comma-delimited "situation".
 | ||||||
|  | @ -243,6 +251,7 @@ pub(crate) fn trailing_commas( | ||||||
|     tokens: &Tokens, |     tokens: &Tokens, | ||||||
|     locator: &Locator, |     locator: &Locator, | ||||||
|     indexer: &Indexer, |     indexer: &Indexer, | ||||||
|  |     settings: &LinterSettings, | ||||||
| ) { | ) { | ||||||
|     let mut fstrings = 0u32; |     let mut fstrings = 0u32; | ||||||
|     let simple_tokens = tokens.iter().filter_map(|token| { |     let simple_tokens = tokens.iter().filter_map(|token| { | ||||||
|  | @ -290,7 +299,7 @@ pub(crate) fn trailing_commas( | ||||||
|         } |         } | ||||||
| 
 | 
 | ||||||
|         // Update the comma context stack.
 |         // Update the comma context stack.
 | ||||||
|         let context = update_context(token, prev, prev_prev, &mut stack); |         let context = update_context(token, prev, prev_prev, &mut stack, settings); | ||||||
| 
 | 
 | ||||||
|         check_token(token, prev, prev_prev, context, locator, lint_context); |         check_token(token, prev, prev_prev, context, locator, lint_context); | ||||||
| 
 | 
 | ||||||
|  | @ -326,6 +335,7 @@ fn check_token( | ||||||
|             ContextType::No => false, |             ContextType::No => false, | ||||||
|             ContextType::FunctionParameters => true, |             ContextType::FunctionParameters => true, | ||||||
|             ContextType::CallArguments => true, |             ContextType::CallArguments => true, | ||||||
|  |             ContextType::TypeParameters => true, | ||||||
|             // `(1)` is not equivalent to `(1,)`.
 |             // `(1)` is not equivalent to `(1,)`.
 | ||||||
|             ContextType::Tuple => context.num_commas != 0, |             ContextType::Tuple => context.num_commas != 0, | ||||||
|             // `x[1]` is not equivalent to `x[1,]`.
 |             // `x[1]` is not equivalent to `x[1,]`.
 | ||||||
|  | @ -408,6 +418,7 @@ fn update_context( | ||||||
|     prev: SimpleToken, |     prev: SimpleToken, | ||||||
|     prev_prev: SimpleToken, |     prev_prev: SimpleToken, | ||||||
|     stack: &mut Vec<Context>, |     stack: &mut Vec<Context>, | ||||||
|  |     settings: &LinterSettings, | ||||||
| ) -> Context { | ) -> Context { | ||||||
|     let new_context = match token.ty { |     let new_context = match token.ty { | ||||||
|         TokenType::OpeningBracket => match (prev.ty, prev_prev.ty) { |         TokenType::OpeningBracket => match (prev.ty, prev_prev.ty) { | ||||||
|  | @ -417,6 +428,17 @@ fn update_context( | ||||||
|             } |             } | ||||||
|             _ => Context::new(ContextType::Tuple), |             _ => Context::new(ContextType::Tuple), | ||||||
|         }, |         }, | ||||||
|  |         TokenType::OpeningSquareBracket if is_trailing_comma_type_params_enabled(settings) => { | ||||||
|  |             match (prev.ty, prev_prev.ty) { | ||||||
|  |                 (TokenType::Named, TokenType::Def | TokenType::Class | TokenType::Type) => { | ||||||
|  |                     Context::new(ContextType::TypeParameters) | ||||||
|  |                 } | ||||||
|  |                 (TokenType::ClosingBracket | TokenType::Named | TokenType::String, _) => { | ||||||
|  |                     Context::new(ContextType::Subscript) | ||||||
|  |                 } | ||||||
|  |                 _ => Context::new(ContextType::List), | ||||||
|  |             } | ||||||
|  |         } | ||||||
|         TokenType::OpeningSquareBracket => match prev.ty { |         TokenType::OpeningSquareBracket => match prev.ty { | ||||||
|             TokenType::ClosingBracket | TokenType::Named | TokenType::String => { |             TokenType::ClosingBracket | TokenType::Named | TokenType::String => { | ||||||
|                 Context::new(ContextType::Subscript) |                 Context::new(ContextType::Subscript) | ||||||
|  |  | ||||||
										
											
												File diff suppressed because it is too large
												Load diff
											
										
									
								
							|  | @ -0,0 +1,30 @@ | ||||||
|  | --- | ||||||
|  | source: crates/ruff_linter/src/rules/flake8_commas/mod.rs | ||||||
|  | --- | ||||||
|  | COM81_syntax_error.py:3:5: SyntaxError: Starred expression cannot be used here | ||||||
|  |   | | ||||||
|  | 1 | # Check for `flake8-commas` violation for a file containing syntax errors. | ||||||
|  | 2 | ( | ||||||
|  | 3 |     *args | ||||||
|  |   |     ^^^^^ | ||||||
|  | 4 | ) | ||||||
|  |   | | ||||||
|  | 
 | ||||||
|  | COM81_syntax_error.py:6:9: SyntaxError: Type parameter list cannot be empty | ||||||
|  |   | | ||||||
|  | 4 | ) | ||||||
|  | 5 | | ||||||
|  | 6 | def foo[(param1='test', param2='test',): | ||||||
|  |   |         ^ | ||||||
|  | 7 |     pass | ||||||
|  |   | | ||||||
|  | 
 | ||||||
|  | COM81_syntax_error.py:6:38: COM819 Trailing comma prohibited | ||||||
|  |   | | ||||||
|  | 4 | ) | ||||||
|  | 5 | | ||||||
|  | 6 | def foo[(param1='test', param2='test',): | ||||||
|  |   |                                      ^ COM819 | ||||||
|  | 7 |     pass | ||||||
|  |   | | ||||||
|  |   = help: Remove trailing comma | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Dan Parizher
						Dan Parizher