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`
This commit is contained in:
Charlie Marsh 2024-01-02 23:04:52 -04:00 committed by GitHub
parent 154d3b9f4b
commit fd36754beb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 25 additions and 25 deletions

View file

@ -51,3 +51,5 @@ if (True) == TrueElement or x == TrueElement:
assert (not foo) in bar assert (not foo) in bar
assert {"x": not foo} in bar assert {"x": not foo} in bar
assert [42, not foo] in bar assert [42, not foo] in bar
assert x in c > 0 == None

View file

@ -200,42 +200,40 @@ pub(crate) fn literal_comparisons(checker: &mut Checker, compare: &ast::ExprComp
continue; continue;
} }
let Some(op) = EqCmpOp::try_from(*op) else { if let Some(op) = EqCmpOp::try_from(*op) {
continue; if checker.enabled(Rule::NoneComparison) && next.is_none_literal_expr() {
};
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 {
match op { match op {
EqCmpOp::Eq => { EqCmpOp::Eq => {
let diagnostic = let diagnostic = Diagnostic::new(NoneComparison(op), next.range());
Diagnostic::new(TrueFalseComparison(*value, op), next.range());
bad_ops.insert(index, CmpOp::Is); bad_ops.insert(index, CmpOp::Is);
diagnostics.push(diagnostic); diagnostics.push(diagnostic);
} }
EqCmpOp::NotEq => { EqCmpOp::NotEq => {
let diagnostic = let diagnostic = Diagnostic::new(NoneComparison(op), next.range());
Diagnostic::new(TrueFalseComparison(*value, op), next.range());
bad_ops.insert(index, CmpOp::IsNot); bad_ops.insert(index, CmpOp::IsNot);
diagnostics.push(diagnostic); 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; comparator = next;