Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

format! should be able to parse arbitrary inline expressions and provide a targeted error #96999

Open
estebank opened this issue May 13, 2022 · 3 comments
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The parsing of Rust source code to an AST. D-papercut Diagnostics: An error or lint that needs small tweaks. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@estebank
Copy link
Contributor

estebank commented May 13, 2022

format! currently doesn't support inline expressions, like format!("{binding.field}");, but the error it emits while serviceable isn't great:

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

It would be much better experience if we started parsing first field access expressions, potentially method calls and later arbitrary expressions and explicitly told the user "you can't do that, move the expression to its own binding". If we were to ever attempt to support this, prototyping the machinery for diagnostics first would also be informative about its limits or potential problems.

error: invalid format string: field access isn't supported
  --> src/main.rs:10:23
   |
10 |     println!("{binding.field}");
   |                ^^^^^^^^^^^^^ not supported
help: consider using a positional formatting argument instead
   |
10 |     println!("{}", binding.field);
   |               --   +++++++++++++
@estebank estebank added A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The parsing of Rust source code to an AST. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. D-papercut Diagnostics: An error or lint that needs small tweaks. labels May 13, 2022
@Mark-Simulacrum
Copy link
Member

Mark-Simulacrum commented May 13, 2022

Do we really need to parse the inside of the format string? It seems like a potentially reasonable first step here is to add a help: message indicating that only identifiers are supported, not arbitrary expressions, and suggest the position formatting argument.

We could probably have a simple heuristic of longer than 3 characters or similar to avoid most malformed {} contents that aren't actually Rust expressions.

@estebank
Copy link
Contributor Author

If we want to provide a structured suggestion, we do need to parse the argument. The heuristic is a good stop gap that might be "enough". Being able to parse fn calls, method and field chains and binops could be done "cheaply" and cover the vast majority of likely user mistakes.

@Milo123459
Copy link
Contributor

Could a suggestion be implemented where it is replaced with a format argument and replaced with the argument number? I think that could be a good idea.

@TaKO8Ki TaKO8Ki self-assigned this May 27, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 4, 2022
…ing-argument-instead-of-format-args-capture, r=estebank

Suggest a positional formatting argument instead of a captured argument

This patch fixes a part of rust-lang#96999.

fixes rust-lang#98241
fixes rust-lang#97311

r? `@estebank`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The parsing of Rust source code to an AST. D-papercut Diagnostics: An error or lint that needs small tweaks. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants