Add @Override annotation where appropriate

[realityforge]

It serves no real purpose in generated code other than communicating to the developer reading the source code. However this seems worth it, particularly if it is not hard to do. Presumably this will simply be adding annotation in java where the equivalent is present in closure.

[jDramaix]

Hi Peter, it seems to be a good idea (translating @override closure to @Override java annotations). Do you plan to work on that?

[realityforge]

It is on my radar but not in the near future. I did the initial implementation but ran into a few issues:

• The externs were "wrong" in that they declared @override but we were not including the externs that declared methods in the parent class. I believe that the last commit that needed to land to fix this was google/closure-compiler#3399 which landed in the last 24 hours. I have not checked if any other similar examples have cropped up since. I have slowly been working through fixing this.
  • The @override requirements are different between closure and java. In google/closure-compiler#3399 is an example where the open method in HTMLDocument overrides the method of the same name in Document but does in a way that does not satisfy javas/jsinterops rules for overrides. The super method is not even generated in this scenario. I believe that many of these could be detected and worked around but I have not done an in-depth analysis.
  • The closure externs have @overrides for types/methods that are handled specially in jsinterop-generator. We could special case these overrides or detect that they should not be applied but haven't done the work to do so.

I do plan to get back to it but not sure when so feel free to pick it up.

The short term plan is to cleanup the closure externs and our usage of it so that there is no warnings. Looking at my code spike it seems I increased the warnings in jsinterop-generator at https://github.com/google/jsinterop-generator/blob/586803654200b235dd54d8478f6dfebcf59d9d6c/java/jsinterop/generator/closure/ClosureJsInteropGenerator.java#L188-L190 by adding some code:

    options.setWarningLevel(DiagnosticGroups.MISSING_CONST_PROPERTY, CheckLevel.ERROR);
    options.setWarningLevel(DiagnosticGroups.MISSING_OVERRIDE, CheckLevel.ERROR);
    options.setWarningLevel(DiagnosticGroups.LINT_CHECKS, CheckLevel.ERROR);
    options.setWarningLevel(DiagnosticGroups.ANALYZER_CHECKS, CheckLevel.ERROR);
    options.setWarningLevel(DiagnosticGroups.forName( "checkTypes" ), CheckLevel.ERROR);

(I am not sure which of these overlap - I was just poking around when I was adding these in).

I then made any of these warnings/errors into fatal for the tool. (Ultimately based off #31).

Once that was done, my plan was to do #25 and then take another swing at this issue

[gkdn]

I would not invest much time for this unless this is as simple as converting @override without adding new requirement to Closure. i.e. pls avoid adding complexity to generator and putting extra constraints to Closure externs for this,

[realityforge]

I don't believe it is possible to do this without adding complexity to the generator so I will close this issue. I still plan on fixing the other issues above but they can be tracked independently