Skip to content

[Macros] generate diagnostics from thrown error #1318

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 2 commits into from
Feb 8, 2023

Conversation

elizablock
Copy link
Contributor

Macro expansion methods throw an error to indicate that the expansion failed. This patch allows the error thrown to add full-fledged diagnostics, rather than autogenerating a diagnostic based on the error's description.

It adds an error protocol:

public protocol DiagnosticsProviding: Error {
  var diagnostics: [Diagnostic] { get }
}

...and a utility method on MacroExpansionContext to add diagnostics from an error. The implementation of this method checks if the error is diagnostic providing; if not, it generates the same diagnostic as before. If so, it adds all the provided diagnostics.

@elizablock
Copy link
Contributor Author

@swift-ci please test

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

This looks good to me. I’m not a huge fan of the name DiagnosticProviding but I don’t have any better names so it’s good with me.

@elizablock
Copy link
Contributor Author

@swift-ci please test

@elizablock
Copy link
Contributor Author

This looks good to me. I’m not a huge fan of the name DiagnosticProviding but I don’t have any better names so it’s good with me.

Happy to rename ... DiagnosticError? DiagnosticGenerating?

@elizablock
Copy link
Contributor Author

@swift-ci please test

@elizablock elizablock merged commit 35ff56c into swiftlang:main Feb 8, 2023
@elizablock elizablock deleted the DiagnosticProviding branch February 8, 2023 03:26
@ahoppen
Copy link
Member

ahoppen commented Feb 8, 2023

What about naming it DiagnosticsError? If we do that, we can also assert in the initializer of DiagnosticsError that at least one diagnostics with severity error is provided.

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