7156633: (javac) incorrect errors when parsing variable declaration in block statements.

Reviewed-by: jjg
Contributed-by: jan.lahoda@oracle.com

src/share/classes/com/sun/tools/javac/parser/JavacParser.java

 /*
- * Copyright (c) 1999, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2012, 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
@@ -1798,92 +1798,126 @@ public class JavacParser implements Parser {
      */
     @SuppressWarnings("fallthrough")
     List<JCStatement> blockStatements() {
-//todo: skip to anchor on error(?)
-        int lastErrPos = -1;
+        //todo: skip to anchor on error(?)
         ListBuffer<JCStatement> stats = new ListBuffer<JCStatement>();
         while (true) {
-            int pos = token.pos;
-            switch (token.kind) {
-            case RBRACE: case CASE: case DEFAULT: case EOF:
+            List<JCStatement> stat = blockStatement();
+            if (stat.isEmpty()) {
                 return stats.toList();
-            case LBRACE: case IF: case FOR: case WHILE: case DO: case TRY:
-            case SWITCH: case SYNCHRONIZED: case RETURN: case THROW: case BREAK:
-            case CONTINUE: case SEMI: case ELSE: case FINALLY: case CATCH:
-                stats.append(parseStatement());
-                break;
-            case MONKEYS_AT:
-            case FINAL: {
-                String dc = token.comment(CommentStyle.JAVADOC);
-                JCModifiers mods = modifiersOpt();
-                if (token.kind == INTERFACE ||
-                    token.kind == CLASS ||
-                    allowEnums && token.kind == ENUM) {
-                    stats.append(classOrInterfaceOrEnumDeclaration(mods, dc));
-                } else {
-                    JCExpression t = parseType();
-                    stats.appendList(variableDeclarators(mods, t,
-                                                         new ListBuffer<JCStatement>()));
-                    // A "LocalVariableDeclarationStatement" subsumes the terminating semicolon
-                    storeEnd(stats.elems.last(), token.endPos);
-                    accept(SEMI);
+            } else {
+                if (token.pos <= endPosTable.errorEndPos) {
+                    skip(false, true, true, true);
                 }
-                break;
+                stats.addAll(stat);
             }
-            case ABSTRACT: case STRICTFP: {
-                String dc = token.comment(CommentStyle.JAVADOC);
-                JCModifiers mods = modifiersOpt();
-                stats.append(classOrInterfaceOrEnumDeclaration(mods, dc));
+        }
+    }
+
+    /*
+     * This method parses a statement treating it as a block, relaxing the
+     * JLS restrictions, allows us to parse more faulty code, doing so
+     * enables us to provide better and accurate diagnostics to the user.
+     */
+    JCStatement parseStatementAsBlock() {
+        int pos = token.pos;
+        List<JCStatement> stats = blockStatement();
+        if (stats.isEmpty()) {
+            JCErroneous e = F.at(pos).Erroneous();
+            error(e, "illegal.start.of.stmt");
+            return F.at(pos).Exec(e);
+        } else {
+            JCStatement first = stats.head;
+            String error = null;
+            switch (first.getTag()) {
+            case CLASSDEF:
+                error = "class.not.allowed";
                 break;
-            }
-            case INTERFACE:
-            case CLASS:
-                String dc = token.comment(CommentStyle.JAVADOC);
-                stats.append(classOrInterfaceOrEnumDeclaration(modifiersOpt(), dc));
+            case VARDEF:
+                error = "variable.not.allowed";
                 break;
-            case ENUM:
-            case ASSERT:
-                if (allowEnums && token.kind == ENUM) {
-                    error(token.pos, "local.enum");
-                    dc = token.comment(CommentStyle.JAVADOC);
-                    stats.append(classOrInterfaceOrEnumDeclaration(modifiersOpt(), dc));
-                    break;
-                } else if (allowAsserts && token.kind == ASSERT) {
-                    stats.append(parseStatement());
-                    break;
-                }
-                /* fall through to default */
-            default:
-                Token prevToken = token;
-                JCExpression t = term(EXPR | TYPE);
-                if (token.kind == COLON && t.hasTag(IDENT)) {
-                    nextToken();
-                    JCStatement stat = parseStatement();
-                    stats.append(F.at(pos).Labelled(prevToken.name(), stat));
-                } else if ((lastmode & TYPE) != 0 &&
-                           (token.kind == IDENTIFIER ||
-                            token.kind == ASSERT ||
-                            token.kind == ENUM)) {
-                    pos = token.pos;
-                    JCModifiers mods = F.at(Position.NOPOS).Modifiers(0);
-                    F.at(pos);
-                    stats.appendList(variableDeclarators(mods, t,
-                                                         new ListBuffer<JCStatement>()));
-                    // A "LocalVariableDeclarationStatement" subsumes the terminating semicolon
-                    storeEnd(stats.elems.last(), token.endPos);
-                    accept(SEMI);
-                } else {
-                    // This Exec is an "ExpressionStatement"; it subsumes the terminating semicolon
-                    stats.append(to(F.at(pos).Exec(checkExprStat(t))));
-                    accept(SEMI);
-                }
             }
+            if (error != null) {
+                error(first, error);
+                List<JCBlock> blist = List.of(F.at(first.pos).Block(0, stats));
+                return toP(F.at(pos).Exec(F.at(first.pos).Erroneous(blist)));
+            }
+            return first;
+        }
+    }
 
-            // error recovery
-            if (token.pos == lastErrPos)
+    @SuppressWarnings("fallthrough")
+    List<JCStatement> blockStatement() {
+        //todo: skip to anchor on error(?)
+        int pos = token.pos;
+        switch (token.kind) {
+        case RBRACE: case CASE: case DEFAULT: case EOF:
+            return List.nil();
+        case LBRACE: case IF: case FOR: case WHILE: case DO: case TRY:
+        case SWITCH: case SYNCHRONIZED: case RETURN: case THROW: case BREAK:
+        case CONTINUE: case SEMI: case ELSE: case FINALLY: case CATCH:
+            return List.of(parseStatement());
+        case MONKEYS_AT:
+        case FINAL: {
+            String dc = token.comment(CommentStyle.JAVADOC);
+            JCModifiers mods = modifiersOpt();
+            if (token.kind == INTERFACE ||
+                token.kind == CLASS ||
+                allowEnums && token.kind == ENUM) {
+                return List.of(classOrInterfaceOrEnumDeclaration(mods, dc));
+            } else {
+                JCExpression t = parseType();
+                ListBuffer<JCStatement> stats =
+                        variableDeclarators(mods, t, new ListBuffer<JCStatement>());
+                // A "LocalVariableDeclarationStatement" subsumes the terminating semicolon
+                storeEnd(stats.elems.last(), token.endPos);
+                accept(SEMI);
                 return stats.toList();
-            if (token.pos <= endPosTable.errorEndPos) {
-                skip(false, true, true, true);
-                lastErrPos = token.pos;
+            }
+        }
+        case ABSTRACT: case STRICTFP: {
+            String dc = token.comment(CommentStyle.JAVADOC);
+            JCModifiers mods = modifiersOpt();
+            return List.of(classOrInterfaceOrEnumDeclaration(mods, dc));
+        }
+        case INTERFACE:
+        case CLASS:
+            String dc = token.comment(CommentStyle.JAVADOC);
+            return List.of(classOrInterfaceOrEnumDeclaration(modifiersOpt(), dc));
+        case ENUM:
+        case ASSERT:
+            if (allowEnums && token.kind == ENUM) {
+                error(token.pos, "local.enum");
+                dc = token.comment(CommentStyle.JAVADOC);
+                return List.of(classOrInterfaceOrEnumDeclaration(modifiersOpt(), dc));
+            } else if (allowAsserts && token.kind == ASSERT) {
+                return List.of(parseStatement());
+            }
+            /* fall through to default */
+        default:
+            Token prevToken = token;
+            JCExpression t = term(EXPR | TYPE);
+            if (token.kind == COLON && t.hasTag(IDENT)) {
+                nextToken();
+                JCStatement stat = parseStatement();
+                return List.<JCStatement>of(F.at(pos).Labelled(prevToken.name(), stat));
+            } else if ((lastmode & TYPE) != 0 &&
+                       (token.kind == IDENTIFIER ||
+                        token.kind == ASSERT ||
+                        token.kind == ENUM)) {
+                pos = token.pos;
+                JCModifiers mods = F.at(Position.NOPOS).Modifiers(0);
+                F.at(pos);
+                ListBuffer<JCStatement> stats =
+                        variableDeclarators(mods, t, new ListBuffer<JCStatement>());
+                // A "LocalVariableDeclarationStatement" subsumes the terminating semicolon
+                storeEnd(stats.elems.last(), token.endPos);
+                accept(SEMI);
+                return stats.toList();
+            } else {
+                // This Exec is an "ExpressionStatement"; it subsumes the terminating semicolon
+                JCExpressionStatement expr = to(F.at(pos).Exec(checkExprStat(t)));
+                accept(SEMI);
+                return List.<JCStatement>of(expr);
             }
         }
     }
@@ -1917,11 +1951,11 @@ public class JavacParser implements Parser {
         case IF: {
             nextToken();
             JCExpression cond = parExpression();
-            JCStatement thenpart = parseStatement();
+            JCStatement thenpart = parseStatementAsBlock();
             JCStatement elsepart = null;
             if (token.kind == ELSE) {
                 nextToken();
-                elsepart = parseStatement();
+                elsepart = parseStatementAsBlock();
             }
             return F.at(pos).If(cond, thenpart, elsepart);
         }
@@ -1938,7 +1972,7 @@ public class JavacParser implements Parser {
                 accept(COLON);
                 JCExpression expr = parseExpression();
                 accept(RPAREN);
-                JCStatement body = parseStatement();
+                JCStatement body = parseStatementAsBlock();
                 return F.at(pos).ForeachLoop(var, expr, body);
             } else {
                 accept(SEMI);
@@ -1946,19 +1980,19 @@ public class JavacParser implements Parser {
                 accept(SEMI);
                 List<JCExpressionStatement> steps = token.kind == RPAREN ? List.<JCExpressionStatement>nil() : forUpdate();
                 accept(RPAREN);
-                JCStatement body = parseStatement();
+                JCStatement body = parseStatementAsBlock();
                 return F.at(pos).ForLoop(inits, cond, steps, body);
             }
         }
         case WHILE: {
             nextToken();
             JCExpression cond = parExpression();
-            JCStatement body = parseStatement();
+            JCStatement body = parseStatementAsBlock();
             return F.at(pos).WhileLoop(cond, body);
         }
         case DO: {
             nextToken();
-            JCStatement body = parseStatement();
+            JCStatement body = parseStatementAsBlock();
             accept(WHILE);
             JCExpression cond = parExpression();
             JCDoWhileLoop t = to(F.at(pos).DoLoop(body, cond));

src/share/classes/com/sun/tools/javac/resources/compiler.properties

@@ -196,6 +196,9 @@ compiler.err.catch.without.try=\
 compiler.err.clash.with.pkg.of.same.name=\
     {0} {1} clashes with package of same name
 
+compiler.err.class.not.allowed=\
+    class, interface or enum declaration not allowed here
+
 compiler.err.const.expr.req=\
     constant expression required
 
@@ -385,6 +388,9 @@ compiler.err.illegal.qual.not.icls=\
 compiler.err.illegal.start.of.expr=\
     illegal start of expression
 
+compiler.err.illegal.start.of.stmt=\
+    illegal start of statement
+
 compiler.err.illegal.start.of.type=\
     illegal start of type
 
@@ -446,6 +452,9 @@ compiler.err.invalid.meth.decl.ret.type.req=\
 compiler.err.varargs.and.old.array.syntax=\
     legacy array notation not allowed on variable-arity parameter
 
+compiler.err.variable.not.allowed=\
+    variable declaration not allowed here
+
 # 0: name
 compiler.err.label.already.in.use=\
     label {0} already in use

test/tools/javac/diags/examples/IllegalStartOfStmt.java

+/*
+ * Copyright (c) 2012, 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.illegal.start.of.stmt
+// key: compiler.err.expected3
+
+class IllegalStartOfStmt {
+    void m() {
+        if (true) }
+    }
+}

test/tools/javac/diags/examples/NotAllowedClass.java

+/*
+ * Copyright (c) 2012, 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.class.not.allowed
+
+class NotAllowedClass {
+    void t1() {
+        if (true)
+            class X {}
+    }
+}

test/tools/javac/diags/examples/NotAllowedVariable.java

+/*
+ * Copyright (c) 2012, 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.variable.not.allowed
+
+class NotAllowedVariable {
+    void t1() {
+        if (true)
+            int x = 0;
+    }
+}

test/tools/javac/parser/JavacParserTest.java

 /*
- * Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2012, 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
@@ -596,8 +596,8 @@ public class JavacParserTest extends TestCase {
     public void testVariableInIfThen3() throws IOException {
 
         String code = "package t; class Test { "+
-                "private static void t(String name) { " +
-                "if (name != null) abstract } }";
+                "private static void t() { " +
+                "if (true) abstract class F {} }}";
         DiagnosticCollector<JavaFileObject> coll =
                 new DiagnosticCollector<JavaFileObject>();
         JavacTaskImpl ct = (JavacTaskImpl) tool.getTask(null, null, coll, null,
@@ -612,7 +612,51 @@ public class JavacParserTest extends TestCase {
         }
 
         assertEquals("testVariableInIfThen3",
-                Arrays.<String>asList("compiler.err.illegal.start.of.expr"),
+                Arrays.<String>asList("compiler.err.class.not.allowed"), codes);
+    }
+
+    public void testVariableInIfThen4() throws IOException {
+
+        String code = "package t; class Test { "+
+                "private static void t(String name) { " +
+                "if (name != null) interface X {} } }";
+        DiagnosticCollector<JavaFileObject> coll =
+                new DiagnosticCollector<JavaFileObject>();
+        JavacTaskImpl ct = (JavacTaskImpl) tool.getTask(null, null, coll, null,
+                null, Arrays.asList(new MyFileObject(code)));
+
+        ct.parse();
+
+        List<String> codes = new LinkedList<String>();
+
+        for (Diagnostic<? extends JavaFileObject> d : coll.getDiagnostics()) {
+            codes.add(d.getCode());
+        }
+
+        assertEquals("testVariableInIfThen4",
+                Arrays.<String>asList("compiler.err.class.not.allowed"), codes);
+    }
+
+    public void testVariableInIfThen5() throws IOException {
+
+        String code = "package t; class Test { "+
+                "private static void t() { " +
+                "if (true) } }";
+        DiagnosticCollector<JavaFileObject> coll =
+                new DiagnosticCollector<JavaFileObject>();
+        JavacTaskImpl ct = (JavacTaskImpl) tool.getTask(null, null, coll, null,
+                null, Arrays.asList(new MyFileObject(code)));
+
+        ct.parse();
+
+        List<String> codes = new LinkedList<String>();
+
+        for (Diagnostic<? extends JavaFileObject> d : coll.getDiagnostics()) {
+            codes.add(d.getCode());
+        }
+
+        assertEquals("testVariableInIfThen5",
+                Arrays.<String>asList("compiler.err.illegal.start.of.stmt"),
                 codes);
     }
 
@@ -808,8 +852,6 @@ public class JavacParserTest extends TestCase {
         testPositionBrokenSource126732b();
 
         // Fails, these tests yet to be addressed
-        testVariableInIfThen1();
-        testVariableInIfThen2();
         testPositionForEnumModifiers();
         testStartPositionEnumConstantInit();
     }
@@ -821,7 +863,11 @@ public class JavacParserTest extends TestCase {
         testPreferredPositionForBinaryOp();
         testStartPositionForMethodWithoutModifiers();
         testVarPos();
+        testVariableInIfThen1();
+        testVariableInIfThen2();
         testVariableInIfThen3();
+        testVariableInIfThen4();
+        testVariableInIfThen5();
         testMissingExponent();
         testTryResourcePos();
         testOperatorMissingError();