Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't trigger assertions_on_constants on debug_assert!(false) #3989

Merged
merged 6 commits into from
Apr 19, 2019
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
61 changes: 30 additions & 31 deletions clippy_lints/src/assertions_on_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ use if_chain::if_chain;
use rustc::hir::{Expr, ExprKind};
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use rustc::{declare_lint_pass, declare_tool_lint};
use syntax_pos::Span;

use crate::consts::{constant, Constant};
use crate::syntax::ast::LitKind;
use crate::utils::{in_macro, is_direct_expn_of, span_help_and_lint};

declare_clippy_lint! {
Expand Down Expand Up @@ -33,41 +33,40 @@ declare_lint_pass!(AssertionsOnConstants => [ASSERTIONS_ON_CONSTANTS]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
let mut is_debug_assert = false;
let debug_assert_not_in_macro = |span: Span| {
is_debug_assert = true;
// Check that `debug_assert!` itself is not inside a macro
!in_macro(span)
};
if_chain! {
if let Some(assert_span) = is_direct_expn_of(e.span, "assert");
if !in_macro(assert_span)
|| is_direct_expn_of(assert_span, "debug_assert").map_or(false, |span| !in_macro(span));
|| is_direct_expn_of(assert_span, "debug_assert")
.map_or(false, debug_assert_not_in_macro);
if let ExprKind::Unary(_, ref lit) = e.node;
if let Some(bool_const) = constant(cx, cx.tables, lit);
then {
if let ExprKind::Lit(ref inner) = lit.node {
match inner.node {
LitKind::Bool(true) => {
span_help_and_lint(cx, ASSERTIONS_ON_CONSTANTS, e.span,
"assert!(true) will be optimized out by the compiler",
"remove it");
},
LitKind::Bool(false) => {
span_help_and_lint(
cx, ASSERTIONS_ON_CONSTANTS, e.span,
"assert!(false) should probably be replaced",
"use panic!() or unreachable!()");
},
_ => (),
}
} else if let Some(bool_const) = constant(cx, cx.tables, lit) {
match bool_const.0 {
Constant::Bool(true) => {
span_help_and_lint(cx, ASSERTIONS_ON_CONSTANTS, e.span,
"assert!(const: true) will be optimized out by the compiler",
"remove it");
},
Constant::Bool(false) => {
span_help_and_lint(cx, ASSERTIONS_ON_CONSTANTS, e.span,
"assert!(const: false) should probably be replaced",
"use panic!() or unreachable!()");
},
_ => (),
}
match bool_const.0 {
Constant::Bool(true) => {
span_help_and_lint(
cx,
ASSERTIONS_ON_CONSTANTS,
e.span,
"`assert!(true)` will be optimized out by the compiler",
"remove it"
);
},
Constant::Bool(false) if !is_debug_assert => {
span_help_and_lint(
cx,
ASSERTIONS_ON_CONSTANTS,
e.span,
"`assert!(false)` should probably be replaced",
"use `panic!()` or `unreachable!()`"
);
},
_ => (),
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/assertions_on_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ fn main() {
assert!(C);

debug_assert!(true);
// Don't lint this, since there is no better way for expressing "Only panic in debug mode".
debug_assert!(false); // #3948
phansch marked this conversation as resolved.
Show resolved Hide resolved
assert_const!(3);
assert_const!(-1);
}
20 changes: 10 additions & 10 deletions tests/ui/assertions_on_constants.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: assert!(true) will be optimized out by the compiler
error: `assert!(true)` will be optimized out by the compiler
--> $DIR/assertions_on_constants.rs:9:5
|
LL | assert!(true);
Expand All @@ -7,47 +7,47 @@ LL | assert!(true);
= note: `-D clippy::assertions-on-constants` implied by `-D warnings`
= help: remove it

error: assert!(false) should probably be replaced
error: `assert!(false)` should probably be replaced
--> $DIR/assertions_on_constants.rs:10:5
|
LL | assert!(false);
| ^^^^^^^^^^^^^^^
|
= help: use panic!() or unreachable!()
= help: use `panic!()` or `unreachable!()`

error: assert!(true) will be optimized out by the compiler
error: `assert!(true)` will be optimized out by the compiler
--> $DIR/assertions_on_constants.rs:11:5
|
LL | assert!(true, "true message");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: remove it

error: assert!(false) should probably be replaced
error: `assert!(false)` should probably be replaced
--> $DIR/assertions_on_constants.rs:12:5
|
LL | assert!(false, "false message");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: use panic!() or unreachable!()
= help: use `panic!()` or `unreachable!()`

error: assert!(const: true) will be optimized out by the compiler
error: `assert!(true)` will be optimized out by the compiler
--> $DIR/assertions_on_constants.rs:15:5
|
LL | assert!(B);
| ^^^^^^^^^^^
|
= help: remove it

error: assert!(const: false) should probably be replaced
error: `assert!(false)` should probably be replaced
--> $DIR/assertions_on_constants.rs:18:5
|
LL | assert!(C);
| ^^^^^^^^^^^
|
= help: use panic!() or unreachable!()
= help: use `panic!()` or `unreachable!()`

error: assert!(true) will be optimized out by the compiler
error: `assert!(true)` will be optimized out by the compiler
--> $DIR/assertions_on_constants.rs:20:5
|
LL | debug_assert!(true);
Expand Down