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::>(); + // 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::>(); + // 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