From d9ac170eb45660b8ac3e61a81b168202b76f1582 Mon Sep 17 00:00:00 2001 From: Auguste Lalande Date: Thu, 21 Mar 2024 04:13:37 -0400 Subject: [PATCH] Fix `E231` bug: Inconsistent catch compared to pycodestyle, such as when dict nested in list (#10469) ## Summary Fix `E231` bug: Inconsistent catch compared to pycodestyle, such as when dict nested in list. Resolves #10113. ## Test Plan Example from #10113 added to test fixture. --- .../test/fixtures/pycodestyle/E23.py | 58 ++- .../rules/logical_lines/missing_whitespace.rs | 18 +- ...ules__pycodestyle__tests__E231_E23.py.snap | 336 +++++++++++++++++- 3 files changed, 401 insertions(+), 11 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pycodestyle/E23.py b/crates/ruff_linter/resources/test/fixtures/pycodestyle/E23.py index 2d7e70e99d4ac..04f420a8b9fa1 100644 --- a/crates/ruff_linter/resources/test/fixtures/pycodestyle/E23.py +++ b/crates/ruff_linter/resources/test/fixtures/pycodestyle/E23.py @@ -47,4 +47,60 @@ def foo() -> None: {len(f's3://{self.s3_bucket_name}/'):1} #: Okay -a = (1, +a = (1,) + + +# https://github.com/astral-sh/ruff/issues/10113 +"""Minimal repo.""" + +def main() -> None: + """Primary function.""" + results = { + "k1": [1], + "k2":[2], + } + results_in_tuple = ( + { + "k1": [1], + "k2":[2], + }, + ) + results_in_list = [ + { + "k1": [1], + "k2":[2], + } + ] + results_in_list_first = [ + { + "k2":[2], + } + ] + +x = [ + { + "k1":[2], # E231 + "k2": [2:4], + "k3":[2], # E231 + "k4": [2], + "k5": [2], + "k6": [1, 2, 3, 4,5,6,7] # E231 + }, + { + "k1": [ + { + "ka":[2,3], # E231 + }, + { + "kb": [2,3], # E231 + }, + { + "ka":[2, 3], # E231 + "kb": [2, 3], # Ok + "kc": [2, 3], # Ok + "kd": [2,3], # E231 + "ke":[2,3], # E231 + }, + ] + } +] diff --git a/crates/ruff_linter/src/rules/pycodestyle/rules/logical_lines/missing_whitespace.rs b/crates/ruff_linter/src/rules/pycodestyle/rules/logical_lines/missing_whitespace.rs index 2da0664dc7ead..b9a7eb8aab6e4 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/rules/logical_lines/missing_whitespace.rs +++ b/crates/ruff_linter/src/rules/pycodestyle/rules/logical_lines/missing_whitespace.rs @@ -2,7 +2,7 @@ use ruff_diagnostics::Edit; use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_parser::TokenKind; -use ruff_text_size::{Ranged, TextSize}; +use ruff_text_size::Ranged; use crate::checkers::logical_lines::LogicalLinesContext; @@ -53,10 +53,8 @@ impl AlwaysFixableViolation for MissingWhitespace { /// E231 pub(crate) fn missing_whitespace(line: &LogicalLine, context: &mut LogicalLinesContext) { - let mut open_parentheses = 0u32; let mut fstrings = 0u32; - let mut prev_lsqb = TextSize::default(); - let mut prev_lbrace = TextSize::default(); + let mut brackets = Vec::new(); let mut iter = line.tokens().iter().peekable(); while let Some(token) = iter.next() { @@ -65,14 +63,16 @@ pub(crate) fn missing_whitespace(line: &LogicalLine, context: &mut LogicalLinesC TokenKind::FStringStart => fstrings += 1, TokenKind::FStringEnd => fstrings = fstrings.saturating_sub(1), TokenKind::Lsqb if fstrings == 0 => { - open_parentheses = open_parentheses.saturating_add(1); - prev_lsqb = token.start(); + brackets.push(kind); } TokenKind::Rsqb if fstrings == 0 => { - open_parentheses = open_parentheses.saturating_sub(1); + brackets.pop(); } TokenKind::Lbrace if fstrings == 0 => { - prev_lbrace = token.start(); + brackets.push(kind); + } + TokenKind::Rbrace if fstrings == 0 => { + brackets.pop(); } TokenKind::Colon if fstrings > 0 => { // Colon in f-string, no space required. This will yield false @@ -97,7 +97,7 @@ pub(crate) fn missing_whitespace(line: &LogicalLine, context: &mut LogicalLinesC if let Some(next_token) = iter.peek() { match (kind, next_token.kind()) { (TokenKind::Colon, _) - if open_parentheses > 0 && prev_lsqb > prev_lbrace => + if matches!(brackets.last(), Some(TokenKind::Lsqb)) => { continue; // Slice syntax, no space required } diff --git a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E231_E23.py.snap b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E231_E23.py.snap index f6c0a3a4aed82..28636eda80789 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E231_E23.py.snap +++ b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E231_E23.py.snap @@ -139,6 +139,340 @@ E23.py:47:37: E231 [*] Missing whitespace after ':' 47 |+{len(f's3://{self.s3_bucket_name}/'): 1} 48 48 | 49 49 | #: Okay -50 50 | a = (1, +50 50 | a = (1,) +E23.py:60:13: E231 [*] Missing whitespace after ':' + | +58 | results = { +59 | "k1": [1], +60 | "k2":[2], + | ^ E231 +61 | } +62 | results_in_tuple = ( + | + = help: Add missing whitespace + +ℹ Safe fix +57 57 | """Primary function.""" +58 58 | results = { +59 59 | "k1": [1], +60 |- "k2":[2], + 60 |+ "k2": [2], +61 61 | } +62 62 | results_in_tuple = ( +63 63 | { + +E23.py:65:17: E231 [*] Missing whitespace after ':' + | +63 | { +64 | "k1": [1], +65 | "k2":[2], + | ^ E231 +66 | }, +67 | ) + | + = help: Add missing whitespace + +ℹ Safe fix +62 62 | results_in_tuple = ( +63 63 | { +64 64 | "k1": [1], +65 |- "k2":[2], + 65 |+ "k2": [2], +66 66 | }, +67 67 | ) +68 68 | results_in_list = [ + +E23.py:71:17: E231 [*] Missing whitespace after ':' + | +69 | { +70 | "k1": [1], +71 | "k2":[2], + | ^ E231 +72 | } +73 | ] + | + = help: Add missing whitespace + +ℹ Safe fix +68 68 | results_in_list = [ +69 69 | { +70 70 | "k1": [1], +71 |- "k2":[2], + 71 |+ "k2": [2], +72 72 | } +73 73 | ] +74 74 | results_in_list_first = [ + +E23.py:76:17: E231 [*] Missing whitespace after ':' + | +74 | results_in_list_first = [ +75 | { +76 | "k2":[2], + | ^ E231 +77 | } +78 | ] + | + = help: Add missing whitespace + +ℹ Safe fix +73 73 | ] +74 74 | results_in_list_first = [ +75 75 | { +76 |- "k2":[2], + 76 |+ "k2": [2], +77 77 | } +78 78 | ] +79 79 | + +E23.py:82:13: E231 [*] Missing whitespace after ':' + | +80 | x = [ +81 | { +82 | "k1":[2], # E231 + | ^ E231 +83 | "k2": [2:4], +84 | "k3":[2], # E231 + | + = help: Add missing whitespace + +ℹ Safe fix +79 79 | +80 80 | x = [ +81 81 | { +82 |- "k1":[2], # E231 + 82 |+ "k1": [2], # E231 +83 83 | "k2": [2:4], +84 84 | "k3":[2], # E231 +85 85 | "k4": [2], + +E23.py:84:13: E231 [*] Missing whitespace after ':' + | +82 | "k1":[2], # E231 +83 | "k2": [2:4], +84 | "k3":[2], # E231 + | ^ E231 +85 | "k4": [2], +86 | "k5": [2], + | + = help: Add missing whitespace + +ℹ Safe fix +81 81 | { +82 82 | "k1":[2], # E231 +83 83 | "k2": [2:4], +84 |- "k3":[2], # E231 + 84 |+ "k3": [2], # E231 +85 85 | "k4": [2], +86 86 | "k5": [2], +87 87 | "k6": [1, 2, 3, 4,5,6,7] # E231 + +E23.py:87:26: E231 [*] Missing whitespace after ',' + | +85 | "k4": [2], +86 | "k5": [2], +87 | "k6": [1, 2, 3, 4,5,6,7] # E231 + | ^ E231 +88 | }, +89 | { + | + = help: Add missing whitespace + +ℹ Safe fix +84 84 | "k3":[2], # E231 +85 85 | "k4": [2], +86 86 | "k5": [2], +87 |- "k6": [1, 2, 3, 4,5,6,7] # E231 + 87 |+ "k6": [1, 2, 3, 4, 5,6,7] # E231 +88 88 | }, +89 89 | { +90 90 | "k1": [ +E23.py:87:28: E231 [*] Missing whitespace after ',' + | +85 | "k4": [2], +86 | "k5": [2], +87 | "k6": [1, 2, 3, 4,5,6,7] # E231 + | ^ E231 +88 | }, +89 | { + | + = help: Add missing whitespace + +ℹ Safe fix +84 84 | "k3":[2], # E231 +85 85 | "k4": [2], +86 86 | "k5": [2], +87 |- "k6": [1, 2, 3, 4,5,6,7] # E231 + 87 |+ "k6": [1, 2, 3, 4,5, 6,7] # E231 +88 88 | }, +89 89 | { +90 90 | "k1": [ + +E23.py:87:30: E231 [*] Missing whitespace after ',' + | +85 | "k4": [2], +86 | "k5": [2], +87 | "k6": [1, 2, 3, 4,5,6,7] # E231 + | ^ E231 +88 | }, +89 | { + | + = help: Add missing whitespace + +ℹ Safe fix +84 84 | "k3":[2], # E231 +85 85 | "k4": [2], +86 86 | "k5": [2], +87 |- "k6": [1, 2, 3, 4,5,6,7] # E231 + 87 |+ "k6": [1, 2, 3, 4,5,6, 7] # E231 +88 88 | }, +89 89 | { +90 90 | "k1": [ + +E23.py:92:21: E231 [*] Missing whitespace after ':' + | +90 | "k1": [ +91 | { +92 | "ka":[2,3], # E231 + | ^ E231 +93 | }, +94 | { + | + = help: Add missing whitespace + +ℹ Safe fix +89 89 | { +90 90 | "k1": [ +91 91 | { +92 |- "ka":[2,3], # E231 + 92 |+ "ka": [2,3], # E231 +93 93 | }, +94 94 | { +95 95 | "kb": [2,3], # E231 + +E23.py:92:24: E231 [*] Missing whitespace after ',' + | +90 | "k1": [ +91 | { +92 | "ka":[2,3], # E231 + | ^ E231 +93 | }, +94 | { + | + = help: Add missing whitespace + +ℹ Safe fix +89 89 | { +90 90 | "k1": [ +91 91 | { +92 |- "ka":[2,3], # E231 + 92 |+ "ka":[2, 3], # E231 +93 93 | }, +94 94 | { +95 95 | "kb": [2,3], # E231 + +E23.py:95:25: E231 [*] Missing whitespace after ',' + | +93 | }, +94 | { +95 | "kb": [2,3], # E231 + | ^ E231 +96 | }, +97 | { + | + = help: Add missing whitespace + +ℹ Safe fix +92 92 | "ka":[2,3], # E231 +93 93 | }, +94 94 | { +95 |- "kb": [2,3], # E231 + 95 |+ "kb": [2, 3], # E231 +96 96 | }, +97 97 | { +98 98 | "ka":[2, 3], # E231 + +E23.py:98:21: E231 [*] Missing whitespace after ':' + | + 96 | }, + 97 | { + 98 | "ka":[2, 3], # E231 + | ^ E231 + 99 | "kb": [2, 3], # Ok +100 | "kc": [2, 3], # Ok + | + = help: Add missing whitespace + +ℹ Safe fix +95 95 | "kb": [2,3], # E231 +96 96 | }, +97 97 | { +98 |- "ka":[2, 3], # E231 + 98 |+ "ka": [2, 3], # E231 +99 99 | "kb": [2, 3], # Ok +100 100 | "kc": [2, 3], # Ok +101 101 | "kd": [2,3], # E231 + +E23.py:101:25: E231 [*] Missing whitespace after ',' + | + 99 | "kb": [2, 3], # Ok +100 | "kc": [2, 3], # Ok +101 | "kd": [2,3], # E231 + | ^ E231 +102 | "ke":[2,3], # E231 +103 | }, + | + = help: Add missing whitespace + +ℹ Safe fix +98 98 | "ka":[2, 3], # E231 +99 99 | "kb": [2, 3], # Ok +100 100 | "kc": [2, 3], # Ok +101 |- "kd": [2,3], # E231 + 101 |+ "kd": [2, 3], # E231 +102 102 | "ke":[2,3], # E231 +103 103 | }, +104 104 | ] + +E23.py:102:21: E231 [*] Missing whitespace after ':' + | +100 | "kc": [2, 3], # Ok +101 | "kd": [2,3], # E231 +102 | "ke":[2,3], # E231 + | ^ E231 +103 | }, +104 | ] + | + = help: Add missing whitespace + +ℹ Safe fix +99 99 | "kb": [2, 3], # Ok +100 100 | "kc": [2, 3], # Ok +101 101 | "kd": [2,3], # E231 +102 |- "ke":[2,3], # E231 + 102 |+ "ke": [2,3], # E231 +103 103 | }, +104 104 | ] +105 105 | } + +E23.py:102:24: E231 [*] Missing whitespace after ',' + | +100 | "kc": [2, 3], # Ok +101 | "kd": [2,3], # E231 +102 | "ke":[2,3], # E231 + | ^ E231 +103 | }, +104 | ] + | + = help: Add missing whitespace + +ℹ Safe fix +99 99 | "kb": [2, 3], # Ok +100 100 | "kc": [2, 3], # Ok +101 101 | "kd": [2,3], # E231 +102 |- "ke":[2,3], # E231 + 102 |+ "ke":[2, 3], # E231 +103 103 | }, +104 104 | ] +105 105 | }