diff options
| -rw-r--r-- | clippy_lints/src/matches.rs | 6 | ||||
| -rw-r--r-- | clippy_lints/src/utils/sugg.rs | 79 | ||||
| -rw-r--r-- | tests/ui/matches.rs | 27 | ||||
| -rw-r--r-- | tests/ui/matches.stderr | 16 |
4 files changed, 91 insertions, 37 deletions
diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index fcff1e16f38..bb337c067ba 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -570,13 +570,15 @@ fn check_match_ref_pats(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm], expr: if has_only_ref_pats(arms) { let mut suggs = Vec::new(); let (title, msg) = if let ExprKind::AddrOf(Mutability::MutImmutable, ref inner) = ex.node { - suggs.push((ex.span, Sugg::hir(cx, inner, "..").to_string())); + let span = ex.span.source_callsite(); + suggs.push((span, Sugg::hir_with_macro_callsite(cx, inner, "..").to_string())); ( "you don't need to add `&` to both the expression and the patterns", "try", ) } else { - suggs.push((ex.span, Sugg::hir(cx, ex, "..").deref().to_string())); + let span = ex.span.source_callsite(); + suggs.push((span, Sugg::hir_with_macro_callsite(cx, ex, "..").deref().to_string())); ( "you don't need to add `&` to all patterns", "instead of prefixing all patterns with `&`, you can dereference the expression", diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs index 166470876c9..7ff7b71ffa4 100644 --- a/clippy_lints/src/utils/sugg.rs +++ b/clippy_lints/src/utils/sugg.rs @@ -1,7 +1,7 @@ //! Contains utility functions to generate suggestions. #![deny(clippy::missing_docs_in_private_items)] -use crate::utils::{higher, in_macro, snippet, snippet_opt}; +use crate::utils::{higher, in_macro, snippet, snippet_opt, snippet_with_macro_callsite}; use matches::matches; use rustc::hir; use rustc::lint::{EarlyContext, LateContext, LintContext}; @@ -46,38 +46,7 @@ impl<'a> Sugg<'a> { pub fn hir_opt(cx: &LateContext<'_, '_>, expr: &hir::Expr) -> Option<Self> { snippet_opt(cx, expr.span).map(|snippet| { let snippet = Cow::Owned(snippet); - match expr.node { - hir::ExprKind::AddrOf(..) - | hir::ExprKind::Box(..) - | hir::ExprKind::Closure(.., _) - | hir::ExprKind::If(..) - | hir::ExprKind::Unary(..) - | hir::ExprKind::Match(..) => Sugg::MaybeParen(snippet), - hir::ExprKind::Continue(..) - | hir::ExprKind::Yield(..) - | hir::ExprKind::Array(..) - | hir::ExprKind::Block(..) - | hir::ExprKind::Break(..) - | hir::ExprKind::Call(..) - | hir::ExprKind::Field(..) - | hir::ExprKind::Index(..) - | hir::ExprKind::InlineAsm(..) - | hir::ExprKind::Lit(..) - | hir::ExprKind::Loop(..) - | hir::ExprKind::MethodCall(..) - | hir::ExprKind::Path(..) - | hir::ExprKind::Repeat(..) - | hir::ExprKind::Ret(..) - | hir::ExprKind::Struct(..) - | hir::ExprKind::Tup(..) - | hir::ExprKind::While(..) - | hir::ExprKind::Err => Sugg::NonParen(snippet), - hir::ExprKind::Assign(..) => Sugg::BinOp(AssocOp::Assign, snippet), - hir::ExprKind::AssignOp(op, ..) => Sugg::BinOp(hirbinop2assignop(op), snippet), - hir::ExprKind::Binary(op, ..) => Sugg::BinOp(AssocOp::from_ast_binop(higher::binop(op.node)), snippet), - hir::ExprKind::Cast(..) => Sugg::BinOp(AssocOp::As, snippet), - hir::ExprKind::Type(..) => Sugg::BinOp(AssocOp::Colon, snippet), - } + Self::hir_from_snippet(expr, snippet) }) } @@ -111,6 +80,50 @@ impl<'a> Sugg<'a> { }) } + /// Same as `hir`, but will use the pre expansion span if the `expr` was in a macro. + pub fn hir_with_macro_callsite(cx: &LateContext<'_, '_>, expr: &hir::Expr, default: &'a str) -> Self { + let snippet = snippet_with_macro_callsite(cx, expr.span, default); + + Self::hir_from_snippet(expr, snippet) + } + + /// Generate a suggestion for an expression with the given snippet. This is used by the `hir_*` + /// function variants of `Sugg`, since these use different snippet functions. + fn hir_from_snippet(expr: &hir::Expr, snippet: Cow<'a, str>) -> Self { + match expr.node { + hir::ExprKind::AddrOf(..) + | hir::ExprKind::Box(..) + | hir::ExprKind::Closure(.., _) + | hir::ExprKind::If(..) + | hir::ExprKind::Unary(..) + | hir::ExprKind::Match(..) => Sugg::MaybeParen(snippet), + hir::ExprKind::Continue(..) + | hir::ExprKind::Yield(..) + | hir::ExprKind::Array(..) + | hir::ExprKind::Block(..) + | hir::ExprKind::Break(..) + | hir::ExprKind::Call(..) + | hir::ExprKind::Field(..) + | hir::ExprKind::Index(..) + | hir::ExprKind::InlineAsm(..) + | hir::ExprKind::Lit(..) + | hir::ExprKind::Loop(..) + | hir::ExprKind::MethodCall(..) + | hir::ExprKind::Path(..) + | hir::ExprKind::Repeat(..) + | hir::ExprKind::Ret(..) + | hir::ExprKind::Struct(..) + | hir::ExprKind::Tup(..) + | hir::ExprKind::While(..) + | hir::ExprKind::Err => Sugg::NonParen(snippet), + hir::ExprKind::Assign(..) => Sugg::BinOp(AssocOp::Assign, snippet), + hir::ExprKind::AssignOp(op, ..) => Sugg::BinOp(hirbinop2assignop(op), snippet), + hir::ExprKind::Binary(op, ..) => Sugg::BinOp(AssocOp::from_ast_binop(higher::binop(op.node)), snippet), + hir::ExprKind::Cast(..) => Sugg::BinOp(AssocOp::As, snippet), + hir::ExprKind::Type(..) => Sugg::BinOp(AssocOp::Colon, snippet), + } + } + /// Prepare a suggestion from an expression. pub fn ast(cx: &EarlyContext<'_>, expr: &ast::Expr, default: &'a str) -> Self { use syntax::ast::RangeLimits; diff --git a/tests/ui/matches.rs b/tests/ui/matches.rs index a40b80378ef..c7c84b5d9c0 100644 --- a/tests/ui/matches.rs +++ b/tests/ui/matches.rs @@ -150,4 +150,29 @@ fn match_as_ref() { }; } -fn main() {} +macro_rules! foo_variant( + ($idx:expr) => (Foo::get($idx).unwrap()) +); + +enum Foo { + A, + B, +} + +impl Foo { + fn get(idx: u8) -> Option<&'static Self> { + match idx { + 0 => Some(&Foo::A), + 1 => Some(&Foo::B), + _ => None, + } + } +} + +fn main() { + // ICE #3719 + match foo_variant!(0) { + &Foo::A => println!("A"), + _ => println!("Wild"), + } +} diff --git a/tests/ui/matches.stderr b/tests/ui/matches.stderr index a3714a69e6e..b4159f7a68d 100644 --- a/tests/ui/matches.stderr +++ b/tests/ui/matches.stderr @@ -278,5 +278,19 @@ LL | | Some(ref mut v) => Some(v), LL | | }; | |_____^ help: try this: `mut_owned.as_mut()` -error: aborting due to 19 previous errors +error: you don't need to add `&` to all patterns + --> $DIR/matches.rs:174:5 + | +LL | / match foo_variant!(0) { +LL | | &Foo::A => println!("A"), +LL | | _ => println!("Wild"), +LL | | } + | |_____^ +help: instead of prefixing all patterns with `&`, you can dereference the expression + | +LL | match *foo_variant!(0) { +LL | Foo::A => println!("A"), + | + +error: aborting due to 20 previous errors |