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

update parsing to better handle unclosed delimeters in input file #4472

Merged
merged 1 commit into from
Oct 16, 2020

Conversation

calebcartwright
Copy link
Member

Fixes #4466

We have historically assumed that the initial crate parsing (rustc_parse's parse_crate_mod) was successful and that we have a formattable root AST node when all of the following conditions are true:

  • the call did not panic
  • the call returns successfully with a Crate
  • the session does not contain any parsing errors

However, this is apparently not a completely safe assumption. This is because unclosed delim errors are buffered and not guaranteed to have been emitted until after the parser has been dropped. In cases where the input file has unclosed delims, and no other parser errors (such as #4466 which could easily occur in editor-format-on-save scenarios) then the parse session was still empty of errors when we do our initial check on the result of parse_crate_mod. As such rustfmt was assuming the crate had been parsed without errors and the AST could be used for formatting, which was incorrect.

AFAICT there's nothing available outside of rustc_parse that grants us insight into the unclosed delims, so this PR refactors our parsing just a bit to ensure that the parser has indeed been dropped (and thus unclosed delim errors emitted) before we check for session errors.

I think it may be worth pursuing some tweaks upstream in rustc_parse, for example ensuring (or at least conditionally supporting) the emission of delim errors as part parse_crate_mod, or adding a helper function on the parser has_unclosed_delims(), but I do think we should go ahead with a rustfmt fix anyway

Copy link
Contributor

@topecongiro topecongiro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one looks a tough one to figure out; thank you! LGTM.

@topecongiro topecongiro merged commit c9aebea into rust-lang:master Oct 16, 2020
@calebcartwright calebcartwright deleted the root-unclosed-delims branch November 20, 2020 02:31
@karyon
Copy link
Contributor

karyon commented Oct 26, 2021

backported in #4503

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustfmt inserts two closing brackets when missing closing paren
3 participants