Skip to content

Commit

Permalink
8337976: Insufficient error recovery in parser for switch inside clas…
Browse files Browse the repository at this point in the history
…s body

Reviewed-by: vromero
  • Loading branch information
lahodaj committed Aug 14, 2024
1 parent 38bd8a3 commit fbe4f05
Show file tree
Hide file tree
Showing 9 changed files with 156 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5251,7 +5251,18 @@ public void visitAnnotatedType(JCAnnotatedType tree) {

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()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ protected JCErroneous syntaxError(int pos, Error errorKey) {
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);
Expand Down Expand Up @@ -4733,6 +4733,12 @@ protected List<JCTree> classOrInterfaceOrRecordBodyDeclaration(JCModifiers mods,
}
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);
}
Expand Down Expand Up @@ -4910,7 +4916,19 @@ protected boolean isDeclaration() {
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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ protected JCErroneous syntaxError(int pos, Error errorKey) {
}

@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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
78 changes: 76 additions & 2 deletions test/langtools/tools/javac/parser/JavacParserTest.java
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down Expand Up @@ -2409,6 +2409,80 @@ void testPartialTopLevelModifiers() throws IOException {
(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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1358,7 +1358,7 @@ void testAcceptRecordId() {
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" +
Expand Down
10 changes: 10 additions & 0 deletions test/langtools/tools/javac/recovery/T8337976.java
Original file line number Diff line number Diff line change
@@ -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; }
}
5 changes: 5 additions & 0 deletions test/langtools/tools/javac/recovery/T8337976.out
Original file line number Diff line number Diff line change
@@ -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

1 comment on commit fbe4f05

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.