From fbe4f05636c8f692bd40bbe11fb5bb8b77b77042 Mon Sep 17 00:00:00 2001
From: Jan Lahoda <jlahoda@openjdk.org>
Date: Wed, 14 Aug 2024 12:20:17 +0000
Subject: [PATCH] 8337976: Insufficient error recovery in parser for switch
 inside class body

Reviewed-by: vromero
---
 .../com/sun/tools/javac/comp/Attr.java        | 13 +++-
 .../sun/tools/javac/parser/JavacParser.java   | 22 +++++-
 .../sun/tools/javac/parser/VirtualParser.java |  2 +-
 .../tools/javac/resources/compiler.properties |  3 +
 .../diags/examples/StatementNotExpected.java  | 28 +++++++
 .../tools/javac/parser/JavacParserTest.java   | 78 ++++++++++++++++++-
 .../javac/records/RecordCompilationTests.java |  2 +-
 .../tools/javac/recovery/T8337976.java        | 10 +++
 .../tools/javac/recovery/T8337976.out         |  5 ++
 9 files changed, 156 insertions(+), 7 deletions(-)
 create mode 100644 test/langtools/tools/javac/diags/examples/StatementNotExpected.java
 create mode 100644 test/langtools/tools/javac/recovery/T8337976.java
 create mode 100644 test/langtools/tools/javac/recovery/T8337976.out

diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
index 9b79846ba40..54edc19560b 100644
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
@@ -5251,7 +5251,18 @@ public class Attr extends JCTree.Visitor {
 
     public void visitErroneous(JCErroneous tree) {
         if (tree.errs != null) {
-            Env<AttrContext> errEnv = env.dup(env.tree, env.info.dup());
+            WriteableScope newScope = env.info.scope;
+
+            if (env.tree instanceof JCClassDecl) {
+                Symbol fakeOwner =
+                    new MethodSymbol(BLOCK, names.empty, null,
+                        env.info.scope.owner);
+                newScope = newScope.dupUnshared(fakeOwner);
+            }
+
+            Env<AttrContext> errEnv =
+                    env.dup(env.tree,
+                            env.info.dup(newScope));
             errEnv.info.returnResult = unknownExprInfo;
             for (JCTree err : tree.errs)
                 attribTree(err, errEnv, new ResultInfo(KindSelector.ERR, pt()));
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
index f570ad54c58..6fc93fdc591 100644
--- 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
@@ -455,7 +455,7 @@ public class JavacParser implements Parser {
         return syntaxError(pos, List.nil(), errorKey);
     }
 
-    protected JCErroneous syntaxError(int pos, List<JCTree> errs, Error errorKey) {
+    protected JCErroneous syntaxError(int pos, List<? extends JCTree> errs, Error errorKey) {
         setErrorEndPos(pos);
         JCErroneous err = F.at(pos).Erroneous(errs);
         reportSyntaxError(err, errorKey);
@@ -4733,6 +4733,12 @@ public class JavacParser implements Parser {
                 }
                 ignoreDanglingComments();   // no declaration with which dangling comments can be associated
                 return List.of(block(pos, mods.flags));
+            } else if (isDefiniteStatementStartToken()) {
+                int startPos = token.pos;
+                List<JCStatement> statements = blockStatement();
+                return List.of(syntaxError(startPos,
+                                           statements,
+                                           Errors.StatementNotExpected));
             } else {
                 return constructorOrMethodOrFieldDeclaration(mods, className, isInterface, isRecord, dc);
             }
@@ -4910,7 +4916,19 @@ public class JavacParser implements Parser {
                token.kind == INTERFACE ||
                token.kind == ENUM ||
                isRecordStart() && allowRecords;
-        }
+    }
+
+    /**
+     * {@return true if and only if the current token is definitelly a token that
+     *  starts a statement.}
+     */
+    private boolean isDefiniteStatementStartToken() {
+        return switch (token.kind) {
+            case IF, WHILE, DO, SWITCH, RETURN, TRY, FOR, ASSERT, BREAK,
+                 CONTINUE, THROW -> true;
+            default -> false;
+        };
+    }
 
     protected boolean isRecordStart() {
         if (token.kind == IDENTIFIER && token.name() == names.record && peekToken(TokenKind.IDENTIFIER)) {
diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/VirtualParser.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/VirtualParser.java
index f32c1bf4756..ec3a373ab4e 100644
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/VirtualParser.java
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/VirtualParser.java
@@ -62,7 +62,7 @@ public class VirtualParser extends JavacParser {
     }
 
     @Override
-    protected JCErroneous syntaxError(int pos, List<JCTree> errs, Error errorKey) {
+    protected JCErroneous syntaxError(int pos, List<? extends JCTree> errs, Error errorKey) {
         hasErrors = true;
         return F.Erroneous();
     }
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
index 10f06ad7017..6fd59438220 100644
--- 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
@@ -1616,6 +1616,9 @@ compiler.err.file.sb.on.source.or.patch.path.for.module=\
 compiler.err.no.java.lang=\
     Unable to find package java.lang in platform classes
 
+compiler.err.statement.not.expected=\
+    statements not expected outside of methods and initializers
+
 #####
 
 # Fatal Errors
diff --git a/test/langtools/tools/javac/diags/examples/StatementNotExpected.java b/test/langtools/tools/javac/diags/examples/StatementNotExpected.java
new file mode 100644
index 00000000000..a6b965a8044
--- /dev/null
+++ b/test/langtools/tools/javac/diags/examples/StatementNotExpected.java
@@ -0,0 +1,28 @@
+/*
+ * Copyright (c) 2024, 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.statement.not.expected
+
+class StatementNotExpected {
+    return null;
+}
diff --git a/test/langtools/tools/javac/parser/JavacParserTest.java b/test/langtools/tools/javac/parser/JavacParserTest.java
index 93b2b51a8fa..547de1088b4 100644
--- a/test/langtools/tools/javac/parser/JavacParserTest.java
+++ b/test/langtools/tools/javac/parser/JavacParserTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2024, 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
@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 7073631 7159445 7156633 8028235 8065753 8205418 8205913 8228451 8237041 8253584 8246774 8256411 8256149 8259050 8266436 8267221 8271928 8275097 8293897 8295401 8304671 8310326 8312093 8312204 8315452
+ * @bug 7073631 7159445 7156633 8028235 8065753 8205418 8205913 8228451 8237041 8253584 8246774 8256411 8256149 8259050 8266436 8267221 8271928 8275097 8293897 8295401 8304671 8310326 8312093 8312204 8315452 8337976
  * @summary tests error and diagnostics positions
  * @author  Jan Lahoda
  * @modules jdk.compiler/com.sun.tools.javac.api
@@ -2409,6 +2409,80 @@ public class JavacParserTest extends TestCase {
                      (ERROR: public )""");
     }
 
+    @Test //JDK-8337976
+    void testStatementsInClass() throws IOException {
+        String code = """
+                      package test;
+                      public class Test {
+                          if (true);
+                          while (true);
+                          do {} while (true);
+                          for ( ; ; );
+                          switch (0) { default: }
+                          assert true;
+                          break;
+                          continue;
+                          return ;
+                          throw new RuntimeException();
+                          try {
+                          } catch (RuntimeException ex) {}
+                      }
+                      """;
+        DiagnosticCollector<JavaFileObject> coll =
+                new DiagnosticCollector<>();
+        JavacTaskImpl ct = (JavacTaskImpl) tool.getTask(null, fm, coll,
+                List.of("--enable-preview", "--source", SOURCE_VERSION),
+                null, Arrays.asList(new MyFileObject(code)));
+        CompilationUnitTree cut = ct.parse().iterator().next();
+
+        String result = toStringWithErrors(cut).replaceAll("\\R", "\n");
+        System.out.println("RESULT\n" + result);
+        assertEquals("incorrect AST",
+                     result,
+                     """
+                     package test;
+                     \n\
+                     public class Test {
+                         (ERROR: if (true) ;)
+                         (ERROR: while (true) ;)
+                         (ERROR: do {
+                     } while (true);)
+                         (ERROR: for (; ; ) ;)
+                         (ERROR: switch (0) {
+                     default:
+
+                     })
+                         (ERROR: assert true;)
+                         (ERROR: break;)
+                         (ERROR: continue;)
+                         (ERROR: return;)
+                         (ERROR: throw new RuntimeException();)
+                         (ERROR: try {
+                     } catch (RuntimeException ex) {
+                     })
+                     }""");
+
+        List<String> codes = new LinkedList<>();
+
+        for (Diagnostic<? extends JavaFileObject> d : coll.getDiagnostics()) {
+            codes.add(d.getLineNumber() + ":" + d.getColumnNumber() + ":" + d.getCode());
+        }
+
+        assertEquals("testStatementsInClass: " + codes,
+                     List.of("3:5:compiler.err.statement.not.expected",
+                             "4:5:compiler.err.statement.not.expected",
+                             "5:5:compiler.err.statement.not.expected",
+                             "6:5:compiler.err.statement.not.expected",
+                             "7:5:compiler.err.statement.not.expected",
+                             "8:5:compiler.err.statement.not.expected",
+                             "9:5:compiler.err.statement.not.expected",
+                             "10:5:compiler.err.statement.not.expected",
+                             "11:5:compiler.err.statement.not.expected",
+                             "12:5:compiler.err.statement.not.expected",
+                             "13:5:compiler.err.statement.not.expected"),
+                     codes);
+    }
+
     void run(String[] args) throws Exception {
         int passed = 0, failed = 0;
         final Pattern p = (args != null && args.length > 0)
diff --git a/test/langtools/tools/javac/records/RecordCompilationTests.java b/test/langtools/tools/javac/records/RecordCompilationTests.java
index cbd00f9c607..d80ade50a3b 100644
--- a/test/langtools/tools/javac/records/RecordCompilationTests.java
+++ b/test/langtools/tools/javac/records/RecordCompilationTests.java
@@ -1358,7 +1358,7 @@ class RecordCompilationTests extends CompilationTestCase {
         try {
             String[] testOptions = {};
             setCompileOptions(testOptions);
-            assertFail("compiler.err.illegal.start.of.type",
+            assertFail("compiler.err.statement.not.expected",
                     "class R {\n" +
                             "    record RR(int i) {\n" +
                             "        return null;\n" +
diff --git a/test/langtools/tools/javac/recovery/T8337976.java b/test/langtools/tools/javac/recovery/T8337976.java
new file mode 100644
index 00000000000..cac987f4083
--- /dev/null
+++ b/test/langtools/tools/javac/recovery/T8337976.java
@@ -0,0 +1,10 @@
+/**
+ * @test /nodynamiccopyright/
+ * @bug 8337976
+ * @summary Verify javac does not crash and produces nice errors for certain erroneous code.
+ * @compile/fail/ref=T8337976.out -XDrawDiagnostics -XDshould-stop.at=FLOW -XDdev T8337976.java
+ */
+public class T8337976 {
+    switch (0) { default: undefined u;}
+    if (true) { undefined u; }
+}
diff --git a/test/langtools/tools/javac/recovery/T8337976.out b/test/langtools/tools/javac/recovery/T8337976.out
new file mode 100644
index 00000000000..bae578cecf3
--- /dev/null
+++ b/test/langtools/tools/javac/recovery/T8337976.out
@@ -0,0 +1,5 @@
+T8337976.java:8:5: compiler.err.statement.not.expected
+T8337976.java:9:5: compiler.err.statement.not.expected
+T8337976.java:8:27: compiler.err.cant.resolve.location: kindname.class, undefined, , , (compiler.misc.location: kindname.class, T8337976, null)
+T8337976.java:9:17: compiler.err.cant.resolve.location: kindname.class, undefined, , , (compiler.misc.location: kindname.class, T8337976, null)
+4 errors