Skip to content

Commit

Permalink
improve suggestion for bool_assert_comparaison
Browse files Browse the repository at this point in the history
  • Loading branch information
@ABouttefeux
ABouttefeux committed Sep 1, 2021
1 parent 87c375f commit bcfe69d
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 40 deletions.
64 changes: 46 additions & 18 deletions clippy_lints/src/bool_assert_comparison.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use clippy_utils::{diagnostics::span_lint_and_sugg, higher, is_direct_expn_of, ty::implements_trait};
use clippy_utils::{diagnostics::span_lint_and_sugg, higher, is_direct_expn_of, source, ty::implements_trait};
use rustc_ast::ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, Lit};
Expand Down Expand Up @@ -30,14 +30,19 @@ declare_clippy_lint! {

declare_lint_pass!(BoolAssertComparison => [BOOL_ASSERT_COMPARISON]);

fn is_bool_lit(e: &Expr<'_>) -> bool {
matches!(
e.kind,
fn bool_lit(e: &Expr<'_>) -> Option<bool> {
match e.kind {
ExprKind::Lit(Lit {
node: LitKind::Bool(_),
..
})
) && !e.span.from_expansion()
node: LitKind::Bool(b), ..
}) => {
if e.span.from_expansion() {
None
} else {
Some(b)
}
},
_ => None,
}
}

fn is_impl_not_trait_with_bool_out(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> bool {
Expand Down Expand Up @@ -68,19 +73,27 @@ impl<'tcx> LateLintPass<'tcx> for BoolAssertComparison {
let macros = ["assert_eq", "debug_assert_eq"];
let inverted_macros = ["assert_ne", "debug_assert_ne"];

for mac in macros.iter().chain(inverted_macros.iter()) {
for (mac, is_eq) in macros
.iter()
.map(|el| (el, true))
.chain(inverted_macros.iter().map(|el| (el, false)))
{
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 nb_bool_args = is_bool_lit(a) as usize + is_bool_lit(b) as usize;

if nb_bool_args != 1 {
// If there are two boolean arguments, we definitely don't understand
// what's going on, so better leave things as is...
//
// Or there is simply no boolean and then we can leave things as is!
return;
}
let (lit_value, other_expr) = match (bool_lit(a), bool_lit(b)) {
(Some(lit), None) => (lit, b),
(None, Some(lit)) => (lit, a),
_ => {
// If there are two boolean arguments, we definitely don't understand
// what's going on, so better leave things as is...
//
// Or there is simply no boolean and then we can leave things as is!
return;
},
};

if !is_impl_not_trait_with_bool_out(cx, a) || !is_impl_not_trait_with_bool_out(cx, b) {
// At this point the expression which is not a boolean
Expand All @@ -90,13 +103,28 @@ impl<'tcx> LateLintPass<'tcx> for BoolAssertComparison {
}

let non_eq_mac = &mac[..mac.len() - 3];
let expr_string = if lit_value && is_eq {
source::snippet(cx, other_expr.span, "").to_string()
} else {
format!("!({})", source::snippet(cx, other_expr.span, ""))
};

let suggestion = if args.len() <= 2 {
format!("{}!({})", non_eq_mac, expr_string)
} else {
let mut str = format!("{}!({}", non_eq_mac, expr_string);
for el in &args[2..] {
str = format!("{}, {}", str, source::snippet(cx, el.span, ""));
}
format!("{})", str)
};
span_lint_and_sugg(
cx,
BOOL_ASSERT_COMPARISON,
span,
&format!("used `{}!` with a literal bool", mac),
"replace it with",
format!("{}!(..)", non_eq_mac),
suggestion,
Applicability::MaybeIncorrect,
);
return;
Expand Down
44 changes: 22 additions & 22 deletions tests/ui/bool_assert_comparison.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,135 +2,135 @@ 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`

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

0 comments on commit bcfe69d

Please sign in to comment.