From 1a86d80c1085c7415f97c8fb446f39f2ed80c6b4 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Sat, 16 Feb 2019 19:34:51 +0100 Subject: [PATCH 1/4] Implement Sugg::hir_with_macro_callsite --- clippy_lints/src/utils/sugg.rs | 76 +++++++++++++++++++--------------- 1 file changed, 43 insertions(+), 33 deletions(-) diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs index 166470876c98..f9e7bf0c9712 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 { 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,47 @@ impl<'a> Sugg<'a> { }) } + 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) + } + + 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; From 87ae6c8bc989c99435afec478a31fca7e319a17d Mon Sep 17 00:00:00 2001 From: flip1995 Date: Sat, 16 Feb 2019 19:35:15 +0100 Subject: [PATCH 2/4] Fix ice-3719 --- clippy_lints/src/matches.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index fcff1e16f383..bb337c067bae 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", From 0ce564a7e19ac17a7dfad8f94ad009d5e03200bc Mon Sep 17 00:00:00 2001 From: flip1995 Date: Sat, 16 Feb 2019 19:37:58 +0100 Subject: [PATCH 3/4] Add test related to the ICE This test doesn't reproduce the ICE since it only happens, when the macro is defined in another file. Currently we can't add tests with multiple files AFAIK Also using the auxiliary folder didn't help --- tests/ui/matches.rs | 27 ++++++++++++++++++++++++++- tests/ui/matches.stderr | 16 +++++++++++++++- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/tests/ui/matches.rs b/tests/ui/matches.rs index a40b80378efc..c7c84b5d9c0a 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 a3714a69e6ef..b4159f7a68d0 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 From 75f39881d4ad76a2aa2bfacdfbd8d49e5d92a0e8 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Sat, 16 Feb 2019 22:52:20 +0100 Subject: [PATCH 4/4] Document the new `Sugg` functions --- clippy_lints/src/utils/sugg.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs index f9e7bf0c9712..7ff7b71ffa41 100644 --- a/clippy_lints/src/utils/sugg.rs +++ b/clippy_lints/src/utils/sugg.rs @@ -80,12 +80,15 @@ 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(..)