8027682: javac wrongly accepts semicolons in package and import decls

archiecobbs · Vicente Romero · commit 4b8f7db6be80 · 2023-03-23T16:04:35.000Z
Reviewed-by: vromero
diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java
@@ -3839,16 +3839,32 @@ public JCTree.JCCompilationUnit parseCompilationUnit() {
             defs.append(pd);
         }
 
-        boolean checkForImports = true;
-        boolean firstTypeDecl = true;
+        boolean firstTypeDecl = true;   // have we see a class, enum, or interface declaration yet?
         while (token.kind != EOF) {
             if (token.pos <= endPosTable.errorEndPos) {
                 // error recovery
-                skip(checkForImports, false, false, false);
+                skip(firstTypeDecl, false, false, false);
                 if (token.kind == EOF)
                     break;
             }
-            if (checkForImports && mods == null && token.kind == IMPORT) {
+            // JLS 7.3 doesn't allow extra semicolons after package or import declarations,
+            // but here we try to provide a more helpful error message if we encounter any.
+            // Do that by slurping in as many semicolons as possible, and then seeing what
+            // comes after before deciding how best to handle them.
+            ListBuffer<JCTree> semiList = new ListBuffer<>();
+            while (firstTypeDecl && mods == null && token.kind == SEMI) {
+                semiList.append(toP(F.at(token.pos).Skip()));
+                nextToken();
+                if (token.kind == EOF)
+                    break;
+            }
+            if (firstTypeDecl && mods == null && token.kind == IMPORT) {
+                if (!semiList.isEmpty()) {
+                    if (source.compareTo(Source.JDK21) >= 0)
+                        reportSyntaxError(semiList.first().pos, Errors.ExtraneousSemicolon);
+                    else
+                        log.warning(semiList.first().pos, Warnings.ExtraneousSemicolon);
+                }
                 seenImport = true;
                 defs.append(importDeclaration());
             } else {
@@ -3860,6 +3876,12 @@ public JCTree.JCCompilationUnit parseCompilationUnit() {
                 if (mods != null || token.kind != SEMI)
                     mods = modifiersOpt(mods);
                 if (firstTypeDecl && token.kind == IDENTIFIER) {
+                    if (!semiList.isEmpty()) {
+                        if (source.compareTo(Source.JDK21) >= 0)
+                            reportSyntaxError(semiList.first().pos, Errors.ExtraneousSemicolon);
+                        else
+                            log.warning(semiList.first().pos, Warnings.ExtraneousSemicolon);
+                    }
                     ModuleKind kind = ModuleKind.STRONG;
                     if (token.name() == names.open) {
                         kind = ModuleKind.OPEN;
@@ -3876,12 +3898,11 @@ public JCTree.JCCompilationUnit parseCompilationUnit() {
                         reportSyntaxError(token.pos, Errors.ExpectedModule);
                     }
                 }
+                defs.appendList(semiList.toList());
                 JCTree def = typeDeclaration(mods, docComment);
                 if (def instanceof JCExpressionStatement statement)
                     def = statement.expr;
                 defs.append(def);
-                if (def instanceof JCClassDecl)
-                    checkForImports = false;
                 mods = null;
                 firstTypeDecl = false;
             }
diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties
@@ -2358,6 +2358,12 @@ compiler.err.enum.constant.expected=\
 compiler.err.enum.constant.not.expected=\
     enum constant not expected here
 
+compiler.err.extraneous.semicolon=\
+    extraneous semicolon
+
+compiler.warn.extraneous.semicolon=\
+    extraneous semicolon
+
 ## The following are related in form, but do not easily fit the above paradigm.
 compiler.err.expected.module.or.open=\
     ''module'' or ''open'' expected
diff --git a/test/jdk/com/sun/jndi/dns/Parser.java b/test/jdk/com/sun/jndi/dns/Parser.java
@@ -30,7 +30,7 @@
 
 import com.sun.jndi.dns.ResourceRecord;
 import javax.naming.CommunicationException;
-import javax.naming.InvalidNameException;;
+import javax.naming.InvalidNameException;
 
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
diff --git a/test/jdk/java/lang/constant/methodTypeDesc/ResolveConstantDesc.java b/test/jdk/java/lang/constant/methodTypeDesc/ResolveConstantDesc.java
@@ -34,8 +34,8 @@
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodHandles.Lookup;
 import java.lang.invoke.MethodType;
-import java.security.AccessControlException;;
-import java.security.Permission;;
+import java.security.AccessControlException;
+import java.security.Permission;
 
 import static jdk.test.lib.Asserts.*;
 
diff --git a/test/jdk/java/lang/management/MemoryMXBean/CollectionUsageThreshold.java b/test/jdk/java/lang/management/MemoryMXBean/CollectionUsageThreshold.java
@@ -45,7 +45,7 @@
 import javax.management.*;
 import javax.management.openmbean.CompositeData;
 import java.lang.management.*;
-import static java.lang.management.MemoryNotificationInfo.*;;
+import static java.lang.management.MemoryNotificationInfo.*;
 import static java.lang.management.ManagementFactory.*;
 
 import jdk.test.whitebox.code.Compiler;
diff --git a/test/jdk/java/nio/channels/AsynchronousFileChannel/Basic.java b/test/jdk/java/nio/channels/AsynchronousFileChannel/Basic.java
@@ -51,7 +51,7 @@
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Future;
 import java.util.concurrent.ThreadFactory;
-import java.util.concurrent.TimeoutException;;
+import java.util.concurrent.TimeoutException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 import static java.nio.file.StandardOpenOption.*;
diff --git a/test/jdk/jdk/jfr/tool/ExecuteHelper.java b/test/jdk/jdk/jfr/tool/ExecuteHelper.java
@@ -33,7 +33,7 @@
 import jdk.test.lib.JDKToolLauncher;
 import jdk.test.lib.Utils;
 import jdk.test.lib.process.OutputAnalyzer;
-import jdk.test.lib.process.ProcessTools;;
+import jdk.test.lib.process.ProcessTools;
 
 final class ExecuteHelper {
 
diff --git a/test/langtools/tools/javac/annotations/typeAnnotations/failures/AnnotatedImport.out b/test/langtools/tools/javac/annotations/typeAnnotations/failures/AnnotatedImport.out
@@ -1,7 +1,7 @@
 AnnotatedImport.java:10:13: compiler.err.expected: token.identifier
 AnnotatedImport.java:10:16: compiler.err.expected4: class, interface, enum, record
-AnnotatedImport.java:11:7: compiler.err.expected: token.identifier
+AnnotatedImport.java:11:1: compiler.err.expected4: class, interface, enum, record
 AnnotatedImport.java:11:11: compiler.err.expected4: class, interface, enum, record
-AnnotatedImport.java:12:18: compiler.err.expected: token.identifier
+AnnotatedImport.java:12:1: compiler.err.expected4: class, interface, enum, record
 AnnotatedImport.java:12:21: compiler.err.expected4: class, interface, enum, record
 6 errors
diff --git a/test/langtools/tools/javac/annotations/typeAnnotations/failures/AnnotatedPackage1.out b/test/langtools/tools/javac/annotations/typeAnnotations/failures/AnnotatedPackage1.out
@@ -1,3 +1,4 @@
 AnnotatedPackage1.java:9:14: compiler.err.expected: token.identifier
 AnnotatedPackage1.java:9:17: compiler.err.expected4: class, interface, enum, record
-2 errors
+AnnotatedPackage1.java:11:1: compiler.err.expected4: class, interface, enum, record
+3 errors
diff --git a/test/langtools/tools/javac/annotations/typeAnnotations/failures/AnnotatedPackage2.out b/test/langtools/tools/javac/annotations/typeAnnotations/failures/AnnotatedPackage2.out
@@ -1,3 +1,4 @@
 AnnotatedPackage2.java:9:8: compiler.err.expected: token.identifier
 AnnotatedPackage2.java:9:12: compiler.err.expected4: class, interface, enum, record
-2 errors
+AnnotatedPackage2.java:11:1: compiler.err.expected4: class, interface, enum, record
+3 errors
diff --git a/test/langtools/tools/javac/diags/examples/ExtraImportSemicolonError.java b/test/langtools/tools/javac/diags/examples/ExtraImportSemicolonError.java
@@ -0,0 +1,30 @@
+/*
+ * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+// key: compiler.err.extraneous.semicolon
+
+import java.util.Map;;      // NOTE: extra semicolon
+import java.util.Set;
+
+class ExtraImportSemicolonError {
+}
diff --git a/test/langtools/tools/javac/diags/examples/ExtraImportSemicolonWarning.java b/test/langtools/tools/javac/diags/examples/ExtraImportSemicolonWarning.java
@@ -0,0 +1,31 @@
+/*
+ * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+// key: compiler.warn.extraneous.semicolon
+// options: --release 20
+
+import java.util.Map;;      // NOTE: extra semicolon
+import java.util.Set;
+
+class ExtraImportSemicolonWarning {
+}
diff --git a/test/langtools/tools/javac/parser/ExtraImportSemicolon.java b/test/langtools/tools/javac/parser/ExtraImportSemicolon.java
@@ -0,0 +1,13 @@
+/*
+ * @test /nodynamiccopyright/
+ * @bug 5059679
+ * @summary Verify proper error reporting of extra semicolon before import statement
+ * @compile/fail/ref=ExtraImportSemicolon.out1 -XDrawDiagnostics ExtraImportSemicolon.java
+ * @compile/ref=ExtraImportSemicolon.out2 --release 20 -XDrawDiagnostics ExtraImportSemicolon.java
+ */
+
+import java.util.Map;;      // NOTE: extra semicolon
+import java.util.Set;
+
+class ExtraImportSemicolon {
+}
diff --git a/test/langtools/tools/javac/parser/ExtraImportSemicolon.out1 b/test/langtools/tools/javac/parser/ExtraImportSemicolon.out1
@@ -0,0 +1,2 @@
+ExtraImportSemicolon.java:9:22: compiler.err.extraneous.semicolon
+1 error
diff --git a/test/langtools/tools/javac/parser/ExtraImportSemicolon.out2 b/test/langtools/tools/javac/parser/ExtraImportSemicolon.out2
@@ -0,0 +1,2 @@
+ExtraImportSemicolon.java:9:22: compiler.warn.extraneous.semicolon
+1 warning
diff --git a/test/langtools/tools/javac/tree/T6963934.java b/test/langtools/tools/javac/tree/T6963934.java
diff --git a/test/langtools/tools/jdeps/modules/src/unsupported/q/Counter.java b/test/langtools/tools/jdeps/modules/src/unsupported/q/Counter.java
@@ -23,7 +23,7 @@
 
 package q;
 
-import jdk.internal.perf.PerfCounter;;
+import jdk.internal.perf.PerfCounter;
 
 public class Counter {
      public static void create(String name) {
diff --git a/test/langtools/tools/lib/types/TypeHarness.java b/test/langtools/tools/lib/types/TypeHarness.java
@@ -464,7 +464,7 @@ class JavaSource extends SimpleJavaFileObject {
 
             String id;
             String type;
-            String template = "#Package;\n" +
+            String template = "#Package\n" +
                     "#Imports\n" +
                     "class G#Id#TypeVars {\n" +
                     "   #FieldType var;" +
