Reversed order of `?#` in format string gives unhelpful error message

[wesleywiser]

Code

#[derive(Debug)]
struct Foo(u8, u8);

fn main() {
    let f = Foo(1, 2);
    println!("{f:?#}");
}

Current output

error: invalid format string: expected `'}'`, found `'#'`
 --> src/main.rs:6:19
  |
6 |     println!("{f:?#}");
  |               -   ^ expected `'}'` in format string
  |               |
  |               because of this opening brace
  |
  = note: if you intended to print `{`, you can escape it using `{{`

Desired output

error: invalid format string: expected `'}'`, found `'#'`
 --> src/main.rs:6:19
  |
6 |     println!("{f:?#}");
  |                  ^^ help: the format parameter should be written `#?`
  |
  = note: if you intended to print `{`, you can escape it using `{{`

Rationale and extra context

It would be helpful to point out to the user that the format parameter should be written the other way.

Other cases

No response

Rust Version

rustc 1.83.0-nightly (d6c8169c1 2024-09-03)
binary: rustc
commit-hash: d6c8169c186ab16a3404cd0d0866674018e8a19e
commit-date: 2024-09-03
host: x86_64-pc-windows-msvc
release: 1.83.0-nightly
LLVM version: 19.1.0

Anything else?

No response

[grimmfl]

Hello,
I would like to tackle this issue.

It would be my first contribution to the rust compiler so it would be nice to know, if this would be feasible as a first issue.
I checked the E-easy labels, but all easy marked as such seam to be assigned already.

Cheers!

[lukas-code]

Hi @grimmfl, thanks for your interest in contributing to Rust!

Yes, this looks like a good first issue and the diagnostic should be reasonably easy to add. The parser for format strings is located in compiler/rustc_parse_format and there you need to add the error recovery and a custom ParseError. To give the error a machine-applicable suggestion (for cargo fix and your favorite IDE), you also have to add another variant to the Suggestion enum and some extra code to rustc_builtin_macros -- check out the existing Suggestion::UsePositional and Suggestion::RemoveRawIdent for examples.

Also, if you haven't already, check out the rustc dev guide, especially the chapters on building/running/testing the compiler and adding new tests. And if you have any questions, don't hesitate to ask me or anyone else here or on the zulip chat.

[grimmfl]

Thanks for your guidance. I will look into it :)

[eduardorittner]

Hi @grimmfl, are you still working on this?

[gagan-bhullar-tech]

@lukas-code i would like to work on this. Can you please assign to me if it is still available.

[lukas-code]

Can you please assign to me

You can assign yourself by commenting @rustbot claim.

[deniskilseev]

@rustbot claim

[deniskilseev]

@lukas-code

I've started looking into the code, and it is going to be my first issue in the Rust repo, so please bear with me here :)

My understanding is that when parsing the formatted string parser will go into fn argument() and then into fn format() where ? will be consumed as part of formatting. Because parsing of ? happens in fn format(), it kind of makes sense to check for following # in there and throw an error too but desired output suggests changing the fn consume_closing_brace() where we can't check that ? exists because it either doesn't or was already parsed.

Could you guide me here? I don't think in Parser there exists a peek_prev() or a function like such, so what's the best approach?

[lukas-code]

Generally, our error recovery works by eagerly consuming tokens in places where they are not allowed, emitting an error, and then continuing as if the user had written the correct thing.

Because parsing of ? happens in fn format(), it kind of makes sense to check for following # in there and throw an error too

That's what I would do. After consuming the ?s around here

rust/compiler/rustc_parse_format/src/lib.rs

Lines 718 to 734 in f2becdf

	if self.consume('x') {
	if self.consume('?') {
	spec.debug_hex = Some(DebugHex::Lower);
	spec.ty = "?";
	} else {
	spec.ty = "x";
	}
	} else if self.consume('X') {
	if self.consume('?') {
	spec.debug_hex = Some(DebugHex::Upper);
	spec.ty = "?";
	} else {
	spec.ty = "X";
	}
	} else if self.consume('?') {
	spec.ty = "?";
	} else {

you can try consuming # (and possibly also X after ? and other similar mistakes) and emit an error when the token was consumed successfully. You can look at suggest_format_debug or the recovery for r#ident in position for inspiration. But please try to keep the "error path" in a separate method if possible, even if position doesn't do that.