From 439284b58400be98036117d053815eb24472fee7 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sun, 18 Aug 2019 01:46:44 +0700 Subject: [PATCH 1/3] 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/3] 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); } } } From b313d25350bf8df41e65485a65b4536fe267bec0 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Sun, 18 Aug 2019 13:14:47 +0200 Subject: [PATCH 3/3] Fix breakage due to rust-lang/rust#61708 --- clippy_lints/src/copies.rs | 8 ++++---- clippy_lints/src/utils/author.rs | 6 ++++++ clippy_lints/src/utils/inspector.rs | 6 ++++++ clippy_lints/src/utils/mod.rs | 2 +- 4 files changed, 17 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs index 64239bb4dd42..01989625b45a 100644 --- a/clippy_lints/src/copies.rs +++ b/clippy_lints/src/copies.rs @@ -298,14 +298,14 @@ fn bindings<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, pat: &Pat) -> FxHashMap { + PatKind::Or(ref fields) | PatKind::Tuple(ref fields, _) => { for pat in fields { - bindings_impl(cx, &pat.pat, map); + bindings_impl(cx, pat, map); } }, - PatKind::Tuple(ref fields, _) => { + PatKind::Struct(_, ref fields, _) => { for pat in fields { - bindings_impl(cx, pat, map); + bindings_impl(cx, &pat.pat, map); } }, PatKind::Slice(ref lhs, ref mid, ref rhs) => { diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs index 6d9eddac8948..4cf52ddac929 100644 --- a/clippy_lints/src/utils/author.rs +++ b/clippy_lints/src/utils/author.rs @@ -549,6 +549,12 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor { println!(" if {}.len() == {};", fields_pat, fields.len()); println!(" // unimplemented: field checks"); }, + PatKind::Or(ref fields) => { + let fields_pat = self.next("fields"); + println!("Or(ref {}) = {};", fields_pat, current); + println!(" if {}.len() == {};", fields_pat, fields.len()); + println!(" // unimplemented: field checks"); + }, PatKind::TupleStruct(ref path, ref fields, skip_pos) => { let path_pat = self.next("path"); let fields_pat = self.next("fields"); diff --git a/clippy_lints/src/utils/inspector.rs b/clippy_lints/src/utils/inspector.rs index 309b5165c69e..b48ef7d293b4 100644 --- a/clippy_lints/src/utils/inspector.rs +++ b/clippy_lints/src/utils/inspector.rs @@ -410,6 +410,12 @@ fn print_pat(cx: &LateContext<'_, '_>, pat: &hir::Pat, indent: usize) { print_pat(cx, inner, indent + 1); } }, + hir::PatKind::Or(ref fields) => { + println!("{}Or", ind); + for field in fields { + print_pat(cx, field, indent + 1); + } + }, hir::PatKind::Struct(ref path, ref fields, ignore) => { println!("{}Struct", ind); println!( diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index a25f25041272..020068d4633a 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -791,7 +791,7 @@ pub fn is_refutable(cx: &LateContext<'_, '_>, pat: &Pat) -> bool { PatKind::Box(ref pat) | PatKind::Ref(ref pat, _) => is_refutable(cx, pat), PatKind::Lit(..) | PatKind::Range(..) => true, PatKind::Path(ref qpath) => is_enum_variant(cx, qpath, pat.hir_id), - PatKind::Tuple(ref pats, _) => are_refutable(cx, pats.iter().map(|pat| &**pat)), + PatKind::Or(ref pats) | PatKind::Tuple(ref pats, _) => are_refutable(cx, pats.iter().map(|pat| &**pat)), PatKind::Struct(ref qpath, ref fields, _) => { if is_enum_variant(cx, qpath, pat.hir_id) { true