Tweak diagnostic for use suggestion to blank text surrounding span.

Our suggestion system is oriented around diff's to code, and in that
context, if your spans are perfect, everything works out. But our
spans are not always perfect, due to macros.

In the specific case of issue 87613, the #[tokio::main] attribute
macro works by rewriting an `async fn main` into an `fn main`, by just
removing the `async` token.  The problem is that the `async` text
remains in the source code, and the span of the code rewritten by
suggestion system happily transcribes all the text on the line of that
`fn main` when generating a `use` suggestion, yielding the absurd
looking `async use std::process::Command;` suggestion.

This patch works around this whole problem by adding a way for
suggestions to indicate that their transcriptions should not include
the original source code; just its *whitespace*. This leads to a happy
place where the suggested line numbers are correct and the indentation
is usually also correct, while avoiding the `async use` output we
would see before.

Author: pnkfelix

compiler/rustc_errors/src/diagnostic.rs

@@ -350,6 +350,7 @@ impl Diagnostic {
             msg: msg.to_owned(),
             style,
             applicability,
+            transcription: Default::default(),
             tool_metadata: Default::default(),
         });
         self
@@ -378,6 +379,7 @@ impl Diagnostic {
             msg: msg.to_owned(),
             style: SuggestionStyle::CompletelyHidden,
             applicability,
+            transcription: Default::default(),
             tool_metadata: Default::default(),
         });
         self
@@ -433,6 +435,7 @@ impl Diagnostic {
             msg: msg.to_owned(),
             style,
             applicability,
+            transcription: Default::default(),
             tool_metadata: Default::default(),
         });
         self
@@ -476,6 +479,7 @@ impl Diagnostic {
             msg: msg.to_owned(),
             style: SuggestionStyle::ShowCode,
             applicability,
+            transcription: Default::default(),
             tool_metadata: Default::default(),
         });
         self
@@ -501,6 +505,7 @@ impl Diagnostic {
             msg: msg.to_owned(),
             style: SuggestionStyle::ShowCode,
             applicability,
+            transcription: Default::default(),
             tool_metadata: Default::default(),
         });
         self
@@ -583,6 +588,7 @@ impl Diagnostic {
             msg: msg.to_owned(),
             style: SuggestionStyle::CompletelyHidden,
             applicability,
+            transcription: Default::default(),
             tool_metadata: ToolMetadata::new(tool_metadata),
         })
     }

compiler/rustc_errors/src/lib.rs

@@ -31,7 +31,7 @@ pub use rustc_lint_defs::{pluralize, Applicability};
 use rustc_serialize::json::Json;
 use rustc_serialize::{Decodable, Decoder, Encodable, Encoder};
 use rustc_span::source_map::SourceMap;
-use rustc_span::{Loc, MultiSpan, Span};
+use rustc_span::{Loc, MultiSpan, SourceFile, Span};
 
 use std::borrow::Cow;
 use std::hash::{Hash, Hasher};
