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

Fix assertions on const lint #4406

Closed
wants to merge 2 commits into from
Closed
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
45 changes: 20 additions & 25 deletions clippy_lints/src/assertions_on_constants.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,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::utils::{in_macro_or_desugar, is_direct_expn_of, span_help_and_lint};
use crate::utils::{in_macro_or_desugar, is_direct_expn_of, is_expn_of, span_help_and_lint};

declare_clippy_lint! {
/// **What it does:** Checks for `assert!(true)` and `assert!(false)` calls.
Expand Down Expand Up @@ -33,42 +31,39 @@ 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_or_desugar = |span: Span| {
is_debug_assert = true;
// Check that `debug_assert!` itself is not inside a macro
!in_macro_or_desugar(span)
};
if_chain! {
if let Some(assert_span) = is_direct_expn_of(e.span, "assert");
if !in_macro_or_desugar(assert_span)
|| is_direct_expn_of(assert_span, "debug_assert")
.map_or(false, debug_assert_not_in_macro_or_desugar);
if let ExprKind::Unary(_, ref lit) = e.node;
if let Some(bool_const) = constant(cx, cx.tables, lit);
then {
match bool_const.0 {
Constant::Bool(true) => {
let lint_assert_cb = |is_debug_assert: bool| {
if let ExprKind::Unary(_, ref lit) = e.node {
if let Some((Constant::Bool(is_true), _)) = constant(cx, cx.tables, lit) {
if is_true {
span_help_and_lint(
cx,
ASSERTIONS_ON_CONSTANTS,
e.span,
"`assert!(true)` will be optimized out by the compiler",
"remove it"
"remove it",
);
},
Constant::Bool(false) if !is_debug_assert => {
} else if !is_debug_assert {
span_help_and_lint(
cx,
ASSERTIONS_ON_CONSTANTS,
e.span,
"`assert!(false)` should probably be replaced",
"use `panic!()` or `unreachable!()`"
"use `panic!()` or `unreachable!()`",
);
},
_ => (),
}
}
}
};
if let Some(debug_assert_span) = is_expn_of(e.span, "debug_assert") {
if in_macro_or_desugar(debug_assert_span) {
return;
}
lint_assert_cb(true);
} else if let Some(assert_span) = is_direct_expn_of(e.span, "assert") {
if in_macro_or_desugar(assert_span) {
return;
}
lint_assert_cb(false);
}
}
}