Skip to content

Commit

Permalink
Parser: Suggest Placing the Return Type After Function Parameters
Browse files Browse the repository at this point in the history
  • Loading branch information
veera-sivarajan committed Jul 5, 2024
1 parent 5834772 commit 86d6f13
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 19 deletions.
2 changes: 2 additions & 0 deletions compiler/rustc_parse/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,8 @@ parse_mismatched_closing_delimiter = mismatched closing delimiter: `{$delimiter}
.label_opening_candidate = closing delimiter possibly meant for this
.label_unclosed = unclosed delimiter
parse_misplaced_return_type = place the return type after the function parameters
parse_missing_comma_after_match_arm = expected `,` following `match` arm
.suggestion = missing a comma here to end this `match` arm
Expand Down
22 changes: 15 additions & 7 deletions compiler/rustc_parse/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1434,6 +1434,20 @@ pub(crate) struct FnPtrWithGenerics {
pub sugg: Option<FnPtrWithGenericsSugg>,
}

#[derive(Subdiagnostic)]
#[multipart_suggestion(
parse_misplaced_return_type,
style = "verbose",
applicability = "maybe-incorrect"
)]
pub(crate) struct MisplacedReturnType {
#[suggestion_part(code = " {snippet}")]
pub fn_params_end: Span,
pub snippet: String,
#[suggestion_part(code = "")]
pub ret_ty_span: Span,
}

#[derive(Subdiagnostic)]
#[multipart_suggestion(parse_suggestion, applicability = "maybe-incorrect")]
pub(crate) struct FnPtrWithGenericsSugg {
Expand All @@ -1448,7 +1462,6 @@ pub(crate) struct FnPtrWithGenericsSugg {

pub(crate) struct FnTraitMissingParen {
pub span: Span,
pub machine_applicable: bool,
}

impl Subdiagnostic for FnTraitMissingParen {
Expand All @@ -1458,16 +1471,11 @@ impl Subdiagnostic for FnTraitMissingParen {
_: &F,
) {
diag.span_label(self.span, crate::fluent_generated::parse_fn_trait_missing_paren);
let applicability = if self.machine_applicable {
Applicability::MachineApplicable
} else {
Applicability::MaybeIncorrect
};
diag.span_suggestion_short(
self.span.shrink_to_hi(),
crate::fluent_generated::parse_add_paren,
"()",
applicability,
Applicability::MachineApplicable,
);
}
}
Expand Down
73 changes: 61 additions & 12 deletions compiler/rustc_parse/src/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2332,10 +2332,20 @@ impl<'a> Parser<'a> {
}
}
};

// Store the end of function parameters to give better diagnostics
// inside `parse_fn_body()`.
let fn_params_end = self.prev_token.span.shrink_to_hi();

generics.where_clause = self.parse_where_clause()?; // `where T: Ord`

// `fn_params_end` is needed only when it's followed by a where clause.
let fn_params_end =
if generics.where_clause.has_where_token { Some(fn_params_end) } else { None };

let mut sig_hi = self.prev_token.span;
let body = self.parse_fn_body(attrs, &ident, &mut sig_hi, fn_parse_mode.req_body)?; // `;` or `{ ... }`.
let body =
self.parse_fn_body(attrs, &ident, &mut sig_hi, fn_parse_mode.req_body, fn_params_end)?; // `;` or `{ ... }`.
let fn_sig_span = sig_lo.to(sig_hi);
Ok((ident, FnSig { header, decl, span: fn_sig_span }, generics, body))
}
Expand All @@ -2349,6 +2359,7 @@ impl<'a> Parser<'a> {
ident: &Ident,
sig_hi: &mut Span,
req_body: bool,
fn_params_end: Option<Span>,
) -> PResult<'a, Option<P<Block>>> {
let has_semi = if req_body {
self.token.kind == TokenKind::Semi
Expand Down Expand Up @@ -2388,20 +2399,58 @@ impl<'a> Parser<'a> {
// the AST for typechecking.
err.span_label(ident.span, "while parsing this `fn`");
err.emit();
} else {
// check for typo'd Fn* trait bounds such as
// fn foo<F>() where F: FnOnce -> () {}
if self.token.kind == token::RArrow {
let machine_applicable = [sym::FnOnce, sym::FnMut, sym::Fn]
.into_iter()
.any(|s| self.prev_token.is_ident_named(s));

err.subdiagnostic(errors::FnTraitMissingParen {
span: self.prev_token.span,
machine_applicable,
} else if self.token.kind == token::RArrow
&& let Some(fn_params_end) = fn_params_end
{
// Instead of a function body, the parser has encountered a right arrow
// preceded by a where clause.

// Find whether token behind the right arrow is a function trait and
// store its span.
let prev_token_is_fn_trait = [sym::FnOnce, sym::FnMut, sym::Fn]
.into_iter()
.any(|s| self.prev_token.is_ident_named(s));
let fn_trait_span = self.prev_token.span;

// Parse the return type (along with the right arrow) and store its span.
// If there's a parse error, cancel it and return the existing error
// as we are primarily concerned with the
// expected-function-body-but-found-something-else error here.
let arrow_span = self.token.span;
let ty_span = match self.parse_ret_ty(
AllowPlus::Yes,
RecoverQPath::Yes,
RecoverReturnSign::Yes,
) {
Ok(ty_span) => ty_span.span().shrink_to_hi(),
Err(parse_error) => {
parse_error.cancel();
return Err(err);
}
};
let ret_ty_span = arrow_span.to(ty_span);

if prev_token_is_fn_trait {
// Typo'd Fn* trait bounds such as
// fn foo<F>() where F: FnOnce -> () {}
err.subdiagnostic(errors::FnTraitMissingParen { span: fn_trait_span });
} else if let Ok(snippet) = self.psess.source_map().span_to_snippet(ret_ty_span)
{
// If token behind right arrow is not a Fn* trait, the programmer
// probably misplaced the return type after the where clause like
// `fn foo<T>() where T: Default -> u8 {}`
err.primary_message(
"return type should be specified after the function parameters",
);
err.subdiagnostic(errors::MisplacedReturnType {
fn_params_end,
snippet,
ret_ty_span,
});
}
return Err(err);
} else {
return Err(err);
}
}
(AttrVec::new(), None)
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_parse/src/parser/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ use rustc_span::symbol::{kw, sym, Ident};
use rustc_span::{ErrorGuaranteed, Span, Symbol};
use thin_vec::{thin_vec, ThinVec};

/// Signals whether parsing a type should allow `+`.
///
/// For example, let T be the type `impl Default + 'static`
/// With `AllowPlus::Yes`, T will be parsed successfully
/// With `AllowPlus::No`, parsing T will return a parse error
#[derive(Copy, Clone, PartialEq)]
pub(super) enum AllowPlus {
Yes,
Expand Down

0 comments on commit 86d6f13

Please sign in to comment.