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 #9374. ## Test Plan `cargo test`
astral-sh · Jan 3, 2024 · fd36754 · fd36754
1 parent 154d3b9
commit fd36754
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 25 deletions.
diff --git a/crates/ruff_linter/resources/test/fixtures/pycodestyle/E711.py b/crates/ruff_linter/resources/test/fixtures/pycodestyle/E711.py
@@ -51,3 +51,5 @@
 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
@@ -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;