mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Rollup merge of #127350 - veera-sivarajan:bugfix-126311, r=lcnr

Browse Source 
Parser: Suggest Placing the Return Type After Function Parameters

Fixes #126311

This PR suggests placing the return type after the function parameters when it's misplaced after a `where` clause.

This also tangentially improves diagnostics for cases like [this](86d6f1312a/tests/ui/parser/issues/misplaced-return-type-without-where-issue-126311.rs (L1C1-L1C28)) and adds doc comments for `parser::AllowPlus`.
This commit is contained in:
Matthias Krüger 2024-07-19 10:48:03 +02:00 committed by GitHub
commit c86e13f330
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
16 changed files with 215 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
compiler/rustc_parse/messages.ftl
View File

@ -524,6 +524,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

22
compiler/rustc_parse/src/errors.rs
View File

@ -1502,6 +1502,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 {
@ -1516,7 +1530,6 @@ pub(crate) struct FnPtrWithGenericsSugg {
pub(crate) struct FnTraitMissingParen {
pub span: Span,
pub machine_applicable: bool,
}
impl Subdiagnostic for FnTraitMissingParen {
@ -1526,16 +1539,11 @@ fn add_to_diag_with<G: EmissionGuarantee, F: SubdiagMessageOp<G>>(
_: &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,
);
}
}

8
compiler/rustc_parse/src/parser/diagnostics.rs
View File

@ -430,7 +430,7 @@ pub(super) fn expected_one_of_not_found(
&mut self,
edible: &[TokenKind],
inedible: &[TokenKind],
) -> PResult<'a, Recovered> {
) -> PResult<'a, ErrorGuaranteed> {
debug!("expected_one_of_not_found(edible: {:?}, inedible: {:?})", edible, inedible);
fn tokens_to_string(tokens: &[TokenType]) -> String {
let mut i = tokens.iter();
@ -533,7 +533,7 @@ fn is_ident_eq_keyword(found: &TokenKind, expected: &TokenType) -> bool {
sugg: ExpectedSemiSugg::ChangeToSemi(self.token.span),
});
self.bump();
return Ok(Recovered::Yes(guar));
return Ok(guar);
} else if self.look_ahead(0, |t| {
t == &token::CloseDelim(Delimiter::Brace)
|| ((t.can_begin_expr() || t.can_begin_item())
@ -557,7 +557,7 @@ fn is_ident_eq_keyword(found: &TokenKind, expected: &TokenType) -> bool {
unexpected_token_label: Some(self.token.span),
sugg: ExpectedSemiSugg::AddSemi(span),
});
return Ok(Recovered::Yes(guar));
return Ok(guar);
}
}
@ -712,7 +712,7 @@ fn is_ident_eq_keyword(found: &TokenKind, expected: &TokenType) -> bool {
if self.check_too_many_raw_str_terminators(&mut err) {
if expected.contains(&TokenType::Token(token::Semi)) && self.eat(&token::Semi) {
let guar = err.emit();
return Ok(Recovered::Yes(guar));
return Ok(guar);
} else {
return Err(err);
}

124
compiler/rustc_parse/src/parser/item.rs
View File

@ -17,6 +17,7 @@
use rustc_span::edition::Edition;
use rustc_span::source_map;
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::ErrorGuaranteed;
use rustc_span::{Span, DUMMY_SP};
use std::fmt::Write;
use std::mem;
@ -2332,14 +2333,106 @@ fn parse_fn(
}
}
};
// 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 `{ ... }`.
// Either `;` or `{ ... }`.
let body =
self.parse_fn_body(attrs, &ident, &mut sig_hi, fn_parse_mode.req_body, fn_params_end)?;
let fn_sig_span = sig_lo.to(sig_hi);
Ok((ident, FnSig { header, decl, span: fn_sig_span }, generics, body))
}
/// Provide diagnostics when function body is not found
fn error_fn_body_not_found(
&mut self,
ident_span: Span,
req_body: bool,
fn_params_end: Option<Span>,
) -> PResult<'a, ErrorGuaranteed> {
let expected = if req_body {
&[token::OpenDelim(Delimiter::Brace)][..]
} else {
&[token::Semi, token::OpenDelim(Delimiter::Brace)]
};
match self.expected_one_of_not_found(&[], expected) {
Ok(error_guaranteed) => Ok(error_guaranteed),
Err(mut err) => {
if self.token.kind == token::CloseDelim(Delimiter::Brace) {
// The enclosing `mod`, `trait` or `impl` is being closed, so keep the `fn` in
// the AST for typechecking.
err.span_label(ident_span, "while parsing this `fn`");
Ok(err.emit())
} 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 fn_trait_span =
[sym::FnOnce, sym::FnMut, sym::Fn].into_iter().find_map(|symbol| {
if self.prev_token.is_ident_named(symbol) {
Some(self.prev_token.span)
} else {
None
}
});
// 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 let Some(fn_trait_span) = fn_trait_span {
// 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,
});
}
Err(err)
} else {
Err(err)
}
}
}
}
/// Parse the "body" of a function.
/// This can either be `;` when there's no body,
/// or e.g. a block when the function is a provided one.
@ -2349,6 +2442,7 @@ fn parse_fn_body(
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
@ -2377,33 +2471,7 @@ fn parse_fn_body(
});
(AttrVec::new(), Some(self.mk_block_err(span, guar)))
} else {
let expected = if req_body {
&[token::OpenDelim(Delimiter::Brace)][..]
} else {
&[token::Semi, token::OpenDelim(Delimiter::Brace)]
};
if let Err(mut err) = self.expected_one_of_not_found(&[], expected) {
if self.token.kind == token::CloseDelim(Delimiter::Brace) {
// The enclosing `mod`, `trait` or `impl` is being closed, so keep the `fn` in
// 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,
});
}
return Err(err);
}
}
self.error_fn_body_not_found(ident.span, req_body, fn_params_end)?;
(AttrVec::new(), None)
};
attrs.extend(inner_attrs);

