From 439284b58400be98036117d053815eb24472fee7 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sun, 18 Aug 2019 01:46:44 +0700 Subject: [PATCH 1/2] Make code cleaner and more readable --- clippy_lints/src/assertions_on_constants.rs | 64 ++++++++++----------- 1 file changed, 31 insertions(+), 33 deletions(-) diff --git a/clippy_lints/src/assertions_on_constants.rs b/clippy_lints/src/assertions_on_constants.rs index 0f0c0dbc15d0..22dbd3ecc9c1 100644 --- a/clippy_lints/src/assertions_on_constants.rs +++ b/clippy_lints/src/assertions_on_constants.rs @@ -34,39 +34,37 @@ 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) => { - 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!()`" - ); - }, - _ => (), + if let Some(assert_span) = is_direct_expn_of(e.span, "assert") { + if in_macro_or_desugar(assert_span) { + return; + } + if let Some(debug_assert_span) = is_direct_expn_of(assert_span, "debug_assert") { + if in_macro_or_desugar(debug_assert_span) { + return; + } + is_debug_assert = true; + } + if let ExprKind::Unary(_, ref lit) = e.node { + if let Some((bool_const, _)) = constant(cx, cx.tables, lit) { + if let Constant::Bool(is_true) bool_const { + if is_true { + span_help_and_lint( + cx, + ASSERTIONS_ON_CONSTANTS, + e.span, + "`assert!(true)` will be optimized out by the compiler", + "remove it" + ); + } 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!()`" + ); + } + } } } } From 533bdfbdbc52c87d03959a4d58be6301f157da6e Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sun, 18 Aug 2019 01:46:44 +0700 Subject: [PATCH 2/2] Fix assertions_on_constants lint --- clippy_lints/src/assertions_on_constants.rs | 63 ++++++++++----------- 1 file changed, 30 insertions(+), 33 deletions(-) diff --git a/clippy_lints/src/assertions_on_constants.rs b/clippy_lints/src/assertions_on_constants.rs index 22dbd3ecc9c1..32ca1cc1bb99 100644 --- a/clippy_lints/src/assertions_on_constants.rs +++ b/clippy_lints/src/assertions_on_constants.rs @@ -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. @@ -33,40 +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; - if let Some(assert_span) = is_direct_expn_of(e.span, "assert") { - if in_macro_or_desugar(assert_span) { - return; - } - if let Some(debug_assert_span) = is_direct_expn_of(assert_span, "debug_assert") { - if in_macro_or_desugar(debug_assert_span) { - return; - } - is_debug_assert = true; - } + let lint_assert_cb = |is_debug_assert: bool| { if let ExprKind::Unary(_, ref lit) = e.node { - if let Some((bool_const, _)) = constant(cx, cx.tables, lit) { - if let Constant::Bool(is_true) bool_const { - if is_true { - span_help_and_lint( - cx, - ASSERTIONS_ON_CONSTANTS, - e.span, - "`assert!(true)` will be optimized out by the compiler", - "remove it" - ); - } 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!()`" - ); - } + 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", + ); + } 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!()`", + ); } } } + }; + 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); } } }