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

Tue Feb 7 13:10:47 UTC 2023

On Tue, 7 Feb 2023 00:01:47 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
>
> Archie L. Cobbs has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Fix some extraneous semi-colons that are now disallowed.
>  - Give a more helpful error for spurious semi-colons before import statements.

Just for information. I've search for the `import [\w.]+;;\s+import` regex across Java sources corpus. The whole corpus is ~31,600,000 files from ~245,000 repositories. The query matched 896 files from 434 repositories.

Here are particular examples:

https://github.com/openjdk/jdk/blob/09b8a1959771213cb982d062f0a913285e4a0c6e/test/jdk/com/sun/jndi/dns/Parser.java#L33 (in OpenJDK, it's still there!)

https://github.com/bpupadhyaya/openjdk-8/blob/45af329463a45955ea2759b89cb0ebfe40570c3f/jdk/src/macosx/classes/sun/font/CFont.java#L31 (ok, this one was fixed later)

https://github.com/mvpleung/QingTingRadio/blob/83c48010f596db82fe09c9f5619987be5e76f14c/QingTingFM/src/com/google/protobuf/GeneratedMessageLite.java#L3

https://github.com/wallysonlima/mypocket/blob/30038ad6a2b3f18ce0af17e7c4a0f07bbf5db8b8/app/src/main/java/wallyson/com/br/mypocket/view/SpendingYearActivity.java#L4

https://github.com/l33tnoob/MT65x2_kernel_lk/blob/f48b462fe90862b30ea067698167c0a55458eec3/mediatek/packages/apps/Stk1/src/com/android/stk/StkAppService.java#L112

https://github.com/alibaba/dragonwell17/blob/586085bb39516e39ffb616ece3e2ca60fd59d225/test/jdk/java/lang/constant/methodTypeDesc/ResolveConstantDesc.java#L38

https://github.com/ninethousand9000/ninehackv1/blob/6da59fb5d25c52fab5ed2978c298d3392133dc7d/src/main/java/me/ninethousand/ninehack/feature/features/client/HUD.java#L7

https://github.com/erincandescent/Impeller/blob/b4c30710975cb49a4223c9352fba032842e91a28/src/eu/e43/impeller/activity/SettingsActivity.java#L10

https://github.com/swarmcom/jSynapse/blob/70587ce0c16d4cb9e318d6cc0cfd414f2a6d4a84/src/main/java/org/swarmcom/jsynapse/service/accesstoken/AccessTokenServiceImpl.java#L21

https://github.com/ging/fiware-draco/blob/c0fe547f95a0e475aaa892ff8f72185901e88bcc/nifi-ngsi-bundle/nifi-ngsi-processors/src/main/java/org/apache/nifi/processors/ngsi/NGSIToPostgreSQL.java#L21

https://github.com/anyaudio/anyaudio-android-app/blob/31b7d033a5b7a303c786ea7f02849e302588b6f0/app/src/main/java/any/audio/Activity/UpdateThemedActivity.java#L10

https://github.com/HewlettPackard/mds/blob/1883702cbe11ae9866a486a938ffe306dda67fbe/java-api/src/com/hpl/mds/impl/TaskOptionImpl.java#L31

Some of them are outdated and was fixed later, some are Android-specific, so probably we don't care much, but there are also totally legitimate cases.

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

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