Skip to content

Fix ignore-unparsable-files flag so that it works. #425

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

Merged
merged 1 commit into from
Oct 12, 2022

Conversation

dylansturg
Copy link
Contributor

The flag was broken after SwiftSyntax switched how it represents unsupported syntax nodes, from Unknown* type nodes to Missing* nodes. Now the tool exits early whenever there are parsing diagnostics, and --ignore-unparsable-files disables emitting those diagnostics. Regardless of the flag's value, the formatter won't try to operate on a file with any parsing diagnostics.

The flag was broken after SwiftSyntax switched how it represents unsupported syntax nodes, from Unknown* type nodes to Missing* nodes. Now the tool exits early whenever there are parsing diagnostics, and `--ignore-unparsable-files` disables emitting those diagnostics. Regardless of the flag's value, the formatter won't try to operate on a file with any parsing diagnostics.
for diagnostic in diagnostics {
let location = diagnostic.location(converter: expectedConverter)
parsingDiagnosticHandler(diagnostic, location)
}
}

guard diagnostics.isEmpty else {
throw SwiftFormatError.fileContainsInvalidSyntax
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should eventually ditch the parsingDiagnosticsConsumer model and just throw the array of diagnostics as part of this error, since we don't plan on doing anything else after that.

That's a bigger API change though so we can save it for later, to make sure there aren't any other weird side effects.

@allevato allevato merged commit 07c7442 into swiftlang:main Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants