Skip to content

Commit 522e3c7

Browse files
ABouttefeuxABouttefeux
committed
improve suggestion for bool_assert_comparaison
1 parent 87c375f commit 522e3c7

File tree

2 files changed

+68
-40
lines changed

2 files changed

+68
-40
lines changed

‎clippy_lints/src/bool_assert_comparison.rs

+46-18
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use clippy_utils::{diagnostics::span_lint_and_sugg, higher, is_direct_expn_of, ty::implements_trait};
1+
use clippy_utils::{diagnostics::span_lint_and_sugg, higher, is_direct_expn_of, source, ty::implements_trait};
22
use rustc_ast::ast::LitKind;
33
use rustc_errors::Applicability;
44
use rustc_hir::{Expr, ExprKind, Lit};
@@ -30,14 +30,19 @@ declare_clippy_lint! {
3030

3131
declare_lint_pass!(BoolAssertComparison => [BOOL_ASSERT_COMPARISON]);
3232

33-
fn is_bool_lit(e: &Expr<'_>) -> bool {
34-
matches!(
35-
e.kind,
33+
fn bool_lit(e: &Expr<'_>) -> Option<bool> {
34+
match e.kind {
3635
ExprKind::Lit(Lit {
37-
node: LitKind::Bool(_),
38-
..
39-
})
40-
) && !e.span.from_expansion()
36+
node: LitKind::Bool(b), ..
37+
}) => {
38+
if e.span.from_expansion() {
39+
None
40+
} else {
41+
Some(b)
42+
}
43+
},
44+
_ => None,
45+
}
4146
}
4247

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

71-
for mac in macros.iter().chain(inverted_macros.iter()) {
76+
for (mac, is_eq) in macros
77+
.iter()
78+
.map(|el| (el, true))
79+
.chain(inverted_macros.iter().map(|el| (el, false)))
80+
{
7281
if let Some(span) = is_direct_expn_of(expr.span, mac) {
7382
if let Some(args) = higher::extract_assert_macro_args(expr) {
7483
if let [a, b, ..] = args[..] {
75-
let nb_bool_args = is_bool_lit(a) as usize + is_bool_lit(b) as usize;
84+
//let nb_bool_args = is_bool_lit(a) as usize + is_bool_lit(b) as usize;
7685

77-
if nb_bool_args != 1 {
78-
// If there are two boolean arguments, we definitely don't understand
79-
// what's going on, so better leave things as is...
80-
//
81-
// Or there is simply no boolean and then we can leave things as is!
82-
return;
83-
}
86+
let (lit_value, other_expr) = match (bool_lit(a), bool_lit(b)) {
87+
(Some(lit), None) => (lit, b),
88+
(None, Some(lit)) => (lit, a),
89+
_ => {
90+
// If there are two boolean arguments, we definitely don't understand
91+
// what's going on, so better leave things as is...
92+
//
93+
// Or there is simply no boolean and then we can leave things as is!
94+
return;
95+
},
96+
};
8497

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

92105
let non_eq_mac = &mac[..mac.len() - 3];
106+
let expr_string = if lit_value ^ is_eq {
107+
format!("!({})", source::snippet(cx, other_expr.span, ""))
108+
} else {
109+
source::snippet(cx, other_expr.span, "").to_string()
110+
};
111+
112+
let suggestion = if args.len() <= 2 {
113+
format!("{}!({})", non_eq_mac, expr_string)
114+
} else {
115+
let mut str = format!("{}!({}", non_eq_mac, expr_string);
116+
for el in &args[2..] {
117+
str = format!("{}, {}", str, source::snippet(cx, el.span, ""));
118+
}
119+
format!("{})", str)
120+
};
93121
span_lint_and_sugg(
94122
cx,
95123
BOOL_ASSERT_COMPARISON,
96124
span,
97125
&format!("used `{}!` with a literal bool", mac),
98126
"replace it with",
99-
format!("{}!(..)", non_eq_mac),
127+
suggestion,
100128
Applicability::MaybeIncorrect,
101129
);
102130
return;

‎tests/ui/bool_assert_comparison.stderr

+22-22
Original file line numberDiff line numberDiff line change
@@ -2,135 +2,135 @@ error: used `assert_eq!` with a literal bool
22
--> $DIR/bool_assert_comparison.rs:69:5
33
|
44
LL | assert_eq!("a".is_empty(), false);
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)`
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(!("a".is_empty()))`
66
|
77
= note: `-D clippy::bool-assert-comparison` implied by `-D warnings`
88

99
error: used `assert_eq!` with a literal bool
1010
--> $DIR/bool_assert_comparison.rs:70:5
1111
|
1212
LL | assert_eq!("".is_empty(), true);
13-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)`
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!("".is_empty())`
1414

1515
error: used `assert_eq!` with a literal bool
1616
--> $DIR/bool_assert_comparison.rs:71:5
1717
|
1818
LL | assert_eq!(true, "".is_empty());
19-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)`
19+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!("".is_empty())`
2020

2121
error: used `assert_eq!` with a literal bool
2222
--> $DIR/bool_assert_comparison.rs:76:5
2323
|
2424
LL | assert_eq!(b, true);
25-
| ^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)`
25+
| ^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(b)`
2626

2727
error: used `assert_ne!` with a literal bool
2828
--> $DIR/bool_assert_comparison.rs:79:5
2929
|
3030
LL | assert_ne!("a".is_empty(), false);
31-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)`
31+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!("a".is_empty())`
3232

3333
error: used `assert_ne!` with a literal bool
3434
--> $DIR/bool_assert_comparison.rs:80:5
3535
|
3636
LL | assert_ne!("".is_empty(), true);
37-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)`
37+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(!("".is_empty()))`
3838

3939
error: used `assert_ne!` with a literal bool
4040
--> $DIR/bool_assert_comparison.rs:81:5
4141
|
4242
LL | assert_ne!(true, "".is_empty());
43-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)`
43+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(!("".is_empty()))`
4444

4545
error: used `assert_ne!` with a literal bool
4646
--> $DIR/bool_assert_comparison.rs:86:5
4747
|
4848
LL | assert_ne!(b, true);
49-
| ^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)`
49+
| ^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(!(b))`
5050

5151
error: used `debug_assert_eq!` with a literal bool
5252
--> $DIR/bool_assert_comparison.rs:89:5
5353
|
5454
LL | debug_assert_eq!("a".is_empty(), false);
55-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)`
55+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(!("a".is_empty()))`
5656

5757
error: used `debug_assert_eq!` with a literal bool
5858
--> $DIR/bool_assert_comparison.rs:90:5
5959
|
6060
LL | debug_assert_eq!("".is_empty(), true);
61-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)`
61+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!("".is_empty())`
6262

6363
error: used `debug_assert_eq!` with a literal bool
6464
--> $DIR/bool_assert_comparison.rs:91:5
6565
|
6666
LL | debug_assert_eq!(true, "".is_empty());
67-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)`
67+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!("".is_empty())`
6868

6969
error: used `debug_assert_eq!` with a literal bool
7070
--> $DIR/bool_assert_comparison.rs:96:5
7171
|
7272
LL | debug_assert_eq!(b, true);
73-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)`
73+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(b)`
7474

7575
error: used `debug_assert_ne!` with a literal bool
7676
--> $DIR/bool_assert_comparison.rs:99:5
7777
|
7878
LL | debug_assert_ne!("a".is_empty(), false);
79-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)`
79+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!("a".is_empty())`
8080

8181
error: used `debug_assert_ne!` with a literal bool
8282
--> $DIR/bool_assert_comparison.rs:100:5
8383
|
8484
LL | debug_assert_ne!("".is_empty(), true);
85-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)`
85+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(!("".is_empty()))`
8686

8787
error: used `debug_assert_ne!` with a literal bool
8888
--> $DIR/bool_assert_comparison.rs:101:5
8989
|
9090
LL | debug_assert_ne!(true, "".is_empty());
91-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)`
91+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(!("".is_empty()))`
9292

9393
error: used `debug_assert_ne!` with a literal bool
9494
--> $DIR/bool_assert_comparison.rs:106:5
9595
|
9696
LL | debug_assert_ne!(b, true);
97-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)`
97+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(!(b))`
9898

9999
error: used `assert_eq!` with a literal bool
100100
--> $DIR/bool_assert_comparison.rs:111:5
101101
|
102102
LL | assert_eq!("a".is_empty(), false, "tadam {}", 1);
103-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)`
103+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(!("a".is_empty()))`
104104

105105
error: used `assert_eq!` with a literal bool
106106
--> $DIR/bool_assert_comparison.rs:112:5
107107
|
108108
LL | assert_eq!("a".is_empty(), false, "tadam {}", true);
109-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)`
109+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(!("a".is_empty()))`
110110

111111
error: used `assert_eq!` with a literal bool
112112
--> $DIR/bool_assert_comparison.rs:113:5
113113
|
114114
LL | assert_eq!(false, "a".is_empty(), "tadam {}", true);
115-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)`
115+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(!("a".is_empty()))`
116116

117117
error: used `debug_assert_eq!` with a literal bool
118118
--> $DIR/bool_assert_comparison.rs:118:5
119119
|
120120
LL | debug_assert_eq!("a".is_empty(), false, "tadam {}", 1);
121-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)`
121+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(!("a".is_empty()))`
122122

123123
error: used `debug_assert_eq!` with a literal bool
124124
--> $DIR/bool_assert_comparison.rs:119:5
125125
|
126126
LL | debug_assert_eq!("a".is_empty(), false, "tadam {}", true);
127-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)`
127+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(!("a".is_empty()))`
128128

129129
error: used `debug_assert_eq!` with a literal bool
130130
--> $DIR/bool_assert_comparison.rs:120:5
131131
|
132132
LL | debug_assert_eq!(false, "a".is_empty(), "tadam {}", true);
133-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)`
133+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(!("a".is_empty()))`
134134

135135
error: aborting due to 22 previous errors
136136

0 commit comments

Comments
 (0)