From 475be10dbd5bef7c37d58f2d5f1905123cb6e19e Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Tue, 16 Oct 2018 21:36:02 -0700 Subject: [PATCH] in which unused-parens suggestions heed what the user actually wrote MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Aaron Hill pointed out that unnecessary parens around a macro call (paradigmatically, `format!`) yielded a suggestion of hideous macro-expanded code. (The slightly unusual choice of using the pretty-printer to compose suggestions was quite recently commented on in the commit message for 1081bbbfc ("abolish ICE when pretty-printing async block"), but without any grounds to condemn it as a 𝘣𝘢𝘥 choice. Hill's report provides the grounds.) `span_to_snippet` is fallable as far as the type system is concerned (because, who knows, macros or something), so the pretty-printing can live on in the oft-neglected `else` branch. Resolves #55109. --- src/librustc_lint/unused.rs | 18 ++++++++++++++---- src/test/ui/lint/suggestions.rs | 4 ++-- src/test/ui/lint/suggestions.stderr | 10 +++++----- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 76717548521b7..e690969732291 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -281,8 +281,13 @@ impl UnusedParens { let necessary = struct_lit_needs_parens && parser::contains_exterior_struct_lit(&inner); if !necessary { - let pattern = pprust::expr_to_string(value); - Self::remove_outer_parens(cx, value.span, &pattern, msg); + let expr_text = if let Ok(snippet) = cx.sess().source_map() + .span_to_snippet(value.span) { + snippet + } else { + pprust::expr_to_string(value) + }; + Self::remove_outer_parens(cx, value.span, &expr_text, msg); } } } @@ -292,8 +297,13 @@ impl UnusedParens { value: &ast::Pat, msg: &str) { if let ast::PatKind::Paren(_) = value.node { - let pattern = pprust::pat_to_string(value); - Self::remove_outer_parens(cx, value.span, &pattern, msg); + let pattern_text = if let Ok(snippet) = cx.sess().source_map() + .span_to_snippet(value.span) { + snippet + } else { + pprust::pat_to_string(value) + }; + Self::remove_outer_parens(cx, value.span, &pattern_text, msg); } } diff --git a/src/test/ui/lint/suggestions.rs b/src/test/ui/lint/suggestions.rs index ff50b3b1ab68f..77d546a00ecd4 100644 --- a/src/test/ui/lint/suggestions.rs +++ b/src/test/ui/lint/suggestions.rs @@ -56,7 +56,7 @@ fn main() { while true { //~^ WARN denote infinite loops //~| HELP use `loop` - let mut a = (1); + let mut registry_no = (format!("NX-{}", 74205)); //~^ WARN does not need to be mutable //~| HELP remove this `mut` //~| WARN unnecessary parentheses @@ -72,6 +72,6 @@ fn main() { //~^ WARN this pattern is redundant //~| HELP remove this } - println!("{} {}", a, b); + println!("{} {}", registry_no, b); } } diff --git a/src/test/ui/lint/suggestions.stderr b/src/test/ui/lint/suggestions.stderr index 340a4a48512e2..73704614815be 100644 --- a/src/test/ui/lint/suggestions.stderr +++ b/src/test/ui/lint/suggestions.stderr @@ -1,8 +1,8 @@ warning: unnecessary parentheses around assigned value - --> $DIR/suggestions.rs:59:21 + --> $DIR/suggestions.rs:59:31 | -LL | let mut a = (1); - | ^^^ help: remove these parentheses +LL | let mut registry_no = (format!("NX-{}", 74205)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove these parentheses | note: lint level defined here --> $DIR/suggestions.rs:13:21 @@ -21,8 +21,8 @@ LL | #[no_debug] // should suggest removal of deprecated attribute warning: variable does not need to be mutable --> $DIR/suggestions.rs:59:13 | -LL | let mut a = (1); - | ----^ +LL | let mut registry_no = (format!("NX-{}", 74205)); + | ----^^^^^^^^^^^ | | | help: remove this `mut` |