RFR: 8294020: improve errors for record declarations

[Vicente Romero]

On Thu, 3 Nov 2022 02:39:07 GMT, Vicente Romero <vromero at openjdk.org> wrote:

> Although the reporter originally complained about the error message for records with no header, I think the issue is deeper. We intentionally didn't follow the same path for parsing record declarations as we do for, for example, classes. This is mainly because `class` is a keyword but `record` is a contextual keyword. So when we find `record` we are not sure if it is an identifier or a record declaration. Although I think that given a context where the compiler expects a type declaration, we can be more aggressive than before and if we find `record` + `identifier` consider it a record declaration. The current implementation of `JavacParser::isRecordStart` is trying to be too clever but it is actually leaving several cases uncovered. So the proposed simpler version should be more stable and make record related errors more similar to those for other class declarations. Test `RecordDeclarationSyntaxTest.java` has been added just to have a golden file that stores the error position.
> 
> TIA

> Hello Vicente, I built this PR locally and compiled the same previous code that was reporting that odd error message:
> 
> ```
> public record Record {
> 
> }
> ```
> 
> With the change in this PR, the error message is much more understandable:
> 
> ```
> javac /tmp/Record.java 
> /tmp/Record.java:1: error: '(' expected
> public record Record {
>                     ^
> /tmp/Record.java:1: error: illegal start of type
> public record Record {
>                      ^
> 2 errors
> ```
> 
> Thank you.

well this is a syntax error, detected by the parser, which doesn't have all the information to know what a user is trying to do so it just says: hey I was expecting something and I can't find it in the input you gave me. What this fix is doing is trying to make the treatment for records the same as that for classes. The compiler points to the right position and shows what it is expecting. If we go down the road to trying to propose what the right thing should be then we will have a lot more complains from users and it will be a never ending loop. Plus as I was saying parser is the one generating the error message, it shouldn't assume that the user is defining a record or not. I don't think that over complicating the parser is a good approach plus we will need to do the same for enums, other classes etc.

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

PR: https://git.openjdk.org/jdk/pull/10963