Skip to content

Commit

Permalink
[pycodestyle] Do not trigger E225 and E275 when the next token …
Browse files Browse the repository at this point in the history
…is a ')' (#10315)

## Summary

Fixes #10295.

`E225` (`Missing whitespace around operator`) and `E275` (`Missing
whitespace after keyword`) try to add a white space even when the next
character is a `)` (which is a syntax error in most cases, the
exceptions already being handled). This causes `E202` (`Whitespace
before close bracket`) to try to remove the added whitespace, resulting
in an infinite loop when `E225`/`E275` re-add it.
This PR adds an exception in `E225` and `E275` to not trigger in case
the next token is a `)`. It is a bit simplistic, but it solves the
example given in the issue without introducing a change in behavior
(according to the fixtures).

## Test Plan

`cargo test` and the `ruff-ecosystem` check were used to check that the
PR's changes do not have side-effects.
A new fixture was added to check that running the 3 rules on the example
given in the issue does not cause ruff to fail to converge.
  • Loading branch information
hoel-bagard committed Mar 11, 2024
1 parent bc693ea commit 8d73866
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
a = (1 or)
17 changes: 17 additions & 0 deletions crates/ruff_linter/src/rules/pycodestyle/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,23 @@ mod tests {
Ok(())
}

/// Tests the compatibility of E2 rules (E202, E225 and E275) on syntactically incorrect code.
#[test]
fn white_space_syntax_error_compatibility() -> Result<()> {
let diagnostics = test_path(
Path::new("pycodestyle").join("E2_syntax_error.py"),
&settings::LinterSettings {
..settings::LinterSettings::for_rules([
Rule::MissingWhitespaceAroundOperator,
Rule::MissingWhitespaceAfterKeyword,
Rule::WhitespaceBeforeCloseBracket,
])
},
)?;
assert_messages!(diagnostics);
Ok(())
}

#[test_case(Rule::BlankLineBetweenMethods, Path::new("E30.py"))]
#[test_case(Rule::BlankLinesTopLevel, Path::new("E30.py"))]
#[test_case(Rule::TooManyBlankLines, Path::new("E30.py"))]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,13 @@ pub(crate) fn missing_whitespace_after_keyword(
|| tok0_kind == TokenKind::Yield && tok1_kind == TokenKind::Rpar
|| matches!(
tok1_kind,
TokenKind::Colon | TokenKind::Newline | TokenKind::NonLogicalNewline
TokenKind::Colon
| TokenKind::Newline
| TokenKind::NonLogicalNewline
// In the event of a syntax error, do not attempt to add a whitespace.
| TokenKind::Rpar
| TokenKind::Rsqb
| TokenKind::Rbrace
))
&& tok0.end() == tok1.start()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,21 @@ pub(crate) fn missing_whitespace_around_operator(
} else {
NeedsSpace::No
}
} else if tokens.peek().is_some_and(|token| {
matches!(
token.kind(),
TokenKind::Rpar | TokenKind::Rsqb | TokenKind::Rbrace
)
}) {
// There should not be a closing bracket directly after a token, as it is a syntax
// error. For example:
// ```
// 1+)
// ```
//
// However, allow it in order to prevent entering an infinite loop in which E225 adds a
// space only for E202 to remove it.
NeedsSpace::No
} else if is_whitespace_needed(kind) {
NeedsSpace::Yes
} else {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
---

0 comments on commit 8d73866

Please sign in to comment.