From fd36754bebf108dc630cf55ee3923dbd0f9d50da Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 2 Jan 2024 23:04:52 -0400 Subject: [PATCH] Avoid infinite loop in constant vs. `None` comparisons (#9376) ## Summary We had an early `continue` in this loop, and we weren't setting `comparator = next;` when continuing... This PR removes the early continue altogether for clarity. Closes https://github.com/astral-sh/ruff/issues/9374. ## Test Plan `cargo test` --- .../test/fixtures/pycodestyle/E711.py | 2 + .../pycodestyle/rules/literal_comparisons.rs | 48 +++++++++---------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pycodestyle/E711.py b/crates/ruff_linter/resources/test/fixtures/pycodestyle/E711.py index f0b2eb8494..bc4d3d4519 100644 --- a/crates/ruff_linter/resources/test/fixtures/pycodestyle/E711.py +++ b/crates/ruff_linter/resources/test/fixtures/pycodestyle/E711.py @@ -51,3 +51,5 @@ if (True) == TrueElement or x == TrueElement: assert (not foo) in bar assert {"x": not foo} in bar assert [42, not foo] in bar + +assert x in c > 0 == None diff --git a/crates/ruff_linter/src/rules/pycodestyle/rules/literal_comparisons.rs b/crates/ruff_linter/src/rules/pycodestyle/rules/literal_comparisons.rs index 8d650d4a25..5abcbde90f 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/rules/literal_comparisons.rs +++ b/crates/ruff_linter/src/rules/pycodestyle/rules/literal_comparisons.rs @@ -200,42 +200,40 @@ pub(crate) fn literal_comparisons(checker: &mut Checker, compare: &ast::ExprComp continue; } - let Some(op) = EqCmpOp::try_from(*op) else { - continue; - }; - - if checker.enabled(Rule::NoneComparison) && next.is_none_literal_expr() { - match op { - EqCmpOp::Eq => { - let diagnostic = Diagnostic::new(NoneComparison(op), next.range()); - bad_ops.insert(index, CmpOp::Is); - diagnostics.push(diagnostic); - } - EqCmpOp::NotEq => { - let diagnostic = Diagnostic::new(NoneComparison(op), next.range()); - bad_ops.insert(index, CmpOp::IsNot); - diagnostics.push(diagnostic); - } - } - } - - if checker.enabled(Rule::TrueFalseComparison) { - if let Expr::BooleanLiteral(ast::ExprBooleanLiteral { value, .. }) = next { + if let Some(op) = EqCmpOp::try_from(*op) { + if checker.enabled(Rule::NoneComparison) && next.is_none_literal_expr() { match op { EqCmpOp::Eq => { - let diagnostic = - Diagnostic::new(TrueFalseComparison(*value, op), next.range()); + let diagnostic = Diagnostic::new(NoneComparison(op), next.range()); bad_ops.insert(index, CmpOp::Is); diagnostics.push(diagnostic); } EqCmpOp::NotEq => { - let diagnostic = - Diagnostic::new(TrueFalseComparison(*value, op), next.range()); + let diagnostic = Diagnostic::new(NoneComparison(op), next.range()); bad_ops.insert(index, CmpOp::IsNot); diagnostics.push(diagnostic); } } } + + if checker.enabled(Rule::TrueFalseComparison) { + if let Expr::BooleanLiteral(ast::ExprBooleanLiteral { value, .. }) = next { + match op { + EqCmpOp::Eq => { + let diagnostic = + Diagnostic::new(TrueFalseComparison(*value, op), next.range()); + bad_ops.insert(index, CmpOp::Is); + diagnostics.push(diagnostic); + } + EqCmpOp::NotEq => { + let diagnostic = + Diagnostic::new(TrueFalseComparison(*value, op), next.range()); + bad_ops.insert(index, CmpOp::IsNot); + diagnostics.push(diagnostic); + } + } + } + } } comparator = next;