From 6f649d657988fd3207b560c0ca1b0a8c8841b51f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Gon=C3=A7alves?= <43336371+carlosmiei@users.noreply.github.com> Date: Thu, 2 Mar 2023 22:48:04 +0000 Subject: [PATCH] feat(E211): add rule + autofix (#3313) --- .../test/fixtures/pycodestyle/E21.py | 14 ++++ crates/ruff/src/checkers/logical_lines.rs | 24 +++++- crates/ruff/src/codes.rs | 2 + crates/ruff/src/linter.rs | 8 +- crates/ruff/src/registry.rs | 3 + crates/ruff/src/rules/pycodestyle/helpers.rs | 4 + crates/ruff/src/rules/pycodestyle/mod.rs | 1 + .../rules/extraneous_whitespace.rs | 2 +- .../rules/pycodestyle/rules/indentation.rs | 2 +- .../rules/missing_whitespace_after_keyword.rs | 2 +- .../ruff/src/rules/pycodestyle/rules/mod.rs | 3 + .../rules/space_around_operator.rs | 2 +- .../rules/whitespace_around_keywords.rs | 2 +- ...hitespace_around_named_parameter_equals.rs | 2 +- .../rules/whitespace_before_comment.rs | 2 +- .../rules/whitespace_before_parameters.rs | 77 +++++++++++++++++++ ...ules__pycodestyle__tests__E211_E21.py.snap | 77 +++++++++++++++++++ 17 files changed, 216 insertions(+), 11 deletions(-) create mode 100644 crates/ruff/resources/test/fixtures/pycodestyle/E21.py create mode 100644 crates/ruff/src/rules/pycodestyle/rules/whitespace_before_parameters.rs create mode 100644 crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E211_E21.py.snap diff --git a/crates/ruff/resources/test/fixtures/pycodestyle/E21.py b/crates/ruff/resources/test/fixtures/pycodestyle/E21.py new file mode 100644 index 0000000000..96b55b8790 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/pycodestyle/E21.py @@ -0,0 +1,14 @@ +#: E211 +spam (1) +#: E211 E211 +dict ['key'] = list [index] +#: E211 +dict['key'] ['subkey'] = list[index] +#: Okay +spam(1) +dict['key'] = list[index] + + +# This is not prohibited by PEP8, but avoid it. +class Foo (Bar, Baz): + pass diff --git a/crates/ruff/src/checkers/logical_lines.rs b/crates/ruff/src/checkers/logical_lines.rs index d4ece00f9c..ae521622e0 100644 --- a/crates/ruff/src/checkers/logical_lines.rs +++ b/crates/ruff/src/checkers/logical_lines.rs @@ -1,17 +1,19 @@ +#![allow(dead_code, unused_imports, unused_variables)] + use bisection::bisect_left; use itertools::Itertools; use rustpython_parser::ast::Location; use rustpython_parser::lexer::LexResult; use crate::ast::types::Range; -use crate::registry::Diagnostic; +use crate::registry::{Diagnostic, Rule}; 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_around_named_parameter_equals, - whitespace_before_comment, + whitespace_before_comment, whitespace_before_parameters, }; -use crate::settings::Settings; +use crate::settings::{flags, Settings}; use crate::source_code::{Locator, Stylist}; /// Return the amount of indentation, expanding tabs to the next multiple of 8. @@ -40,6 +42,7 @@ pub fn check_logical_lines( locator: &Locator, stylist: &Stylist, settings: &Settings, + autofix: flags::Autofix, ) -> Vec { let mut diagnostics = vec![]; @@ -149,6 +152,21 @@ pub fn check_logical_lines( } } + if line.flags.contains(TokenFlags::BRACKET) { + #[cfg(feature = "logical_lines")] + let should_fix = + autofix.into() && settings.rules.should_fix(&Rule::WhitespaceBeforeParameters); + + #[cfg(not(feature = "logical_lines"))] + let should_fix = false; + + for diagnostic in whitespace_before_parameters(&line.tokens, should_fix) { + if settings.rules.enabled(diagnostic.kind.rule()) { + diagnostics.push(diagnostic); + } + } + } + for (index, kind) in indentation( &line, prev_line.as_ref(), diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index cade353efc..84c024ad35 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -29,6 +29,8 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option { #[cfg(feature = "logical_lines")] (Pycodestyle, "E203") => Rule::WhitespaceBeforePunctuation, #[cfg(feature = "logical_lines")] + (Pycodestyle, "E211") => Rule::WhitespaceBeforeParameters, + #[cfg(feature = "logical_lines")] (Pycodestyle, "E221") => Rule::MultipleSpacesBeforeOperator, #[cfg(feature = "logical_lines")] (Pycodestyle, "E222") => Rule::MultipleSpacesAfterOperator, diff --git a/crates/ruff/src/linter.rs b/crates/ruff/src/linter.rs index 799a53921b..ac36d5a0e4 100644 --- a/crates/ruff/src/linter.rs +++ b/crates/ruff/src/linter.rs @@ -101,7 +101,13 @@ pub fn check_path( .iter_enabled() .any(|rule_code| rule_code.lint_source().is_logical_lines()) { - diagnostics.extend(check_logical_lines(&tokens, locator, stylist, settings)); + diagnostics.extend(check_logical_lines( + &tokens, + locator, + stylist, + settings, + flags::Autofix::Enabled, + )); } // Run the AST-based rules. diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 2c6e7ef9c7..7effcb2b9c 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -63,6 +63,8 @@ ruff_macros::register_rules!( #[cfg(feature = "logical_lines")] rules::pycodestyle::rules::MissingWhitespaceAroundParameterEquals, #[cfg(feature = "logical_lines")] + rules::pycodestyle::rules::WhitespaceBeforeParameters, + #[cfg(feature = "logical_lines")] rules::pycodestyle::rules::TabBeforeKeyword, rules::pycodestyle::rules::MultipleImportsOnOneLine, rules::pycodestyle::rules::ModuleImportNotAtTopOfFile, @@ -857,6 +859,7 @@ impl Rule { | Rule::WhitespaceBeforeCloseBracket | Rule::UnexpectedSpacesAroundKeywordParameterEquals | Rule::MissingWhitespaceAroundParameterEquals + | Rule::WhitespaceBeforeParameters | 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 e57227a2ff..2a44139724 100644 --- a/crates/ruff/src/rules/pycodestyle/helpers.rs +++ b/crates/ruff/src/rules/pycodestyle/helpers.rs @@ -156,3 +156,7 @@ pub fn is_op_token(token: &Tok) -> bool { | Tok::Colon ) } + +pub fn is_soft_keyword_token(token: &Tok) -> bool { + matches!(token, Tok::Match | Tok::Case) +} diff --git a/crates/ruff/src/rules/pycodestyle/mod.rs b/crates/ruff/src/rules/pycodestyle/mod.rs index ce10996f80..4c4d5d3efe 100644 --- a/crates/ruff/src/rules/pycodestyle/mod.rs +++ b/crates/ruff/src/rules/pycodestyle/mod.rs @@ -101,6 +101,7 @@ 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::WhitespaceBeforeParameters, Path::new("E21.py"))] #[test_case( Rule::UnexpectedSpacesAroundKeywordParameterEquals, Path::new("E25.py") diff --git a/crates/ruff/src/rules/pycodestyle/rules/extraneous_whitespace.rs b/crates/ruff/src/rules/pycodestyle/rules/extraneous_whitespace.rs index 79ac7cc151..478180bca3 100644 --- a/crates/ruff/src/rules/pycodestyle/rules/extraneous_whitespace.rs +++ b/crates/ruff/src/rules/pycodestyle/rules/extraneous_whitespace.rs @@ -1,4 +1,4 @@ -#![allow(dead_code)] +#![allow(dead_code, unused_imports, unused_variables)] use once_cell::sync::Lazy; use regex::Regex; diff --git a/crates/ruff/src/rules/pycodestyle/rules/indentation.rs b/crates/ruff/src/rules/pycodestyle/rules/indentation.rs index 59047a8533..91dbbb9186 100644 --- a/crates/ruff/src/rules/pycodestyle/rules/indentation.rs +++ b/crates/ruff/src/rules/pycodestyle/rules/indentation.rs @@ -1,4 +1,4 @@ -#![allow(dead_code)] +#![allow(dead_code, unused_imports, unused_variables)] use ruff_macros::{define_violation, derive_message_formats}; diff --git a/crates/ruff/src/rules/pycodestyle/rules/missing_whitespace_after_keyword.rs b/crates/ruff/src/rules/pycodestyle/rules/missing_whitespace_after_keyword.rs index dab4df3fde..0262744760 100644 --- a/crates/ruff/src/rules/pycodestyle/rules/missing_whitespace_after_keyword.rs +++ b/crates/ruff/src/rules/pycodestyle/rules/missing_whitespace_after_keyword.rs @@ -1,4 +1,4 @@ -#![allow(dead_code, unused_imports)] +#![allow(dead_code, unused_imports, unused_variables)] use rustpython_parser::ast::Location; use rustpython_parser::Tok; diff --git a/crates/ruff/src/rules/pycodestyle/rules/mod.rs b/crates/ruff/src/rules/pycodestyle/rules/mod.rs index 12ba1ed57a..d7700788d9 100644 --- a/crates/ruff/src/rules/pycodestyle/rules/mod.rs +++ b/crates/ruff/src/rules/pycodestyle/rules/mod.rs @@ -55,6 +55,8 @@ pub use whitespace_around_named_parameter_equals::{ UnexpectedSpacesAroundKeywordParameterEquals, }; +pub use whitespace_before_parameters::{whitespace_before_parameters, WhitespaceBeforeParameters}; + mod ambiguous_class_name; mod ambiguous_function_name; mod ambiguous_variable_name; @@ -80,3 +82,4 @@ mod type_comparison; mod whitespace_around_keywords; mod whitespace_around_named_parameter_equals; mod whitespace_before_comment; +mod whitespace_before_parameters; diff --git a/crates/ruff/src/rules/pycodestyle/rules/space_around_operator.rs b/crates/ruff/src/rules/pycodestyle/rules/space_around_operator.rs index e1db233fb3..adfee0d4bd 100644 --- a/crates/ruff/src/rules/pycodestyle/rules/space_around_operator.rs +++ b/crates/ruff/src/rules/pycodestyle/rules/space_around_operator.rs @@ -1,4 +1,4 @@ -#![allow(dead_code)] +#![allow(dead_code, unused_imports, unused_variables)] use once_cell::sync::Lazy; use regex::Regex; diff --git a/crates/ruff/src/rules/pycodestyle/rules/whitespace_around_keywords.rs b/crates/ruff/src/rules/pycodestyle/rules/whitespace_around_keywords.rs index 9f1990b0f0..f887df81b7 100644 --- a/crates/ruff/src/rules/pycodestyle/rules/whitespace_around_keywords.rs +++ b/crates/ruff/src/rules/pycodestyle/rules/whitespace_around_keywords.rs @@ -1,4 +1,4 @@ -#![allow(dead_code)] +#![allow(dead_code, unused_imports, unused_variables)] use once_cell::sync::Lazy; use regex::Regex; 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 index d7e6a41af7..7b257caae9 100644 --- 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 @@ -1,4 +1,4 @@ -#![allow(dead_code)] +#![allow(dead_code, unused_imports, unused_variables)] use rustpython_parser::ast::Location; use rustpython_parser::Tok; diff --git a/crates/ruff/src/rules/pycodestyle/rules/whitespace_before_comment.rs b/crates/ruff/src/rules/pycodestyle/rules/whitespace_before_comment.rs index 60f059d27c..133aa3735c 100644 --- a/crates/ruff/src/rules/pycodestyle/rules/whitespace_before_comment.rs +++ b/crates/ruff/src/rules/pycodestyle/rules/whitespace_before_comment.rs @@ -1,4 +1,4 @@ -#![allow(dead_code)] +#![allow(dead_code, unused_imports, unused_variables)] use ruff_macros::{define_violation, derive_message_formats}; use rustpython_parser::ast::Location; diff --git a/crates/ruff/src/rules/pycodestyle/rules/whitespace_before_parameters.rs b/crates/ruff/src/rules/pycodestyle/rules/whitespace_before_parameters.rs new file mode 100644 index 0000000000..ecc139dec0 --- /dev/null +++ b/crates/ruff/src/rules/pycodestyle/rules/whitespace_before_parameters.rs @@ -0,0 +1,77 @@ +#![allow(dead_code, unused_imports, unused_variables)] + +use rustpython_parser::ast::Location; +use rustpython_parser::Tok; + +use ruff_macros::{define_violation, derive_message_formats}; + +use crate::ast::types::Range; +use crate::fix::Fix; +use crate::registry::Diagnostic; +use crate::rules::pycodestyle::helpers::{is_keyword_token, is_op_token, is_soft_keyword_token}; +use crate::violation::AlwaysAutofixableViolation; + +define_violation!( + pub struct WhitespaceBeforeParameters { + pub bracket: String, + } +); + +impl AlwaysAutofixableViolation for WhitespaceBeforeParameters { + #[derive_message_formats] + fn message(&self) -> String { + let WhitespaceBeforeParameters { bracket } = self; + format!("Whitespace before {bracket}") + } + + fn autofix_title(&self) -> String { + let WhitespaceBeforeParameters { bracket } = self; + format!("Removed whitespace before {bracket}") + } +} + +/// E211 +#[cfg(feature = "logical_lines")] +pub fn whitespace_before_parameters( + tokens: &[(Location, &Tok, Location)], + autofix: bool, +) -> Vec { + let mut diagnostics = vec![]; + let (_, mut prev_token, mut prev_end) = tokens.first().unwrap(); + for (idx, (start, tok, end)) in tokens.iter().enumerate() { + if is_op_token(tok) + && (**tok == Tok::Lpar || **tok == Tok::Lsqb) + && *start != prev_end + && (matches!(prev_token, Tok::Name { .. }) + || matches!(prev_token, Tok::Rpar | Tok::Rsqb | Tok::Rbrace)) + && (idx < 2 || *(tokens[idx - 2].1) != Tok::Class) + && !is_keyword_token(tok) + && !is_soft_keyword_token(tok) + { + let start = Location::new(prev_end.row(), prev_end.column()); + let end = Location::new(end.row(), end.column() - 1); + + let kind: WhitespaceBeforeParameters = WhitespaceBeforeParameters { + bracket: tok.to_string(), + }; + + let mut diagnostic = Diagnostic::new(kind, Range::new(start, end)); + + if autofix { + diagnostic.amend(Fix::deletion(start, end)); + } + diagnostics.push(diagnostic); + } + prev_token = *tok; + prev_end = *end; + } + diagnostics +} + +#[cfg(not(feature = "logical_lines"))] +pub fn whitespace_before_parameters( + _tokens: &[(Location, &Tok, Location)], + _autofix: bool, +) -> Vec { + vec![] +} diff --git a/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E211_E21.py.snap b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E211_E21.py.snap new file mode 100644 index 0000000000..4351420747 --- /dev/null +++ b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E211_E21.py.snap @@ -0,0 +1,77 @@ +--- +source: crates/ruff/src/rules/pycodestyle/mod.rs +expression: diagnostics +--- +- kind: + WhitespaceBeforeParameters: + bracket: "'('" + location: + row: 2 + column: 4 + end_location: + row: 2 + column: 5 + fix: + content: "" + location: + row: 2 + column: 4 + end_location: + row: 2 + column: 5 + parent: ~ +- kind: + WhitespaceBeforeParameters: + bracket: "'['" + location: + row: 4 + column: 4 + end_location: + row: 4 + column: 5 + fix: + content: "" + location: + row: 4 + column: 4 + end_location: + row: 4 + column: 5 + parent: ~ +- kind: + WhitespaceBeforeParameters: + bracket: "'['" + location: + row: 4 + column: 19 + end_location: + row: 4 + column: 20 + fix: + content: "" + location: + row: 4 + column: 19 + end_location: + row: 4 + column: 20 + parent: ~ +- kind: + WhitespaceBeforeParameters: + bracket: "'['" + location: + row: 6 + column: 11 + end_location: + row: 6 + column: 12 + fix: + content: "" + location: + row: 6 + column: 11 + end_location: + row: 6 + column: 12 + parent: ~ +