Auto merge of #50724 - zackmdavis:applicability_rush, r=Manishearth

add suggestion applicabilities to librustc and libsyntax

A down payment on #50723. Interested in feedback on whether my `MaybeIncorrect` vs. `MachineApplicable` judgement calls are well-calibrated (and that we have a consensus on what this means).

r? @Manishearth
cc @killercup @estebank

4 files changed, 166 insertions, 67 deletions

diff --git a/src/libsyntax/attr.rs b/src/libsyntax/attr.rs
index fcda6ce9b16..076b6d17658 100644
--- a/src/libsyntax/attr.rs
+++ b/src/libsyntax/attr.rs
@@ -20,7 +20,7 @@ use ast::{MetaItem, MetaItemKind, NestedMetaItem, NestedMetaItemKind};
 use ast::{Lit, LitKind, Expr, ExprKind, Item, Local, Stmt, StmtKind};
 use codemap::{BytePos, Spanned, respan, dummy_spanned};
 use syntax_pos::Span;
-use errors::Handler;
+use errors::{Applicability, Handler};
 use feature_gate::{Features, GatedCfg};
 use parse::lexer::comments::{doc_comment_style, strip_doc_comment_decoration};
 use parse::parser::Parser;
@@ -1067,14 +1067,20 @@ pub fn find_repr_attrs(diagnostic: &Handler, attr: &Attribute) -> Vec<ReprAttr>
                                     "incorrect `repr(align)` attribute format");
                                 match value.node {
                                     ast::LitKind::Int(int, ast::LitIntType::Unsuffixed) => {
-                                        err.span_suggestion(item.span,
-                                                            "use parentheses instead",
-                                                            format!("align({})", int));
+                                        err.span_suggestion_with_applicability(
+                                            item.span,
+                                            "use parentheses instead",
+                                            format!("align({})", int),
+                                            Applicability::MachineApplicable
+                                        );
                                     }
                                     ast::LitKind::Str(s, _) => {
-                                        err.span_suggestion(item.span,
-                                                            "use parentheses instead",
-                                                            format!("align({})", s));
+                                        err.span_suggestion_with_applicability(
+                                            item.span,
+                                            "use parentheses instead",
+                                            format!("align({})", s),
+                                            Applicability::MachineApplicable
+                                        );
                                     }
                                     _ => {}
                                 }
diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs
index 667105a534a..83e7dd84cbf 100644
--- a/src/libsyntax/ext/expand.rs
+++ b/src/libsyntax/ext/expand.rs
@@ -13,7 +13,7 @@ use ast::{MacStmtStyle, StmtKind, ItemKind};
 use attr::{self, HasAttrs};
 use codemap::{ExpnInfo, NameAndSpan, MacroBang, MacroAttribute, dummy_spanned, respan};
 use config::{is_test_or_bench, StripUnconfigured};
-use errors::FatalError;
+use errors::{Applicability, FatalError};
 use ext::base::*;
 use ext::derive::{add_derived_markers, collect_derives};
 use ext::hygiene::{self, Mark, SyntaxContext};
@@ -331,7 +331,11 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
                             let trait_list = traits.iter()
                                 .map(|t| format!("{}", t)).collect::<Vec<_>>();
                             let suggestion = format!("#[derive({})]", trait_list.join(", "));
-                            err.span_suggestion(span, "try an outer attribute", suggestion);
+                            err.span_suggestion_with_applicability(
+                                span, "try an outer attribute", suggestion,
+                                // We don't 𝑘𝑛𝑜𝑤 that the following item is an ADT
+                                Applicability::MaybeIncorrect
+                            );
                         }
                         err.emit();
                     }
diff --git a/src/libsyntax/parse/lexer/mod.rs b/src/libsyntax/parse/lexer/mod.rs
index c39eb1594b2..c396fb0fbc0 100644
--- a/src/libsyntax/parse/lexer/mod.rs
+++ b/src/libsyntax/parse/lexer/mod.rs
@@ -11,7 +11,7 @@
 use ast::{self, Ident};
 use syntax_pos::{self, BytePos, CharPos, Pos, Span, NO_EXPANSION};
 use codemap::{CodeMap, FilePathMapping};
-use errors::{FatalError, DiagnosticBuilder};
+use errors::{Applicability, FatalError, DiagnosticBuilder};
 use parse::{token, ParseSess};
 use str::char_at;
 use symbol::{Symbol, keywords};
