From 0b4a7d534204b7b3b041f5117282dd13b1c7c62f Mon Sep 17 00:00:00 2001 From: Jan Lahoda Date: Thu, 29 Aug 2024 06:25:27 +0000 Subject: [PATCH] 8324859: Improve error recovery Reviewed-by: mcimadamore --- .../sun/tools/javac/parser/JavacParser.java | 93 ++- .../tools/javac/parser/JavacParserTest.java | 533 +++++++++++++++++- 2 files changed, 622 insertions(+), 4 deletions(-) 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 6fc93fdc591ab..27cb44bebba15 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 @@ -59,6 +59,7 @@ import static com.sun.tools.javac.parser.Tokens.TokenKind.GT; import static com.sun.tools.javac.parser.Tokens.TokenKind.IMPORT; import static com.sun.tools.javac.parser.Tokens.TokenKind.LT; +import com.sun.tools.javac.parser.VirtualParser.VirtualScanner; import static com.sun.tools.javac.tree.JCTree.Tag.*; import static com.sun.tools.javac.resources.CompilerProperties.Fragments.ImplicitAndExplicitNotAllowed; import static com.sun.tools.javac.resources.CompilerProperties.Fragments.VarAndExplicitNotAllowed; @@ -5019,13 +5020,17 @@ protected JCTree methodDeclaratorRest(int pos, // Parsing formalParameters sets the receiverParam, if present List params = List.nil(); List thrown = List.nil(); + boolean unclosedParameterList; if (!isRecord || name != names.init || token.kind == LPAREN) { params = formalParameters(); + unclosedParameterList = token.pos == endPosTable.errorEndPos; if (!isVoid) type = bracketsOpt(type); if (token.kind == THROWS) { nextToken(); thrown = qualidentList(true); } + } else { + unclosedParameterList = false; } saveDanglingDocComments(dc); @@ -5039,14 +5044,18 @@ protected JCTree methodDeclaratorRest(int pos, if (token.kind == DEFAULT) { accept(DEFAULT); defaultValue = annotationValue(); + accept(SEMI); } else { defaultValue = null; + accept(SEMI, tk -> Errors.Expected2(LBRACE, SEMI)); } - accept(SEMI); if (token.pos <= endPosTable.errorEndPos) { // error recovery - skip(false, true, false, false); - if (token.kind == LBRACE) { + // look if there is a probable missing opening brace, + // and if yes, parse as a block + boolean parseAsBlock = openingBraceMissing(unclosedParameterList); + + if (parseAsBlock) { body = block(); } } @@ -5063,6 +5072,84 @@ protected JCTree methodDeclaratorRest(int pos, } } + /** + * After seeing a method header, and not seeing an opening left brace, + * attempt to estimate if acting as if the left brace was present and + * parsing the upcoming code will get better results than not parsing + * the code as a block. + * + * The estimate is as follows: + * - tokens are skipped until member, statement or identifier is found, + * - then, if there is a left brace, parse as a block, + * - otherwise, if the head was broken, do not parse as a block, + * - otherwise, look at the next token and: + * - if it definitelly starts a statement, parse as a block, + * - otherwise, if it is a closing/right brace, count opening and closing + * braces in the rest of the file, to see if imaginarily "adding" an opening + * brace would lead to a balanced count - if yes, parse as a block, + * - otherwise, speculatively parse the following code as a block, and if + * it contains statements that cannot be members, parse as a block, + * - otherwise, don't parse as a block. + * + * @param unclosedParameterList whether there was a serious problem in the + * parameters list + * @return true if and only if the following code should be parsed as a block. + */ + private boolean openingBraceMissing(boolean unclosedParameterList) { + skip(false, true, !unclosedParameterList, !unclosedParameterList); + + if (token.kind == LBRACE) { + return true; + } else if (unclosedParameterList) { + return false; + } else { + return switch (token.kind) { + //definitelly sees a statement: + case CASE, DEFAULT, IF, FOR, WHILE, DO, TRY, SWITCH, + RETURN, THROW, BREAK, CONTINUE, ELSE, FINALLY, + CATCH, THIS, SUPER, NEW -> true; + case RBRACE -> { + //check if adding an opening brace would balance out + //the opening and closing braces: + int braceBalance = 1; + VirtualScanner virtualScanner = new VirtualScanner(S); + + virtualScanner.nextToken(); + + while (virtualScanner.token().kind != TokenKind.EOF) { + switch (virtualScanner.token().kind) { + case LBRACE -> braceBalance++; + case RBRACE -> braceBalance--; + } + virtualScanner.nextToken(); + } + + yield braceBalance == 0; + } + default -> { + //speculatively try to parse as a block, and check + //if the result would suggest there is a block + //e.g.: it contains a statement that is not + //a member declaration + JavacParser speculative = new VirtualParser(this); + JCBlock speculativeResult = + speculative.block(); + if (!speculativeResult.stats.isEmpty()) { + JCStatement last = speculativeResult.stats.last(); + yield !speculativeResult.stats.stream().allMatch(s -> s.hasTag(VARDEF) || + s.hasTag(CLASSDEF) || + s.hasTag(BLOCK) || + s == last) || + !(last instanceof JCExpressionStatement exprStatement && + exprStatement.expr.hasTag(ERRONEOUS)); + } else { + yield false; + } + } + }; + } + } + /** QualidentList = [Annotations] Qualident {"," [Annotations] Qualident} */ List qualidentList(boolean allowAnnos) { diff --git a/test/langtools/tools/javac/parser/JavacParserTest.java b/test/langtools/tools/javac/parser/JavacParserTest.java index 547de1088b460..40ab577a5d135 100644 --- a/test/langtools/tools/javac/parser/JavacParserTest.java +++ b/test/langtools/tools/javac/parser/JavacParserTest.java @@ -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 8337976 + * @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 8324859 * @summary tests error and diagnostics positions * @author Jan Lahoda * @modules jdk.compiler/com.sun.tools.javac.api @@ -2483,6 +2483,537 @@ public class Test { codes); } + @Test //JDK-8324859 + void testImplicitlyDeclaredClassesConfusion1() throws IOException { + String code = """ + package tests; + public class TestB { + public static boolean test() // missing open brace + return true; + } + public static boolean test2() { + return true; + } + }"""; + DiagnosticCollector 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(); + + List codes = new LinkedList<>(); + + for (Diagnostic d : coll.getDiagnostics()) { + codes.add(d.getLineNumber() + ":" + d.getColumnNumber() + ":" + d.getCode()); + } + + assertEquals("testImplicitlyDeclaredClassesConfusion1: " + codes, + List.of("3:33:compiler.err.expected2"), + codes); + String result = toStringWithErrors(cut).replaceAll("\\R", "\n"); + System.out.println("RESULT\n" + result); + assertEquals("incorrect AST", + result, + """ + package tests; + \n\ + public class TestB { + \n\ + public static boolean test() { + return true; + } + \n\ + public static boolean test2() { + return true; + } + }"""); + } + + @Test //JDK-8324859 + void testImplicitlyDeclaredClassesConfusion2() throws IOException { + String code = """ + package tests; + public class TestB { + public static boolean test() // missing open brace + + public static boolean test2() { + return true; + } + } """; + DiagnosticCollector 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(); + + List codes = new LinkedList<>(); + + for (Diagnostic d : coll.getDiagnostics()) { + codes.add(d.getLineNumber() + ":" + d.getColumnNumber() + ":" + d.getCode()); + } + + assertEquals("testImplicitlyDeclaredClassesConfusion2: " + codes, + List.of("3:33:compiler.err.expected2"), + codes); + String result = toStringWithErrors(cut).replaceAll("\\R", "\n"); + System.out.println("RESULT\n" + result); + assertEquals("incorrect AST", + result, + """ + package tests; + \n\ + public class TestB { + \n\ + public static boolean test(); + \n\ + public static boolean test2() { + return true; + } + }"""); + } + + @Test //JDK-8324859 + void testImplicitlyDeclaredClassesConfusion3() throws IOException { + String code = """ + package tests; + public class TestB { + public static boolean test() // missing open brace + } + """; + DiagnosticCollector 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(); + + List codes = new LinkedList<>(); + + for (Diagnostic d : coll.getDiagnostics()) { + codes.add(d.getLineNumber() + ":" + d.getColumnNumber() + ":" + d.getCode()); + } + + assertEquals("testImplicitlyDeclaredClassesConfusion3: " + codes, + List.of("3:33:compiler.err.expected2"), + codes); + String result = toStringWithErrors(cut).replaceAll("\\R", "\n"); + System.out.println("RESULT\n" + result); + assertEquals("incorrect AST", + result, + """ + package tests; + \n\ + public class TestB { + \n\ + public static boolean test(); + }"""); + } + + @Test //JDK-8324859 + void testImplicitlyDeclaredClassesConfusion4() throws IOException { + String code = """ + package tests; + public class TestB { + public static boolean test() // missing open brace + } + } + """; + DiagnosticCollector 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(); + + List codes = new LinkedList<>(); + + for (Diagnostic d : coll.getDiagnostics()) { + codes.add(d.getLineNumber() + ":" + d.getColumnNumber() + ":" + d.getCode()); + } + + assertEquals("testImplicitlyDeclaredClassesConfusion4: " + codes, + List.of("3:33:compiler.err.expected2"), + codes); + String result = toStringWithErrors(cut).replaceAll("\\R", "\n"); + System.out.println("RESULT\n" + result); + assertEquals("incorrect AST", + result, + """ + package tests; + \n\ + public class TestB { + \n\ + public static boolean test() { + } + }"""); + } + + @Test //JDK-8324859 + void testImplicitlyDeclaredClassesConfusion5() throws IOException { + String code = """ + package tests; + public class TestB { + public static boolean test(String, + } + class T {} + """; + DiagnosticCollector 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(); + + List codes = new LinkedList<>(); + + for (Diagnostic d : coll.getDiagnostics()) { + codes.add(d.getLineNumber() + ":" + d.getColumnNumber() + ":" + d.getCode()); + } + + assertEquals("testImplicitlyDeclaredClassesConfusion5: " + codes, + List.of("3:38:compiler.err.expected", + "4:1:compiler.err.illegal.start.of.type"), + codes); + String result = toStringWithErrors(cut).replaceAll("\\R", "\n"); + System.out.println("RESULT\n" + result); + assertEquals("incorrect AST", + result, + """ + package tests; + \n\ + public class TestB { + \n\ + public static boolean test(String , (ERROR: ) ); + } + class T { + }"""); + } + + @Test //JDK-8324859 + void testImplicitlyDeclaredClassesConfusion6() throws IOException { + String code = """ + package tests; + public class TestB { + private Object testMethod(final String arg1 final String arg2) { + return null; + } + } + """; + DiagnosticCollector 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(); + + List codes = new LinkedList<>(); + + for (Diagnostic d : coll.getDiagnostics()) { + codes.add(d.getLineNumber() + ":" + d.getColumnNumber() + ":" + d.getCode()); + } + + assertEquals("testImplicitlyDeclaredClassesConfusion5: " + codes, + List.of("3:48:compiler.err.expected3", + "3:66:compiler.err.expected"), + codes); + String result = toStringWithErrors(cut).replaceAll("\\R", "\n"); + System.out.println("RESULT\n" + result); + assertEquals("incorrect AST", + result, + """ + package tests; + \n\ + public class TestB { + \n\ + private Object testMethod(final String arg1); + final String arg2; + { + return null; + } + }"""); + } + + @Test //JDK-8324859 + void testImplicitlyDeclaredClassesConfusion7() throws IOException { + //after 'default' attribute value, only semicolon (';') is expected, + //not left brace ('{'): + String code = """ + package tests; + public @interface A { + public String value() default "" + } + """; + DiagnosticCollector 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(); + + List codes = new LinkedList<>(); + + for (Diagnostic d : coll.getDiagnostics()) { + codes.add(d.getLineNumber() + ":" + d.getColumnNumber() + ":" + d.getCode()); + } + + assertEquals("testImplicitlyDeclaredClassesConfusion5: " + codes, + List.of("3:37:compiler.err.expected"), + codes); + String result = toStringWithErrors(cut).replaceAll("\\R", "\n"); + System.out.println("RESULT\n" + result); + assertEquals("incorrect AST", + result, + """ + package tests; + \n\ + public @interface A { + \n\ + public String value() default ""; + }"""); + } + + @Test //JDK-8324859 + void testImplicitlyDeclaredClassesConfusion10() throws IOException { + String code = """ + package tests; + public class TestB { + public static boolean test() // missing open brace + String s = ""; + return s.isEmpty(); + } + public static boolean test2() { + return true; + } + }"""; + DiagnosticCollector 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(); + + List codes = new LinkedList<>(); + + for (Diagnostic d : coll.getDiagnostics()) { + codes.add(d.getLineNumber() + ":" + d.getColumnNumber() + ":" + d.getCode()); + } + + assertEquals("testImplicitlyDeclaredClassesConfusion1: " + codes, + List.of("3:33:compiler.err.expected2"), + codes); + String result = toStringWithErrors(cut).replaceAll("\\R", "\n"); + System.out.println("RESULT\n" + result); + assertEquals("incorrect AST", + result, + """ + package tests; + + public class TestB { + \n\ + public static boolean test() { + String s = ""; + return s.isEmpty(); + } + \n\ + public static boolean test2() { + return true; + } + }"""); + } + + @Test //JDK-8324859 + void testImplicitlyDeclaredClassesConfusion11() throws IOException { + String code = """ + package tests; + public class TestB { + public static boolean test() // missing open brace + String s = ""; //field declaration + public static boolean test2() { + return true; + } + }"""; + DiagnosticCollector 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(); + + List codes = new LinkedList<>(); + + for (Diagnostic d : coll.getDiagnostics()) { + codes.add(d.getLineNumber() + ":" + d.getColumnNumber() + ":" + d.getCode()); + } + + assertEquals("testImplicitlyDeclaredClassesConfusion1: " + codes, + List.of("3:33:compiler.err.expected2"), + codes); + String result = toStringWithErrors(cut).replaceAll("\\R", "\n"); + System.out.println("RESULT\n" + result); + assertEquals("incorrect AST", + result, + """ + package tests; + \n\ + public class TestB { + \n\ + public static boolean test(); + String s = ""; + \n\ + public static boolean test2() { + return true; + } + }"""); + } + + @Test //JDK-8324859 + void testImplicitlyDeclaredClassesConfusion12() throws IOException { + String code = """ + package tests; + public class TestB { + public static boolean test() // missing open brace + final String s = ""; + return s.isEmpty(); + } + public static boolean test2() { + return true; + } + }"""; + DiagnosticCollector 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(); + + List codes = new LinkedList<>(); + + for (Diagnostic d : coll.getDiagnostics()) { + codes.add(d.getLineNumber() + ":" + d.getColumnNumber() + ":" + d.getCode()); + } + + assertEquals("testImplicitlyDeclaredClassesConfusion1: " + codes, + List.of("3:33:compiler.err.expected2"), + codes); + String result = toStringWithErrors(cut).replaceAll("\\R", "\n"); + System.out.println("RESULT\n" + result); + assertEquals("incorrect AST", + result, + """ + package tests; + \n\ + public class TestB { + \n\ + public static boolean test() { + final String s = ""; + return s.isEmpty(); + } + \n\ + public static boolean test2() { + return true; + } + }"""); + } + + @Test //JDK-8324859 + void testImplicitlyDeclaredClassesConfusion13() throws IOException { + String code = """ + package tests; + public class TestB { + public static boolean test() // missing open brace + final String s = ""; //field declaration? + public static boolean test2() { + return true; + } + }"""; + DiagnosticCollector 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(); + + List codes = new LinkedList<>(); + + for (Diagnostic d : coll.getDiagnostics()) { + codes.add(d.getLineNumber() + ":" + d.getColumnNumber() + ":" + d.getCode()); + } + + assertEquals("testImplicitlyDeclaredClassesConfusion1: " + codes, + List.of("3:33:compiler.err.expected2"), + codes); + String result = toStringWithErrors(cut).replaceAll("\\R", "\n"); + System.out.println("RESULT\n" + result); + assertEquals("incorrect AST", + result, + """ + package tests; + \n\ + public class TestB { + \n\ + public static boolean test(); + final String s = ""; + \n\ + public static boolean test2() { + return true; + } + }"""); + } + + @Test //JDK-8324859 + void testImplicitlyDeclaredClassesConfusion14() throws IOException { + String code = """ + package tests; + public class TestB { + public static boolean test() // missing open brace + String s = ""; + s.length(); + if (true); //force parse as block + public static boolean test2() { + return true; + } + }"""; + DiagnosticCollector 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(); + + List codes = new LinkedList<>(); + + for (Diagnostic d : coll.getDiagnostics()) { + codes.add(d.getLineNumber() + ":" + d.getColumnNumber() + ":" + d.getCode()); + } + + assertEquals("testImplicitlyDeclaredClassesConfusion1: " + codes, + List.of("3:33:compiler.err.expected2", + "7:5:compiler.err.illegal.start.of.expr"), + codes); + String result = toStringWithErrors(cut).replaceAll("\\R", "\n"); + System.out.println("RESULT\n" + result); + assertEquals("incorrect AST", + result, + """ + package tests; + \n\ + public class TestB { + \n\ + public static boolean test() { + String s = ""; + s.length(); + if (true) ; + (ERROR: ); + } + \n\ + public static boolean test2() { + return true; + } + }"""); + } + void run(String[] args) throws Exception { int passed = 0, failed = 0; final Pattern p = (args != null && args.length > 0)