From df828b9554e3a6f936bfec1092b7080c7dc95853 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Sat, 6 Oct 2018 20:43:19 +0200 Subject: [PATCH 1/2] Fix single_match suggestions with expanded macros --- clippy_lints/src/matches.rs | 18 ++++++++++--- tests/ui/matches.stderr | 2 +- tests/ui/single_match.rs | 9 +++++++ tests/ui/single_match.stderr | 52 ++++++++++++++++++++++-------------- 4 files changed, 57 insertions(+), 24 deletions(-) diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index e46615f4da2f..048e60ca7f12 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -13,12 +13,13 @@ use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass, in_exte use crate::rustc::{declare_tool_lint, lint_array}; use if_chain::if_chain; use crate::rustc::ty::{self, Ty}; +use std::borrow::Cow; use std::cmp::Ordering; use std::collections::Bound; use crate::syntax::ast::LitKind; use crate::syntax::source_map::Span; use crate::utils::paths; -use crate::utils::{expr_block, is_allowed, is_expn_of, match_qpath, match_type, multispan_sugg, +use crate::utils::{expr_block, in_macro, is_allowed, is_expn_of, match_qpath, match_type, multispan_sugg, remove_blocks, snippet, span_lint_and_sugg, span_lint_and_then, span_note_and_lint, walk_ptrs_ty}; use crate::utils::sugg::Sugg; use crate::consts::{constant, Constant}; @@ -254,7 +255,18 @@ fn report_single_match_single_pattern(cx: &LateContext<'_, '_>, ex: &Expr, arms: } else { SINGLE_MATCH }; - let els_str = els.map_or(String::new(), |els| format!(" else {}", expr_block(cx, els, None, ".."))); + let els_str = els.map_or(String::new(), |els| { + if in_macro(els.span) { + " else { .. }".to_string() + } else { + format!(" else {}", expr_block(cx, els, None, "..")) + } + }); + let expr_block = if in_macro(arms[0].body.span) { + Cow::Owned("{ .. }".to_string()) + } else { + expr_block(cx, &arms[0].body, None, "..") + }; span_lint_and_sugg( cx, lint, @@ -266,7 +278,7 @@ fn report_single_match_single_pattern(cx: &LateContext<'_, '_>, ex: &Expr, arms: "if let {} = {} {}{}", snippet(cx, arms[0].pats[0].span, ".."), snippet(cx, ex.span, ".."), - expr_block(cx, &arms[0].body, None, ".."), + expr_block, els_str ), ); diff --git a/tests/ui/matches.stderr b/tests/ui/matches.stderr index bed903faf1a1..7c4b84f6f7ed 100644 --- a/tests/ui/matches.stderr +++ b/tests/ui/matches.stderr @@ -33,7 +33,7 @@ error: you seem to be trying to use match for destructuring a single pattern. Co 51 | | &(v, 1) => println!("{}", v), 52 | | _ => println!("none"), 53 | | } - | |_____^ help: try this: `if let &(v, 1) = tup { $ crate :: io :: _print ( format_args_nl ! ( $ ( $ arg ) * ) ) ; } else { $ crate :: io :: _print ( format_args_nl ! ( $ ( $ arg ) * ) ) ; }` + | |_____^ help: try this: `if let &(v, 1) = tup { .. } else { .. }` error: you don't need to add `&` to all patterns --> $DIR/matches.rs:50:5 diff --git a/tests/ui/single_match.rs b/tests/ui/single_match.rs index 5c7cae249b4f..dca68e179e74 100644 --- a/tests/ui/single_match.rs +++ b/tests/ui/single_match.rs @@ -23,6 +23,15 @@ fn single_match(){ _ => () }; + let x = Some(1u8); + match x { + // Note the missing block braces. + // We suggest `if let Some(y) = x { .. }` because the macro + // is expanded before we can do anything. + Some(y) => println!("{:?}", y), + _ => () + } + let z = (1u8,1u8); match z { (2...3, 7...9) => dummy(), diff --git a/tests/ui/single_match.stderr b/tests/ui/single_match.stderr index 74448391ca52..a050134d5769 100644 --- a/tests/ui/single_match.stderr +++ b/tests/ui/single_match.stderr @@ -12,38 +12,50 @@ error: you seem to be trying to use match for destructuring a single pattern. Co error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let` --> $DIR/single_match.rs:27:5 | -27 | / match z { -28 | | (2...3, 7...9) => dummy(), -29 | | _ => {} -30 | | }; +27 | / match x { +28 | | // Note the missing block braces. +29 | | // We suggest `if let Some(y) = x { .. }` because the macro +30 | | // is expanded before we can do anything. +31 | | Some(y) => println!("{:?}", y), +32 | | _ => () +33 | | } + | |_____^ help: try this: `if let Some(y) = x { .. }` + +error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let` + --> $DIR/single_match.rs:36:5 + | +36 | / match z { +37 | | (2...3, 7...9) => dummy(), +38 | | _ => {} +39 | | }; | |_____^ help: try this: `if let (2...3, 7...9) = z { dummy() }` error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let` - --> $DIR/single_match.rs:53:5 + --> $DIR/single_match.rs:62:5 | -53 | / match x { -54 | | Some(y) => dummy(), -55 | | None => () -56 | | }; +62 | / match x { +63 | | Some(y) => dummy(), +64 | | None => () +65 | | }; | |_____^ help: try this: `if let Some(y) = x { dummy() }` error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let` - --> $DIR/single_match.rs:58:5 + --> $DIR/single_match.rs:67:5 | -58 | / match y { -59 | | Ok(y) => dummy(), -60 | | Err(..) => () -61 | | }; +67 | / match y { +68 | | Ok(y) => dummy(), +69 | | Err(..) => () +70 | | }; | |_____^ help: try this: `if let Ok(y) = y { dummy() }` error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let` - --> $DIR/single_match.rs:65:5 + --> $DIR/single_match.rs:74:5 | -65 | / match c { -66 | | Cow::Borrowed(..) => dummy(), -67 | | Cow::Owned(..) => (), -68 | | }; +74 | / match c { +75 | | Cow::Borrowed(..) => dummy(), +76 | | Cow::Owned(..) => (), +77 | | }; | |_____^ help: try this: `if let Cow::Borrowed(..) = c { dummy() }` -error: aborting due to 5 previous errors +error: aborting due to 6 previous errors From dfe08fb0f98590c0c10972a735c769ec844cba1d Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Sat, 27 Oct 2018 09:50:53 +0200 Subject: [PATCH 2/2] Include macro callsites in suggestions --- clippy_lints/src/matches.rs | 9 +++++---- tests/ui/matches.stderr | 2 +- tests/ui/single_match.stderr | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 048e60ca7f12..ba080f860272 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -13,7 +13,6 @@ use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass, in_exte use crate::rustc::{declare_tool_lint, lint_array}; use if_chain::if_chain; use crate::rustc::ty::{self, Ty}; -use std::borrow::Cow; use std::cmp::Ordering; use std::collections::Bound; use crate::syntax::ast::LitKind; @@ -257,15 +256,17 @@ fn report_single_match_single_pattern(cx: &LateContext<'_, '_>, ex: &Expr, arms: }; let els_str = els.map_or(String::new(), |els| { if in_macro(els.span) { - " else { .. }".to_string() + let source_callsite_snip = snippet(cx, els.span.source_callsite(), ".."); + format!(" else {{ {} }}", source_callsite_snip) } else { format!(" else {}", expr_block(cx, els, None, "..")) } }); let expr_block = if in_macro(arms[0].body.span) { - Cow::Owned("{ .. }".to_string()) + let source_callsite_snip = snippet(cx, arms[0].body.span.source_callsite(), ".."); + format!("{{ {} }}", source_callsite_snip) } else { - expr_block(cx, &arms[0].body, None, "..") + expr_block(cx, &arms[0].body, None, "..").to_string() }; span_lint_and_sugg( cx, diff --git a/tests/ui/matches.stderr b/tests/ui/matches.stderr index 7c4b84f6f7ed..b5f1f2ab0e73 100644 --- a/tests/ui/matches.stderr +++ b/tests/ui/matches.stderr @@ -33,7 +33,7 @@ error: you seem to be trying to use match for destructuring a single pattern. Co 51 | | &(v, 1) => println!("{}", v), 52 | | _ => println!("none"), 53 | | } - | |_____^ help: try this: `if let &(v, 1) = tup { .. } else { .. }` + | |_____^ help: try this: `if let &(v, 1) = tup { println!("{}", v) } else { println!("none") }` error: you don't need to add `&` to all patterns --> $DIR/matches.rs:50:5 diff --git a/tests/ui/single_match.stderr b/tests/ui/single_match.stderr index a050134d5769..df614ad201d1 100644 --- a/tests/ui/single_match.stderr +++ b/tests/ui/single_match.stderr @@ -19,7 +19,7 @@ error: you seem to be trying to use match for destructuring a single pattern. Co 31 | | Some(y) => println!("{:?}", y), 32 | | _ => () 33 | | } - | |_____^ help: try this: `if let Some(y) = x { .. }` + | |_____^ help: try this: `if let Some(y) = x { println!("{:?}", y) }` error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let` --> $DIR/single_match.rs:36:5