@@ -142,6 +142,10 @@ pub struct CodeSuggestion {
     pub msg: String,
     /// Visual representation of this suggestion.
     pub style: SuggestionStyle,
+    /// Role of context of spans. Usually the context around every
+    /// span should be included in the output. But sometimes the
+    /// suggestion stands entirely on its own.
+    pub transcription: Transcription,
     /// Whether or not the suggestion is approximate
     ///
     /// Sometimes we may show suggestions with placeholders,
@@ -152,6 +156,24 @@ pub struct CodeSuggestion {
     pub tool_metadata: ToolMetadata,
 }
 
+#[derive(Clone, Debug, PartialEq, Hash, Encodable, Decodable)]
+pub enum Transcription {
+    /// The characters in the line(s) around each span should be
+    /// included in the output.
+    Copy,
+
+    /// The span is solely providing the indentation and lexical
+    /// context; the source code contents of the surrounding code is
+    /// irrelevant and should not be included in the suggestion.
+    Blank,
+}
+
+impl Default for Transcription {
+    fn default() -> Self {
+        Transcription::Copy
+    }
+}
+
 #[derive(Clone, Debug, PartialEq, Hash, Encodable, Decodable)]
 /// See the docs on `CodeSuggestion::substitutions`
 pub struct Substitution {
@@ -193,6 +215,19 @@ impl SubstitutionPart {
 }
 
 impl CodeSuggestion {
+    fn get_line<'a>(&self, sf: &'a SourceFile, line_number: usize) -> Option<Cow<'a, str>> {
+        self.transcribe(sf.get_line(line_number))
+    }
+
+    fn transcribe<'a>(&self, text: Option<Cow<'a, str>>) -> Option<Cow<'a, str>> {
+        match self.transcription {
+            Transcription::Copy => text,
+            Transcription::Blank => text.map(|s| {
+                s.chars().map(|c| if c.is_whitespace() { c } else { ' ' }).collect()
+            }),
+        }
+    }
+
     /// Returns the assembled code suggestions, whether they should be shown with an underline
     /// and whether the substitution only differs in capitalization.
     pub fn splice_lines(
@@ -283,7 +318,7 @@ impl CodeSuggestion {
                 let mut prev_hi = sm.lookup_char_pos(bounding_span.lo());
                 prev_hi.col = CharPos::from_usize(0);
                 let mut prev_line =
-                    lines.lines.get(0).and_then(|line0| sf.get_line(line0.line_index));
+                    lines.lines.get(0).and_then(|line0| self.get_line(sf, line0.line_index));
                 let mut buf = String::new();
 
                 let mut line_highlight = vec![];
@@ -310,13 +345,13 @@ impl CodeSuggestion {
                         }
                         // push lines between the previous and current span (if any)
                         for idx in prev_hi.line..(cur_lo.line - 1) {
-                            if let Some(line) = sf.get_line(idx) {
+                            if let Some(line) = self.get_line(sf, idx) {
                                 buf.push_str(line.as_ref());
                                 buf.push('\n');
                                 highlights.push(std::mem::take(&mut line_highlight));
                             }
                         }
-                        if let Some(cur_line) = sf.get_line(cur_lo.line - 1) {
+                        if let Some(cur_line) = self.get_line(sf, cur_lo.line - 1) {
                             let end = match cur_line.char_indices().nth(cur_lo.col.to_usize()) {
                                 Some((i, _)) => i,
                                 None => cur_line.len(),
@@ -349,7 +384,7 @@ impl CodeSuggestion {
                         acc += len as isize - (cur_hi.col.0 - cur_lo.col.0) as isize;
                     }
                     prev_hi = cur_hi;
-                    prev_line = sf.get_line(prev_hi.line - 1);
+                    prev_line = self.get_line(sf, prev_hi.line - 1);
                     for line in part.snippet.split('\n').skip(1) {
                         acc = 0;
                         highlights.push(std::mem::take(&mut line_highlight));

compiler/rustc_resolve/src/diagnostics.rs

@@ -4,7 +4,7 @@ use std::ptr;
 use rustc_ast::{self as ast, Path};
 use rustc_ast_pretty::pprust;
 use rustc_data_structures::fx::FxHashSet;
-use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder};
+use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder, Transcription};
 use rustc_feature::BUILTIN_ATTRIBUTES;
 use rustc_hir::def::Namespace::{self, *};
 use rustc_hir::def::{self, CtorKind, CtorOf, DefKind, NonMacroAttrKind};
@@ -1843,6 +1843,11 @@ crate fn show_candidates(
                 accessible_path_strings.into_iter().map(|a| a.0),
                 Applicability::Unspecified,
             );
+            let last_idx = err.suggestions.len() - 1;
+            // rust#87613: for `use` suggestions, transcribed source
+            // code can yield incorrect suggestions. Instead, use
+            // source span solely to establish line number and indent.
+            err.suggestions[last_idx].transcription = Transcription::Blank;
         } else {
             msg.push(':');
 

src/test/ui/proc-macro/amputate-span.rs

@@ -0,0 +1,63 @@
+// aux-build:amputate-span.rs
+// edition:2018
+// compile-flags: --extern amputate_span
+
+// This test has been crafted to ensure the following things:
+//
+// 1. There's a resolution error that prompts the compiler to suggest
+//    adding a `use` item.
+//
+// 2. There are no `use` or `extern crate` items in the source
+//    code. In fact, there is only one item, the `fn main`
+//    declaration.
+//
+// 3. The single `fn main` declaration has an attribute attached to it
+//    that just deletes the first token from the given item.
+//
+// You need all of these conditions to hold in order to replicate the
+// scenario that yielded issue 87613, where the compiler's suggestion
+// looks like:
+//
+// ```
+// help: consider importing this struct
+//    |
+// 47 | hey */ async use std::process::Command;
+//    |              ++++++++++++++++++++++++++
+// ```
+//
+// The first condition is necessary to force the compiler issue a
+// suggestion. The second condition is necessary to force the
+// suggestion to be issued at a span associated with the sole
+// `fn`-item of this crate. The third condition is necessary in order
+// to yield the weird state where the associated span of the `fn`-item
+// does not actually cover all of the original source code of the
+// `fn`-item (which is why we are calling it an "amputated" span
+// here).
+//
+// Note that satisfying conditions 2 and 3 requires the use of the
+// `--extern` compile flag.
+//
+// You might ask yourself: What code would do such a thing?  The
+// answer is: the #[tokio::main] attribute does *exactly* this (as
+// well as injecting some other code into the `fn main` that it
+// constructs).
+
+#[amputate_span::drop_first_token]
+/* what the
+hey */ async fn main() {
+    Command::new("git"); //~ ERROR [E0433]
+}
+
+// (The /* ... */ comment in the above is not part of the original
+// bug. It is just meant to illustrate one particular facet of the
+// original non-ideal behavior, where we were transcribing the
+// trailing comment as part of the emitted suggestion, for better or
+// for worse.)
+
+mod inner {
+    #[amputate_span::drop_first_token]
+    /* another interesting
+    case */ async fn foo() {
+        Command::new("git"); //~ ERROR [E0433]
+    }
+}

src/test/ui/proc-macro/amputate-span.stderr

@@ -0,0 +1,25 @@
+error[E0433]: failed to resolve: use of undeclared type `Command`
+  --> $DIR/amputate-span.rs:48:5
+   |
+LL |     Command::new("git");
+   |     ^^^^^^^ not found in this scope
+   |
+help: consider importing this struct
+   |
+LL |              use std::process::Command;
+   |
+
+error[E0433]: failed to resolve: use of undeclared type `Command`
+  --> $DIR/amputate-span.rs:61:2
+   |
+LL |     Command::new("git");
+   |     ^^^^^^^ not found in this scope
+   |
+help: consider importing this struct
+   |
+LL |                   use std::process::Command;
+   |
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0433`.

src/test/ui/proc-macro/auxiliary/amputate-span.rs

@@ -0,0 +1,14 @@
+// force-host
+// no-prefer-dynamic
+
+#![crate_type = "proc-macro"]
+
+extern crate proc_macro;
+
+use proc_macro::TokenStream;
+
+#[proc_macro_attribute]
+pub fn drop_first_token(attr: TokenStream, input: TokenStream) -> TokenStream {
+    assert!(attr.is_empty());
+    input.into_iter().skip(1).collect()
+}