RFR: 8338301: Error recovery and reporting should be improved for erroneous implicitly declared classes

[Christian Stein]

On Thu, 15 Aug 2024 08:41:14 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

> Consider code like:
> 
> $ cat /tmp/M.java 
> main() {}
> 
> 
> which is compiled like:
> 
> $ javac --enable-preview --source 24 /tmp/M.java 
> /tmp/M.java:1: error: class, interface, enum, or record expected
> main() {}
> ^
> 1 error
> 
> 
> This error is problematic for two reasons:
>  - this is clearly a method with missing return type, but the error recovery does not see it as such
>  - with implicitly declared classes, not only classes/interfaces/enum/records are expected here, but also methods and fields as permitted
> 
> Note that if a method inside a class is missing a return type, the error is much better, so this is a problem specifically for methods on the top-level:
> 
> $ cat /tmp/C.java
> public class C {
>     main() {}
> }
> $ javac --enable-preview --source 24 /tmp/C.java 
> /tmp/C.java:2: error: invalid method declaration; return type required
>     main() {}
>     ^
> 1 error
> 
> 
> The immediate reason is that the javac's parser will speculatively parse the code to see if the code looks-like a method or a field, but this speculative parse does not special-case methods without return types (unlike class-member parsing, which has this special case, partly because of constructors).
> 
> I tried to remove this speculative parsing, reusing the code to parse class members, but I wasn't successful in that, sadly.
> 
> So, the proposal here is three-fold:
>  - the speculative parse will special-case methods without return type, as member parsing does
>  - when the parser cannot figure out the at the top-level code, the error will be "class, interface, annotation type, enum, record, method or field expected", instead of the current "class, interface, enum, or record expected", to cover all possibilities. (This is only when implicitly declared classes are permitted, i.e. when preview is enabled and source level is correct, otherwise the existing error will be emitted. The idea here is not to suggest something that will lead to another compile-time error.)
>  - (definite) statements at the top-level are special-cased, similarly to how they are special-cased for members after https://github.com/openjdk/jdk/pull/20526
> 
> With this patch, errors like this are produced:
> 
> $ cat /tmp/M.java 
> main() {}
> $ javac --enable-preview -source 24 /tmp/M.java 
> /tmp/M.java:1: error: invalid method declaration; return type required
> main() {}
> ^
> Note: /tmp/M.java uses preview features of Java SE 24.
> Note: Recompile with -Xlint:preview for details.
> 1 error
> 
> 
> 
> $ cat /tmp/S.java 
> if (true);
> $ javac --enable-preview -source 24 /tmp/S.java 
> /tmp/S.j...

Looks good to me - error messages are much better now.

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

Marked as reviewed by cstein (Committer).

PR Review: https://git.openjdk.org/jdk/pull/20591#pullrequestreview-2240025301