feat(e231): add rule + autofix (#3344)

crates/ruff/resources/test/fixtures/pycodestyle/E23.py

@@ -0,0 +1,15 @@
+#: E231
+a = (1,2)
+#: E231
+a[b1,:]
+#: E231
+a = [{'a':''}]
+#: Okay
+a = (4,)
+b = (5, )
+c = {'text': text[5:]}
+
+result = {
+    'key1': 'value',
+    'key2': 'value',
+}

crates/ruff/src/checkers/logical_lines.rs

@@ -9,7 +9,7 @@ use crate::ast::types::Range;
 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,
+    extraneous_whitespace, indentation, missing_whitespace, missing_whitespace_after_keyword,
     missing_whitespace_around_operator, space_around_operator, whitespace_around_keywords,
     whitespace_around_named_parameter_equals, whitespace_before_comment,
     whitespace_before_parameters,
@@ -162,6 +162,18 @@ pub fn check_logical_lines(
                     });
                 }
             }
+
+            #[cfg(feature = "logical_lines")]
+            let should_fix = autofix.into() && settings.rules.should_fix(&Rule::MissingWhitespace);
+
+            #[cfg(not(feature = "logical_lines"))]
+            let should_fix = false;
+
+            for diagnostic in missing_whitespace(&line.text, start_loc.row(), should_fix) {
+                if settings.rules.enabled(diagnostic.kind.rule()) {
+                    diagnostics.push(diagnostic);
+                }
+            }
         }
 
         if line.flags.contains(TokenFlags::BRACKET) {
@@ -291,7 +303,7 @@ f()"#;
             .into_iter()
             .map(|line| line.text)
             .collect();
-        let expected = vec!["def f():", "\"xxx\"", "", "x = 1", "f()"];
+        let expected = vec!["def f():", "\"xxxxxxxxxxxxxxxxxxxx\"", "", "x = 1", "f()"];
         assert_eq!(actual, expected);
     }
 }

crates/ruff/src/codes.rs

@@ -47,6 +47,8 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
         #[cfg(feature = "logical_lines")]
         (Pycodestyle, "E228") => Rule::MissingWhitespaceAroundModuloOperator,
         #[cfg(feature = "logical_lines")]
+        (Pycodestyle, "E231") => Rule::MissingWhitespace,
+        #[cfg(feature = "logical_lines")]
         (Pycodestyle, "E251") => Rule::UnexpectedSpacesAroundKeywordParameterEquals,
         #[cfg(feature = "logical_lines")]
         (Pycodestyle, "E252") => Rule::MissingWhitespaceAroundParameterEquals,

crates/ruff/src/registry.rs

