From 074a343a6386e319605fa3f19ea573e7a3e75d59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Gon=C3=A7alves?= <43336371+carlosmiei@users.noreply.github.com> Date: Tue, 28 Feb 2023 17:02:36 +0000 Subject: [PATCH] feat(E251,E252): add rules (#3274) --- .../test/fixtures/pycodestyle/E25.py | 50 ++++++++ crates/ruff/src/checkers/logical_lines.rs | 18 ++- crates/ruff/src/codes.rs | 4 + crates/ruff/src/registry.rs | 6 + crates/ruff/src/rules/pycodestyle/helpers.rs | 120 +++++++++++++----- .../src/rules/pycodestyle/logical_lines.rs | 72 +---------- crates/ruff/src/rules/pycodestyle/mod.rs | 5 + .../ruff/src/rules/pycodestyle/rules/mod.rs | 6 + ...hitespace_around_named_parameter_equals.rs | 114 +++++++++++++++++ ...ules__pycodestyle__tests__E251_E25.py.snap | 115 +++++++++++++++++ ...ules__pycodestyle__tests__E252_E25.py.snap | 45 +++++++ 11 files changed, 452 insertions(+), 103 deletions(-) create mode 100644 crates/ruff/resources/test/fixtures/pycodestyle/E25.py create mode 100644 crates/ruff/src/rules/pycodestyle/rules/whitespace_around_named_parameter_equals.rs create mode 100644 crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E251_E25.py.snap create mode 100644 crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E252_E25.py.snap diff --git a/crates/ruff/resources/test/fixtures/pycodestyle/E25.py b/crates/ruff/resources/test/fixtures/pycodestyle/E25.py new file mode 100644 index 0000000000..88981576ef --- /dev/null +++ b/crates/ruff/resources/test/fixtures/pycodestyle/E25.py @@ -0,0 +1,50 @@ +#: E251 E251 +def foo(bar = False): + '''Test function with an error in declaration''' + pass +#: E251 +foo(bar= True) +#: E251 +foo(bar =True) +#: E251 E251 +foo(bar = True) +#: E251 +y = bar(root= "sdasd") +#: E251:2:29 +parser.add_argument('--long-option', + default= + "/rather/long/filesystem/path/here/blah/blah/blah") +#: E251:1:45 +parser.add_argument('--long-option', default + ="/rather/long/filesystem/path/here/blah/blah/blah") +#: E251:3:8 E251:3:10 +foo(True, + baz=(1, 2), + biz = 'foo' + ) +#: Okay +foo(bar=(1 == 1)) +foo(bar=(1 != 1)) +foo(bar=(1 >= 1)) +foo(bar=(1 <= 1)) +(options, args) = parser.parse_args() +d[type(None)] = _deepcopy_atomic + +# Annotated Function Definitions +#: Okay +def munge(input: AnyStr, sep: AnyStr = None, limit=1000, + extra: Union[str, dict] = None) -> AnyStr: + pass +#: Okay +async def add(a: int = 0, b: int = 0) -> int: + return a + b +# Previously E251 four times +#: E271:1:6 +async def add(a: int = 0, b: int = 0) -> int: + return a + b +#: E252:1:15 E252:1:16 E252:1:27 E252:1:36 +def add(a: int=0, b: int =0, c: int= 0) -> int: + return a + b + c +#: Okay +def add(a: int = _default(name='f')): + return a diff --git a/crates/ruff/src/checkers/logical_lines.rs b/crates/ruff/src/checkers/logical_lines.rs index 2bcce8e1ff..d4ece00f9c 100644 --- a/crates/ruff/src/checkers/logical_lines.rs +++ b/crates/ruff/src/checkers/logical_lines.rs @@ -8,7 +8,8 @@ use crate::registry::Diagnostic; use crate::rules::pycodestyle::logical_lines::{iter_logical_lines, TokenFlags}; use crate::rules::pycodestyle::rules::{ extraneous_whitespace, indentation, missing_whitespace_after_keyword, space_around_operator, - whitespace_around_keywords, whitespace_before_comment, + whitespace_around_keywords, whitespace_around_named_parameter_equals, + whitespace_before_comment, }; use crate::settings::Settings; use crate::source_code::{Locator, Stylist}; @@ -132,6 +133,21 @@ pub fn check_logical_lines( } } } + if line.flags.contains(TokenFlags::OPERATOR) { + for (location, kind) in + whitespace_around_named_parameter_equals(&line.tokens, &line.text) + { + if settings.rules.enabled(kind.rule()) { + diagnostics.push(Diagnostic { + kind, + location, + end_location: location, + fix: None, + parent: None, + }); + } + } + } for (index, kind) in indentation( &line, diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index 0f29da15d2..e424e0173c 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -37,6 +37,10 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option { #[cfg(feature = "logical_lines")] (Pycodestyle, "E224") => Rule::TabAfterOperator, #[cfg(feature = "logical_lines")] + (Pycodestyle, "E251") => Rule::UnexpectedSpacesAroundKeywordParameterEquals, + #[cfg(feature = "logical_lines")] + (Pycodestyle, "E252") => Rule::MissingWhitespaceAroundParameterEquals, + #[cfg(feature = "logical_lines")] (Pycodestyle, "E261") => Rule::TooFewSpacesBeforeInlineComment, #[cfg(feature = "logical_lines")] (Pycodestyle, "E262") => Rule::NoSpaceAfterInlineComment, diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index a8c9f4417b..7583340025 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -59,6 +59,10 @@ ruff_macros::register_rules!( #[cfg(feature = "logical_lines")] rules::pycodestyle::rules::TabAfterKeyword, #[cfg(feature = "logical_lines")] + rules::pycodestyle::rules::UnexpectedSpacesAroundKeywordParameterEquals, + #[cfg(feature = "logical_lines")] + rules::pycodestyle::rules::MissingWhitespaceAroundParameterEquals, + #[cfg(feature = "logical_lines")] rules::pycodestyle::rules::TabBeforeKeyword, rules::pycodestyle::rules::MultipleImportsOnOneLine, rules::pycodestyle::rules::ModuleImportNotAtTopOfFile, @@ -850,6 +854,8 @@ impl Rule { | Rule::UnexpectedIndentationComment | Rule::WhitespaceAfterOpenBracket | Rule::WhitespaceBeforeCloseBracket + | Rule::UnexpectedSpacesAroundKeywordParameterEquals + | Rule::MissingWhitespaceAroundParameterEquals | Rule::WhitespaceBeforePunctuation => &LintSource::LogicalLines, _ => &LintSource::Ast, } diff --git a/crates/ruff/src/rules/pycodestyle/helpers.rs b/crates/ruff/src/rules/pycodestyle/helpers.rs index 01c7c34431..e57227a2ff 100644 --- a/crates/ruff/src/rules/pycodestyle/helpers.rs +++ b/crates/ruff/src/rules/pycodestyle/helpers.rs @@ -61,40 +61,40 @@ pub fn is_overlong( pub fn is_keyword_token(token: &Tok) -> bool { matches!( token, - Tok::False { .. } - | Tok::True { .. } - | Tok::None { .. } - | Tok::And { .. } - | Tok::As { .. } - | Tok::Assert { .. } - | Tok::Await { .. } - | Tok::Break { .. } - | Tok::Class { .. } - | Tok::Continue { .. } - | Tok::Def { .. } - | Tok::Del { .. } - | Tok::Elif { .. } - | Tok::Else { .. } - | Tok::Except { .. } - | Tok::Finally { .. } - | Tok::For { .. } - | Tok::From { .. } - | Tok::Global { .. } - | Tok::If { .. } - | Tok::Import { .. } - | Tok::In { .. } - | Tok::Is { .. } - | Tok::Lambda { .. } - | Tok::Nonlocal { .. } - | Tok::Not { .. } - | Tok::Or { .. } - | Tok::Pass { .. } - | Tok::Raise { .. } - | Tok::Return { .. } - | Tok::Try { .. } - | Tok::While { .. } - | Tok::With { .. } - | Tok::Yield { .. } + Tok::False + | Tok::True + | Tok::None + | Tok::And + | Tok::As + | Tok::Assert + | Tok::Await + | Tok::Break + | Tok::Class + | Tok::Continue + | Tok::Def + | Tok::Del + | Tok::Elif + | Tok::Else + | Tok::Except + | Tok::Finally + | Tok::For + | Tok::From + | Tok::Global + | Tok::If + | Tok::Import + | Tok::In + | Tok::Is + | Tok::Lambda + | Tok::Nonlocal + | Tok::Not + | Tok::Or + | Tok::Pass + | Tok::Raise + | Tok::Return + | Tok::Try + | Tok::While + | Tok::With + | Tok::Yield ) } @@ -104,3 +104,55 @@ pub fn is_singleton_token(token: &Tok) -> bool { Tok::False { .. } | Tok::True { .. } | Tok::None { .. }, ) } + +pub fn is_op_token(token: &Tok) -> bool { + matches!( + token, + Tok::Lpar + | Tok::Rpar + | Tok::Lsqb + | Tok::Rsqb + | Tok::Comma + | Tok::Semi + | Tok::Plus + | Tok::Minus + | Tok::Star + | Tok::Slash + | Tok::Vbar + | Tok::Amper + | Tok::Less + | Tok::Greater + | Tok::Equal + | Tok::Dot + | Tok::Percent + | Tok::Lbrace + | Tok::Rbrace + | Tok::NotEqual + | Tok::LessEqual + | Tok::GreaterEqual + | Tok::Tilde + | Tok::CircumFlex + | Tok::LeftShift + | Tok::RightShift + | Tok::DoubleStar + | Tok::PlusEqual + | Tok::MinusEqual + | Tok::StarEqual + | Tok::SlashEqual + | Tok::PercentEqual + | Tok::AmperEqual + | Tok::VbarEqual + | Tok::CircumflexEqual + | Tok::LeftShiftEqual + | Tok::RightShiftEqual + | Tok::DoubleStarEqual + | Tok::DoubleSlash + | Tok::DoubleSlashEqual + | Tok::At + | Tok::AtEqual + | Tok::Rarrow + | Tok::Ellipsis + | Tok::ColonEqual + | Tok::Colon + ) +} diff --git a/crates/ruff/src/rules/pycodestyle/logical_lines.rs b/crates/ruff/src/rules/pycodestyle/logical_lines.rs index cc84fefd09..bfa4f5bb39 100644 --- a/crates/ruff/src/rules/pycodestyle/logical_lines.rs +++ b/crates/ruff/src/rules/pycodestyle/logical_lines.rs @@ -3,6 +3,8 @@ use rustpython_parser::ast::Location; use rustpython_parser::lexer::LexResult; use rustpython_parser::Tok; +use crate::rules::pycodestyle::helpers::{is_keyword_token, is_op_token}; + use crate::ast::types::Range; use crate::source_code::Locator; @@ -62,36 +64,7 @@ fn build_line<'a>( continue; } - if matches!( - tok, - Tok::Amper - | Tok::AmperEqual - | Tok::CircumFlex - | Tok::CircumflexEqual - | Tok::Colon - | Tok::ColonEqual - | Tok::DoubleSlash - | Tok::DoubleSlashEqual - | Tok::DoubleStar - | Tok::Equal - | Tok::Greater - | Tok::GreaterEqual - | Tok::Less - | Tok::LessEqual - | Tok::Minus - | Tok::MinusEqual - | Tok::NotEqual - | Tok::Percent - | Tok::PercentEqual - | Tok::Plus - | Tok::PlusEqual - | Tok::Slash - | Tok::SlashEqual - | Tok::Star - | Tok::StarEqual - | Tok::Vbar - | Tok::VbarEqual - ) { + if is_op_token(tok) { flags.insert(TokenFlags::OPERATOR); } @@ -106,44 +79,7 @@ fn build_line<'a>( flags.insert(TokenFlags::PUNCTUATION); } - if matches!( - tok, - Tok::False - | Tok::None - | Tok::True - | Tok::And - | Tok::As - | Tok::Assert - | Tok::Async - | Tok::Await - | Tok::Break - | Tok::Class - | Tok::Continue - | Tok::Def - | Tok::Del - | Tok::Elif - | Tok::Else - | Tok::Except - | Tok::Finally - | Tok::For - | Tok::From - | Tok::Global - | Tok::If - | Tok::Import - | Tok::In - | Tok::Is - | Tok::Lambda - | Tok::Nonlocal - | Tok::Not - | Tok::Or - | Tok::Pass - | Tok::Raise - | Tok::Return - | Tok::Try - | Tok::While - | Tok::With - | Tok::Yield - ) { + if is_keyword_token(tok) { flags.insert(TokenFlags::KEYWORD); } diff --git a/crates/ruff/src/rules/pycodestyle/mod.rs b/crates/ruff/src/rules/pycodestyle/mod.rs index 2bc6e1662d..ce10996f80 100644 --- a/crates/ruff/src/rules/pycodestyle/mod.rs +++ b/crates/ruff/src/rules/pycodestyle/mod.rs @@ -101,6 +101,11 @@ mod tests { #[test_case(Rule::WhitespaceAfterOpenBracket, Path::new("E20.py"))] #[test_case(Rule::WhitespaceBeforeCloseBracket, Path::new("E20.py"))] #[test_case(Rule::WhitespaceBeforePunctuation, Path::new("E20.py"))] + #[test_case( + Rule::UnexpectedSpacesAroundKeywordParameterEquals, + Path::new("E25.py") + )] + #[test_case(Rule::MissingWhitespaceAroundParameterEquals, Path::new("E25.py"))] fn logical(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff/src/rules/pycodestyle/rules/mod.rs b/crates/ruff/src/rules/pycodestyle/rules/mod.rs index 0ad18a550c..12ba1ed57a 100644 --- a/crates/ruff/src/rules/pycodestyle/rules/mod.rs +++ b/crates/ruff/src/rules/pycodestyle/rules/mod.rs @@ -50,6 +50,11 @@ pub use whitespace_before_comment::{ NoSpaceAfterInlineComment, TooFewSpacesBeforeInlineComment, }; +pub use whitespace_around_named_parameter_equals::{ + whitespace_around_named_parameter_equals, MissingWhitespaceAroundParameterEquals, + UnexpectedSpacesAroundKeywordParameterEquals, +}; + mod ambiguous_class_name; mod ambiguous_function_name; mod ambiguous_variable_name; @@ -73,4 +78,5 @@ mod space_around_operator; mod trailing_whitespace; mod type_comparison; mod whitespace_around_keywords; +mod whitespace_around_named_parameter_equals; mod whitespace_before_comment; diff --git a/crates/ruff/src/rules/pycodestyle/rules/whitespace_around_named_parameter_equals.rs b/crates/ruff/src/rules/pycodestyle/rules/whitespace_around_named_parameter_equals.rs new file mode 100644 index 0000000000..d7e6a41af7 --- /dev/null +++ b/crates/ruff/src/rules/pycodestyle/rules/whitespace_around_named_parameter_equals.rs @@ -0,0 +1,114 @@ +#![allow(dead_code)] + +use rustpython_parser::ast::Location; +use rustpython_parser::Tok; + +use once_cell::sync::Lazy; +use regex::Regex; +use ruff_macros::{define_violation, derive_message_formats}; + +#[cfg(feature = "logical_lines")] +use crate::rules::pycodestyle::helpers::is_op_token; + +use crate::registry::DiagnosticKind; +use crate::violation::Violation; + +define_violation!( + pub struct UnexpectedSpacesAroundKeywordParameterEquals; +); +impl Violation for UnexpectedSpacesAroundKeywordParameterEquals { + #[derive_message_formats] + fn message(&self) -> String { + format!("Unexpected spaces around keyword / parameter equals") + } +} + +define_violation!( + pub struct MissingWhitespaceAroundParameterEquals; +); +impl Violation for MissingWhitespaceAroundParameterEquals { + #[derive_message_formats] + fn message(&self) -> String { + format!("Missing whitespace around parameter equals") + } +} + +static STARTSWITH_DEF_REGEX: Lazy = + Lazy::new(|| Regex::new(r"^(async\s+def|def)\b").unwrap()); + +/// E251, E252 +#[cfg(feature = "logical_lines")] +pub fn whitespace_around_named_parameter_equals( + tokens: &[(Location, &Tok, Location)], + line: &str, +) -> Vec<(Location, DiagnosticKind)> { + let mut diagnostics = vec![]; + let mut parens = 0; + let mut require_space = false; + let mut no_space = false; + let mut annotated_func_arg = false; + let mut prev_end: Option<&Location> = None; + + let in_def = STARTSWITH_DEF_REGEX.is_match(line); + + for (start, token, end) in tokens { + if **token == Tok::NonLogicalNewline { + continue; + } + if no_space { + no_space = false; + if Some(start) != prev_end { + diagnostics.push(( + *(prev_end.unwrap()), + UnexpectedSpacesAroundKeywordParameterEquals.into(), + )); + } + } + if require_space { + require_space = false; + if Some(start) == prev_end { + diagnostics.push((*start, MissingWhitespaceAroundParameterEquals.into())); + } + } + if is_op_token(token) { + if **token == Tok::Lpar || **token == Tok::Lsqb { + parens += 1; + } else if **token == Tok::Rpar || **token == Tok::Rsqb { + parens -= 1; + } else if in_def && **token == Tok::Colon && parens == 1 { + annotated_func_arg = true; + } else if parens == 1 && **token == Tok::Comma { + annotated_func_arg = false; + } else if parens > 0 && **token == Tok::Equal { + if annotated_func_arg && parens == 1 { + require_space = true; + if Some(start) == prev_end { + diagnostics.push((*start, MissingWhitespaceAroundParameterEquals.into())); + } + } else { + no_space = true; + if Some(start) != prev_end { + diagnostics.push(( + *(prev_end.unwrap()), + UnexpectedSpacesAroundKeywordParameterEquals.into(), + )); + } + } + } + + if parens < 1 { + annotated_func_arg = false; + } + } + prev_end = Some(end); + } + diagnostics +} + +#[cfg(not(feature = "logical_lines"))] +pub fn whitespace_around_named_parameter_equals( + _tokens: &[(Location, &Tok, Location)], + _line: &str, +) -> Vec<(Location, DiagnosticKind)> { + vec![] +} diff --git a/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E251_E25.py.snap b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E251_E25.py.snap new file mode 100644 index 0000000000..274b074d2a --- /dev/null +++ b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E251_E25.py.snap @@ -0,0 +1,115 @@ +--- +source: crates/ruff/src/rules/pycodestyle/mod.rs +expression: diagnostics +--- +- kind: + UnexpectedSpacesAroundKeywordParameterEquals: ~ + location: + row: 2 + column: 11 + end_location: + row: 2 + column: 11 + fix: ~ + parent: ~ +- kind: + UnexpectedSpacesAroundKeywordParameterEquals: ~ + location: + row: 2 + column: 13 + end_location: + row: 2 + column: 13 + fix: ~ + parent: ~ +- kind: + UnexpectedSpacesAroundKeywordParameterEquals: ~ + location: + row: 6 + column: 8 + end_location: + row: 6 + column: 8 + fix: ~ + parent: ~ +- kind: + UnexpectedSpacesAroundKeywordParameterEquals: ~ + location: + row: 8 + column: 7 + end_location: + row: 8 + column: 7 + fix: ~ + parent: ~ +- kind: + UnexpectedSpacesAroundKeywordParameterEquals: ~ + location: + row: 10 + column: 7 + end_location: + row: 10 + column: 7 + fix: ~ + parent: ~ +- kind: + UnexpectedSpacesAroundKeywordParameterEquals: ~ + location: + row: 10 + column: 9 + end_location: + row: 10 + column: 9 + fix: ~ + parent: ~ +- kind: + UnexpectedSpacesAroundKeywordParameterEquals: ~ + location: + row: 12 + column: 13 + end_location: + row: 12 + column: 13 + fix: ~ + parent: ~ +- kind: + UnexpectedSpacesAroundKeywordParameterEquals: ~ + location: + row: 15 + column: 28 + end_location: + row: 15 + column: 28 + fix: ~ + parent: ~ +- kind: + UnexpectedSpacesAroundKeywordParameterEquals: ~ + location: + row: 18 + column: 44 + end_location: + row: 18 + column: 44 + fix: ~ + parent: ~ +- kind: + UnexpectedSpacesAroundKeywordParameterEquals: ~ + location: + row: 23 + column: 7 + end_location: + row: 23 + column: 7 + fix: ~ + parent: ~ +- kind: + UnexpectedSpacesAroundKeywordParameterEquals: ~ + location: + row: 23 + column: 9 + end_location: + row: 23 + column: 9 + fix: ~ + parent: ~ + diff --git a/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E252_E25.py.snap b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E252_E25.py.snap new file mode 100644 index 0000000000..2db2258a5f --- /dev/null +++ b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E252_E25.py.snap @@ -0,0 +1,45 @@ +--- +source: crates/ruff/src/rules/pycodestyle/mod.rs +expression: diagnostics +--- +- kind: + MissingWhitespaceAroundParameterEquals: ~ + location: + row: 46 + column: 14 + end_location: + row: 46 + column: 14 + fix: ~ + parent: ~ +- kind: + MissingWhitespaceAroundParameterEquals: ~ + location: + row: 46 + column: 15 + end_location: + row: 46 + column: 15 + fix: ~ + parent: ~ +- kind: + MissingWhitespaceAroundParameterEquals: ~ + location: + row: 46 + column: 26 + end_location: + row: 46 + column: 26 + fix: ~ + parent: ~ +- kind: + MissingWhitespaceAroundParameterEquals: ~ + location: + row: 46 + column: 35 + end_location: + row: 46 + column: 35 + fix: ~ + parent: ~ +