Fix diagnostics for Scala 2.13.12

[scoquelin]

Description

This PR fixes diagnostics issue (i.e. generating empty diagnosticsproto files) when using Scala 2.13.12

Again, this particular change in the Scala library means that we now need to override doReport method instead of info0 in ProtoReporter.java

Moreover, there is also another change introduced in Scala 2.13.12 that modifies the positioning and therefore expectations in the DiagnosticsReporterTest.java reason why we now need 2 versions of that file (before/after Scala 2.13.12)

Motivation

Closes #1484

[aishfenton]

There's another wrinkle to this. It's also broken in 2.12, if you enable SemanticDB. The problem is semanticDB wraps the reporter but only calls doReport on the underlying one (so info0 never gets called).

Problem is here

Could we just override doReport on ProtoReporter instead? And have info0 call doReport for < 2.12.13 support?

[scoquelin]

@aishfenton thanks for reporting! I gave it a shot could you please double-check last commit in this PR and confirm it matches with what you had in mind ?

FYI I had compile issues with 2.11.x since doReport method did not exist back then in ConsoleReporter.scala so I added another conditional branch for 2.11 sources in BUILD.bazel with the original ProtoReporter.java

Also, any chance that you have a way to test that the fix works on your end?

[aishfenton]

LGTM @scoquelin. Taking more careful look at when doReport was added in to FilteringReporter. It looks like it came in 2.12.13, but then the signature was slightly changed for 2.13.12

I wonder if we could split ProtoReporter code on < 2.12.13 (since doReport doesn't exist prior to that), and then override in versions later to that. Even with the change of signature super.doReport(Position, String, Severity) should keep working since they're providing a base version with the old sig in FilterReporter still. A bit brittle though.

[scoquelin]

@aishfenton OK I see thanks for the feedback I got confused when you mentioned 2.12.13 version in your first message and initially thought it was a typo(!) for 2.13.12 but now with added context everything makes sense! I will make the adjustment!

[scoquelin]

@aishfenton I addressed your feedback and added 2.12.13 in the branching logic, however I still had to keep a dedicated ProtoReporter.java file for Scala 2.13.12

I initially tried to simplify the branching logic to have basically 2 branches : before 2.12.13 and after 2.12.13 but unfortunately - and even if the code compiles successfully - the ProtoReporter.java does not produce the .diagnosticsproto files with Scala 2.13.12 and is therefore making this test fail

I believe the reason it fails and we need a dedicated ProtoReporter.java for Scala 2.13.12 is because : 1) the compiler is now invoking doReport(Position, String, Severity, List[CodeAction]) which is the method we have to override in ProtoReporter.java to catch the event 2) trying to invoke super.doReport(Position, String, Severity) from that overridden doReport method will end up with a StackOverflowError since its default implementation will be calling 1) again and create an :infinity: loop :face_with_head_bandage: so we have no choice but to invoke super.doReport(Position, String, Severity, List[CodeAction]) instead

Please kindly review/test my last commit and thanks for the insights I have a better understanding of some of the internals here now after digging a bit more into this!

[simuons]

Hi, @scoquelin, thanks for PR. Is this finished (I see new commit). Are we good to merge?

[scoquelin]

@simuons Yes it's OK to merge! :+1: Thanks!