1
compiler/rustc_parse/src/parser/mod.rs
View File

@ -495,6 +495,7 @@ fn expect_one_of(
FatalError.raise();
} else {
self.expected_one_of_not_found(edible, inedible)
.map(|error_guaranteed| Recovered::Yes(error_guaranteed))
}
}

5
compiler/rustc_parse/src/parser/ty.rs
View File

@ -21,6 +21,11 @@
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,

5
tests/ui/parser/issues/misplaced-return-type-complex-type-issue-126311.rs Normal file
View File

@ -0,0 +1,5 @@
fn foo<T>() where T: Default -> impl Default + 'static {}
//~^ ERROR return type should be specified after the function parameters
//~| HELP place the return type after the function parameters
fn main() {}

14
tests/ui/parser/issues/misplaced-return-type-complex-type-issue-126311.stderr Normal file
View File

@ -0,0 +1,14 @@
error: return type should be specified after the function parameters
--> $DIR/misplaced-return-type-complex-type-issue-126311.rs:1:30
|
LL | fn foo<T>() where T: Default -> impl Default + 'static {}
| ^^ expected one of `(`, `+`, `,`, `::`, `<`, or `{`
|
help: place the return type after the function parameters
|
LL - fn foo<T>() where T: Default -> impl Default + 'static {}
LL + fn foo<T>() -> impl Default + 'static where T: Default {}
|
error: aborting due to 1 previous error

5
tests/ui/parser/issues/misplaced-return-type-issue-126311.rs Normal file
View File

@ -0,0 +1,5 @@
fn foo<T>() where T: Default -> u8 {}
//~^ ERROR return type should be specified after the function parameters
//~| HELP place the return type after the function parameters
fn main() {}

14
tests/ui/parser/issues/misplaced-return-type-issue-126311.stderr Normal file
View File

@ -0,0 +1,14 @@
error: return type should be specified after the function parameters
--> $DIR/misplaced-return-type-issue-126311.rs:1:30
|
LL | fn foo<T>() where T: Default -> u8 {}
| ^^ expected one of `(`, `+`, `,`, `::`, `<`, or `{`
|
help: place the return type after the function parameters
|
LL - fn foo<T>() where T: Default -> u8 {}
LL + fn foo<T>() -> u8 where T: Default {}
|
error: aborting due to 1 previous error

11
tests/ui/parser/issues/misplaced-return-type-where-in-next-line-issue-126311.rs Normal file
View File

@ -0,0 +1,11 @@
fn foo<T, K>()
//~^ HELP place the return type after the function parameters
where
T: Default,
K: Clone, -> Result<u8, String>
//~^ ERROR return type should be specified after the function parameters
{
Ok(0)
}
fn main() {}

17
tests/ui/parser/issues/misplaced-return-type-where-in-next-line-issue-126311.stderr Normal file
View File

@ -0,0 +1,17 @@
error: return type should be specified after the function parameters
--> $DIR/misplaced-return-type-where-in-next-line-issue-126311.rs:5:15
|
LL | K: Clone, -> Result<u8, String>
| ^^ expected one of `{`, lifetime, or type
|
help: place the return type after the function parameters
|
LL ~ fn foo<T, K>() -> Result<u8, String>
LL |
LL | where
LL | T: Default,
LL ~ K: Clone,
|
error: aborting due to 1 previous error

6
tests/ui/parser/issues/misplaced-return-type-without-type-issue-126311.rs Normal file
View File

@ -0,0 +1,6 @@
fn foo<T>() where T: Default -> {
//~^ ERROR expected one of `(`, `+`, `,`, `::`, `<`, or `{`, found `->`
0
}
fn main() {}

8
tests/ui/parser/issues/misplaced-return-type-without-type-issue-126311.stderr Normal file
View File

@ -0,0 +1,8 @@
error: expected one of `(`, `+`, `,`, `::`, `<`, or `{`, found `->`
--> $DIR/misplaced-return-type-without-type-issue-126311.rs:1:30
|
LL | fn foo<T>() where T: Default -> {
| ^^ expected one of `(`, `+`, `,`, `::`, `<`, or `{`
error: aborting due to 1 previous error

4
tests/ui/parser/issues/misplaced-return-type-without-where-issue-126311.rs Normal file
View File

@ -0,0 +1,4 @@
fn bar<T>() -> u8 -> u64 {}
//~^ ERROR expected one of `!`, `(`, `+`, `::`, `<`, `where`, or `{`, found `->`
fn main() {}

8
tests/ui/parser/issues/misplaced-return-type-without-where-issue-126311.stderr Normal file
View File

@ -0,0 +1,8 @@
error: expected one of `!`, `(`, `+`, `::`, `<`, `where`, or `{`, found `->`
--> $DIR/misplaced-return-type-without-where-issue-126311.rs:1:19
|
LL | fn bar<T>() -> u8 -> u64 {}
| ^^ expected one of 7 possible tokens
error: aborting due to 1 previous error