RFR: 8027682: javac wrongly accepts semicolons in package and import decls

[Jonathan Gibbons]

On Mon, 6 Feb 2023 20:32:27 GMT, Archie L. Cobbs <duke at openjdk.org> wrote:

> JLS [§7.3](https://docs.oracle.com/javase/specs/jls/se19/html/jls-7.html#jls-7.3) specifies that while a lone semi-colon is a valid _TopLevelClassOrInterfaceDeclaration_, it is not a valid _ImportDeclaration_. Therefore, if we see a lone semi-colon while looking for the next import statement we have to advance and accept a class declaration, and we can therefore no longer accept any further `import` statements.
> 
> However, the compiler was allowing this, for example:
> 
> package p; import X; ;;; import Y; class Foo {}
> 
> The bug is that the parser was switching out of "look for imports" mode after parsing a valid class declaration, but it was not switching out of "look for imports" mode after parsing a lone semi-colon.
> 
> The fix to `JavacParser.java` is easy, however it also requires these adjustments to unit tests:
> 
> * Test `tree/T6963934.java` must go away, because it is verifying a bug that only happens when the above bogus input is successfully parsed, and this can no longer happen.
> * A bug in `lib/types/TypeHarness.java` was uncovered and fixed; it was inserting an extra semi-colon.
> * The following bugs, which check invalid syntax within import statements, now generate different parse errors and therefor needed their "golden output" updated:
>   * annotations/typeAnnotations/failures/AnnotatedImport.java
>   * annotations/typeAnnotations/failures/AnnotatedPackage1.java
>   * annotations/typeAnnotations/failures/AnnotatedPackage2.java

I would question whether this is the correct/best solution.

A different solution might be to look ahead for the next keyword (i.e. not-a-semi-colon).   If the next keyword is `import`, then you could report an error, ignore the semi-colon and carry on parsing.   In other words, look to improve the error recovery in these situations.  I'm reminded of a similar-but-different change a few years back to allow the compiler to parse (and reject) `if (expr) declaration` because it improved the error recovery.

And, maybe check whether we want to tweak the language rules to permit these semicolons, in the same way that we permit them between top-level declarations.

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

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