@@ -1379,11 +1379,12 @@ impl<'a> StringReader<'a> {
                             self.sess.span_diagnostic
                                 .struct_span_err(span,
                                                  "character literal may only contain one codepoint")
-                                .span_suggestion(span,
-                                                 "if you meant to write a `str` literal, \
-                                                  use double quotes",
-                                                 format!("\"{}\"", &self.src[start..end]))
-                                .emit();
+                                .span_suggestion_with_applicability(
+                                    span,
+                                    "if you meant to write a `str` literal, use double quotes",
+                                    format!("\"{}\"", &self.src[start..end]),
+                                    Applicability::MachineApplicable
+                                ).emit();
                             return Ok(token::Literal(token::Str_(Symbol::intern("??")), None))
                         }
                         if self.ch_is('\n') || self.is_eof() || self.ch_is('/') {
diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs
index 716faaa4e39..28f93328e95 100644
--- a/src/libsyntax/parse/parser.rs
+++ b/src/libsyntax/parse/parser.rs
@@ -43,7 +43,7 @@ use ast::{RangeEnd, RangeSyntax};
 use {ast, attr};
 use codemap::{self, CodeMap, Spanned, respan};
 use syntax_pos::{self, Span, MultiSpan, BytePos, FileName, DUMMY_SP};
-use errors::{self, DiagnosticBuilder};
+use errors::{self, Applicability, DiagnosticBuilder};
 use parse::{self, classify, token};
 use parse::common::SeqSep;
 use parse::lexer::TokenAndSpan;
@@ -1655,8 +1655,12 @@ impl<'a> Parser<'a> {
         if !allow_plus && impl_dyn_multi {
             let sum_with_parens = format!("({})", pprust::ty_to_string(&ty));
             self.struct_span_err(ty.span, "ambiguous `+` in a type")
-                .span_suggestion(ty.span, "use parentheses to disambiguate", sum_with_parens)
-                .emit();
+                .span_suggestion_with_applicability(
+                    ty.span,
+                    "use parentheses to disambiguate",
+                    sum_with_parens,
+                    Applicability::MachineApplicable
+                ).emit();
         }
     }
 
@@ -1686,7 +1690,12 @@ impl<'a> Parser<'a> {
                     s.print_bounds(" +", &bounds)?;
                     s.pclose()
                 });
-                err.span_suggestion(sum_span, "try adding parentheses", sum_with_parens);
+                err.span_suggestion_with_applicability(
+                    sum_span,
+                    "try adding parentheses",
+                    sum_with_parens,
+                    Applicability::MachineApplicable
+                );
             }
             TyKind::Ptr(..) | TyKind::BareFn(..) => {
                 err.span_label(sum_span, "perhaps you forgot parentheses?");
@@ -1724,7 +1733,9 @@ impl<'a> Parser<'a> {
 
         self.diagnostic()
             .struct_span_err(span, "missing angle brackets in associated item path")
-            .span_suggestion(span, "try", recovered.to_string()).emit();
+            .span_suggestion_with_applicability( // this is a best-effort recovery
+                span, "try", recovered.to_string(), Applicability::MaybeIncorrect
+            ).emit();
 
         Ok(recovered)
     }
@@ -2498,7 +2509,12 @@ impl<'a> Parser<'a> {
                         exp_span.to(self.prev_span),
                         "cannot use a comma after the base struct",
                     );
-                    err.span_suggestion_short(self.span, "remove this comma", "".to_owned());
+                    err.span_suggestion_short_with_applicability(
+                        self.span,
+                        "remove this comma",
+                        "".to_owned(),
+                        Applicability::MachineApplicable
+                    );
                     err.note("the base struct must always be the last field");
                     err.emit();
                     self.recover_stmt();
@@ -2671,10 +2687,12 @@ impl<'a> Parser<'a> {
                             s.s.word(".")?;
                             s.s.word(fstr.splitn(2, ".").last().unwrap())
                         });
-                        err.span_suggestion(
+                        err.span_suggestion_with_applicability(
                             lo.to(self.prev_span),
                             "try parenthesizing the first index",
-                            sugg);
+                            sugg,
+                            Applicability::MachineApplicable
+                        );
                     }
                     return Err(err);
 
