Remove `unwrap` from none-comparison rule (#4969)

This commit is contained in:
Charlie Marsh 2023-06-08 14:21:56 -04:00 committed by GitHub
parent 775d247731
commit d042eddccc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 111 additions and 94 deletions

View File

@ -16,12 +16,12 @@ enum EqCmpop {
NotEq, NotEq,
} }
impl From<&Cmpop> for EqCmpop { impl EqCmpop {
fn from(cmpop: &Cmpop) -> Self { fn try_from(value: Cmpop) -> Option<EqCmpop> {
match cmpop { match value {
Cmpop::Eq => EqCmpop::Eq, Cmpop::Eq => Some(EqCmpop::Eq),
Cmpop::NotEq => EqCmpop::NotEq, Cmpop::NotEq => Some(EqCmpop::NotEq),
_ => panic!("Expected Cmpop::Eq | Cmpop::NotEq"), _ => None,
} }
} }
} }
@ -154,6 +154,7 @@ pub(crate) fn literal_comparisons(
let next = &comparators[0]; let next = &comparators[0];
if !helpers::is_constant_non_singleton(next) { if !helpers::is_constant_non_singleton(next) {
if let Some(op) = EqCmpop::try_from(*op) {
if check_none_comparisons if check_none_comparisons
&& matches!( && matches!(
comparator, comparator,
@ -164,21 +165,23 @@ pub(crate) fn literal_comparisons(
}) })
) )
{ {
if matches!(op, Cmpop::Eq) { match op {
let diagnostic = Diagnostic::new(NoneComparison(op.into()), comparator.range()); EqCmpop::Eq => {
let diagnostic = Diagnostic::new(NoneComparison(op), comparator.range());
if checker.patch(diagnostic.kind.rule()) { if checker.patch(diagnostic.kind.rule()) {
bad_ops.insert(0, Cmpop::Is); bad_ops.insert(0, Cmpop::Is);
} }
diagnostics.push(diagnostic); diagnostics.push(diagnostic);
} }
if matches!(op, Cmpop::NotEq) { EqCmpop::NotEq => {
let diagnostic = Diagnostic::new(NoneComparison(op.into()), comparator.range()); let diagnostic = Diagnostic::new(NoneComparison(op), comparator.range());
if checker.patch(diagnostic.kind.rule()) { if checker.patch(diagnostic.kind.rule()) {
bad_ops.insert(0, Cmpop::IsNot); bad_ops.insert(0, Cmpop::IsNot);
} }
diagnostics.push(diagnostic); diagnostics.push(diagnostic);
} }
} }
}
if check_true_false_comparisons { if check_true_false_comparisons {
if let Expr::Constant(ast::ExprConstant { if let Expr::Constant(ast::ExprConstant {
@ -187,17 +190,22 @@ pub(crate) fn literal_comparisons(
range: _, range: _,
}) = comparator }) = comparator
{ {
if matches!(op, Cmpop::Eq) { match op {
let diagnostic = EqCmpop::Eq => {
Diagnostic::new(TrueFalseComparison(*value, op.into()), comparator.range()); let diagnostic = Diagnostic::new(
TrueFalseComparison(*value, op),
comparator.range(),
);
if checker.patch(diagnostic.kind.rule()) { if checker.patch(diagnostic.kind.rule()) {
bad_ops.insert(0, Cmpop::Is); bad_ops.insert(0, Cmpop::Is);
} }
diagnostics.push(diagnostic); diagnostics.push(diagnostic);
} }
if matches!(op, Cmpop::NotEq) { EqCmpop::NotEq => {
let diagnostic = let diagnostic = Diagnostic::new(
Diagnostic::new(TrueFalseComparison(*value, op.into()), comparator.range()); TrueFalseComparison(*value, op),
comparator.range(),
);
if checker.patch(diagnostic.kind.rule()) { if checker.patch(diagnostic.kind.rule()) {
bad_ops.insert(0, Cmpop::IsNot); bad_ops.insert(0, Cmpop::IsNot);
} }
@ -206,6 +214,8 @@ pub(crate) fn literal_comparisons(
} }
} }
} }
}
}
// Check each comparator in order. // Check each comparator in order.
for (idx, (op, next)) in izip!(ops, comparators).enumerate() { for (idx, (op, next)) in izip!(ops, comparators).enumerate() {
@ -214,6 +224,7 @@ pub(crate) fn literal_comparisons(
continue; continue;
} }
if let Some(op) = EqCmpop::try_from(*op) {
if check_none_comparisons if check_none_comparisons
&& matches!( && matches!(
next, next,
@ -224,21 +235,23 @@ pub(crate) fn literal_comparisons(
}) })
) )
{ {
if matches!(op, Cmpop::Eq) { match op {
let diagnostic = Diagnostic::new(NoneComparison(op.into()), next.range()); EqCmpop::Eq => {
let diagnostic = Diagnostic::new(NoneComparison(op), next.range());
if checker.patch(diagnostic.kind.rule()) { if checker.patch(diagnostic.kind.rule()) {
bad_ops.insert(idx, Cmpop::Is); bad_ops.insert(idx, Cmpop::Is);
} }
diagnostics.push(diagnostic); diagnostics.push(diagnostic);
} }
if matches!(op, Cmpop::NotEq) { EqCmpop::NotEq => {
let diagnostic = Diagnostic::new(NoneComparison(op.into()), next.range()); let diagnostic = Diagnostic::new(NoneComparison(op), next.range());
if checker.patch(diagnostic.kind.rule()) { if checker.patch(diagnostic.kind.rule()) {
bad_ops.insert(idx, Cmpop::IsNot); bad_ops.insert(idx, Cmpop::IsNot);
} }
diagnostics.push(diagnostic); diagnostics.push(diagnostic);
} }
} }
}
if check_true_false_comparisons { if check_true_false_comparisons {
if let Expr::Constant(ast::ExprConstant { if let Expr::Constant(ast::ExprConstant {
@ -247,16 +260,18 @@ pub(crate) fn literal_comparisons(
range: _, range: _,
}) = next }) = next
{ {
if op.is_eq() { match op {
EqCmpop::Eq => {
let diagnostic = let diagnostic =
Diagnostic::new(TrueFalseComparison(*value, op.into()), next.range()); Diagnostic::new(TrueFalseComparison(*value, op), next.range());
if checker.patch(diagnostic.kind.rule()) { if checker.patch(diagnostic.kind.rule()) {
bad_ops.insert(idx, Cmpop::Is); bad_ops.insert(idx, Cmpop::Is);
} }
diagnostics.push(diagnostic); diagnostics.push(diagnostic);
} else if op.is_not_eq() { }
EqCmpop::NotEq => {
let diagnostic = let diagnostic =
Diagnostic::new(TrueFalseComparison(*value, op.into()), next.range()); Diagnostic::new(TrueFalseComparison(*value, op), next.range());
if checker.patch(diagnostic.kind.rule()) { if checker.patch(diagnostic.kind.rule()) {
bad_ops.insert(idx, Cmpop::IsNot); bad_ops.insert(idx, Cmpop::IsNot);
} }
@ -264,6 +279,8 @@ pub(crate) fn literal_comparisons(
} }
} }
} }
}
}
comparator = next; comparator = next;
} }