Skip to content

Commit

Permalink
review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
estebank committed Oct 3, 2019
1 parent f1499a8 commit 02f57f8
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 41 deletions.
41 changes: 35 additions & 6 deletions src/librustc_errors/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,9 +298,13 @@ impl Diagnostic {
/// * may contain a name of a function, variable, or type, but not whole expressions
///
/// See `CodeSuggestion` for more information.
pub fn span_suggestion(&mut self, sp: Span, msg: &str,
suggestion: String,
applicability: Applicability) -> &mut Self {
pub fn span_suggestion(
&mut self,
sp: Span,
msg: &str,
suggestion: String,
applicability: Applicability,
) -> &mut Self {
self.suggestions.push(CodeSuggestion {
substitutions: vec![Substitution {
parts: vec![SubstitutionPart {
Expand All @@ -315,10 +319,35 @@ impl Diagnostic {
self
}

pub fn span_suggestion_verbose(
&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(),
style: SuggestionStyle::ShowAlways,
applicability,
});
self
}

/// Prints out a message with multiple suggested edits of the code.
pub fn span_suggestions(&mut self, sp: Span, msg: &str,
suggestions: impl Iterator<Item = String>, applicability: Applicability) -> &mut Self
{
pub fn span_suggestions(
&mut self,
sp: Span,
msg: &str,
suggestions: impl Iterator<Item = String>,
applicability: Applicability,
) -> &mut Self {
self.suggestions.push(CodeSuggestion {
substitutions: suggestions.map(|snippet| Substitution {
parts: vec![SubstitutionPart {
Expand Down
4 changes: 3 additions & 1 deletion src/librustc_errors/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,9 @@ pub trait Emitter {
// when this style is set we want the suggestion to be a message, not inline
sugg.style != SuggestionStyle::HideCodeAlways &&
// trivial suggestion for tooling's sake, never shown
sugg.style != SuggestionStyle::CompletelyHidden
sugg.style != SuggestionStyle::CompletelyHidden &&
// subtle suggestion, never shown inline
sugg.style != SuggestionStyle::ShowAlways
{
let substitution = &sugg.substitutions[0].parts[0].snippet.trim();
let msg = if substitution.len() == 0 || sugg.style.hide_inline() {
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_errors/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ pub enum SuggestionStyle {
/// This will *not* show the code if the suggestion is inline *and* the suggested code is
/// empty.
ShowCode,
/// Always show the suggested code independently.
ShowAlways,
}

impl SuggestionStyle {
Expand Down
57 changes: 32 additions & 25 deletions src/libsyntax/parse/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ use syntax_pos::{Span, DUMMY_SP, MultiSpan, SpanSnippetError};
use log::{debug, trace};
use std::mem;

const TURBOFISH: &'static str = "use the \"turbofish\" `::<...>` instead of `<...>` to specify \
type arguments";
const TURBOFISH: &'static str = "use `::<...>` instead of `<...>` to specify type arguments";
/// Creates a placeholder argument.
crate fn dummy_arg(ident: Ident) -> Param {
let pat = P(Pat {
Expand Down Expand Up @@ -585,7 +584,7 @@ impl<'a> Parser<'a> {
);

let suggest = |err: &mut DiagnosticBuilder<'_>| {
err.span_suggestion(
err.span_suggestion_verbose(
op_span.shrink_to_lo(),
TURBOFISH,
"::".to_string(),
Expand Down Expand Up @@ -647,29 +646,16 @@ impl<'a> Parser<'a> {
// We have high certainty that this was a bad turbofish at this point.
// `foo< bar >(`
suggest(&mut err);

let snapshot = self.clone();
self.bump(); // `(`

// Consume the fn call arguments.
let modifiers = [
(token::OpenDelim(token::Paren), 1),
(token::CloseDelim(token::Paren), -1),
];
self.consume_tts(1, &modifiers[..]);

if self.token.kind == token::Eof {
// Not entirely sure now, but we bubble the error up with the
// suggestion.
mem::replace(self, snapshot);
Err(err)
} else {
// 99% certain that the suggestion is correct, continue parsing.
err.emit();
// FIXME: actually check that the two expressions in the binop are
// paths and resynthesize new fn call expression instead of using
// `ExprKind::Err` placeholder.
mk_err_expr(self, lhs.span.to(self.prev_span))
match self.consume_fn_args() {
Err(()) => Err(err),
Ok(()) => {
err.emit();
// FIXME: actually check that the two expressions in the binop are
// paths and resynthesize new fn call expression instead of using
// `ExprKind::Err` placeholder.
mk_err_expr(self, lhs.span.to(self.prev_span))
}
}
} else {
// All we know is that this is `foo < bar >` and *nothing* else. Try to
Expand All @@ -687,6 +673,27 @@ impl<'a> Parser<'a> {
Ok(None)
}

fn consume_fn_args(&mut self) -> Result<(), ()> {
let snapshot = self.clone();
self.bump(); // `(`

// Consume the fn call arguments.
let modifiers = [
(token::OpenDelim(token::Paren), 1),
(token::CloseDelim(token::Paren), -1),
];
self.consume_tts(1, &modifiers[..]);

if self.token.kind == token::Eof {
// Not entirely sure that what we consumed were fn arguments, rollback.
mem::replace(self, snapshot);
Err(())
} else {
// 99% certain that the suggestion is correct, continue parsing.
Ok(())
}
}

crate fn maybe_report_ambiguous_plus(
&mut self,
allow_plus: bool,
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/did_you_mean/issue-40396.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ error: chained comparison operators require parentheses
|
LL | (0..13).collect<Vec<i32>>();
| ^^^^^
help: use the "turbofish" `::<...>` instead of `<...>` to specify type arguments
help: use `::<...>` instead of `<...>` to specify type arguments
|
LL | (0..13).collect::<Vec<i32>>();
| ^^
Expand All @@ -13,7 +13,7 @@ error: chained comparison operators require parentheses
|
LL | Vec<i32>::new();
| ^^^^^
help: use the "turbofish" `::<...>` instead of `<...>` to specify type arguments
help: use `::<...>` instead of `<...>` to specify type arguments
|
LL | Vec::<i32>::new();
| ^^
Expand All @@ -23,7 +23,7 @@ error: chained comparison operators require parentheses
|
LL | (0..13).collect<Vec<i32>();
| ^^^^^
help: use the "turbofish" `::<...>` instead of `<...>` to specify type arguments
help: use `::<...>` instead of `<...>` to specify type arguments
|
LL | (0..13).collect::<Vec<i32>();
| ^^
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/parser/require-parens-for-chained-comparison.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ fn main() {

f<X>();
//~^ ERROR chained comparison operators require parentheses
//~| HELP use the "turbofish" `::<...>` instead of `<...>` to specify type arguments
//~| HELP use `::<...>` instead of `<...>` to specify type arguments

f<Result<Option<X>, Option<Option<X>>>(1, 2);
//~^ ERROR chained comparison operators require parentheses
//~| HELP use the "turbofish" `::<...>` instead of `<...>` to specify type arguments
//~| HELP use `::<...>` instead of `<...>` to specify type arguments

use std::convert::identity;
let _ = identity<u8>;
//~^ ERROR chained comparison operators require parentheses
//~| HELP use the "turbofish" `::<...>` instead of `<...>` to specify type arguments
//~| HELP use `::<...>` instead of `<...>` to specify type arguments
//~| HELP or use `(...)` if you meant to specify fn arguments
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ error: chained comparison operators require parentheses
|
LL | f<X>();
| ^^^
help: use the "turbofish" `::<...>` instead of `<...>` to specify type arguments
help: use `::<...>` instead of `<...>` to specify type arguments
|
LL | f::<X>();
| ^^
Expand All @@ -25,7 +25,7 @@ error: chained comparison operators require parentheses
|
LL | f<Result<Option<X>, Option<Option<X>>>(1, 2);
| ^^^^^^^^
help: use the "turbofish" `::<...>` instead of `<...>` to specify type arguments
help: use `::<...>` instead of `<...>` to specify type arguments
|
LL | f::<Result<Option<X>, Option<Option<X>>>(1, 2);
| ^^
Expand All @@ -36,7 +36,7 @@ error: chained comparison operators require parentheses
LL | let _ = identity<u8>;
| ^^^^
|
= help: use the "turbofish" `::<...>` instead of `<...>` to specify type arguments
= help: use `::<...>` instead of `<...>` to specify type arguments
= help: or use `(...)` if you meant to specify fn arguments

error[E0308]: mismatched types
Expand Down

0 comments on commit 02f57f8

Please sign in to comment.