Skip to content

Commit

Permalink
wip
Browse files Browse the repository at this point in the history
  • Loading branch information
nyurik committed Sep 20, 2024
1 parent a20cb02 commit 8f4d34e
Show file tree
Hide file tree
Showing 6 changed files with 96 additions and 5 deletions.
4 changes: 4 additions & 0 deletions clippy_lints/src/bool_assert_comparison.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::is_in_const_context;
use clippy_utils::macros::{find_assert_args, find_assert_eq_args, root_macro_call_first_node, MacroCall};
use clippy_utils::sugg::Sugg;
use clippy_utils::ty::{implements_trait, is_copy};
Expand Down Expand Up @@ -161,6 +162,9 @@ fn check_eq<'tcx>(

/// Checks for `assert!(a == b)` and `assert!(a != b)`
fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, macro_call: &MacroCall, macro_name: &str) {
if is_in_const_context(cx) {
return;
}
let Some((cond, _)) = find_assert_args(cx, expr, macro_call.expn) else {
return;
};
Expand Down
64 changes: 64 additions & 0 deletions tests/ui/assertions_on_constants.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
#![allow(non_fmt_panics, clippy::needless_bool, clippy::eq_op)]

macro_rules! assert_const {
($len:expr) => {
assert!($len > 0);
debug_assert!($len < 0);
};
}
fn main() {
assert!(true);
//~^ ERROR: `assert!(true)` will be optimized out by the compiler
assert!(false);
//~^ ERROR: `assert!(false)` should probably be replaced
assert!(true, "true message");
//~^ ERROR: `assert!(true)` will be optimized out by the compiler
assert!(false, "false message");
//~^ ERROR: `assert!(false, ..)` should probably be replaced

let msg = "panic message";
assert!(false, "{}", msg.to_uppercase());
//~^ ERROR: `assert!(false, ..)` should probably be replaced

const B: bool = true;
assert!(B);
//~^ ERROR: `assert!(true)` will be optimized out by the compiler

const C: bool = false;
assert!(C);
//~^ ERROR: `assert!(false)` should probably be replaced
assert!(C, "C message");
//~^ ERROR: `assert!(false, ..)` should probably be replaced

debug_assert!(true);
//~^ ERROR: `debug_assert!(true)` will be optimized out by the compiler
// Don't lint this, since there is no better way for expressing "Only panic in debug mode".
debug_assert!(false); // #3948
assert_const!(3);
assert_const!(-1);

// Don't lint if based on `cfg!(..)`:
assert!(cfg!(feature = "hey") || cfg!(not(feature = "asdf")));

let flag: bool = cfg!(not(feature = "asdf"));
assert!(flag);

const CFG_FLAG: &bool = &cfg!(feature = "hey");
assert!(!CFG_FLAG);

const _: () = assert!(true);
//~^ ERROR: `assert!(true)` will be optimized out by the compiler
#[allow(clippy::bool_assert_comparison)]
assert_eq!(8, (7 + 1));
//~^ ERROR: `assert!(true)` will be optimized out by the compiler

// Don't lint if the value is dependent on a defined constant:
const N: usize = 1024;
const _: () = assert!(N.is_power_of_two());
}

const _: () = {
assert!(true);
//~^ ERROR: `assert!(true)` will be optimized out by the compiler
assert!(8 == (7 + 1));
};
4 changes: 2 additions & 2 deletions tests/ui/assertions_on_constants.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![allow(non_fmt_panics, clippy::needless_bool, clippy::eq_op, clippy::bool_assert_comparison)]
#![allow(non_fmt_panics, clippy::needless_bool, clippy::eq_op)]

macro_rules! assert_const {
($len:expr) => {
Expand Down Expand Up @@ -48,7 +48,7 @@ fn main() {

const _: () = assert!(true);
//~^ ERROR: `assert!(true)` will be optimized out by the compiler

#[allow(clippy::bool_assert_comparison)]
assert!(8 == (7 + 1));
//~^ ERROR: `assert!(true)` will be optimized out by the compiler

Expand Down
15 changes: 14 additions & 1 deletion tests/ui/assertions_on_constants.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,19 @@ LL | assert!(8 == (7 + 1));
|
= help: remove it

error: used `assert!` with an equality comparison
--> tests/ui/assertions_on_constants.rs:52:5
|
LL | assert!(8 == (7 + 1));
| ^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::bool-assert-comparison` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::bool_assert_comparison)]`
help: replace it with `assert_eq!(..)`
|
LL | assert_eq!(8, (7 + 1));
| ~~~~~~~~~ ~

error: `assert!(true)` will be optimized out by the compiler
--> tests/ui/assertions_on_constants.rs:61:5
|
Expand All @@ -96,5 +109,5 @@ LL | assert!(true);
|
= help: remove it

error: aborting due to 12 previous errors
error: aborting due to 13 previous errors

7 changes: 6 additions & 1 deletion tests/ui/bool_assert_comparison.fixed
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![allow(unused, clippy::assertions_on_constants, clippy::const_is_empty)]
#![allow(unused, clippy::assertions_on_constants, clippy::const_is_empty, clippy::eq_op)]
#![warn(clippy::bool_assert_comparison)]

use std::ops::Not;
Expand Down Expand Up @@ -171,4 +171,9 @@ fn main() {
assert_eq!("a", "a".to_ascii_lowercase(), "a==a");
assert_ne!("A", "A".to_ascii_lowercase());
assert_ne!("A", "A".to_ascii_lowercase(), "A!=a");
const _: () = assert!(5 == 2 + 3);
}

const _: () = {
assert!(8 == (7 + 1));
};
7 changes: 6 additions & 1 deletion tests/ui/bool_assert_comparison.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![allow(unused, clippy::assertions_on_constants, clippy::const_is_empty)]
#![allow(unused, clippy::assertions_on_constants, clippy::const_is_empty, clippy::eq_op)]
#![warn(clippy::bool_assert_comparison)]

use std::ops::Not;
Expand Down Expand Up @@ -171,4 +171,9 @@ fn main() {
assert!("a" == "a".to_ascii_lowercase(), "a==a");
assert!("A" != "A".to_ascii_lowercase());
assert!("A" != "A".to_ascii_lowercase(), "A!=a");
const _: () = assert!(5 == 2 + 3);
}

const _: () = {
assert!(8 == (7 + 1));
};

0 comments on commit 8f4d34e

Please sign in to comment.