From a37af06fea574f3d9f4d3cca58a70cd545523486 Mon Sep 17 00:00:00 2001
From: xFrednet <xFrednet@gmail.com>
Date: Wed, 9 Dec 2020 12:25:45 +0000
Subject: [PATCH] Removing false positive for the match_single_binding lint
* Applying suggested changes from the PR
---
clippy_lints/src/matches.rs | 22 +++++++++++++++++--
tests/ui/match_single_binding.fixed | 28 +++++++++++++++++++++++
tests/ui/match_single_binding.rs | 33 ++++++++++++++++++++++++++++
tests/ui/match_single_binding.stderr | 13 ++++++++++-
4 files changed, 93 insertions(+), 3 deletions(-)
diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs
index 2a1a73f98ee..04b35835c6b 100644
--- a/clippy_lints/src/matches.rs
+++ b/clippy_lints/src/matches.rs
@@ -4,8 +4,8 @@ use crate::utils::usage::is_unused;
use crate::utils::{
expr_block, get_arg_name, get_parent_expr, in_macro, indent_of, is_allowed, is_expn_of, is_refutable,
is_type_diagnostic_item, is_wild, match_qpath, match_type, match_var, meets_msrv, multispan_sugg, remove_blocks,
- snippet, snippet_block, snippet_with_applicability, span_lint_and_help, span_lint_and_note, span_lint_and_sugg,
- span_lint_and_then,
+ snippet, snippet_block, snippet_opt, snippet_with_applicability, span_lint_and_help, span_lint_and_note,
+ span_lint_and_sugg, span_lint_and_then,
};
use crate::utils::{paths, search_same, SpanlessEq, SpanlessHash};
use if_chain::if_chain;
@@ -1237,6 +1237,24 @@ fn check_match_single_binding<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[A
if in_macro(expr.span) || arms.len() != 1 || is_refutable(cx, arms[0].pat) {
return;
}
+
+ // HACK:
+ // This is a hack to deal with arms that are excluded by macros like `#[cfg]`. It is only used here
+ // to prevent false positives as there is currently no better way to detect if code was excluded by
+ // a macro. See PR #6435
+ if_chain! {
+ if let Some(match_snippet) = snippet_opt(cx, expr.span);
+ if let Some(arm_snippet) = snippet_opt(cx, arms[0].span);
+ if let Some(ex_snippet) = snippet_opt(cx, ex.span);
+ let rest_snippet = match_snippet.replace(&arm_snippet, "").replace(&ex_snippet, "");
+ if rest_snippet.contains("=>");
+ then {
+ // The code it self contains another thick arrow "=>"
+ // -> Either another arm or a comment
+ return;
+ }
+ }
+
let matched_vars = ex.span;
let bind_names = arms[0].pat.span;
let match_body = remove_blocks(&arms[0].body);
diff --git a/tests/ui/match_single_binding.fixed b/tests/ui/match_single_binding.fixed
index f3627902eec..526e94b10bd 100644
--- a/tests/ui/match_single_binding.fixed
+++ b/tests/ui/match_single_binding.fixed
@@ -87,4 +87,32 @@ fn main() {
unwrapped
})
.collect::<Vec<u8>>();
+ // Ok
+ let x = 1;
+ match x {
+ #[cfg(disabled_feature)]
+ 0 => println!("Disabled branch"),
+ _ => println!("Enabled branch"),
+ }
+ // Lint
+ let x = 1;
+ let y = 1;
+ println!("Single branch");
+ // Ok
+ let x = 1;
+ let y = 1;
+ match match y {
+ 0 => 1,
+ _ => 2,
+ } {
+ #[cfg(disabled_feature)]
+ 0 => println!("Array index start"),
+ _ => println!("Not an array index start"),
+ }
+ // False negative
+ let x = 1;
+ match x {
+ // =>
+ _ => println!("Not an array index start"),
+ }
}
diff --git a/tests/ui/match_single_binding.rs b/tests/ui/match_single_binding.rs
index 8c182148ae1..6a2ca7c5e93 100644
--- a/tests/ui/match_single_binding.rs
+++ b/tests/ui/match_single_binding.rs
@@ -99,4 +99,37 @@ fn main() {
unwrapped => unwrapped,
})
.collect::<Vec<u8>>();
+ // Ok
+ let x = 1;
+ match x {
+ #[cfg(disabled_feature)]
+ 0 => println!("Disabled branch"),
+ _ => println!("Enabled branch"),
+ }
+ // Lint
+ let x = 1;
+ let y = 1;
+ match match y {
+ 0 => 1,
+ _ => 2,
+ } {
+ _ => println!("Single branch"),
+ }
+ // Ok
+ let x = 1;
+ let y = 1;
+ match match y {
+ 0 => 1,
+ _ => 2,
+ } {
+ #[cfg(disabled_feature)]
+ 0 => println!("Array index start"),
+ _ => println!("Not an array index start"),
+ }
+ // False negative
+ let x = 1;
+ match x {
+ // =>
+ _ => println!("Not an array index start"),
+ }
}
diff --git a/tests/ui/match_single_binding.stderr b/tests/ui/match_single_binding.stderr
index 795c8c3e24d..cbbf5d29c02 100644
--- a/tests/ui/match_single_binding.stderr
+++ b/tests/ui/match_single_binding.stderr
@@ -167,5 +167,16 @@ LL | unwrapped
LL | })
|
-error: aborting due to 11 previous errors
+error: this match could be replaced by its body itself
+ --> $DIR/match_single_binding.rs:112:5
+ |
+LL | / match match y {
+LL | | 0 => 1,
+LL | | _ => 2,
+LL | | } {
+LL | | _ => println!("Single branch"),
+LL | | }
+ | |_____^ help: consider using the match body instead: `println!("Single branch");`
+
+error: aborting due to 12 previous errors