From 6e779def625356243535485cf0dbdb020f8da056 Mon Sep 17 00:00:00 2001 From: augustelalande Date: Tue, 19 Mar 2024 00:48:14 -0400 Subject: [PATCH 1/4] fix bug --- .../test/fixtures/pycodestyle/E23.py | 30 ++++++- .../rules/logical_lines/missing_whitespace.rs | 19 ++--- ...ules__pycodestyle__tests__E231_E23.py.snap | 83 ++++++++++++++++++- 3 files changed, 120 insertions(+), 12 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..0530428791e57 100644 --- a/crates/ruff_linter/resources/test/fixtures/pycodestyle/E23.py +++ b/crates/ruff_linter/resources/test/fixtures/pycodestyle/E23.py @@ -47,4 +47,32 @@ 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], + } + ] 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..ee8a08cbb18a7 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 @@ -53,10 +53,9 @@ 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 lsqb_stack = vec![TextSize::default()]; + let mut lbrace_stack = vec![TextSize::default()]; let mut iter = line.tokens().iter().peekable(); while let Some(token) = iter.next() { @@ -65,14 +64,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(); + lsqb_stack.push(token.start()); } TokenKind::Rsqb if fstrings == 0 => { - open_parentheses = open_parentheses.saturating_sub(1); + lsqb_stack.pop(); } TokenKind::Lbrace if fstrings == 0 => { - prev_lbrace = token.start(); + lbrace_stack.push(token.start()); + } + TokenKind::Rbrace if fstrings == 0 => { + lbrace_stack.pop(); } TokenKind::Colon if fstrings > 0 => { // Colon in f-string, no space required. This will yield false @@ -96,9 +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 => - { + (TokenKind::Colon, _) if lsqb_stack.last() > lbrace_stack.last() => { continue; // Slice syntax, no space required } (TokenKind::Comma, TokenKind::Rpar | TokenKind::Rsqb) => { 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..9497a269d32e8 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,87 @@ 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 | ] From 0558714d40056d06209f9a147272bb11497209b7 Mon Sep 17 00:00:00 2001 From: augustelalande Date: Wed, 20 Mar 2024 14:25:38 -0400 Subject: [PATCH 2/4] do not initialize stack --- .../pycodestyle/rules/logical_lines/missing_whitespace.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 ee8a08cbb18a7..42b2676a4546f 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 @@ -54,8 +54,8 @@ impl AlwaysFixableViolation for MissingWhitespace { /// E231 pub(crate) fn missing_whitespace(line: &LogicalLine, context: &mut LogicalLinesContext) { let mut fstrings = 0u32; - let mut lsqb_stack = vec![TextSize::default()]; - let mut lbrace_stack = vec![TextSize::default()]; + let mut lsqb_stack = Vec::new(); + let mut lbrace_stack = Vec::new(); let mut iter = line.tokens().iter().peekable(); while let Some(token) = iter.next() { From ee08766887359dd69a86a3a5a206e0fe43d463b0 Mon Sep 17 00:00:00 2001 From: augustelalande Date: Wed, 20 Mar 2024 14:33:03 -0400 Subject: [PATCH 3/4] add more test cases for nested structures --- .../test/fixtures/pycodestyle/E23.py | 28 ++ .../rules/logical_lines/missing_whitespace.rs | 2 +- ...ules__pycodestyle__tests__E231_E23.py.snap | 253 ++++++++++++++++++ 3 files changed, 282 insertions(+), 1 deletion(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pycodestyle/E23.py b/crates/ruff_linter/resources/test/fixtures/pycodestyle/E23.py index 0530428791e57..04f420a8b9fa1 100644 --- a/crates/ruff_linter/resources/test/fixtures/pycodestyle/E23.py +++ b/crates/ruff_linter/resources/test/fixtures/pycodestyle/E23.py @@ -76,3 +76,31 @@ def main() -> None: "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 42b2676a4546f..c9657f6d0e677 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; 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 9497a269d32e8..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 @@ -223,3 +223,256 @@ E23.py:76:17: E231 [*] Missing whitespace after ':' 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 | } From 37d22709f4cf60b60401d1760c9be38982b1424b Mon Sep 17 00:00:00 2001 From: augustelalande Date: Thu, 21 Mar 2024 01:03:55 -0400 Subject: [PATCH 4/4] reduce to one stack --- .../rules/logical_lines/missing_whitespace.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) 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 c9657f6d0e677..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 @@ -54,8 +54,7 @@ impl AlwaysFixableViolation for MissingWhitespace { /// E231 pub(crate) fn missing_whitespace(line: &LogicalLine, context: &mut LogicalLinesContext) { let mut fstrings = 0u32; - let mut lsqb_stack = Vec::new(); - let mut lbrace_stack = Vec::new(); + let mut brackets = Vec::new(); let mut iter = line.tokens().iter().peekable(); while let Some(token) = iter.next() { @@ -64,16 +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 => { - lsqb_stack.push(token.start()); + brackets.push(kind); } TokenKind::Rsqb if fstrings == 0 => { - lsqb_stack.pop(); + brackets.pop(); } TokenKind::Lbrace if fstrings == 0 => { - lbrace_stack.push(token.start()); + brackets.push(kind); } TokenKind::Rbrace if fstrings == 0 => { - lbrace_stack.pop(); + brackets.pop(); } TokenKind::Colon if fstrings > 0 => { // Colon in f-string, no space required. This will yield false @@ -97,7 +96,9 @@ 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 lsqb_stack.last() > lbrace_stack.last() => { + (TokenKind::Colon, _) + if matches!(brackets.last(), Some(TokenKind::Lsqb)) => + { continue; // Slice syntax, no space required } (TokenKind::Comma, TokenKind::Rpar | TokenKind::Rsqb) => {