@@ -53,6 +53,8 @@ ruff_macros::register_rules!(
     #[cfg(feature = "logical_lines")]
     rules::pycodestyle::rules::MultipleSpacesAfterKeyword,
     #[cfg(feature = "logical_lines")]
+    rules::pycodestyle::rules::MissingWhitespace,
+    #[cfg(feature = "logical_lines")]
     rules::pycodestyle::rules::MissingWhitespaceAfterKeyword,
     #[cfg(feature = "logical_lines")]
     rules::pycodestyle::rules::MultipleSpacesBeforeKeyword,
@@ -847,6 +849,7 @@ impl Rule {
             #[cfg(feature = "logical_lines")]
             Rule::IndentationWithInvalidMultiple
             | Rule::IndentationWithInvalidMultipleComment
+            | Rule::MissingWhitespace
             | Rule::MissingWhitespaceAfterKeyword
             | Rule::MissingWhitespaceAroundArithmeticOperator
             | Rule::MissingWhitespaceAroundBitwiseOrShiftOperator

crates/ruff/src/rules/pycodestyle/logical_lines.rs

@@ -84,8 +84,10 @@ fn build_line<'a>(
         }
 
         // TODO(charlie): "Mute" strings.
-        let text = if let Tok::String { .. } = tok {
+        let s;
-            "\"xxx\""
+        let text = if let Tok::String { value, .. } = tok {
+            s = format!("\"{}\"", "x".repeat(value.len()).clone());
+            &s
         } else {
             locator.slice(&Range {
                 location: *start,

crates/ruff/src/rules/pycodestyle/mod.rs

@@ -103,6 +103,7 @@ mod tests {
         Path::new("E22.py")
     )]
     #[test_case(Rule::MissingWhitespaceAroundModuloOperator, Path::new("E22.py"))]
+    #[test_case(Rule::MissingWhitespace, Path::new("E23.py"))]
     #[test_case(Rule::TooFewSpacesBeforeInlineComment, Path::new("E26.py"))]
     #[test_case(Rule::UnexpectedIndentation, Path::new("E11.py"))]
     #[test_case(Rule::UnexpectedIndentationComment, Path::new("E11.py"))]

crates/ruff/src/rules/pycodestyle/rules/missing_whitespace.rs

@@ -0,0 +1,80 @@
+#![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::registry::DiagnosticKind;
+use crate::rules::pycodestyle::helpers::{is_keyword_token, is_singleton_token};
+use crate::violation::AlwaysAutofixableViolation;
+use crate::violation::Violation;
+
+define_violation!(
+    pub struct MissingWhitespace {
+        pub token: String,
+    }
+);
+impl AlwaysAutofixableViolation for MissingWhitespace {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        let MissingWhitespace { token } = self;
+        format!("Missing whitespace after '{token}'")
+    }
+
+    fn autofix_title(&self) -> String {
+        let MissingWhitespace { token } = self;
+        format!("Added missing whitespace after '{token}'")
+    }
+}
+
+/// E231
+#[cfg(feature = "logical_lines")]
+pub fn missing_whitespace(line: &str, row: usize, autofix: bool) -> Vec<Diagnostic> {
+    let mut diagnostics = vec![];
+    for (idx, char) in line.chars().enumerate() {
+        if idx + 1 == line.len() {
+            break;
+        }
+        let next_char = line.chars().nth(idx + 1).unwrap();
+
+        if ",;:".contains(char) && !char::is_whitespace(next_char) {
+            let before = &line[..idx];
+            if char == ':'
+                && before.matches('[').count() > before.matches(']').count()
+                && before.rfind('{') < before.rfind('[')
+            {
+                continue; // Slice syntax, no space required
+            }
+            if char == ',' && ")]".contains(next_char) {
+                continue; // Allow tuple with only one element: (3,)
+            }
+            if char == ':' && next_char == '=' {
+                continue; // Allow assignment expression
+            }
+
+            let kind: MissingWhitespace = MissingWhitespace {
+                token: char.to_string(),
+            };
+
+            let mut diagnostic = Diagnostic::new(
+                kind,
+                Range::new(Location::new(row, idx), Location::new(row, idx)),
+            );
+
+            if autofix {
+                diagnostic.amend(Fix::insertion(" ".to_string(), Location::new(row, idx + 1)));
+            }
+            diagnostics.push(diagnostic);
+        }
+    }
+    diagnostics
+}
+
+#[cfg(not(feature = "logical_lines"))]
+pub fn missing_whitespace(_line: &str, _row: usize, _autofix: bool) -> Vec<Diagnostic> {
+    vec![]
+}

crates/ruff/src/rules/pycodestyle/rules/mod.rs

@@ -26,6 +26,7 @@ pub use invalid_escape_sequence::{invalid_escape_sequence, InvalidEscapeSequence
 pub use lambda_assignment::{lambda_assignment, LambdaAssignment};
 pub use line_too_long::{line_too_long, LineTooLong};
 pub use literal_comparisons::{literal_comparisons, NoneComparison, TrueFalseComparison};
+pub use missing_whitespace::{missing_whitespace, MissingWhitespace};
 pub use missing_whitespace_after_keyword::{
     missing_whitespace_after_keyword, MissingWhitespaceAfterKeyword,
 };
@@ -74,6 +75,7 @@ mod invalid_escape_sequence;
 mod lambda_assignment;
 mod line_too_long;
 mod literal_comparisons;
+mod missing_whitespace;
 mod missing_whitespace_after_keyword;
 mod missing_whitespace_around_operator;
 mod mixed_spaces_and_tabs;

crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E231_E23.py.snap

@@ -0,0 +1,59 @@
+---
+source: crates/ruff/src/rules/pycodestyle/mod.rs
+expression: diagnostics
+---
+- kind:
+    MissingWhitespace:
+      token: ","
+  location:
+    row: 2
+    column: 6
+  end_location:
+    row: 2
+    column: 6
+  fix:
+    content: " "
+    location:
+      row: 2
+      column: 7
+    end_location:
+      row: 2
+      column: 7
+  parent: ~
+- kind:
+    MissingWhitespace:
+      token: ","
+  location:
+    row: 4
+    column: 4
+  end_location:
+    row: 4
+    column: 4
+  fix:
+    content: " "
+    location:
+      row: 4
+      column: 5
+    end_location:
+      row: 4
+      column: 5
+  parent: ~
+- kind:
+    MissingWhitespace:
+      token: ":"
+  location:
+    row: 6
+    column: 9
+  end_location:
+    row: 6
+    column: 9
+  fix:
+    content: " "
+    location:
+      row: 6
+      column: 10
+    end_location:
+      row: 6
+      column: 10
+  parent: ~
+