error-prone: [Discussion] Serialization of Error Prone diagnostics
A question in three parts:
- Is there any option/flag for serializing Error Prone diagnostics (i.e. the Description object) into some machine parsable format (i.e. some standard like SARIF or even a custom JSON), in addition to emitting compiler errors/warnings?
- If that option doesn’t exist, has it ever been previously discussed?
- Assuming it isn’t already there somewhere, is this something that would be desirable upstream? Any ideas on what it might look like?
I might well be missing previous discussions, but I tried to search the repo and the docs and I didn’t find anything of the sort.
The context around the ask is that we are looking for ways to surface the diagnostics from non-blocking checks (i.e. SUGGESTION and for some configurations WARNING) both inside an IDE and also during code review. We can potentially parse the output of javac, but that means over-emitting stuff (again, see SUGGESTION level checks) and also seems more fragile than just somehow getting the results serialized from Error Prone, with potentially more detailed location information.
About this issue
- Original URL
- State: open
- Created a year ago
- Reactions: 5
- Comments: 22 (19 by maintainers)
I’ve just made a proof of concept here: https://github.com/tbroyer/javac-diagnostics-serializer
It’s a javac plugin that uses internal APIs (like ErrorProne) to setup a Log.DiagnosticHandler such that diagnostics are still printed to the standard output (and it won’t mess up with setups that might use diagnostic listeners). It’s accompanied by a gradle plugin to setup the appropriate options so all you have to do is apply the plugin and add a dependency to the javac plugin (I’ll add a default dependency soon; I’ll also a Maven integration test). It requires JDK 9 at a minimum (maybe 11, I haven’t actually tested with 9) because it depends on a
TaskEvent.Kind.COMPILATIONevent to close the output stream. For now, as a PoC, it just prints diagnostics in plain text with<filename>:<line>: <kind>: <message>format.Now, should an actual such plugin emit SARIF? Checkstyle XML? (tools like reviewdog support the latter but not the former) Should it allow pluggable reporters?
I wanted to chime in to say I haven’t spent a lot of time considering the details, but I am supportive of Error Prone providing better support for structured diagnostics.
(In our our own internal use we’ve been getting by with scraping information from build logs with regexes, which is worse but simpler, and now that things like SARIF are getting some traction it might make sense to revisit that.)
@tbroyer your prototype looks encouraging, it seems like an elegant way to enable this with minimal changes and without committing to particular format in Error Prone itself.
If anyone wants to explore what it would like like to have EP take a flag to specific an output file and have it write e.g. SARIF output directly, I’d also be curious what that would look like.
Ha, looking back into it, I don’t think AppliedFix is really needed actually: a SARIF fix is more or less a direct representation of a Fix, so the approach used in DescriptionBasedDiff should be enough. https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317881
Oh, thanks, that clarifies your
toString()comment for me. Not to say that it was unclear, just that I’m more of a person who throws Error Prone checks over the wall than someone familiar with the architecture 😃Given my unfamiliarity with the architecture, I can’t confidently do anything more than run our internal tests on your commit and make sure that nothing explodes (which I assume it won’t). I’ll at least do that now.
An alternative would be to have a DiagnosticListener-based library that build tools calling javax.tools (Maven, Gradle, etc.) could integrate. It would take more time to integrate there than in ErrorProne, but likely less than in the JDK, and would be backward-compatible with older JDKs.
This issue is related https://github.com/google/error-prone/issues/444. It’s fair to say that there is interest in having something for this based on the number of votes there.
As @Stephan202 mentioned, this comment contains a rough idea on how collecting relevant diagnostics could look like: https://github.com/google/error-prone/pull/1474#issuecomment-1370633206.
Thanks for the nudge 😃
I think
JavacErrorDescriptionListeneris the only thing that usesDescriptionListener.Factory, and there are only two calls toDescriptionListener.Factory#getDescriptionListener. I think we could probably refactor those to take aConsumer<AppliedFix>or something, and havingErrorProneAnalyzerpass theSarifWriterin when it creates theJavacErrorDescriptionListener. Does that sounds plausible?Here’s a PoC of the kind of cooperation I hinted above: