Skip to content

Commit

Permalink
Auto merge of #4977 - krishna-veerareddy:issue-4969-replace-consts-fp…
Browse files Browse the repository at this point in the history
…, r=phansch

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.

Fixes #4969

changelog: Fix false positive in `replace_consts` lint
  • Loading branch information
bors committed Jan 2, 2020
2 parents 99dd0bb + 8b36196 commit 5b710ee
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 3 deletions.
21 changes: 18 additions & 3 deletions clippy_lints/src/replace_consts.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
16 changes: 16 additions & 0 deletions tests/ui/replace_consts.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,22 @@ fn good() {
{ let foo = u32::max_value(); };
{ let foo = u64::max_value(); };
{ let foo = u128::max_value(); };

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() {
Expand Down
16 changes: 16 additions & 0 deletions tests/ui/replace_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,22 @@ fn good() {
{ let foo = u32::max_value(); };
{ let foo = u64::max_value(); };
{ let foo = u128::max_value(); };

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() {
Expand Down

0 comments on commit 5b710ee

Please sign in to comment.