Skip to content

Commit

Permalink
Fix E231 bug: Inconsistent catch compared to pycodestyle, such as w…
Browse files Browse the repository at this point in the history
…hen dict nested in list (#10469)

<!--
Thank you for contributing to Ruff! To help us out with reviewing,
please consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## 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.
  • Loading branch information
augustelalande authored Mar 21, 2024
1 parent c5ea420 commit d9ac170
Show file tree
Hide file tree
Showing 3 changed files with 401 additions and 11 deletions.
58 changes: 57 additions & 1 deletion crates/ruff_linter/resources/test/fixtures/pycodestyle/E23.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
]
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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() {
Expand All @@ -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
Expand All @@ -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
}
Expand Down
Loading

0 comments on commit d9ac170

Please sign in to comment.