7156633: (javac) incorrect errors when parsing variable declaration in block statements.
Reviewed-by: jjg
Contributed-by: jan.lahoda@oracle.com
--- a/langtools/src/share/classes/com/sun/tools/javac/parser/JavacParser.java Fri Apr 06 10:10:44 2012 -0700
+++ b/langtools/src/share/classes/com/sun/tools/javac/parser/JavacParser.java Mon Apr 09 14:31:18 2012 -0700
@@ -1,5 +1,5 @@
/*
- * 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 @@
*/
@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());
+ } else {
+ if (token.pos <= endPosTable.errorEndPos) {
+ skip(false, true, true, true);
+ }
+ stats.addAll(stat);
+ }
+ }
+ }
+
+ /*
+ * 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 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);
- }
- break;
- }
- case ABSTRACT: case STRICTFP: {
- String dc = token.comment(CommentStyle.JAVADOC);
- JCModifiers mods = modifiersOpt();
- stats.append(classOrInterfaceOrEnumDeclaration(mods, dc));
+ case VARDEF:
+ error = "variable.not.allowed";
break;
}
- case INTERFACE:
- case CLASS:
- String dc = token.comment(CommentStyle.JAVADOC);
- stats.append(classOrInterfaceOrEnumDeclaration(modifiersOpt(), dc));
- 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;
+ }
+ }
+
+ @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();
}
-
- // error recovery
- if (token.pos == lastErrPos)
+ }
+ 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();
- if (token.pos <= endPosTable.errorEndPos) {
- skip(false, true, true, true);
- lastErrPos = token.pos;
+ } 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 @@
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 @@
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 @@
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));
--- a/langtools/src/share/classes/com/sun/tools/javac/resources/compiler.properties Fri Apr 06 10:10:44 2012 -0700
+++ b/langtools/src/share/classes/com/sun/tools/javac/resources/compiler.properties Mon Apr 09 14:31:18 2012 -0700
@@ -196,6 +196,9 @@
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.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.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
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/diags/examples/IllegalStartOfStmt.java Mon Apr 09 14:31:18 2012 -0700
@@ -0,0 +1,31 @@
+/*
+ * 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) }
+ }
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/diags/examples/NotAllowedClass.java Mon Apr 09 14:31:18 2012 -0700
@@ -0,0 +1,31 @@
+/*
+ * 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 {}
+ }
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/diags/examples/NotAllowedVariable.java Mon Apr 09 14:31:18 2012 -0700
@@ -0,0 +1,31 @@
+/*
+ * 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;
+ }
+}
--- a/langtools/test/tools/javac/parser/JavacParserTest.java Fri Apr 06 10:10:44 2012 -0700
+++ b/langtools/test/tools/javac/parser/JavacParserTest.java Mon Apr 09 14:31:18 2012 -0700
@@ -1,5 +1,5 @@
/*
- * 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 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 @@
}
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 @@
testPositionBrokenSource126732b();
// Fails, these tests yet to be addressed
- testVariableInIfThen1();
- testVariableInIfThen2();
testPositionForEnumModifiers();
testStartPositionEnumConstantInit();
}
@@ -821,7 +863,11 @@
testPreferredPositionForBinaryOp();
testStartPositionForMethodWithoutModifiers();
testVarPos();
+ testVariableInIfThen1();
+ testVariableInIfThen2();
testVariableInIfThen3();
+ testVariableInIfThen4();
+ testVariableInIfThen5();
testMissingExponent();
testTryResourcePos();
testOperatorMissingError();