@@ -2814,9 +2832,12 @@ impl<'a> Parser<'a> {
                 let span_of_tilde = lo;
                 let mut err = self.diagnostic().struct_span_err(span_of_tilde,
                         "`~` cannot be used as a unary operator");
-                err.span_suggestion_short(span_of_tilde,
-                                          "use `!` to perform bitwise negation",
-                                          "!".to_owned());
+                err.span_suggestion_short_with_applicability(
+                    span_of_tilde,
+                    "use `!` to perform bitwise negation",
+                    "!".to_owned(),
+                    Applicability::MachineApplicable
+                );
                 err.emit();
                 (lo.to(span), self.mk_unary(UnOp::Not, e))
             }
@@ -2884,9 +2905,12 @@ impl<'a> Parser<'a> {
                     // trailing whitespace after the `!` in our suggestion
                     let to_replace = self.sess.codemap()
                         .span_until_non_whitespace(lo.to(self.span));
-                    err.span_suggestion_short(to_replace,
-                                              "use `!` to perform logical negation",
-                                              "!".to_owned());
+                    err.span_suggestion_short_with_applicability(
+                        to_replace,
+                        "use `!` to perform logical negation",
+                        "!".to_owned(),
+                        Applicability::MachineApplicable
+                    );
                     err.emit();
                     // —and recover! (just as if we were in the block
                     // for the `token::Not` arm)
@@ -2981,9 +3005,12 @@ impl<'a> Parser<'a> {
                         let cur_pos = cm.lookup_char_pos(self.span.lo());
                         let op_pos = cm.lookup_char_pos(cur_op_span.hi());
                         if cur_pos.line != op_pos.line {
-                            err.span_suggestion_short(cur_op_span,
-                                                      "did you mean to use `;` here?",
-                                                      ";".to_string());
+                            err.span_suggestion_with_applicability(
+                                cur_op_span,
+                                "try using a semicolon",
+                                ";".to_string(),
+                                Applicability::MaybeIncorrect // speculative
+                            );
                         }
                         return Err(err);
                     }
@@ -3137,9 +3164,12 @@ impl<'a> Parser<'a> {
 
                         let expr_str = self.sess.codemap().span_to_snippet(expr.span)
                                                 .unwrap_or(pprust::expr_to_string(&expr));
-                        err.span_suggestion(expr.span,
-                                            &format!("try {} the cast value", op_verb),
-                                            format!("({})", expr_str));
+                        err.span_suggestion_with_applicability(
+                            expr.span,
+                            &format!("try {} the cast value", op_verb),
+                            format!("({})", expr_str),
+                            Applicability::MachineApplicable
+                        );
                         err.emit();
 
                         Ok(expr)
@@ -3347,7 +3377,11 @@ impl<'a> Parser<'a> {
             let in_span = self.prev_span.between(self.span);
             let mut err = self.sess.span_diagnostic
                 .struct_span_err(in_span, "missing `in` in `for` loop");
-            err.span_suggestion_short(in_span, "try adding `in` here", " in ".into());
+            err.span_suggestion_short_with_applicability(
+                in_span, "try adding `in` here", " in ".into(),
+                // has been misleading, at least in the past (closed Issue #48492)
+                Applicability::MaybeIncorrect
+            );
             err.emit();
         }
         let expr = self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, None)?;
@@ -3413,7 +3447,12 @@ impl<'a> Parser<'a> {
                                                None)?;
         if let Err(mut e) = self.expect(&token::OpenDelim(token::Brace)) {
             if self.token == token::Token::Semi {
-                e.span_suggestion_short(match_span, "try removing this `match`", "".to_owned());
+                e.span_suggestion_short_with_applicability(
+                    match_span,
+                    "try removing this `match`",
+                    "".to_owned(),
+                    Applicability::MaybeIncorrect // speculative
+                );
             }
             return Err(e)
         }
@@ -3485,10 +3524,11 @@ impl<'a> Parser<'a> {
                             //   |      - ^^ self.span
                             //   |      |
                             //   |      parsed until here as `"y" & X`
-                            err.span_suggestion_short(
+                            err.span_suggestion_short_with_applicability(
                                 cm.next_point(arm_start_span),
                                 "missing a comma here to end this `match` arm",
-                                ",".to_owned()
+                                ",".to_owned(),
+                                Applicability::MachineApplicable
                             );
                         }
                         _ => {
@@ -3557,9 +3597,12 @@ impl<'a> Parser<'a> {
             if self.token == token::OrOr {
                 let mut err = self.struct_span_err(self.span,
                                                    "unexpected token `||` after pattern");
-                err.span_suggestion(self.span,
-                                    "use a single `|` to specify multiple patterns",
-                                    "|".to_owned());
+                err.span_suggestion_with_applicability(
+                    self.span,
+                    "use a single `|` to specify multiple patterns",
+                    "|".to_owned(),
+                    Applicability::MachineApplicable
+                );
                 err.emit();
                 self.bump();
             } else if self.check(&token::BinOp(token::Or)) {
@@ -3689,9 +3732,12 @@ impl<'a> Parser<'a> {
                 if self.token == token::DotDotDot { // Issue #46718
                     let mut err = self.struct_span_err(self.span,
                                                        "expected field pattern, found `...`");
-                    err.span_suggestion(self.span,
-                                        "to omit remaining fields, use one fewer `.`",
-                                        "..".to_owned());
+                    err.span_suggestion_with_applicability(
+                        self.span,
+                        "to omit remaining fields, use one fewer `.`",
+                        "..".to_owned(),
+                        Applicability::MachineApplicable
+                    );
                     err.emit();
                 }
 
@@ -3822,8 +3868,12 @@ impl<'a> Parser<'a> {
             let mut err = self.struct_span_err(comma_span,
                                                "unexpected `,` in pattern");
             if let Ok(seq_snippet) = self.sess.codemap().span_to_snippet(seq_span) {
-                err.span_suggestion(seq_span, "try adding parentheses",
-                                    format!("({})", seq_snippet));
+                err.span_suggestion_with_applicability(
+                    seq_span,
+                    "try adding parentheses",
+                    format!("({})", seq_snippet),
+                    Applicability::MachineApplicable
+                );
             }
             return Err(err);
         }
@@ -3882,8 +3932,12 @@ impl<'a> Parser<'a> {
                 let binding_mode = if self.eat_keyword(keywords::Ref) {
                     self.diagnostic()
                         .struct_span_err(mutref_span, "the order of `mut` and `ref` is incorrect")
-                        .span_suggestion(mutref_span, "try switching the order", "ref mut".into())
-                        .emit();
+                        .span_suggestion_with_applicability(
+                            mutref_span,
+                            "try switching the order",
+                            "ref mut".into(),
+                            Applicability::MachineApplicable
+                        ).emit();
                     BindingMode::ByRef(Mutability::Mutable)
                 } else {
                     BindingMode::ByValue(Mutability::Mutable)
@@ -4008,10 +4062,12 @@ impl<'a> Parser<'a> {
                         pat.span,
                         "the range pattern here has ambiguous interpretation",
                     );
-                    err.span_suggestion(
+                    err.span_suggestion_with_applicability(
                         pat.span,
                         "add parentheses to clarify the precedence",
                         format!("({})", pprust::pat_to_string(&pat)),
+                        // "ambiguous interpretation" implies that we have to be guessing
+                        Applicability::MaybeIncorrect
                     );
                     return Err(err);
                 }
@@ -4082,9 +4138,12 @@ impl<'a> Parser<'a> {
             (Ok(init), Some((_, colon_sp, mut err))) => {  // init parsed, ty error
                 // Could parse the type as if it were the initializer, it is likely there was a
                 // typo in the code: `:` instead of `=`. Add suggestion and emit the error.
-                err.span_suggestion_short(colon_sp,
-                                          "use `=` if you meant to assign",
-                                          "=".to_string());
+                err.span_suggestion_short_with_applicability(
+                    colon_sp,
+                    "use `=` if you meant to assign",
+                    "=".to_string(),
+                    Applicability::MachineApplicable
+                );
                 err.emit();
                 // As this was parsed successfully, continue as if the code has been fixed for the
                 // rest of the file. It will still fail due to the emitted error, but we avoid
@@ -4572,7 +4631,13 @@ impl<'a> Parser<'a> {
                         s.print_stmt(&stmt)?;
                         s.bclose_maybe_open(stmt.span, INDENT_UNIT, false)
                     });
-                    e.span_suggestion(stmt_span, "try placing this code inside a block", sugg);
+                    e.span_suggestion_with_applicability(
+                        stmt_span,
+                        "try placing this code inside a block",
+                        sugg,
+                        // speculative, has been misleading in the past (closed Issue #46836)
+                        Applicability::MaybeIncorrect
+                    );
                 }
                 Err(mut e) => {
                     self.recover_stmt_(SemiColonMode::Break, BlockMode::Ignore);
@@ -5416,9 +5481,12 @@ impl<'a> Parser<'a> {
                 if is_macro_rules {
                     let mut err = self.diagnostic()
                         .struct_span_err(sp, "can't qualify macro_rules invocation with `pub`");
-                    err.span_suggestion(sp,
-                                        "try exporting the macro",
-                                        "#[macro_export]".to_owned());
+                    err.span_suggestion_with_applicability(
+                        sp,
+                        "try exporting the macro",
+                        "#[macro_export]".to_owned(),
+                        Applicability::MaybeIncorrect // speculative
+                    );
                     Err(err)
                 } else {
                     let mut err = self.diagnostic()
@@ -5839,7 +5907,12 @@ impl<'a> Parser<'a> {
                 } else {
                     if seen_comma == false {
                         let sp = self.sess.codemap().next_point(previous_span);
-                        err.span_suggestion(sp, "missing comma here", ",".into());
+                        err.span_suggestion_with_applicability(
+                            sp,
+                            "missing comma here",
+                            ",".into(),
+                            Applicability::MachineApplicable
+                        );
                     }
                     return Err(err);
                 }
@@ -5938,7 +6011,9 @@ impl<'a> Parser<'a> {
                 let help_msg = format!("make this visible only to module `{}` with `in`", path);
                 self.expect(&token::CloseDelim(token::Paren))?;  // `)`
                 let mut err = self.span_fatal_help(path_span, msg, suggestion);
-                err.span_suggestion(path_span, &help_msg, format!("in {}", path));
+                err.span_suggestion_with_applicability(
+                    path_span, &help_msg, format!("in {}", path), Applicability::MachineApplicable
+                );
                 err.emit();  // emit diagnostic, but continue with public visibility
             }
         }
@@ -5976,7 +6051,9 @@ impl<'a> Parser<'a> {
             let mut err = self.fatal(&format!("expected item, found `{}`", token_str));
             if token_str == ";" {
                 let msg = "consider removing this semicolon";
-                err.span_suggestion_short(self.span, msg, "".to_string());
+                err.span_suggestion_short_with_applicability(
+                    self.span, msg, "".to_string(), Applicability::MachineApplicable
+                );
             } else {
                 err.span_label(self.span, "expected item");
             }
@@ -6793,7 +6870,9 @@ impl<'a> Parser<'a> {
                                   ident);
                 let mut err = self.diagnostic()
                     .struct_span_err(sp, "missing `struct` for struct definition");
-                err.span_suggestion_short(sp, &msg, " struct ".into());
+                err.span_suggestion_short_with_applicability(
+                    sp, &msg, " struct ".into(), Applicability::MaybeIncorrect // speculative
+                );
                 return Err(err);
             } else if self.look_ahead(1, |t| *t == token::OpenDelim(token::Paren)) {
                 let ident = self.parse_ident().unwrap();
@@ -6816,13 +6895,18 @@ impl<'a> Parser<'a> {
                                              kw,
                                              ident,
                                              kw_name);
-                    err.span_suggestion_short(sp, &suggestion, format!(" {} ", kw));
+                    err.span_suggestion_short_with_applicability(
+                        sp, &suggestion, format!(" {} ", kw), Applicability::MachineApplicable
+                    );
                 } else {
                     if let Ok(snippet) = self.sess.codemap().span_to_snippet(ident_sp) {
-                        err.span_suggestion(
+                        err.span_suggestion_with_applicability(
                             full_sp,
-                            "if you meant to call a macro, write instead",
-                            format!("{}!", snippet));
+                            "if you meant to call a macro, try",
+                            format!("{}!", snippet),
+                            // this is the `ambiguous` conditional branch
+                            Applicability::MaybeIncorrect
+                        );
                     } else {
                         err.help("if you meant to call a macro, remove the `pub` \
                                   and add a trailing `!` after the identifier");
@@ -6848,8 +6932,12 @@ impl<'a> Parser<'a> {
             if self.token.is_keyword(keywords::Const) {
                 self.diagnostic()
                     .struct_span_err(self.span, "extern items cannot be `const`")
-                    .span_suggestion(self.span, "instead try using", "static".to_owned())
-                    .emit();
+                    .span_suggestion_with_applicability(
+                        self.span,
+                        "try using a static value",
+                        "static".to_owned(),
+                        Applicability::MachineApplicable
+                    ).emit();
             }
             self.bump(); // `static` or `const`
             return Ok(Some(self.parse_item_foreign_static(visibility, lo, attrs)?));