From 6bb4aad51f40536447cd7603ab5be7792bab0a3d Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Sat, 12 May 2018 20:44:50 -0700 Subject: [PATCH 1/2] introducing `span_suggestion_short_with_applicability` Some would argue that this 40-character method name is ludicrously unwieldy (even ironic), but it's the unique continuation of the precedent set by the other suggestion methods. (And there is some hope that someday we'll just fold `Applicability` into the signature of the "basic" method `span_suggestion`.) This is in support of #50723. --- src/librustc_errors/diagnostic.rs | 17 +++++++++++++++++ src/librustc_errors/diagnostic_builder.rs | 6 ++++++ 2 files changed, 23 insertions(+) diff --git a/src/librustc_errors/diagnostic.rs b/src/librustc_errors/diagnostic.rs index 75401f21862b..bb2badaf293b 100644 --- a/src/librustc_errors/diagnostic.rs +++ b/src/librustc_errors/diagnostic.rs @@ -311,6 +311,23 @@ pub fn span_suggestions_with_applicability(&mut self, sp: Span, msg: &str, self } + pub fn span_suggestion_short_with_applicability( + &mut self, sp: Span, msg: &str, suggestion: String, applicability: Applicability + ) -> &mut Self { + self.suggestions.push(CodeSuggestion { + substitutions: vec![Substitution { + parts: vec![SubstitutionPart { + snippet: suggestion, + span: sp, + }], + }], + msg: msg.to_owned(), + show_code_when_inline: false, + applicability: applicability, + }); + self + } + pub fn set_span>(&mut self, sp: S) -> &mut Self { self.span = sp.into(); self diff --git a/src/librustc_errors/diagnostic_builder.rs b/src/librustc_errors/diagnostic_builder.rs index 7e9ca8633a53..41c3f7ce841e 100644 --- a/src/librustc_errors/diagnostic_builder.rs +++ b/src/librustc_errors/diagnostic_builder.rs @@ -200,6 +200,12 @@ pub fn span_label>(&mut self, span: Span, label: T) -> &mut Self suggestions: Vec, applicability: Applicability) -> &mut Self); + forward!(pub fn span_suggestion_short_with_applicability(&mut self, + sp: Span, + msg: &str, + suggestion: String, + applicability: Applicability) + -> &mut Self); forward!(pub fn set_span>(&mut self, sp: S) -> &mut Self); forward!(pub fn code(&mut self, s: DiagnosticId) -> &mut Self); From 98a04291e4db92ae86d7e3a20b5763cb926ebfbf Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Sat, 19 May 2018 14:52:24 -0700 Subject: [PATCH 2/2] suggestion applicabilities for libsyntax and librustc, run-rustfix tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consider this a down payment on #50723. To recap, an `Applicability` enum was recently (#50204) added, to convey to Rustfix and other tools whether we think it's OK for them to blindly apply the suggestion, or whether to prompt a human for guidance (because the suggestion might contain placeholders that we can't infer, or because we think it has a sufficiently high probability of being wrong even though it's— presumably—right often enough to be worth emitting in the first place). When a suggestion is marked as `MaybeIncorrect`, we try to use comments to indicate precisely why (although there are a few places where we just say `// speculative` because the present author's subjective judgement balked at the idea that the suggestion has no false positives). The `run-rustfix` directive is opporunistically set on some relevant UI tests (and a couple tests that were in the `test/ui/suggestions` directory, even if the suggestions didn't originate in librustc or libsyntax). This is less trivial than it sounds, because a surprising number of test files aren't equipped to be tested as fixed even when they contain successfully fixable errors, because, e.g., there are more, not-directly-related errors after fixing. Some test files need an attribute or underscore to avoid unused warnings tripping up the "fixed code is still producing diagnostics" check despite the fixes being correct; this is an interesting contrast-to/inconsistency-with the behavior of UI tests (which secretly pass `-A unused`), a behavior which we probably ought to resolve one way or the other (filed issue #50926). A few suggestion labels are reworded (e.g., to avoid phrasing it as a question, which which is discouraged by the style guidelines listed in `.span_suggestion`'s doc-comment). --- src/librustc/infer/error_reporting/mod.rs | 7 +- src/librustc/lint/levels.rs | 7 +- src/librustc/middle/liveness.rs | 13 +- src/librustc/traits/error_reporting.rs | 35 ++-- src/libsyntax/attr.rs | 20 +- src/libsyntax/ext/expand.rs | 8 +- src/libsyntax/parse/lexer/mod.rs | 13 +- src/libsyntax/parse/parser.rs | 192 +++++++++++++----- src/test/ui/extern-const.fixed | 25 +++ src/test/ui/extern-const.rs | 8 +- src/test/ui/extern-const.stderr | 4 +- src/test/ui/issue-42954.fixed | 24 +++ src/test/ui/issue-42954.rs | 4 + src/test/ui/issue-42954.stderr | 2 +- src/test/ui/issue-44406.stderr | 2 +- src/test/ui/issue-48636.fixed | 21 ++ src/test/ui/issue-48636.rs | 4 + src/test/ui/issue-48636.stderr | 2 +- ...-arg-count-expected-type-issue-47244.fixed | 30 +++ ...ure-arg-count-expected-type-issue-47244.rs | 11 +- ...arg-count-expected-type-issue-47244.stderr | 14 +- src/test/ui/repr-align-assign.fixed | 21 ++ src/test/ui/repr-align-assign.rs | 4 + src/test/ui/repr-align-assign.stderr | 4 +- .../issue-32354-suggest-import-rename.fixed | 26 +++ .../issue-32354-suggest-import-rename.rs | 4 + .../issue-32354-suggest-import-rename.stderr | 2 +- .../pub-ident-fn-or-struct-2.stderr | 2 +- .../suggestions/pub-ident-fn-or-struct.stderr | 2 +- src/test/ui/suggestions/pub-ident-fn.fixed | 18 ++ src/test/ui/suggestions/pub-ident-fn.rs | 4 +- src/test/ui/suggestions/pub-ident-fn.stderr | 6 +- ...ascription-instead-of-statement-end.stderr | 2 +- 33 files changed, 424 insertions(+), 117 deletions(-) create mode 100644 src/test/ui/extern-const.fixed create mode 100644 src/test/ui/issue-42954.fixed create mode 100644 src/test/ui/issue-48636.fixed create mode 100644 src/test/ui/mismatched_types/closure-arg-count-expected-type-issue-47244.fixed create mode 100644 src/test/ui/repr-align-assign.fixed create mode 100644 src/test/ui/suggestions/issue-32354-suggest-import-rename.fixed create mode 100644 src/test/ui/suggestions/pub-ident-fn.fixed diff --git a/src/librustc/infer/error_reporting/mod.rs b/src/librustc/infer/error_reporting/mod.rs index da93156b0b0b..4bde363672dc 100644 --- a/src/librustc/infer/error_reporting/mod.rs +++ b/src/librustc/infer/error_reporting/mod.rs @@ -70,7 +70,7 @@ use ty::error::TypeError; use syntax::ast::DUMMY_NODE_ID; use syntax_pos::{Pos, Span}; -use errors::{DiagnosticBuilder, DiagnosticStyledString}; +use errors::{Applicability, DiagnosticBuilder, DiagnosticStyledString}; use rustc_data_structures::indexed_vec::Idx; @@ -1097,7 +1097,10 @@ fn binding_suggestion<'tcx, S: fmt::Display>( if let Some((sp, has_lifetimes)) = type_param_span { let tail = if has_lifetimes { " + " } else { "" }; let suggestion = format!("{}: {}{}", bound_kind, sub, tail); - err.span_suggestion_short(sp, consider, suggestion); + err.span_suggestion_short_with_applicability( + sp, consider, suggestion, + Applicability::MaybeIncorrect // Issue #41966 + ); } else { err.help(consider); } diff --git a/src/librustc/lint/levels.rs b/src/librustc/lint/levels.rs index d158f52c643c..3393a2bf89d4 100644 --- a/src/librustc/lint/levels.rs +++ b/src/librustc/lint/levels.rs @@ -10,7 +10,7 @@ use std::cmp; -use errors::DiagnosticBuilder; +use errors::{Applicability, DiagnosticBuilder}; use hir::HirId; use ich::StableHashingContext; use lint::builtin; @@ -265,10 +265,11 @@ pub fn push(&mut self, attrs: &[ast::Attribute]) -> BuilderPush { store.check_lint_name(&name_lower) { db.emit(); } else { - db.span_suggestion( + db.span_suggestion_with_applicability( li.span, "lowercase the lint name", - name_lower + name_lower, + Applicability::MachineApplicable ).emit(); } } else { diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs index 3db8c746713f..0ac0fdd79cbb 100644 --- a/src/librustc/middle/liveness.rs +++ b/src/librustc/middle/liveness.rs @@ -109,6 +109,7 @@ use hir::def::*; use ty::{self, TyCtxt}; use lint; +use errors::Applicability; use util::nodemap::{NodeMap, NodeSet}; use std::collections::VecDeque; @@ -1541,11 +1542,15 @@ fn warn_about_unused(&self, let mut err = self.ir.tcx .struct_span_lint_node(lint::builtin::UNUSED_VARIABLES, id, sp, &msg); if self.ir.variable_is_shorthand(var) { - err.span_suggestion(sp, "try ignoring the field", - format!("{}: _", name)); + err.span_suggestion_with_applicability(sp, "try ignoring the field", + format!("{}: _", name), + Applicability::MachineApplicable); } else { - err.span_suggestion_short(sp, &suggest_underscore_msg, - format!("_{}", name)); + err.span_suggestion_short_with_applicability( + sp, &suggest_underscore_msg, + format!("_{}", name), + Applicability::MachineApplicable, + ); } err.emit() } diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index 97ce730c59ec..66eee3e7c1ac 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -27,7 +27,7 @@ Overflow, }; -use errors::DiagnosticBuilder; +use errors::{Applicability, DiagnosticBuilder}; use hir; use hir::def_id::DefId; use infer::{self, InferCtxt}; @@ -856,9 +856,12 @@ fn suggest_borrow_on_unsized_slice(&self, if let Some(ref expr) = local.init { if let hir::ExprIndex(_, _) = expr.node { if let Ok(snippet) = self.tcx.sess.codemap().span_to_snippet(expr.span) { - err.span_suggestion(expr.span, - "consider borrowing here", - format!("&{}", snippet)); + err.span_suggestion_with_applicability( + expr.span, + "consider borrowing here", + format!("&{}", snippet), + Applicability::MachineApplicable + ); } } } @@ -901,7 +904,9 @@ fn suggest_remove_reference(&self, let format_str = format!("consider removing {} leading `&`-references", remove_refs); - err.span_suggestion_short(sp, &format_str, String::from("")); + err.span_suggestion_short_with_applicability( + sp, &format_str, String::from(""), Applicability::MachineApplicable + ); break; } } else { @@ -1046,10 +1051,11 @@ pub fn report_arg_count_mismatch( let sugg = fields.iter() .map(|(name, _)| name.to_owned()) .collect::>().join(", "); - err.span_suggestion(found_span, - "change the closure to take multiple arguments instead of \ - a single tuple", - format!("|{}|", sugg)); + err.span_suggestion_with_applicability(found_span, + "change the closure to take multiple \ + arguments instead of a single tuple", + format!("|{}|", sugg), + Applicability::MachineApplicable); } } if let &[ArgKind::Tuple(_, ref fields)] = &expected_args[..] { @@ -1077,10 +1083,13 @@ pub fn report_arg_count_mismatch( "".to_owned() }, ); - err.span_suggestion(found_span, - "change the closure to accept a tuple instead of \ - individual arguments", - sugg); + err.span_suggestion_with_applicability( + found_span, + "change the closure to accept a tuple instead of \ + individual arguments", + sugg, + Applicability::MachineApplicable + ); } } } diff --git a/src/libsyntax/attr.rs b/src/libsyntax/attr.rs index fcda6ce9b164..076b6d176584 100644 --- a/src/libsyntax/attr.rs +++ b/src/libsyntax/attr.rs @@ -20,7 +20,7 @@ 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 "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 146db632c07b..0bee8e20009a 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -13,7 +13,7 @@ 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 @@ fn expand(&mut self, expansion: Expansion) -> Expansion { let trait_list = traits.iter() .map(|t| format!("{}", t)).collect::>(); 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 bbece1ee5e3d..b665f528cfc0 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}; @@ -1345,11 +1345,12 @@ fn next_token_inner(&mut self) -> Result { 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 3e9869494f93..1a7f897802aa 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -43,7 +43,7 @@ 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; @@ -1648,8 +1648,12 @@ fn maybe_report_ambiguous_plus(&mut self, allow_plus: bool, impl_dyn_multi: bool 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(); } } @@ -1679,7 +1683,12 @@ fn maybe_recover_from_bad_type_plus(&mut self, allow_plus: bool, ty: &Ty) -> PRe 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?"); @@ -1714,7 +1723,9 @@ fn maybe_recover_from_bad_qpath(&mut self, base: T, allow_recov 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) } @@ -2465,7 +2476,12 @@ fn parse_struct_expr(&mut self, lo: Span, pth: ast::Path, mut attrs: ThinVec, lo: Span) -> PResult<'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); @@ -2781,9 +2799,12 @@ pub fn parse_prefix_expr(&mut self, 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)) } @@ -2840,9 +2861,12 @@ pub fn parse_prefix_expr(&mut self, // 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) @@ -2937,9 +2961,12 @@ pub fn parse_assoc_expr_with(&mut self, 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); } @@ -3091,9 +3118,12 @@ fn parse_assoc_op_cast(&mut self, lhs: P, lhs_span: Span, 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) @@ -3301,7 +3331,11 @@ pub fn parse_for_expr(&mut self, opt_label: Option