diff --git a/README.md b/README.md index 57845ce3a1..3f131e3e35 100644 --- a/README.md +++ b/README.md @@ -123,6 +123,7 @@ of [Conda](https://docs.conda.io/en/latest/): 1. [pygrep-hooks (PGH)](#pygrep-hooks-pgh) 1. [Pylint (PLC, PLE, PLR, PLW)](#pylint-plc-ple-plr-plw) 1. [flake8-pie (PIE)](#flake8-pie-pie) + 1. [flake8-commas (COM)](#flake8-commas-com) 1. [Ruff-specific rules (RUF)](#ruff-specific-rules-ruf) 1. [Editor Integrations](#editor-integrations) 1. [FAQ](#faq) @@ -1129,6 +1130,16 @@ For more, see [flake8-pie](https://pypi.org/project/flake8-pie/0.16.0/) on PyPI. | PIE794 | DupeClassFieldDefinitions | Class field `...` is defined multiple times | 🛠 | | PIE807 | PreferListBuiltin | Prefer `list()` over useless lambda | 🛠 | +### flake8-commas (COM) + +For more, see [flake8-commas](https://pypi.org/project/flake8-commas/2.1.0/) on PyPI. + +| Code | Name | Message | Fix | +| ---- | ---- | ------- | --- | +| COM812 | TrailingCommaMissing | Trailing comma missing | 🛠 | +| COM818 | TrailingCommaOnBareTupleProhibited | Trailing comma on bare tuple prohibited | | +| COM819 | TrailingCommaProhibited | Trailing comma prohibited | 🛠 | + ### Ruff-specific rules (RUF) | Code | Name | Message | Fix | diff --git a/licenses/LICENSE_PyCQA_flake8-commas b/licenses/LICENSE_PyCQA_flake8-commas new file mode 100644 index 0000000000..96dfb32226 --- /dev/null +++ b/licenses/LICENSE_PyCQA_flake8-commas @@ -0,0 +1,45 @@ +The MIT License (MIT) + +Copyright (c) 2017 Thomas Grainger. + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. + + +Portions of this flake8-commas Software may utilize the following +copyrighted material, the use of which is hereby acknowledged. + +Original flake8-commas: https://github.com/trevorcreech/flake8-commas/commit/e8563b71b1d5442e102c8734c11cb5202284293d + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. diff --git a/resources/test/fixtures/flake8_commas/COM81.py b/resources/test/fixtures/flake8_commas/COM81.py new file mode 100644 index 0000000000..54581ffb91 --- /dev/null +++ b/resources/test/fixtures/flake8_commas/COM81.py @@ -0,0 +1,628 @@ +# ==> bad_function_call.py <== +bad_function_call( + param1='test', + param2='test' +) +# ==> bad_list.py <== +bad_list = [ + 1, + 2, + 3 +] + +bad_list_with_comment = [ + 1, + 2, + 3 + # still needs a comma! +] + +bad_list_with_extra_empty = [ + 1, + 2, + 3 + + + +] + +# ==> bare.py <== +bar = 1, 2 + +foo = 1 + +foo = (1,) + +foo = 1, + +bar = 1; foo = bar, + +foo = ( +3, +4, +) + +foo = 3, + +class A(object): + foo = 3 + bar = 10, + foo_bar = 2 + +a = ('a',) + +from foo import bar, baz + +group_by = function_call('arg'), + +group_by = ('foobar' * 3), + +def foo(): + return False, + +==> callable_before_parenth_form.py <== +def foo( + bar, +): + pass + +{'foo': foo}['foo']( + bar +) + +{'foo': foo}['foo']( + bar, +) + +(foo)( + bar +) + +(foo)[0]( + bar, +) + +[foo][0]( + bar +) + +[foo][0]( + bar, +) + +# ==> comment_good_dict.py <== +multiline_good_dict = { + "good": 123, # this is a good number +} + +# ==> dict_comprehension.py <== +not_a_dict = { + x: y + for x, y in ((1, 2), (3, 4)) +} + +# ==> good_empty_comma_context.py <== +def func2( +): + pass + + +func2( + +) + +func2( +) + +[ +] + +[ + +] + +( +) + +( + +) + +{ + +} + +# ==> good_list.py <== +stuff = [ + 'a', + 'b', + # more stuff will go here +] + +more_stuff = [ + 'a', + 'b', + + + +] + +# ==> keyword_before_parenth_form/base_bad.py <== +from x import ( + y +) + +assert( + SyntaxWarning, + ThrownHere, + Anyway +) + +# async await is fine outside an async def +# ruff: RustPython tokenizer treats async/await as keywords, not applicable. + +# def await( +# foo +# ): +# async( +# foo +# ) + +# def async( +# foo +# ): +# await( +# foo +# ) + +# ==> keyword_before_parenth_form/base.py <== +from x import ( + y, +) + +assert( + SyntaxWarning, + ThrownHere, + Anyway, +) + +assert ( + foo +) + +assert ( + foo and + bar +) + +if( + foo and + bar +): + pass +elif( + foo and + bar +): + pass + +for x in( + [1,2,3] +): + print(x) + + +(x for x in ( + [1, 2, 3] +)) + +( + 'foo' +) is ( + 'foo' +) + +if ( + foo and + bar +) or not ( + foo +) or ( + spam +): + pass + +def xyz(): + raise( + Exception() + ) + +def abc(): + return( + 3 + ) + +while( + False +): + pass + +with( + loop +): + pass + +def foo(): + yield ( + "foo" + ) + +# async await is fine outside an async def +# ruff: RustPython tokenizer treats async/await as keywords, not applicable. + +# def await( +# foo, +# ): +# async( +# foo, +# ) + +# def async( +# foo, +# ): +# await( +# foo, +# ) + +# ==> keyword_before_parenth_form/py3.py <== +# Syntax error in Py2 + +def foo(): + yield from ( + foo + ) + +# ==> list_comprehension.py <== +not_a_list = [ + s.strip() + for s in 'foo, bar, baz'.split(',') +] + +# ==> multiline_bad_dict.py <== +multiline_bad_dict = { + "bad": 123 +} +# ==> multiline_bad_function_def.py <== +def func_good( + a = 3, + b = 2): + pass + + +def func_bad( + a = 3, + b = 2 + ): + pass + +# ==> multiline_bad_function_one_param.py <== +def func( + a = 3 + ): + pass + + +func( + a = 3 +) + +# ==> multiline_bad_or_dict.py <== +multiline_bad_or_dict = { + "good": True or False, + "bad": 123 +} + +# ==> multiline_good_dict.py <== +multiline_good_dict = { + "good": 123, +} +# ==> multiline_good_single_keyed_for_dict.py <== +good_dict = { + "good": x for x in y +} + +# ==> multiline_if.py <== +if ( + foo + and bar +): + print("Baz") + +# ==> multiline_index_access.py <== +multiline_index_access[ + "good" +] + +multiline_index_access_after_function()[ + "good" +] + +multiline_index_access_after_inline_index_access['first'][ + "good" +] + +multiline_index_access[ + "probably fine", +] + +[0, 1, 2][ + "good" +] + +[0, 1, 2][ + "probably fine", +] + +multiline_index_access[ + "probably fine", + "not good" +] + +multiline_index_access[ + "fine", + "fine", + : + "not good" +] + +# ==> multiline_string.py <== +s = ( + 'this' + + 'is a string' +) + +s2 = ( + 'this' + 'is a also a string' +) + +t = ( + 'this' + + 'is a tuple', +) + +t2 = ( + 'this' + 'is also a tuple', +) + +# ==> multiline_subscript_slice.py <== +multiline_index_access[ + "fine", + "fine" + : + "not fine" +] + +multiline_index_access[ + "fine" + "fine" + : + "fine" + : + "fine" +] + +multiline_index_access[ + "fine" + "fine", + : + "fine", + : + "fine", +] + +multiline_index_access[ + "fine" + "fine", + : + "fine" + : + "fine", + "not fine" +] + +multiline_index_access[ + "fine" + "fine", + : + "fine", + "fine" + : + "fine", +] + +multiline_index_access[ + lambda fine, + fine, + fine: (0,) + : + lambda fine, + fine, + fine: (0,), + "fine" + : + "fine", +] + +# ==> one_line_dict.py <== +one_line_dict = {"good": 123} +# ==> parenth_form.py <== +parenth_form = ( + a + + b + + c +) + +parenth_form_with_lambda = ( + lambda x, y: 0 +) + +parenth_form_with_default_lambda = ( + lambda x=( + lambda + x, + y, + : + 0 + ), + y = {a: b}, + : + 0 +) + +# ==> prohibited.py <== +foo = ['a', 'b', 'c',] + +bar = { a: b,} + +def bah(ham, spam,): + pass + +(0,) + +(0, 1,) + +foo = ['a', 'b', 'c', ] + +bar = { a: b, } + +def bah(ham, spam, ): + pass + +(0, ) + +(0, 1, ) + +image[:, :, 0] + +image[:,] + +image[:,:,] + +lambda x, : + +# ==> unpack.py <== +def function( + foo, + bar, + **kwargs +): + pass + +def function( + foo, + bar, + *args +): + pass + +def function( + foo, + bar, + *args, + extra_kwarg + ): + pass + +result = function( + foo, + bar, + **kwargs +) + +result = function( + foo, + bar, + **not_called_kwargs +) + +def foo( + ham, + spam, + *args, + kwarg_only + ): + pass + +# In python 3.5 if it's not a function def, commas are mandatory. + +foo( + **kwargs +) + +{ + **kwargs +} + +( + *args +) + +{ + *args +} + +[ + *args +] + +def foo( + ham, + spam, + *args + ): + pass + +def foo( + ham, + spam, + **kwargs + ): + pass + +def foo( + ham, + spam, + *args, + kwarg_only + ): + pass + +# In python 3.5 if it's not a function def, commas are mandatory. + +foo( + **kwargs, +) + +{ + **kwargs, +} + +( + *args, +) + +{ + *args, +} + +[ + *args, +] + +result = function( + foo, + bar, + **{'ham': spam} +) diff --git a/ruff.schema.json b/ruff.schema.json index fd1eddc1ef..fbfbe1fb9c 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1156,6 +1156,12 @@ "C9", "C90", "C901", + "COM", + "COM8", + "COM81", + "COM812", + "COM818", + "COM819", "D", "D1", "D10", diff --git a/ruff_macros/src/prefixes.rs b/ruff_macros/src/prefixes.rs index adfcb78100..580ad90246 100644 --- a/ruff_macros/src/prefixes.rs +++ b/ruff_macros/src/prefixes.rs @@ -9,6 +9,7 @@ pub const PREFIX_TO_ORIGIN: &[(&str, &str)] = &[ ("B", "Flake8Bugbear"), ("C4", "Flake8Comprehensions"), ("C9", "McCabe"), + ("COM", "Flake8Commas"), ("DTZ", "Flake8Datetimez"), ("D", "Pydocstyle"), ("ERA", "Eradicate"), diff --git a/src/checkers/tokens.rs b/src/checkers/tokens.rs index 4771a08a14..aeb5841aa6 100644 --- a/src/checkers/tokens.rs +++ b/src/checkers/tokens.rs @@ -5,7 +5,9 @@ use rustpython_parser::lexer::{LexResult, Tok}; use crate::lex::docstring_detection::StateMachine; use crate::registry::{Diagnostic, RuleCode}; use crate::rules::ruff::rules::Context; -use crate::rules::{eradicate, flake8_implicit_str_concat, flake8_quotes, pycodestyle, ruff}; +use crate::rules::{ + eradicate, flake8_commas, flake8_implicit_str_concat, flake8_quotes, pycodestyle, ruff, +}; use crate::settings::{flags, Settings}; use crate::source_code::Locator; @@ -28,6 +30,9 @@ pub fn check_tokens( let enforce_invalid_escape_sequence = settings.enabled.contains(&RuleCode::W605); let enforce_implicit_string_concatenation = settings.enabled.contains(&RuleCode::ISC001) || settings.enabled.contains(&RuleCode::ISC002); + let enforce_trailing_comma = settings.enabled.contains(&RuleCode::COM812) + || settings.enabled.contains(&RuleCode::COM818) + || settings.enabled.contains(&RuleCode::COM819); let mut state_machine = StateMachine::default(); for &(start, ref tok, end) in tokens.iter().flatten() { @@ -111,5 +116,14 @@ pub fn check_tokens( ); } + // COM812, COM818, COM819 + if enforce_trailing_comma { + diagnostics.extend( + flake8_commas::rules::trailing_commas(tokens, locator) + .into_iter() + .filter(|diagnostic| settings.enabled.contains(diagnostic.kind.code())), + ); + } + diagnostics } diff --git a/src/registry.rs b/src/registry.rs index 51467ce9b7..f1ab10f61b 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -411,6 +411,10 @@ ruff_macros::define_rule_mapping!( PIE790 => violations::NoUnnecessaryPass, PIE794 => violations::DupeClassFieldDefinitions, PIE807 => violations::PreferListBuiltin, + // flake8-commas + COM812 => violations::TrailingCommaMissing, + COM818 => violations::TrailingCommaOnBareTupleProhibited, + COM819 => violations::TrailingCommaProhibited, // Ruff RUF001 => violations::AmbiguousUnicodeCharacterString, RUF002 => violations::AmbiguousUnicodeCharacterDocstring, @@ -453,6 +457,7 @@ pub enum RuleOrigin { PygrepHooks, Pylint, Flake8Pie, + Flake8Commas, Ruff, } @@ -522,6 +527,7 @@ impl RuleOrigin { RuleOrigin::Pylint => "Pylint", RuleOrigin::Pyupgrade => "pyupgrade", RuleOrigin::Flake8Pie => "flake8-pie", + RuleOrigin::Flake8Commas => "flake8-commas", RuleOrigin::Ruff => "Ruff-specific rules", } } @@ -568,6 +574,7 @@ impl RuleOrigin { ]), RuleOrigin::Pyupgrade => Prefixes::Single(RuleCodePrefix::UP), RuleOrigin::Flake8Pie => Prefixes::Single(RuleCodePrefix::PIE), + RuleOrigin::Flake8Commas => Prefixes::Single(RuleCodePrefix::COM), RuleOrigin::Ruff => Prefixes::Single(RuleCodePrefix::RUF), } } @@ -689,6 +696,10 @@ impl RuleOrigin { "https://pypi.org/project/flake8-pie/0.16.0/", &Platform::PyPI, )), + RuleOrigin::Flake8Commas => Some(( + "https://pypi.org/project/flake8-commas/2.1.0/", + &Platform::PyPI, + )), RuleOrigin::Ruff => None, } } @@ -723,6 +734,9 @@ impl RuleCode { | RuleCode::Q002 | RuleCode::Q003 | RuleCode::W605 + | RuleCode::COM812 + | RuleCode::COM818 + | RuleCode::COM819 | RuleCode::RUF001 | RuleCode::RUF002 | RuleCode::RUF003 => &LintSource::Tokens, diff --git a/src/rules/flake8_commas/mod.rs b/src/rules/flake8_commas/mod.rs new file mode 100644 index 0000000000..382e1caaae --- /dev/null +++ b/src/rules/flake8_commas/mod.rs @@ -0,0 +1,30 @@ +pub(crate) mod rules; + +#[cfg(test)] +mod tests { + use std::path::Path; + + use anyhow::Result; + use test_case::test_case; + + use crate::linter::test_path; + use crate::registry::RuleCode; + use crate::settings; + + #[test_case(Path::new("COM81.py"); "COM81")] + fn rules(path: &Path) -> Result<()> { + let snapshot = path.to_string_lossy().into_owned(); + let diagnostics = test_path( + Path::new("./resources/test/fixtures/flake8_commas") + .join(path) + .as_path(), + &settings::Settings::for_rules(vec![ + RuleCode::COM812, + RuleCode::COM818, + RuleCode::COM819, + ]), + )?; + insta::assert_yaml_snapshot!(snapshot, diagnostics); + Ok(()) + } +} diff --git a/src/rules/flake8_commas/rules.rs b/src/rules/flake8_commas/rules.rs new file mode 100644 index 0000000000..fc39ad7cb0 --- /dev/null +++ b/src/rules/flake8_commas/rules.rs @@ -0,0 +1,284 @@ +use itertools::Itertools; +use rustpython_parser::lexer::{LexResult, Spanned}; +use rustpython_parser::token::Tok; + +use crate::ast::types::Range; +use crate::fix::Fix; +use crate::registry::Diagnostic; +use crate::source_code::Locator; +use crate::violations; + +/// Simplified token type. +#[derive(Copy, Clone, PartialEq, Eq)] +enum TokenType { + Irrelevant, + NonLogicalNewline, + Newline, + Comma, + OpeningBracket, + OpeningSquareBracket, + OpeningCurlyBracket, + ClosingBracket, + For, + Named, + Def, + Lambda, + Colon, +} + +/// Simplified token specialized for the task. +#[derive(Copy, Clone)] +struct Token<'tok> { + type_: TokenType, + // Underlying token. + spanned: Option<&'tok Spanned>, +} + +impl<'tok> Token<'tok> { + fn irrelevant() -> Token<'static> { + Token { + type_: TokenType::Irrelevant, + spanned: None, + } + } + + fn from_spanned(spanned: &'tok Spanned) -> Token<'tok> { + let type_ = match &spanned.1 { + Tok::NonLogicalNewline => TokenType::NonLogicalNewline, + Tok::Newline => TokenType::Newline, + Tok::For => TokenType::For, + Tok::Def => TokenType::Def, + Tok::Lambda => TokenType::Lambda, + // Import treated like a function. + Tok::Import => TokenType::Named, + Tok::Name { .. } => TokenType::Named, + Tok::Comma => TokenType::Comma, + Tok::Lpar => TokenType::OpeningBracket, + Tok::Lsqb => TokenType::OpeningSquareBracket, + Tok::Lbrace => TokenType::OpeningCurlyBracket, + Tok::Rpar | Tok::Rsqb | Tok::Rbrace => TokenType::ClosingBracket, + Tok::Colon => TokenType::Colon, + _ => TokenType::Irrelevant, + }; + Self { + spanned: Some(spanned), + type_, + } + } +} + +/// Comma context type - types of comma-delimited Python constructs. +#[derive(Copy, Clone, PartialEq, Eq)] +enum ContextType { + No, + /// Function definition parameter list, e.g. `def foo(a,b,c)`. + FunctionParameters, + /// Call argument-like item list, e.g. `f(1,2,3)`, `foo()(1,2,3)`. + CallArguments, + /// Tuple-like item list, e.g. `(1,2,3)`. + Tuple, + /// Subscript item list, e.g. `x[1,2,3]`, `foo()[1,2,3]`. + Subscript, + /// List-like item list, e.g. `[1,2,3]`. + List, + /// Dict-/set-like item list, e.g. `{1,2,3}`. + Dict, + /// Lambda parameter list, e.g. `lambda a, b`. + LambdaParameters, +} + +/// Comma context - described a comma-delimited "situation". +#[derive(Copy, Clone)] +struct Context { + type_: ContextType, + num_commas: u32, +} + +impl Context { + fn new(type_: ContextType) -> Self { + Context { + type_, + num_commas: 0, + } + } + + fn inc(&mut self) { + self.num_commas += 1; + } +} + +/// COM812, COM818, COM819 +#[allow(clippy::if_same_then_else, clippy::needless_bool)] +pub fn trailing_commas(tokens: &[LexResult], _locator: &Locator) -> Vec { + let mut diagnostics = vec![]; + + let tokens = tokens + .iter() + .flatten() + // Completely ignore comments -- they just interfere with the logic. + .filter(|&r| !matches!(r, (_, Tok::Comment(_), _))) + .map(Token::from_spanned); + let tokens = [Token::irrelevant(), Token::irrelevant()] + .into_iter() + .chain(tokens); + // Collapse consecutive newlines to the first one -- trailing commas are + // added before the first newline. + let tokens = tokens.coalesce(|previous, current| { + if previous.type_ == TokenType::NonLogicalNewline + && current.type_ == TokenType::NonLogicalNewline + { + Ok(previous) + } else { + Err((previous, current)) + } + }); + + // The current nesting of the comma contexts. + let mut stack = vec![Context::new(ContextType::No)]; + + for (prev_prev, prev, token) in tokens.tuple_windows() { + // Update the comma context stack. + match token.type_ { + TokenType::OpeningBracket => match (prev.type_, prev_prev.type_) { + (TokenType::Named, TokenType::Def) => { + stack.push(Context::new(ContextType::FunctionParameters)); + } + (TokenType::Named | TokenType::ClosingBracket, _) => { + stack.push(Context::new(ContextType::CallArguments)); + } + _ => { + stack.push(Context::new(ContextType::Tuple)); + } + }, + TokenType::OpeningSquareBracket => match prev.type_ { + TokenType::ClosingBracket | TokenType::Named => { + stack.push(Context::new(ContextType::Subscript)); + } + _ => { + stack.push(Context::new(ContextType::List)); + } + }, + TokenType::OpeningCurlyBracket => { + stack.push(Context::new(ContextType::Dict)); + } + TokenType::Lambda => { + stack.push(Context::new(ContextType::LambdaParameters)); + } + TokenType::For => { + let len = stack.len(); + stack[len - 1] = Context::new(ContextType::No); + } + TokenType::Comma => { + let len = stack.len(); + stack[len - 1].inc(); + } + _ => {} + } + let context = &stack[stack.len() - 1]; + + // Is it allowed to have a trailing comma before this token? + let comma_allowed = token.type_ == TokenType::ClosingBracket + && match context.type_ { + ContextType::No => false, + ContextType::FunctionParameters => true, + ContextType::CallArguments => true, + // `(1)` is not equivalent to `(1,)`. + ContextType::Tuple => context.num_commas != 0, + // `x[1]` is not equivalent to `x[1,]`. + ContextType::Subscript => context.num_commas != 0, + ContextType::List => true, + ContextType::Dict => true, + // Lambdas are required to be a single line, trailing comma never makes sense. + ContextType::LambdaParameters => false, + }; + + // Is prev a prohibited trailing comma? + let comma_prohibited = prev.type_ == TokenType::Comma && { + // Is `(1,)` or `x[1,]`? + let is_singleton_tuplish = + matches!(context.type_, ContextType::Subscript | ContextType::Tuple) + && context.num_commas <= 1; + // There was no non-logical newline, so prohibit (except in `(1,)` or `x[1,]`). + if comma_allowed && !is_singleton_tuplish { + true + // Lambdas not handled by comma_allowed so handle it specially. + } else if context.type_ == ContextType::LambdaParameters + && token.type_ == TokenType::Colon + { + true + } else { + false + } + }; + if comma_prohibited { + let comma = prev.spanned.unwrap(); + let mut diagnostic = Diagnostic::new( + violations::TrailingCommaProhibited, + Range { + location: comma.0, + end_location: comma.2, + }, + ); + diagnostic.amend(Fix::deletion(comma.0, comma.2)); + diagnostics.push(diagnostic); + } + + // Is prev a prohibited trailing comma on a bare tuple? + // Approximation: any comma followed by a statement-ending newline. + let bare_comma_prohibited = + prev.type_ == TokenType::Comma && token.type_ == TokenType::Newline; + if bare_comma_prohibited { + let comma = prev.spanned.unwrap(); + let diagnostic = Diagnostic::new( + violations::TrailingCommaOnBareTupleProhibited, + Range { + location: comma.0, + end_location: comma.2, + }, + ); + diagnostics.push(diagnostic); + } + + // Comma is required if: + // - It is allowed, + // - Followed by a newline, + // - Not already present, + // - Not on an empty (), {}, []. + let comma_required = comma_allowed + && prev.type_ == TokenType::NonLogicalNewline + && !matches!( + prev_prev.type_, + TokenType::Comma + | TokenType::OpeningBracket + | TokenType::OpeningSquareBracket + | TokenType::OpeningCurlyBracket + ); + if comma_required { + let missing_comma = prev_prev.spanned.unwrap(); + let mut diagnostic = Diagnostic::new( + violations::TrailingCommaMissing, + Range { + location: missing_comma.2, + end_location: missing_comma.2, + }, + ); + diagnostic.amend(Fix::insertion(",".to_owned(), missing_comma.2)); + diagnostics.push(diagnostic); + } + + // Pop the current context if the current token ended it. + // The top context is never popped (if unbalanced closing brackets). + let pop_context = match context.type_ { + // Lambda terminated by `:`. + ContextType::LambdaParameters => token.type_ == TokenType::Colon, + // All others terminated by a closing bracket. + // flake8-commas doesn't verify that it matches the opening... + _ => token.type_ == TokenType::ClosingBracket, + }; + if pop_context && stack.len() > 1 { + stack.pop(); + } + } + + diagnostics +} diff --git a/src/rules/flake8_commas/snapshots/ruff__rules__flake8_commas__tests__COM81.py.snap b/src/rules/flake8_commas/snapshots/ruff__rules__flake8_commas__tests__COM81.py.snap new file mode 100644 index 0000000000..cd79023e24 --- /dev/null +++ b/src/rules/flake8_commas/snapshots/ruff__rules__flake8_commas__tests__COM81.py.snap @@ -0,0 +1,789 @@ +--- +source: src/rules/flake8_commas/mod.rs +expression: diagnostics +--- +- kind: + TrailingCommaMissing: ~ + location: + row: 4 + column: 17 + end_location: + row: 4 + column: 17 + fix: + content: "," + location: + row: 4 + column: 17 + end_location: + row: 4 + column: 17 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 10 + column: 5 + end_location: + row: 10 + column: 5 + fix: + content: "," + location: + row: 10 + column: 5 + end_location: + row: 10 + column: 5 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 16 + column: 5 + end_location: + row: 16 + column: 5 + fix: + content: "," + location: + row: 16 + column: 5 + end_location: + row: 16 + column: 5 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 23 + column: 5 + end_location: + row: 23 + column: 5 + fix: + content: "," + location: + row: 23 + column: 5 + end_location: + row: 23 + column: 5 + parent: ~ +- kind: + TrailingCommaOnBareTupleProhibited: ~ + location: + row: 36 + column: 7 + end_location: + row: 36 + column: 8 + fix: ~ + parent: ~ +- kind: + TrailingCommaOnBareTupleProhibited: ~ + location: + row: 38 + column: 18 + end_location: + row: 38 + column: 19 + fix: ~ + parent: ~ +- kind: + TrailingCommaOnBareTupleProhibited: ~ + location: + row: 45 + column: 7 + end_location: + row: 45 + column: 8 + fix: ~ + parent: ~ +- kind: + TrailingCommaOnBareTupleProhibited: ~ + location: + row: 49 + column: 9 + end_location: + row: 49 + column: 10 + fix: ~ + parent: ~ +- kind: + TrailingCommaOnBareTupleProhibited: ~ + location: + row: 56 + column: 31 + end_location: + row: 56 + column: 32 + fix: ~ + parent: ~ +- kind: + TrailingCommaOnBareTupleProhibited: ~ + location: + row: 58 + column: 25 + end_location: + row: 58 + column: 26 + fix: ~ + parent: ~ +- kind: + TrailingCommaOnBareTupleProhibited: ~ + location: + row: 61 + column: 16 + end_location: + row: 61 + column: 17 + fix: ~ + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 70 + column: 7 + end_location: + row: 70 + column: 7 + fix: + content: "," + location: + row: 70 + column: 7 + end_location: + row: 70 + column: 7 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 78 + column: 7 + end_location: + row: 78 + column: 7 + fix: + content: "," + location: + row: 78 + column: 7 + end_location: + row: 78 + column: 7 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 86 + column: 7 + end_location: + row: 86 + column: 7 + fix: + content: "," + location: + row: 86 + column: 7 + end_location: + row: 86 + column: 7 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 152 + column: 5 + end_location: + row: 152 + column: 5 + fix: + content: "," + location: + row: 152 + column: 5 + end_location: + row: 152 + column: 5 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 158 + column: 10 + end_location: + row: 158 + column: 10 + fix: + content: "," + location: + row: 158 + column: 10 + end_location: + row: 158 + column: 10 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 293 + column: 14 + end_location: + row: 293 + column: 14 + fix: + content: "," + location: + row: 293 + column: 14 + end_location: + row: 293 + column: 14 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 304 + column: 13 + end_location: + row: 304 + column: 13 + fix: + content: "," + location: + row: 304 + column: 13 + end_location: + row: 304 + column: 13 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 310 + column: 13 + end_location: + row: 310 + column: 13 + fix: + content: "," + location: + row: 310 + column: 13 + end_location: + row: 310 + column: 13 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 316 + column: 9 + end_location: + row: 316 + column: 9 + fix: + content: "," + location: + row: 316 + column: 9 + end_location: + row: 316 + column: 9 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 322 + column: 14 + end_location: + row: 322 + column: 14 + fix: + content: "," + location: + row: 322 + column: 14 + end_location: + row: 322 + column: 14 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 368 + column: 14 + end_location: + row: 368 + column: 14 + fix: + content: "," + location: + row: 368 + column: 14 + end_location: + row: 368 + column: 14 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 375 + column: 14 + end_location: + row: 375 + column: 14 + fix: + content: "," + location: + row: 375 + column: 14 + end_location: + row: 375 + column: 14 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 404 + column: 14 + end_location: + row: 404 + column: 14 + fix: + content: "," + location: + row: 404 + column: 14 + end_location: + row: 404 + column: 14 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 432 + column: 14 + end_location: + row: 432 + column: 14 + fix: + content: "," + location: + row: 432 + column: 14 + end_location: + row: 432 + column: 14 + parent: ~ +- kind: + TrailingCommaProhibited: ~ + location: + row: 485 + column: 20 + end_location: + row: 485 + column: 21 + fix: + content: "" + location: + row: 485 + column: 20 + end_location: + row: 485 + column: 21 + parent: ~ +- kind: + TrailingCommaProhibited: ~ + location: + row: 487 + column: 12 + end_location: + row: 487 + column: 13 + fix: + content: "" + location: + row: 487 + column: 12 + end_location: + row: 487 + column: 13 + parent: ~ +- kind: + TrailingCommaProhibited: ~ + location: + row: 489 + column: 17 + end_location: + row: 489 + column: 18 + fix: + content: "" + location: + row: 489 + column: 17 + end_location: + row: 489 + column: 18 + parent: ~ +- kind: + TrailingCommaProhibited: ~ + location: + row: 494 + column: 5 + end_location: + row: 494 + column: 6 + fix: + content: "" + location: + row: 494 + column: 5 + end_location: + row: 494 + column: 6 + parent: ~ +- kind: + TrailingCommaProhibited: ~ + location: + row: 496 + column: 20 + end_location: + row: 496 + column: 21 + fix: + content: "" + location: + row: 496 + column: 20 + end_location: + row: 496 + column: 21 + parent: ~ +- kind: + TrailingCommaProhibited: ~ + location: + row: 498 + column: 12 + end_location: + row: 498 + column: 13 + fix: + content: "" + location: + row: 498 + column: 12 + end_location: + row: 498 + column: 13 + parent: ~ +- kind: + TrailingCommaProhibited: ~ + location: + row: 500 + column: 17 + end_location: + row: 500 + column: 18 + fix: + content: "" + location: + row: 500 + column: 17 + end_location: + row: 500 + column: 18 + parent: ~ +- kind: + TrailingCommaProhibited: ~ + location: + row: 505 + column: 5 + end_location: + row: 505 + column: 6 + fix: + content: "" + location: + row: 505 + column: 5 + end_location: + row: 505 + column: 6 + parent: ~ +- kind: + TrailingCommaProhibited: ~ + location: + row: 511 + column: 9 + end_location: + row: 511 + column: 10 + fix: + content: "" + location: + row: 511 + column: 9 + end_location: + row: 511 + column: 10 + parent: ~ +- kind: + TrailingCommaProhibited: ~ + location: + row: 513 + column: 8 + end_location: + row: 513 + column: 9 + fix: + content: "" + location: + row: 513 + column: 8 + end_location: + row: 513 + column: 9 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 519 + column: 12 + end_location: + row: 519 + column: 12 + fix: + content: "," + location: + row: 519 + column: 12 + end_location: + row: 519 + column: 12 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 526 + column: 9 + end_location: + row: 526 + column: 9 + fix: + content: "," + location: + row: 526 + column: 9 + end_location: + row: 526 + column: 9 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 534 + column: 15 + end_location: + row: 534 + column: 15 + fix: + content: "," + location: + row: 534 + column: 15 + end_location: + row: 534 + column: 15 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 541 + column: 12 + end_location: + row: 541 + column: 12 + fix: + content: "," + location: + row: 541 + column: 12 + end_location: + row: 541 + column: 12 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 547 + column: 23 + end_location: + row: 547 + column: 23 + fix: + content: "," + location: + row: 547 + column: 23 + end_location: + row: 547 + column: 23 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 554 + column: 14 + end_location: + row: 554 + column: 14 + fix: + content: "," + location: + row: 554 + column: 14 + end_location: + row: 554 + column: 14 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 561 + column: 12 + end_location: + row: 561 + column: 12 + fix: + content: "," + location: + row: 561 + column: 12 + end_location: + row: 561 + column: 12 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 565 + column: 12 + end_location: + row: 565 + column: 12 + fix: + content: "," + location: + row: 565 + column: 12 + end_location: + row: 565 + column: 12 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 573 + column: 9 + end_location: + row: 573 + column: 9 + fix: + content: "," + location: + row: 573 + column: 9 + end_location: + row: 573 + column: 9 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 577 + column: 9 + end_location: + row: 577 + column: 9 + fix: + content: "," + location: + row: 577 + column: 9 + end_location: + row: 577 + column: 9 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 583 + column: 9 + end_location: + row: 583 + column: 9 + fix: + content: "," + location: + row: 583 + column: 9 + end_location: + row: 583 + column: 9 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 590 + column: 12 + end_location: + row: 590 + column: 12 + fix: + content: "," + location: + row: 590 + column: 12 + end_location: + row: 590 + column: 12 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 598 + column: 14 + end_location: + row: 598 + column: 14 + fix: + content: "," + location: + row: 598 + column: 14 + end_location: + row: 598 + column: 14 + parent: ~ +- kind: + TrailingCommaMissing: ~ + location: + row: 627 + column: 19 + end_location: + row: 627 + column: 19 + fix: + content: "," + location: + row: 627 + column: 19 + end_location: + row: 627 + column: 19 + parent: ~ + diff --git a/src/rules/mod.rs b/src/rules/mod.rs index 78765cd76b..98aea59129 100644 --- a/src/rules/mod.rs +++ b/src/rules/mod.rs @@ -6,6 +6,7 @@ pub mod flake8_blind_except; pub mod flake8_boolean_trap; pub mod flake8_bugbear; pub mod flake8_builtins; +pub mod flake8_commas; pub mod flake8_comprehensions; pub mod flake8_datetimez; pub mod flake8_debugger; diff --git a/src/violations.rs b/src/violations.rs index 60eae3e66e..379f73abcd 100644 --- a/src/violations.rs +++ b/src/violations.rs @@ -6027,6 +6027,55 @@ impl AlwaysAutofixableViolation for PreferListBuiltin { } } +// flake8-commas + +define_violation!( + pub struct TrailingCommaMissing; +); +impl AlwaysAutofixableViolation for TrailingCommaMissing { + fn message(&self) -> String { + "Trailing comma missing".to_string() + } + + fn autofix_title(&self) -> String { + "Add trailing comma".to_string() + } + + fn placeholder() -> Self { + TrailingCommaMissing + } +} + +define_violation!( + pub struct TrailingCommaOnBareTupleProhibited; +); +impl Violation for TrailingCommaOnBareTupleProhibited { + fn message(&self) -> String { + "Trailing comma on bare tuple prohibited".to_string() + } + + fn placeholder() -> Self { + TrailingCommaOnBareTupleProhibited + } +} + +define_violation!( + pub struct TrailingCommaProhibited; +); +impl AlwaysAutofixableViolation for TrailingCommaProhibited { + fn message(&self) -> String { + "Trailing comma prohibited".to_string() + } + + fn autofix_title(&self) -> String { + "Remove trailing comma".to_string() + } + + fn placeholder() -> Self { + TrailingCommaProhibited + } +} + // Ruff define_violation!(