Diagnostics: teach the parser API to listen to diagnostics emitted from the compiler(parser). #102
Conversation
…om the compiler(parser). This patch adds a parameter of DiagnosticConsumer to the parser entry point to capture any diagnostics emitted from the compiler(parser). We also convert the c structures collected from the compiler side to an existing Diagnostic type in SwiftSyntax to unify the API. To avoid unnecessarily calculating line/column from a utf8 offset, a property is added to DiagnosticConsumer to indicate whether the users want line/column; by default they are calculated. rdar://48439271
|
@swift-ci please test |
| parseTransition: IncrementalParseTransition? = nil | ||
| file: String = "", | ||
| parseTransition: IncrementalParseTransition? = nil, | ||
| diagConsumer: DiagnosticConsumer? = nil |
There was a problem hiding this comment.
Can you make this take a DiagnosticEngine instead? An engine might have several consumers that would want to receive diagnostics. You'd probably want to check if any of the consumers requested line and column, and compute it once if so. IMO that would make the parser API a little nicer -- 'parse and push the diagnostics into this diagnostic engine' rather than 'deliver them to this one consumer, of multiple which I might have'.
Also, please name it 'diagnosticEngine' over 'diagEngine'.
There was a problem hiding this comment.
hmm, we usually need DiagnosticEngine for issuing new diagnostics. In here, the clients just want to hear diagnostics so adding DiagnosticEngine to the parameter seems to me to complicate the caller site (setting up engine and adding consumers to it). Also, the usefulness of multiple consumers for these diagnostics isn't so clear to me yet.
There was a problem hiding this comment.
In practice, tools that use SwiftSyntax and deal with diagnostics will create a DiagnosticEngine as part of their initial start-up. If we make this method take a single consumer, that removes the ability for a client to use multiple consumers (say, a linter that wants to print diagnostics in CI and also send them to a web service).
The way this reads now (which I still think is valid) is 'parse this file, and if there are diagnostics, tell this consumer'. I don't think it's unreasonable or more difficult for clients to change that to 'parse this file, and if there are diagnostics, use this engine to diagnose them'.
There was a problem hiding this comment.
That's fair. I've updated the function signature to use DiagnosticEngine instead of DiagnosticConsumer.
|
@swift-ci please test |
|
@swift-ci please test |
|
Build failed |
|
@swift-ci please test |
|
Build failed |
Add open breaks around type annotations in optional condition exprs.
This patch adds a parameter of DiagnosticConsumer to the parser entry point
to capture any diagnostics emitted from the compiler(parser). We also convert the
c structures collected from the compiler side to an existing Diagnostic type
in SwiftSyntax to unify the API.
To avoid unnecessarily calculating line/column from a utf8 offset, a property
is added to DiagnosticConsumer to indicate whether the users want line/column;
by default they are calculated.
rdar://48439271