Avoid infinite loop in constant vs. None comparisons

astral-sh · Jan 3, 2024 · f8b8671 · f8b8671
1 parent 154d3b9
commit f8b8671
Show file tree

Hide file tree

Showing 3 changed files with 26 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;

diff --git a/crates/ruff_linter/src/rules/pyflakes/rules/invalid_literal_comparisons.rs b/crates/ruff_linter/src/rules/pyflakes/rules/invalid_literal_comparisons.rs
@@ -95,6 +95,7 @@ pub(crate) fn invalid_literal_comparison(
                     && (helpers::is_mutable_iterable_initializer(left)
                         || helpers::is_mutable_iterable_initializer(right))))
         {
+            println!("Found invalid comparison: {op:?} {right:?}");
             let mut diagnostic = Diagnostic::new(IsLiteral { cmp_op: op.into() }, expr.range());
             if lazy_located.is_none() {
                 lazy_located = Some(locate_cmp_ops(expr, checker.locator().contents()));