From 84a60c3186fe6a0656da43358cea00cea7319b83 Mon Sep 17 00:00:00 2001 From: Krishna Veera Reddy Date: Tue, 31 Dec 2019 10:33:15 -0800 Subject: [PATCH 1/2] Prevent `replace_consts` lint within match patterns Currently `replace_consts` lint applies within match patterns but the suggestion is incorrect as function calls are disallowed in them. To fix this we prevent the lint from firing within patterns. --- clippy_lints/src/replace_consts.rs | 21 ++++++++++++++++++--- tests/ui/replace_consts.fixed | 6 ++++++ tests/ui/replace_consts.rs | 6 ++++++ 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/replace_consts.rs b/clippy_lints/src/replace_consts.rs index 782d5f1c813f..a8259b0265a3 100644 --- a/clippy_lints/src/replace_consts.rs +++ b/clippy_lints/src/replace_consts.rs @@ -1,8 +1,8 @@ use crate::utils::{match_def_path, span_lint_and_sugg}; use if_chain::if_chain; use rustc::declare_lint_pass; -use rustc::hir; use rustc::hir::def::{DefKind, Res}; +use rustc::hir::*; use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use rustc_errors::Applicability; use rustc_session::declare_tool_lint; @@ -34,11 +34,26 @@ declare_clippy_lint! { declare_lint_pass!(ReplaceConsts => [REPLACE_CONSTS]); +fn in_pattern(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool { + let map = &cx.tcx.hir(); + let parent_id = map.get_parent_node(expr.hir_id); + + if let Some(node) = map.find(parent_id) { + if let Node::Pat(_) = node { + return true; + } + } + + false +} + impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ReplaceConsts { - fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr<'_>) { + fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) { if_chain! { - if let hir::ExprKind::Path(ref qp) = expr.kind; + if let ExprKind::Path(ref qp) = expr.kind; if let Res::Def(DefKind::Const, def_id) = cx.tables.qpath_res(qp, expr.hir_id); + // Do not lint within patterns as function calls are disallowed in them + if !in_pattern(cx, expr); then { for &(ref const_path, repl_snip) in &REPLACEMENTS { if match_def_path(cx, def_id, const_path) { diff --git a/tests/ui/replace_consts.fixed b/tests/ui/replace_consts.fixed index db3126654103..a8b11c8e4883 100644 --- a/tests/ui/replace_consts.fixed +++ b/tests/ui/replace_consts.fixed @@ -75,6 +75,12 @@ fn good() { { let foo = u32::max_value(); }; { let foo = u64::max_value(); }; { let foo = u128::max_value(); }; + + let _ = match 42 { + std::i8::MIN => -1, + 1..=std::i8::MAX => 1, + _ => 0 + }; } fn main() { diff --git a/tests/ui/replace_consts.rs b/tests/ui/replace_consts.rs index 0c8440e7675a..fff892b79ca7 100644 --- a/tests/ui/replace_consts.rs +++ b/tests/ui/replace_consts.rs @@ -75,6 +75,12 @@ fn good() { { let foo = u32::max_value(); }; { let foo = u64::max_value(); }; { let foo = u128::max_value(); }; + + let _ = match 42 { + std::i8::MIN => -1, + 1..=std::i8::MAX => 1, + _ => 0 + }; } fn main() { From 8b36196cb610dea86ba425079584f71d78cc218d Mon Sep 17 00:00:00 2001 From: Krishna Veera Reddy Date: Wed, 1 Jan 2020 23:22:57 -0800 Subject: [PATCH 2/2] Add `if let` test case --- tests/ui/replace_consts.fixed | 12 +++++++++++- tests/ui/replace_consts.rs | 12 +++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/tests/ui/replace_consts.fixed b/tests/ui/replace_consts.fixed index a8b11c8e4883..108474408e0f 100644 --- a/tests/ui/replace_consts.fixed +++ b/tests/ui/replace_consts.fixed @@ -76,11 +76,21 @@ fn good() { { let foo = u64::max_value(); }; { let foo = u128::max_value(); }; - let _ = match 42 { + let x = 42; + + let _ = match x { std::i8::MIN => -1, 1..=std::i8::MAX => 1, _ => 0 }; + + let _ = if let std::i8::MIN = x { + -1 + } else if let 1..=std::i8::MAX = x { + 1 + } else { + 0 + }; } fn main() { diff --git a/tests/ui/replace_consts.rs b/tests/ui/replace_consts.rs index fff892b79ca7..dae3422a35f0 100644 --- a/tests/ui/replace_consts.rs +++ b/tests/ui/replace_consts.rs @@ -76,11 +76,21 @@ fn good() { { let foo = u64::max_value(); }; { let foo = u128::max_value(); }; - let _ = match 42 { + let x = 42; + + let _ = match x { std::i8::MIN => -1, 1..=std::i8::MAX => 1, _ => 0 }; + + let _ = if let std::i8::MIN = x { + -1 + } else if let 1..=std::i8::MAX = x { + 1 + } else { + 0 + }; } fn main() {