Diagnostic when Fn*-family trait bounds is missing parens could be improved

[Fishrock123]

Code

fn handle<F>() where F: FnOnce -> () {}

Current output

error: expected one of `(`, `+`, `,`, `::`, `<`, or `{`, found `->`
 --> src/main.rs:1:32
  |
1 | fn handle<F>() where F: FnOnce -> () {}
  |                                ^^ expected one of `(`, `+`, `,`, `::`, `<`, or `{`

Desired output

error: expected `(`, found `->`
 --> src/main.rs:1:32
  |
1 | fn handle<F>() where F: FnOnce -> () {}
  |                               ^ `Fn*`-family trait bounds must be followed by parentheses
help: try using parenthesis here:
  |
1 | fn handle<F>() where F: FnOnce() -> () {}
  |                               ^^

Rationale and extra context

It feels like rustc should be able to determine that a trait followed by a -> is a Fn*-family trait bound and be able to suggest this behavior. If that is too general it could just check for Fn, FnOnce, FnMut.

Other cases

Not that i know of...

Very similar is if parenthesis are missing from regular fn declarations:

error: expected one of `(` or `<`, found `->`
 --> src/lib.rs:3:9
  |
3 | fn main -> () {}
  |         ^^ expected one of `(` or `<`

Anything else?

Also happens on nightly according to Rust Playground.

I'd like to try my hand at implementing this diagnostic, but I'll likely need a little bit of locational guidance. A naive code search show that the message is emitted by this code is the parser diagnostics, but I'm not sure if the adjustment would need to be made within that or from where it is invoked in this case.

rust/compiler/rustc_parse/src/parser/diagnostics.rs

Lines 384 to 388 in c528357

	pub(super) fn expected_one_of_not_found(
	&mut self,
	edible: &[TokenKind],
	inedible: &[TokenKind],
	) -> PResult<'a, bool /* recovered */> {

---

Edit: Updated to use the wording "Fn*-family trait bounds" rather than "HRTBs".

[compiler-errors]

but I'm not sure if the adjustment would need to be made within that or from where it is invoked in this case

Where this function is invoked. Try looking for the code that parses "parenthesized" generics.

[fmease]

@Fishrock123 If you end up writing a fix, note that those trait bounds are usually called Fn*-family trait bounds, not “HRTBs”. The latter would require at least one higher-rank parameter to be involved (e.g. for<'a>).

[Fishrock123]

It seems this diagnostic contextually comes from parse_fn_params() which does this token search by parse_paren_comma_seq().

As corroboration it also seems that declaring a regular fn without parenthesis causes a similar if more contextually understandable error:

error: expected one of `(` or `<`, found `->`
 --> src/lib.rs:3:9
  |
3 | fn main -> () {}
  |         ^^ expected one of `(` or `<`

rust/compiler/rustc_parse/src/parser/item.rs

Lines 2375 to 2396 in 9a7cc6c

	/// Parses the parameter list of a function, including the `(` and `)` delimiters.
	pub(super) fn parse_fn_params(&mut self, req_name: ReqName) -> PResult<'a, Vec<Param>> {
	let mut first_param = true;
	// Parse the arguments, starting out with `self` being allowed...
	let (mut params, _) = self.parse_paren_comma_seq(|p| {
	p.recover_diff_marker();
	let param = p.parse_param_general(req_name, first_param).or_else(|mut e| {
	e.emit();
	let lo = p.prev_token.span;
	// Skip every token until next possible arg or end.
	p.eat_to_tokens(&[&token::Comma, &token::CloseDelim(Delimiter::Parenthesis)]);
	// Create a placeholder argument for proper arg count (issue #34264).
	Ok(dummy_arg(Ident::new(kw::Empty, lo.to(p.prev_token.span))))
	});
	// ...now that we've parsed the first argument, `self` is no longer allowed.
	first_param = false;
	param
	})?;
	// Replace duplicated recovered params with `_` pattern to avoid unnecessary errors.
	self.deduplicate_recovered_params_names(&mut params);
	Ok(params)
	}

I naively think that the adjustment point should probably be parse_fn_params() rather than parse_paren_comma_seq()...

However I don't understand how the output gets multiple possibilities e.g. ( and < in this output:

3 | fn main -> () {}
  |         ^^ expected one of `(` or `<`

or the whole list of

expected one of `(`, `+`, `,`, `::`, `<`, or `{`

Since it's only calling Parser::expect() with Delimiter::Parenthesis from self.parse_delim_comma_seq(Delimiter::Parenthesis, f)...

I guess this requires catching the result in parse_fn_params and handling this result type somehow?

pub type PErr<'a> = DiagnosticBuilder<'a, ErrorGuaranteed>;

[compiler-errors]

Any time you call Parser::check, it stashes the token type you tried probing for and it'll present that in a list whenever you get to expected_one_of_not_found. (Or something like that).

[compiler-errors]

Anyways, it shouldn't matter -- in the caller to parse_fn_params (when parsing Fn ->), you could add a check for -> before calling parse_fn_params and handle the -> token specifically.

[estebank]

Just chiming in to mention: in any parser change you make here make sure you don't start accepting new code and add tests for all combinations you can think of.