Auto merge of #3989 - flip1995:assert_on_const, r=phansch

Don't trigger assertions_on_constants on debug_assert!(false)

Fixes #3948
Fixes #3765

changelog: Fix `debug_assert!` false positive on `assertions_on_constants` lint
This commit is contained in:
bors 2019-04-19 09:57:35 +00:00
commit 12e8075d91
3 changed files with 42 additions and 41 deletions

View File

@ -2,9 +2,9 @@ use if_chain::if_chain;
use rustc::hir::{Expr, ExprKind}; use rustc::hir::{Expr, ExprKind};
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use rustc::{declare_lint_pass, declare_tool_lint}; use rustc::{declare_lint_pass, declare_tool_lint};
use syntax_pos::Span;
use crate::consts::{constant, Constant}; use crate::consts::{constant, Constant};
use crate::syntax::ast::LitKind;
use crate::utils::{in_macro, is_direct_expn_of, span_help_and_lint}; use crate::utils::{in_macro, is_direct_expn_of, span_help_and_lint};
declare_clippy_lint! { declare_clippy_lint! {
@ -33,41 +33,40 @@ declare_lint_pass!(AssertionsOnConstants => [ASSERTIONS_ON_CONSTANTS]);
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) { 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_chain! {
if let Some(assert_span) = is_direct_expn_of(e.span, "assert"); if let Some(assert_span) = is_direct_expn_of(e.span, "assert");
if !in_macro(assert_span) 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 ExprKind::Unary(_, ref lit) = e.node;
if let Some(bool_const) = constant(cx, cx.tables, lit);
then { then {
if let ExprKind::Lit(ref inner) = lit.node { match bool_const.0 {
match inner.node { Constant::Bool(true) => {
LitKind::Bool(true) => { span_help_and_lint(
span_help_and_lint(cx, ASSERTIONS_ON_CONSTANTS, e.span, cx,
"assert!(true) will be optimized out by the compiler", ASSERTIONS_ON_CONSTANTS,
"remove it"); e.span,
}, "`assert!(true)` will be optimized out by the compiler",
LitKind::Bool(false) => { "remove it"
span_help_and_lint( );
cx, ASSERTIONS_ON_CONSTANTS, e.span, },
"assert!(false) should probably be replaced", Constant::Bool(false) if !is_debug_assert => {
"use panic!() or unreachable!()"); span_help_and_lint(
}, cx,
_ => (), ASSERTIONS_ON_CONSTANTS,
} e.span,
} else if let Some(bool_const) = constant(cx, cx.tables, lit) { "`assert!(false)` should probably be replaced",
match bool_const.0 { "use `panic!()` or `unreachable!()`"
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!()");
},
_ => (),
}
} }
} }
} }

View File

@ -18,6 +18,8 @@ fn main() {
assert!(C); assert!(C);
debug_assert!(true); debug_assert!(true);
// 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!(3);
assert_const!(-1); assert_const!(-1);
} }

View File

@ -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 --> $DIR/assertions_on_constants.rs:9:5
| |
LL | assert!(true); LL | assert!(true);
@ -7,15 +7,15 @@ LL | assert!(true);
= note: `-D clippy::assertions-on-constants` implied by `-D warnings` = note: `-D clippy::assertions-on-constants` implied by `-D warnings`
= help: remove it = help: remove it
error: assert!(false) should probably be replaced error: `assert!(false)` should probably be replaced
--> $DIR/assertions_on_constants.rs:10:5 --> $DIR/assertions_on_constants.rs:10:5
| |
LL | assert!(false); 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 --> $DIR/assertions_on_constants.rs:11:5
| |
LL | assert!(true, "true message"); LL | assert!(true, "true message");
@ -23,15 +23,15 @@ LL | assert!(true, "true message");
| |
= help: remove it = help: remove it
error: assert!(false) should probably be replaced error: `assert!(false)` should probably be replaced
--> $DIR/assertions_on_constants.rs:12:5 --> $DIR/assertions_on_constants.rs:12:5
| |
LL | assert!(false, "false message"); 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 --> $DIR/assertions_on_constants.rs:15:5
| |
LL | assert!(B); LL | assert!(B);
@ -39,15 +39,15 @@ LL | assert!(B);
| |
= help: remove it = 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 --> $DIR/assertions_on_constants.rs:18:5
| |
LL | assert!(C); 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 --> $DIR/assertions_on_constants.rs:20:5
| |
LL | debug_assert!(true); LL | debug_assert!(true);