RFR: 8286057: Make javac error on a generic enum friendlier [v2]

[Jan Lahoda]

On Thu, 12 May 2022 18:06:23 GMT, Aggelos Biboudis <abimpoudis at openjdk.org> wrote:

>> Improves the error message for generic enumerations.
>
> Aggelos Biboudis has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
> 
>   8286057: Make javac error on a generic enum friendlier

Overall, I think this is good, thanks!

Besides Pavel's comment on the location, I added some nits.

Also - we usually have tests beside the diagnostic example. E.g. a golden file test, or something like this. This is typically for two reasons: a) such tests allow to cover all needed combination (which is not really relevant here); b) such tests allow to check/verify the specific location of the error.

src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java line 4036:

> 4034:         Name name = typeName();
> 4035: 
> 4036:         if(typeParametersOpt().size() > 0) {

Nit: we usually have a space between `if` and `(`.
Nit: using `!.isEmpty()` might be better here (it would also be slightly faster for non-empty Lists, but since that would only happen in erroneous cases, it is not so critical).

src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java line 4037:

> 4035: 
> 4036:         if(typeParametersOpt().size() > 0) {
> 4037:             log.error(DiagnosticFlag.SYNTAX, S.prevToken().endPos, Errors.EnumCantBeGeneric);

I agree with Pavel the location of the error could be better - probably store the `typeParametersOpt` into a variable, and then use the content to provide a better location. Or, you could store some meaningful position based on tokens.

-------------

Changes requested by jlahoda (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8677