Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2024-11-13 08:27:14 +00:00
Code Issues Projects Releases Packages Wiki Activity

Auto merge of #7605 - xordi:issue-7548-fix, r=giraffate

Browse source 
Issue 7548 fix

Close #7548

changelog: [`bool_assert_comparison`] fixes should be emitted only in case they are comparing a value of a type that implements the `Not` trait with an output of type `bool` against a boolean literal.
This commit is contained in:
bors 2021-08-31 13:36:20 +00:00
commit 87c375fa62
4 changed files with 169 additions and 48 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

90
clippy_lints/src/bool_assert_comparison.rs
View file

@ -1,9 +1,11 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::{ast_utils, is_direct_expn_of};
use rustc_ast::ast::{Expr, ExprKind, Lit, LitKind};
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;
use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_hir::{Expr, ExprKind, Lit};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::symbol::Ident;
declare_clippy_lint! {
/// ### What it does
@ -28,45 +30,77 @@ declare_clippy_lint! {
declare_lint_pass!(BoolAssertComparison => [BOOL_ASSERT_COMPARISON]);
fn is_bool_lit(e: &Expr) -> bool {
fn is_bool_lit(e: &Expr<'_>) -> bool {
matches!(
e.kind,
ExprKind::Lit(Lit {
kind: LitKind::Bool(_),
node: LitKind::Bool(_),
..
})
) && !e.span.from_expansion()
}
impl EarlyLintPass for BoolAssertComparison {
fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &Expr) {
fn is_impl_not_trait_with_bool_out(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> bool {
let ty = cx.typeck_results().expr_ty(e);
cx.tcx
.lang_items()
.not_trait()
.filter(|trait_id| implements_trait(cx, ty, *trait_id, &[]))
.and_then(|trait_id| {
cx.tcx.associated_items(trait_id).find_by_name_and_kind(
cx.tcx,
Ident::from_str("Output"),
ty::AssocKind::Type,
trait_id,
)
})
.map_or(false, |assoc_item| {
let proj = cx.tcx.mk_projection(assoc_item.def_id, cx.tcx.mk_substs_trait(ty, &[]));
let nty = cx.tcx.normalize_erasing_regions(cx.param_env, proj);
nty.is_bool()
})
}
impl<'tcx> LateLintPass<'tcx> for BoolAssertComparison {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
let macros = ["assert_eq", "debug_assert_eq"];
let inverted_macros = ["assert_ne", "debug_assert_ne"];
for mac in macros.iter().chain(inverted_macros.iter()) {
if let Some(span) = is_direct_expn_of(e.span, mac) {
if let Some([a, b]) = ast_utils::extract_assert_macro_args(e) {
let nb_bool_args = is_bool_lit(a) as usize + is_bool_lit(b) as usize;
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;
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!
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;
}
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
// literal does not implement Not trait with a bool output,
// so we cannot suggest to rewrite our code
return;
}
let non_eq_mac = &mac[..mac.len() - 3];
span_lint_and_sugg(
cx,
BOOL_ASSERT_COMPARISON,
span,
&format!("used `{}!` with a literal bool", mac),
"replace it with",
format!("{}!(..)", non_eq_mac),
Applicability::MaybeIncorrect,
);
return;
}
let non_eq_mac = &mac[..mac.len() - 3];
span_lint_and_sugg(
cx,
BOOL_ASSERT_COMPARISON,
span,
&format!("used `{}!` with a literal bool", mac),
"replace it with",
format!("{}!(..)", non_eq_mac),
Applicability::MaybeIncorrect,
);
return;
}
}
}

2
clippy_lints/src/lib.rs
View file

@ -2115,7 +2115,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box from_str_radix_10::FromStrRadix10);
store.register_late_pass(|| box manual_map::ManualMap);
store.register_late_pass(move || box if_then_some_else_none::IfThenSomeElseNone::new(msrv));
store.register_early_pass(|| box bool_assert_comparison::BoolAssertComparison);
store.register_late_pass(|| box bool_assert_comparison::BoolAssertComparison);
store.register_early_pass(move || box module_style::ModStyle);
store.register_late_pass(|| box unused_async::UnusedAsync);
let disallowed_types = conf.disallowed_types.iter().cloned().collect::<FxHashSet<_>>();

63
tests/ui/bool_assert_comparison.rs
View file

@ -1,5 +1,7 @@
#![warn(clippy::bool_assert_comparison)]
use std::ops::Not;
macro_rules! a {
() => {
true
@ -11,7 +13,58 @@ macro_rules! b {
};
}
// Implements the Not trait but with an output type
// that's not bool. Should not suggest a rewrite
#[derive(Debug)]
enum ImplNotTraitWithoutBool {
VariantX(bool),
VariantY(u32),
}
impl PartialEq<bool> for ImplNotTraitWithoutBool {
fn eq(&self, other: &bool) -> bool {
match *self {
ImplNotTraitWithoutBool::VariantX(b) => b == *other,
_ => false,
}
}
}
impl Not for ImplNotTraitWithoutBool {
type Output = Self;
fn not(self) -> Self::Output {
match self {
ImplNotTraitWithoutBool::VariantX(b) => ImplNotTraitWithoutBool::VariantX(!b),
ImplNotTraitWithoutBool::VariantY(0) => ImplNotTraitWithoutBool::VariantY(1),
ImplNotTraitWithoutBool::VariantY(_) => ImplNotTraitWithoutBool::VariantY(0),
}
}
}
// This type implements the Not trait with an Output of
// type bool. Using assert!(..) must be suggested
#[derive(Debug)]
struct ImplNotTraitWithBool;
impl PartialEq<bool> for ImplNotTraitWithBool {
fn eq(&self, other: &bool) -> bool {
false
}
}
impl Not for ImplNotTraitWithBool {
type Output = bool;
fn not(self) -> Self::Output {
true
}
}
fn main() {
let a = ImplNotTraitWithoutBool::VariantX(true);
let b = ImplNotTraitWithBool;
assert_eq!("a".len(), 1);
assert_eq!("a".is_empty(), false);
assert_eq!("".is_empty(), true);
@ -19,6 +72,8 @@ fn main() {
assert_eq!(a!(), b!());
assert_eq!(a!(), "".is_empty());
assert_eq!("".is_empty(), b!());
assert_eq!(a, true);
assert_eq!(b, true);
assert_ne!("a".len(), 1);
assert_ne!("a".is_empty(), false);
@ -27,6 +82,8 @@ fn main() {
assert_ne!(a!(), b!());
assert_ne!(a!(), "".is_empty());
assert_ne!("".is_empty(), b!());
assert_ne!(a, true);
assert_ne!(b, true);
debug_assert_eq!("a".len(), 1);
debug_assert_eq!("a".is_empty(), false);
@ -35,6 +92,8 @@ fn main() {
debug_assert_eq!(a!(), b!());
debug_assert_eq!(a!(), "".is_empty());
debug_assert_eq!("".is_empty(), b!());
debug_assert_eq!(a, true);
debug_assert_eq!(b, true);
debug_assert_ne!("a".len(), 1);
debug_assert_ne!("a".is_empty(), false);
@ -43,6 +102,8 @@ fn main() {
debug_assert_ne!(a!(), b!());
debug_assert_ne!(a!(), "".is_empty());
debug_assert_ne!("".is_empty(), b!());
debug_assert_ne!(a, true);
debug_assert_ne!(b, true);
// assert with error messages
assert_eq!("a".len(), 1, "tadam {}", 1);
@ -50,10 +111,12 @@ fn main() {
assert_eq!("a".is_empty(), false, "tadam {}", 1);
assert_eq!("a".is_empty(), false, "tadam {}", true);
assert_eq!(false, "a".is_empty(), "tadam {}", true);
assert_eq!(a, true, "tadam {}", false);
debug_assert_eq!("a".len(), 1, "tadam {}", 1);
debug_assert_eq!("a".len(), 1, "tadam {}", true);
debug_assert_eq!("a".is_empty(), false, "tadam {}", 1);
debug_assert_eq!("a".is_empty(), false, "tadam {}", true);
debug_assert_eq!(false, "a".is_empty(), "tadam {}", true);
debug_assert_eq!(a, true, "tadam {}", false);
}

62
tests/ui/bool_assert_comparison.stderr
View file

@ -1,5 +1,5 @@
error: used `assert_eq!` with a literal bool
--> $DIR/bool_assert_comparison.rs:16:5
--> $DIR/bool_assert_comparison.rs:69:5
|
LL | assert_eq!("a".is_empty(), false);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)`
@ -7,106 +7,130 @@ LL | assert_eq!("a".is_empty(), false);
= note: `-D clippy::bool-assert-comparison` implied by `-D warnings`
error: used `assert_eq!` with a literal bool
--> $DIR/bool_assert_comparison.rs:17:5
--> $DIR/bool_assert_comparison.rs:70:5
|
LL | assert_eq!("".is_empty(), true);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)`
error: used `assert_eq!` with a literal bool
--> $DIR/bool_assert_comparison.rs:18:5
--> $DIR/bool_assert_comparison.rs:71:5
|
LL | assert_eq!(true, "".is_empty());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)`
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!(..)`
error: used `assert_ne!` with a literal bool
--> $DIR/bool_assert_comparison.rs:24:5
--> $DIR/bool_assert_comparison.rs:79:5
|
LL | assert_ne!("a".is_empty(), false);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)`
error: used `assert_ne!` with a literal bool
--> $DIR/bool_assert_comparison.rs:25:5
--> $DIR/bool_assert_comparison.rs:80:5
|
LL | assert_ne!("".is_empty(), true);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)`
error: used `assert_ne!` with a literal bool
--> $DIR/bool_assert_comparison.rs:26:5
--> $DIR/bool_assert_comparison.rs:81:5
|
LL | assert_ne!(true, "".is_empty());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)`
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!(..)`
error: used `debug_assert_eq!` with a literal bool
--> $DIR/bool_assert_comparison.rs:32:5
--> $DIR/bool_assert_comparison.rs:89:5
|
LL | debug_assert_eq!("a".is_empty(), false);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)`
error: used `debug_assert_eq!` with a literal bool
--> $DIR/bool_assert_comparison.rs:33:5
--> $DIR/bool_assert_comparison.rs:90:5
|
LL | debug_assert_eq!("".is_empty(), true);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)`
error: used `debug_assert_eq!` with a literal bool
--> $DIR/bool_assert_comparison.rs:34:5
--> $DIR/bool_assert_comparison.rs:91:5
|
LL | debug_assert_eq!(true, "".is_empty());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)`
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!(..)`
error: used `debug_assert_ne!` with a literal bool
--> $DIR/bool_assert_comparison.rs:40:5
--> $DIR/bool_assert_comparison.rs:99:5
|
LL | debug_assert_ne!("a".is_empty(), false);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)`
error: used `debug_assert_ne!` with a literal bool
--> $DIR/bool_assert_comparison.rs:41:5
--> $DIR/bool_assert_comparison.rs:100:5
|
LL | debug_assert_ne!("".is_empty(), true);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)`
error: used `debug_assert_ne!` with a literal bool
--> $DIR/bool_assert_comparison.rs:42:5
--> $DIR/bool_assert_comparison.rs:101:5
|
LL | debug_assert_ne!(true, "".is_empty());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)`
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!(..)`
error: used `assert_eq!` with a literal bool
--> $DIR/bool_assert_comparison.rs:50:5
--> $DIR/bool_assert_comparison.rs:111:5
|
LL | assert_eq!("a".is_empty(), false, "tadam {}", 1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)`
error: used `assert_eq!` with a literal bool
--> $DIR/bool_assert_comparison.rs:51:5
--> $DIR/bool_assert_comparison.rs:112:5
|
LL | assert_eq!("a".is_empty(), false, "tadam {}", true);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)`
error: used `assert_eq!` with a literal bool
--> $DIR/bool_assert_comparison.rs:52:5
--> $DIR/bool_assert_comparison.rs:113:5
|
LL | assert_eq!(false, "a".is_empty(), "tadam {}", true);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `assert!(..)`
error: used `debug_assert_eq!` with a literal bool
--> $DIR/bool_assert_comparison.rs:56:5
--> $DIR/bool_assert_comparison.rs:118:5
|
LL | debug_assert_eq!("a".is_empty(), false, "tadam {}", 1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)`
error: used `debug_assert_eq!` with a literal bool
--> $DIR/bool_assert_comparison.rs:57:5
--> $DIR/bool_assert_comparison.rs:119:5
|
LL | debug_assert_eq!("a".is_empty(), false, "tadam {}", true);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)`
error: used `debug_assert_eq!` with a literal bool
--> $DIR/bool_assert_comparison.rs:58:5
--> $DIR/bool_assert_comparison.rs:120:5
|
LL | debug_assert_eq!(false, "a".is_empty(), "tadam {}", true);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert!(..)`
error: aborting due to 18 previous errors
error: aborting due to 22 previous errors