diff --git a/clippy_lints/src/bool_assert_comparison.rs b/clippy_lints/src/bool_assert_comparison.rs index cdc192a47e48..8693a8e8fb4d 100644 --- a/clippy_lints/src/bool_assert_comparison.rs +++ b/clippy_lints/src/bool_assert_comparison.rs @@ -1,3 +1,4 @@ +use clippy_utils::source::snippet; use clippy_utils::{diagnostics::span_lint_and_sugg, higher, is_direct_expn_of, ty::implements_trait}; use rustc_ast::ast::LitKind; use rustc_errors::Applicability; @@ -30,14 +31,13 @@ declare_clippy_lint! { declare_lint_pass!(BoolAssertComparison => [BOOL_ASSERT_COMPARISON]); -fn is_bool_lit(e: &Expr<'_>) -> bool { - matches!( - e.kind, +fn as_bool_lit(e: &Expr<'_>) -> Option { + match e.kind { ExprKind::Lit(Lit { - node: LitKind::Bool(_), - .. - }) - ) && !e.span.from_expansion() + node: LitKind::Bool(f), .. + }) if !e.span.from_expansion() => Some(f), + _ => None, + } } fn is_impl_not_trait_with_bool_out(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> bool { @@ -72,7 +72,9 @@ impl<'tcx> LateLintPass<'tcx> for BoolAssertComparison { if let Some(span) = is_direct_expn_of(expr.span, mac) { if let Some(args) = higher::extract_assert_macro_args(expr) { if let [a, b, ..] = args[..] { - let nb_bool_args = is_bool_lit(a) as usize + is_bool_lit(b) as usize; + let a_literal = as_bool_lit(a); + let b_literal = as_bool_lit(b); + let nb_bool_args = a_literal.is_some() as usize + b_literal.is_some() as usize; if nb_bool_args != 1 { // If there are two boolean arguments, we definitely don't understand @@ -89,6 +91,9 @@ impl<'tcx> LateLintPass<'tcx> for BoolAssertComparison { return; } + let is_false_literal = a_literal == Some(false) || b_literal == Some(false); + let need_negation = inverted_macros.contains(mac) ^ is_false_literal; + let non_literal_expr = if a_literal.is_none() { a } else { b }; let non_eq_mac = &mac[..mac.len() - 3]; span_lint_and_sugg( cx, @@ -96,7 +101,12 @@ impl<'tcx> LateLintPass<'tcx> for BoolAssertComparison { span, &format!("used `{}!` with a literal bool", mac), "replace it with", - format!("{}!(..)", non_eq_mac), + format!( + "{}!({}{})", + non_eq_mac, + if need_negation { "!" } else { "" }, + snippet(cx, non_literal_expr.span, "..") + ), Applicability::MaybeIncorrect, ); return; diff --git a/tests/ui/bool_assert_comparison.stderr b/tests/ui/bool_assert_comparison.stderr index 377d51be4cde..4ebcbefe2071 100644 --- a/tests/ui/bool_assert_comparison.stderr +++ b/tests/ui/bool_assert_comparison.stderr @@ -2,7 +2,7 @@ error: used `assert_eq!` with a literal bool --> $DIR/bool_assert_comparison.rs:69:5 | LL | assert_eq!("a".is_empty(), false); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(!"a".is_empty())` | = note: `-D clippy::bool-assert-comparison` implied by `-D warnings` @@ -10,127 +10,127 @@ error: used `assert_eq!` with a literal bool --> $DIR/bool_assert_comparison.rs:70:5 | LL | assert_eq!("".is_empty(), true); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!("".is_empty())` error: used `assert_eq!` with a literal bool --> $DIR/bool_assert_comparison.rs:71:5 | LL | assert_eq!(true, "".is_empty()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!("".is_empty())` error: used `assert_eq!` with a literal bool --> $DIR/bool_assert_comparison.rs:76:5 | LL | assert_eq!(b, true); - | ^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)` + | ^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(b)` error: used `assert_ne!` with a literal bool --> $DIR/bool_assert_comparison.rs:79:5 | LL | assert_ne!("a".is_empty(), false); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!("a".is_empty())` error: used `assert_ne!` with a literal bool --> $DIR/bool_assert_comparison.rs:80:5 | LL | assert_ne!("".is_empty(), true); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(!"".is_empty())` error: used `assert_ne!` with a literal bool --> $DIR/bool_assert_comparison.rs:81:5 | LL | assert_ne!(true, "".is_empty()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(!"".is_empty())` error: used `assert_ne!` with a literal bool --> $DIR/bool_assert_comparison.rs:86:5 | LL | assert_ne!(b, true); - | ^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)` + | ^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(!b)` error: used `debug_assert_eq!` with a literal bool --> $DIR/bool_assert_comparison.rs:89:5 | LL | debug_assert_eq!("a".is_empty(), false); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(!"a".is_empty())` error: used `debug_assert_eq!` with a literal bool --> $DIR/bool_assert_comparison.rs:90:5 | LL | debug_assert_eq!("".is_empty(), true); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!("".is_empty())` error: used `debug_assert_eq!` with a literal bool --> $DIR/bool_assert_comparison.rs:91:5 | LL | debug_assert_eq!(true, "".is_empty()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!("".is_empty())` error: used `debug_assert_eq!` with a literal bool --> $DIR/bool_assert_comparison.rs:96:5 | LL | debug_assert_eq!(b, true); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(b)` error: used `debug_assert_ne!` with a literal bool --> $DIR/bool_assert_comparison.rs:99:5 | LL | debug_assert_ne!("a".is_empty(), false); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!("a".is_empty())` error: used `debug_assert_ne!` with a literal bool --> $DIR/bool_assert_comparison.rs:100:5 | LL | debug_assert_ne!("".is_empty(), true); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(!"".is_empty())` error: used `debug_assert_ne!` with a literal bool --> $DIR/bool_assert_comparison.rs:101:5 | LL | debug_assert_ne!(true, "".is_empty()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(!"".is_empty())` error: used `debug_assert_ne!` with a literal bool --> $DIR/bool_assert_comparison.rs:106:5 | LL | debug_assert_ne!(b, true); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(!b)` error: used `assert_eq!` with a literal bool --> $DIR/bool_assert_comparison.rs:111:5 | LL | assert_eq!("a".is_empty(), false, "tadam {}", 1); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(!"a".is_empty())` error: used `assert_eq!` with a literal bool --> $DIR/bool_assert_comparison.rs:112:5 | LL | assert_eq!("a".is_empty(), false, "tadam {}", true); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(!"a".is_empty())` error: used `assert_eq!` with a literal bool --> $DIR/bool_assert_comparison.rs:113:5 | LL | assert_eq!(false, "a".is_empty(), "tadam {}", true); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(!"a".is_empty())` error: used `debug_assert_eq!` with a literal bool --> $DIR/bool_assert_comparison.rs:118:5 | LL | debug_assert_eq!("a".is_empty(), false, "tadam {}", 1); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(!"a".is_empty())` error: used `debug_assert_eq!` with a literal bool --> $DIR/bool_assert_comparison.rs:119:5 | LL | debug_assert_eq!("a".is_empty(), false, "tadam {}", true); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(!"a".is_empty())` error: used `debug_assert_eq!` with a literal bool --> $DIR/bool_assert_comparison.rs:120:5 | LL | debug_assert_eq!(false, "a".is_empty(), "tadam {}", true); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(!"a".is_empty())` error: aborting due to 22 previous errors