Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 54 additions & 48 deletions clippy_lints/src/assertions_on_constants.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
use clippy_config::Conf;
use clippy_utils::consts::{ConstEvalCtxt, Constant};
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::is_inside_always_const_context;
use clippy_utils::macros::{PanicExpn, find_assert_args, root_macro_call_first_node};
use clippy_utils::macros::{find_assert_args, root_macro_call_first_node};
use clippy_utils::msrvs::Msrv;
use clippy_utils::{is_inside_always_const_context, msrvs};
use rustc_ast::LitKind;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
use rustc_session::impl_lint_pass;
use rustc_span::sym;

declare_clippy_lint! {
Expand All @@ -28,56 +31,59 @@ declare_clippy_lint! {
"`assert!(true)` / `assert!(false)` will be optimized out by the compiler, and should probably be replaced by a `panic!()` or `unreachable!()`"
}

declare_lint_pass!(AssertionsOnConstants => [ASSERTIONS_ON_CONSTANTS]);
impl_lint_pass!(AssertionsOnConstants => [ASSERTIONS_ON_CONSTANTS]);
pub struct AssertionsOnConstants {
msrv: Msrv,
}
impl AssertionsOnConstants {
pub fn new(conf: &Conf) -> Self {
Self { msrv: conf.msrv }
}
}

impl<'tcx> LateLintPass<'tcx> for AssertionsOnConstants {
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
let Some(macro_call) = root_macro_call_first_node(cx, e) else {
return;
};
let is_debug = match cx.tcx.get_diagnostic_name(macro_call.def_id) {
Some(sym::debug_assert_macro) => true,
Some(sym::assert_macro) => false,
_ => return,
};
let Some((condition, panic_expn)) = find_assert_args(cx, e, macro_call.expn) else {
return;
};
let Some(Constant::Bool(val)) = ConstEvalCtxt::new(cx).eval(condition) else {
return;
};
if let Some(macro_call) = root_macro_call_first_node(cx, e)
&& let is_debug = match cx.tcx.get_diagnostic_name(macro_call.def_id) {
Some(sym::debug_assert_macro) => true,
Some(sym::assert_macro) => false,
_ => return,
}
&& let Some((condition, _)) = find_assert_args(cx, e, macro_call.expn)
&& let Some((Constant::Bool(assert_val), const_src)) = ConstEvalCtxt::new(cx).eval_with_source(condition)
&& let in_const_context = is_inside_always_const_context(cx.tcx, e.hir_id)
&& (const_src.is_local() || !in_const_context)
&& !(is_debug && as_bool_lit(condition) == Some(false))
{
let (msg, help) = if !const_src.is_local() {
let help = if self.msrv.meets(cx, msrvs::CONST_BLOCKS) {
"consider moving this into a const block: `const { assert!(..) }`"
} else if self.msrv.meets(cx, msrvs::CONST_PANIC) {
"consider moving this to an anonymous constant: `const _: () = { assert!(..); }`"
} else {
return;
};
("this assertion has a constant value", help)
} else if assert_val {
("this assertion is always `true`", "remove the assertion")
} else {
(
"this assertion is always `false`",
"replace this with `panic!()` or `unreachable!()`",
)
};

match condition.kind {
ExprKind::Path(..) | ExprKind::Lit(_) => {},
_ if is_inside_always_const_context(cx.tcx, e.hir_id) => return,
_ => {},
span_lint_and_help(cx, ASSERTIONS_ON_CONSTANTS, macro_call.span, msg, None, help);
}
}
}

if val {
span_lint_and_help(
cx,
ASSERTIONS_ON_CONSTANTS,
macro_call.span,
format!(
"`{}!(true)` will be optimized out by the compiler",
cx.tcx.item_name(macro_call.def_id)
),
None,
"remove it",
);
} else if !is_debug {
let (assert_arg, panic_arg) = match panic_expn {
PanicExpn::Empty => ("", ""),
_ => (", ..", ".."),
};
span_lint_and_help(
cx,
ASSERTIONS_ON_CONSTANTS,
macro_call.span,
format!("`assert!(false{assert_arg})` should probably be replaced"),
None,
format!("use `panic!({panic_arg})` or `unreachable!({panic_arg})`"),
);
}
fn as_bool_lit(e: &Expr<'_>) -> Option<bool> {
if let ExprKind::Lit(l) = e.kind
&& let LitKind::Bool(b) = l.node
{
Some(b)
} else {
None
}
}
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
store.register_late_pass(|_| Box::new(redundant_clone::RedundantClone));
store.register_late_pass(|_| Box::new(slow_vector_initialization::SlowVectorInit));
store.register_late_pass(move |_| Box::new(unnecessary_wraps::UnnecessaryWraps::new(conf)));
store.register_late_pass(|_| Box::new(assertions_on_constants::AssertionsOnConstants));
store.register_late_pass(|_| Box::new(assertions_on_constants::AssertionsOnConstants::new(conf)));
store.register_late_pass(|_| Box::new(assertions_on_result_states::AssertionsOnResultStates));
store.register_late_pass(|_| Box::new(inherent_to_string::InherentToString));
store.register_late_pass(move |_| Box::new(trait_bounds::TraitBounds::new(conf)));
Expand Down
3 changes: 2 additions & 1 deletion clippy_utils/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ msrv_aliases! {
1,82,0 { IS_NONE_OR, REPEAT_N, RAW_REF_OP }
1,81,0 { LINT_REASONS_STABILIZATION, ERROR_IN_CORE, EXPLICIT_SELF_TYPE_ELISION, DURATION_ABS_DIFF }
1,80,0 { BOX_INTO_ITER, LAZY_CELL }
1,79,0 { CONST_BLOCKS }
1,77,0 { C_STR_LITERALS }
1,76,0 { PTR_FROM_REF, OPTION_RESULT_INSPECT }
1,75,0 { OPTION_AS_SLICE }
Expand All @@ -46,7 +47,7 @@ msrv_aliases! {
1,60,0 { ABS_DIFF }
1,59,0 { THREAD_LOCAL_CONST_INIT }
1,58,0 { FORMAT_ARGS_CAPTURE, PATTERN_TRAIT_CHAR_ARRAY, CONST_RAW_PTR_DEREF }
1,57,0 { MAP_WHILE }
1,57,0 { MAP_WHILE, CONST_PANIC }
1,56,0 { CONST_FN_UNION }
1,55,0 { SEEK_REWIND }
1,54,0 { INTO_KEYS }
Expand Down
29 changes: 28 additions & 1 deletion tests/ui/assertions_on_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ fn main() {
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"));
Expand All @@ -62,9 +61,37 @@ fn main() {
const _: () = assert!(N.is_power_of_two());
}

const C: bool = true;

const _: () = {
assert!(true);
//~^ assertions_on_constants

assert!(8 == (7 + 1));
//~^ assertions_on_constants

assert!(C);
};

#[clippy::msrv = "1.57"]
fn _f1() {
assert!(C);
//~^ assertions_on_constants
}

#[clippy::msrv = "1.56"]
fn _f2() {
assert!(C);
}

#[clippy::msrv = "1.79"]
fn _f3() {
assert!(C);
//~^ assertions_on_constants
}

#[clippy::msrv = "1.78"]
fn _f4() {
assert!(C);
//~^ assertions_on_constants
}
88 changes: 60 additions & 28 deletions tests/ui/assertions_on_constants.stderr
Original file line number Diff line number Diff line change
@@ -1,100 +1,132 @@
error: `assert!(true)` will be optimized out by the compiler
error: this assertion is always `true`
--> tests/ui/assertions_on_constants.rs:10:5
|
LL | assert!(true);
| ^^^^^^^^^^^^^
|
= help: remove it
= help: remove the assertion
= note: `-D clippy::assertions-on-constants` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::assertions_on_constants)]`

error: `assert!(false)` should probably be replaced
error: this assertion is always `false`
--> tests/ui/assertions_on_constants.rs:13:5
|
LL | assert!(false);
| ^^^^^^^^^^^^^^
|
= help: use `panic!()` or `unreachable!()`
= help: replace this with `panic!()` or `unreachable!()`

error: `assert!(true)` will be optimized out by the compiler
error: this assertion is always `true`
--> tests/ui/assertions_on_constants.rs:16:5
|
LL | assert!(true, "true message");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: remove it
= help: remove the assertion

error: `assert!(false, ..)` should probably be replaced
error: this assertion is always `false`
--> tests/ui/assertions_on_constants.rs:19:5
|
LL | assert!(false, "false message");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: use `panic!(..)` or `unreachable!(..)`
= help: replace this with `panic!()` or `unreachable!()`

error: `assert!(false, ..)` should probably be replaced
error: this assertion is always `false`
--> tests/ui/assertions_on_constants.rs:23:5
|
LL | assert!(false, "{}", msg.to_uppercase());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: use `panic!(..)` or `unreachable!(..)`
= help: replace this with `panic!()` or `unreachable!()`

error: `assert!(true)` will be optimized out by the compiler
error: this assertion has a constant value
--> tests/ui/assertions_on_constants.rs:27:5
|
LL | assert!(B);
| ^^^^^^^^^^
|
= help: remove it
= help: consider moving this into a const block: `const { assert!(..) }`

error: `assert!(false)` should probably be replaced
error: this assertion has a constant value
--> tests/ui/assertions_on_constants.rs:31:5
|
LL | assert!(C);
| ^^^^^^^^^^
|
= help: use `panic!()` or `unreachable!()`
= help: consider moving this into a const block: `const { assert!(..) }`

error: `assert!(false, ..)` should probably be replaced
error: this assertion has a constant value
--> tests/ui/assertions_on_constants.rs:34:5
|
LL | assert!(C, "C message");
| ^^^^^^^^^^^^^^^^^^^^^^^
|
= help: use `panic!(..)` or `unreachable!(..)`
= help: consider moving this into a const block: `const { assert!(..) }`

error: `debug_assert!(true)` will be optimized out by the compiler
error: this assertion is always `true`
--> tests/ui/assertions_on_constants.rs:37:5
|
LL | debug_assert!(true);
| ^^^^^^^^^^^^^^^^^^^
|
= help: remove it
= help: remove the assertion

error: `assert!(true)` will be optimized out by the compiler
--> tests/ui/assertions_on_constants.rs:54:19
error: this assertion is always `true`
--> tests/ui/assertions_on_constants.rs:53:19
|
LL | const _: () = assert!(true);
| ^^^^^^^^^^^^^
|
= help: remove it
= help: remove the assertion

error: `assert!(true)` will be optimized out by the compiler
--> tests/ui/assertions_on_constants.rs:57:5
error: this assertion is always `true`
--> tests/ui/assertions_on_constants.rs:56:5
|
LL | assert!(8 == (7 + 1));
| ^^^^^^^^^^^^^^^^^^^^^
|
= help: remove it
= help: remove the assertion

error: `assert!(true)` will be optimized out by the compiler
--> tests/ui/assertions_on_constants.rs:66:5
error: this assertion is always `true`
--> tests/ui/assertions_on_constants.rs:67:5
|
LL | assert!(true);
| ^^^^^^^^^^^^^
|
= help: remove it
= help: remove the assertion

error: aborting due to 12 previous errors
error: this assertion is always `true`
--> tests/ui/assertions_on_constants.rs:70:5
|
LL | assert!(8 == (7 + 1));
| ^^^^^^^^^^^^^^^^^^^^^
|
= help: remove the assertion

error: this assertion has a constant value
--> tests/ui/assertions_on_constants.rs:78:5
|
LL | assert!(C);
| ^^^^^^^^^^
|
= help: consider moving this to an anonymous constant: `const _: () = { assert!(..); }`

error: this assertion has a constant value
--> tests/ui/assertions_on_constants.rs:89:5
|
LL | assert!(C);
| ^^^^^^^^^^
|
= help: consider moving this into a const block: `const { assert!(..) }`

error: this assertion has a constant value
--> tests/ui/assertions_on_constants.rs:95:5
|
LL | assert!(C);
| ^^^^^^^^^^
|
= help: consider moving this to an anonymous constant: `const _: () = { assert!(..); }`

error: aborting due to 16 previous errors