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

Consider allowing formatting in the presence of non-syntactic errors from the parser #59923

Open
stereotype441 opened this issue Jan 16, 2025 · 2 comments
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. cfe-error-recovery Error recovery in the CFE. type-enhancement A request for a change that isn't a bug

Comments

@stereotype441
Copy link
Member

Currently the formatter refuses to format if the file being formatted has any errors that came from the parser. The reason for this is because in general, the parser's error recovery process may lead to malformed ASTs, ASTs that don't reflect the token stream, or ASTs that don't reflect the user's intent, and trying to format based on such an AST would probably lead to a poor user experience.

However, there are some errors that are reported by the parser not because they are syntax errors per se, but simply because they are errors that are easy for the parser to detect and report. Prohibiting formatting in the presence of these non-syntactic errors causes its own user experience problems, by preventing the user from getting the benefit of auto-formatting until they have fixed them. We should consider allowing formatting even in the presence of these non-syntactic errors.

Note that deciding precisely which errors are safe to allow might require careful consideration. To name a particular subtle example that came up in discussion this morning, we believe that the parser recovers from a syntax error like final static int? foo; by assuming it's equivalent to static final int? foo;. In a sense, we can think of this as the parser accepting a broader grammar than what the language specification strictly allows, and so we can think of this as a non-syntactic error. But the AST that it produces in response would cause the formatter to output static final int? foo; (essentially "correcting" the error without user consent). We think that having the formatter make corrections like this is a risky proposition. For example, what if the reason the user's code says final static int? foo; is because they have a field static int? foo;, and they are in the process of adding another field above it, that starts with final? In this case if the formatter "fixed" the code by rewriting it to static final int? foo;, that would be frustrating.

@dart-github-bot
Copy link
Collaborator

Summary: The Dart formatter currently refuses to format code with parser errors. This issue proposes allowing formatting even with certain non-syntactic parser errors to improve user experience.

@dart-github-bot dart-github-bot added area-front-end Use area-front-end for front end / CFE / kernel format related issues. triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. type-enhancement A request for a change that isn't a bug labels Jan 16, 2025
@johnniwinther johnniwinther added the cfe-error-recovery Error recovery in the CFE. label Jan 17, 2025
@lrhn
Copy link
Member

lrhn commented Jan 18, 2025

What is an example of such an error?
The way the language grammar is specified is partly grammatic, partly "prose restrictions", but that separation is entirely for convenience. It allows, fx, using the same grammar for parameters everywhere, and then simply saying "it's an error if an initializing formal occurs in anything but a non-redirecting generative constructor", rather than having to specify two separate, mostly parallel, grammars for parameters. If we had a parameterized grammar, we might have solved it that way instead, so one is <parameterList>[<parameter>] and the other is <parameterList>[<parameter>|<constructorParameter>].
While the formatter could probably work with initializing formals in non-constructor parameter lists, it's also reasonable to assume that they don't happen, so the code could choke if it saw one. Or to actually have separate representation classes in the implementation.
I don't know if I'd categorize that as a syntactic or semantic error.

If such errors do exist, they should probably be recognized in the parser, having the parser categorize all errors as being either "syntactic errors" or "semantic errors".
If all errors are semantic, then the syntax is valid, it's just recognized as something that will fail for other reasons, and it's just eagerly detected already while parsing.
If there is a syntax error, you can't trust the parsed syntax, because it might not represent the source.

@lrhn lrhn removed the triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. label Jan 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. cfe-error-recovery Error recovery in the CFE. type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants