This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch GROOVY-9192 in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 0fb62e46f9bc68e0e1d8082563a264f3be19a724 Author: Daniel Sun <[email protected]> AuthorDate: Sun Jul 19 22:53:14 2026 +0900 GROOVY-9192: Implement optional parser error recovery for Parrot parser --- .../apache/groovy/parser/antlr4/AstBuilder.java | 36 ++- .../internal/AbstractFriendlyErrorStrategy.java | 102 +++++++ .../antlr4/internal/DescriptiveErrorStrategy.java | 144 +++++----- .../RecoveringDescriptiveErrorStrategy.java | 49 ++++ .../parser/antlr4/internal/package-info.java | 4 +- .../groovy/control/CompilerConfiguration.java | 35 ++- .../org/codehaus/groovy/control/SourceUnit.java | 7 + .../apache/groovy/parser/antlr4/Groovy9192.groovy | 310 +++++++++++++++++++++ .../groovy/control/CompilerConfigurationTest.java | 20 ++ 9 files changed, 623 insertions(+), 84 deletions(-) diff --git a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java index deaaf9988d..644277f693 100644 --- a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java +++ b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java @@ -188,7 +188,12 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { this.lexer = new GroovyLangLexer(charStream); this.parser = new GroovyLangParser(new CommonTokenStream(this.lexer)); - this.parser.setErrorHandler(new DescriptiveErrorStrategy(charStream)); + + // Opt-in recovery (CompilerConfiguration.ERROR_RECOVERY): resync after syntax + // errors so IDEs can collect multiple diagnostics in one pass. Default is fail-fast. + // Multi-error truth for hosts is ErrorCollector; recovery may still yield a partial tree. + this.errorRecovery = sourceUnit.getConfiguration().isErrorRecoveryEnabled(); + this.parser.setErrorHandler(DescriptiveErrorStrategy.create(charStream, this.errorRecovery)); this.groovydocManager = new GroovydocManager(groovydocEnabled, runtimeGroovydocEnabled); this.tryWithResourcesASTTransformation = new TryWithResourcesASTTransformation(this); @@ -216,7 +221,12 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { AtnManager.READ_LOCK.lock(); try { final TokenStream tokenStream = parser.getInputStream(); - if (SLL_THRESHOLD >= 0 && tokenStream.size() > SLL_THRESHOLD) { + if (errorRecovery) { + // Recovery needs LL + listeners so every syntax error is reported. + // Skip SLL: recovering under SLL can "succeed" with a degraded tree + // and never re-run LL. + result = buildCST(PredictionMode.LL); + } else if (SLL_THRESHOLD >= 0 && tokenStream.size() > SLL_THRESHOLD) { // The more tokens to parse, the more possibility SLL will fail and the more parsing time will waste. // The option `groovy.antlr4.sll.threshold` could be tuned for better parsing performance, but it is disabled by default. // If the token count is greater than `groovy.antlr4.sll.threshold`, use LL directly. @@ -272,7 +282,15 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { public ModuleNode buildAST() { try { - return (ModuleNode) this.visit(this.buildCST()); + ModuleNode module = (ModuleNode) this.visit(this.buildCST()); + // Recovery can finish CST/AST with errors already recorded; surface failure + // so CompilationUnit / IDE hosts see a failed compilation with all diagnostics. + if (errorRecovery && sourceUnit.getErrorCollector().hasErrors()) { + throw new CompilationFailedException(CompilePhase.PARSING.getPhaseNumber(), this.sourceUnit); + } + return module; + } catch (CompilationFailedException e) { + throw e; } catch (Throwable t) { throw convertException(t); } @@ -4874,7 +4892,6 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { if (t instanceof SyntaxException) { this.collectSyntaxError((SyntaxException) t); } else if (t instanceof GroovySyntaxError groovySyntaxError) { - this.collectSyntaxError( new SyntaxException( groovySyntaxError.getMessage(), @@ -4892,7 +4909,14 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { } private void collectSyntaxError(final SyntaxException e) { - sourceUnit.getErrorCollector().addFatalError(new SyntaxErrorMessage(e, sourceUnit)); + SyntaxErrorMessage message = new SyntaxErrorMessage(e, sourceUnit); + if (errorRecovery) { + // Accumulate diagnostics so ANTLR resync can surface further errors in one pass. + // Fail-fast still uses addFatalError so the first error stops compilation immediately. + sourceUnit.getErrorCollector().addErrorAndContinue(message); + } else { + sourceUnit.getErrorCollector().addFatalError(message); + } } private void collectException(final Exception e) { @@ -4968,6 +4992,8 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { private final GroovyLangParser parser; private final GroovydocManager groovydocManager; private final TryWithResourcesASTTransformation tryWithResourcesASTTransformation; + /** {@code true} when {@link org.codehaus.groovy.control.CompilerConfiguration#ERROR_RECOVERY} is enabled. */ + private final boolean errorRecovery; private final List<ClassNode> classNodeList = new ArrayList<>(); private final Deque<ClassNode> classNodeStack = new ArrayDeque<>(); diff --git a/src/main/java/org/apache/groovy/parser/antlr4/internal/AbstractFriendlyErrorStrategy.java b/src/main/java/org/apache/groovy/parser/antlr4/internal/AbstractFriendlyErrorStrategy.java new file mode 100644 index 0000000000..1b9098dc58 --- /dev/null +++ b/src/main/java/org/apache/groovy/parser/antlr4/internal/AbstractFriendlyErrorStrategy.java @@ -0,0 +1,102 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.groovy.parser.antlr4.internal; + +import org.antlr.v4.runtime.CharStream; +import org.antlr.v4.runtime.DefaultErrorStrategy; +import org.antlr.v4.runtime.FailedPredicateException; +import org.antlr.v4.runtime.InputMismatchException; +import org.antlr.v4.runtime.NoViableAltException; +import org.antlr.v4.runtime.Parser; +import org.antlr.v4.runtime.RecognitionException; +import org.antlr.v4.runtime.Token; +import org.antlr.v4.runtime.TokenStream; +import org.antlr.v4.runtime.misc.Interval; + +/** + * Shared friendly recognition diagnostics for Parrot error strategies. + * <p> + * Subclasses choose control flow ({@link DescriptiveErrorStrategy} fail-fast vs + * {@link RecoveringDescriptiveErrorStrategy} multi-error resync). Reporting — + * including {@link MissingDelimiterDiagnostic} — stays here so both modes share + * one message path. + * </p> + */ +abstract class AbstractFriendlyErrorStrategy extends DefaultErrorStrategy { + + private final CharStream charStream; + + AbstractFriendlyErrorStrategy(final CharStream charStream) { + this.charStream = charStream; + } + + /** + * Prefer a precise "Missing …" delimiter diagnostic when the token stream + * clearly indicates an unclosed / incomplete construct; otherwise fall + * back to the generic message. + */ + private void reportFriendlyError(final Parser recognizer, final RecognitionException e, final String fallbackMessage) { + MissingDelimiterDiagnostic.Hit hit = + MissingDelimiterDiagnostic.locate(recognizer.getInputStream(), e); + if (hit != null) { + recognizer.notifyErrorListeners(hit.at, hit.message, e); + return; + } + notifyErrorListeners(recognizer, fallbackMessage, e); + } + + protected String createNoViableAlternativeErrorMessage(final Parser recognizer, final NoViableAltException e) { + TokenStream tokens = recognizer.getInputStream(); + String input; + if (tokens != null) { + if (e.getStartToken().getType() == Token.EOF) { + input = "<EOF>"; + } else { + input = charStream.getText(Interval.of(e.getStartToken().getStartIndex(), e.getOffendingToken().getStopIndex())); + } + } else { + input = "<unknown input>"; + } + + return "Unexpected input: " + escapeWSAndQuote(input); + } + + @Override + protected void reportNoViableAlternative(final Parser recognizer, final NoViableAltException e) { + reportFriendlyError(recognizer, e, createNoViableAlternativeErrorMessage(recognizer, e)); + } + + protected String createInputMismatchErrorMessage(final Parser recognizer, final InputMismatchException e) { + return "Unexpected input: " + getTokenErrorDisplay(e.getOffendingToken(recognizer)); + } + + @Override + protected void reportInputMismatch(final Parser recognizer, final InputMismatchException e) { + reportFriendlyError(recognizer, e, createInputMismatchErrorMessage(recognizer, e)); + } + + protected String createFailedPredicateErrorMessage(final Parser recognizer, final FailedPredicateException e) { + return e.getMessage(); + } + + @Override + protected void reportFailedPredicate(final Parser recognizer, final FailedPredicateException e) { + notifyErrorListeners(recognizer, createFailedPredicateErrorMessage(recognizer, e), e); + } +} diff --git a/src/main/java/org/apache/groovy/parser/antlr4/internal/DescriptiveErrorStrategy.java b/src/main/java/org/apache/groovy/parser/antlr4/internal/DescriptiveErrorStrategy.java index aa581e1d36..08b731f850 100644 --- a/src/main/java/org/apache/groovy/parser/antlr4/internal/DescriptiveErrorStrategy.java +++ b/src/main/java/org/apache/groovy/parser/antlr4/internal/DescriptiveErrorStrategy.java @@ -18,7 +18,7 @@ */ package org.apache.groovy.parser.antlr4.internal; -import org.antlr.v4.runtime.BailErrorStrategy; +import org.antlr.v4.runtime.ANTLRErrorStrategy; import org.antlr.v4.runtime.CharStream; import org.antlr.v4.runtime.FailedPredicateException; import org.antlr.v4.runtime.InputMismatchException; @@ -27,113 +27,103 @@ import org.antlr.v4.runtime.Parser; import org.antlr.v4.runtime.ParserRuleContext; import org.antlr.v4.runtime.RecognitionException; import org.antlr.v4.runtime.Token; -import org.antlr.v4.runtime.TokenStream; import org.antlr.v4.runtime.atn.PredictionMode; -import org.antlr.v4.runtime.misc.Interval; import org.antlr.v4.runtime.misc.ParseCancellationException; /** - * Provide friendly error messages when parsing errors occurred. + * Fail-fast parser error strategy with friendly diagnostics (default Parrot mode). * <p> - * Extends {@link BailErrorStrategy} so the successful-parse path stays - * allocation-free and ATN-light (no grammar-level error alternatives — - * those were removed after GROOVY-9588). Missing-delimiter diagnostics - * ({@code ')'}, {@code ']'}, {@code '}'}) run only after a recognition - * failure via {@link MissingDelimiterDiagnostic}. + * Cancels the parse with {@link ParseCancellationException} after reporting, + * matching {@link org.antlr.v4.runtime.BailErrorStrategy}. {@link #sync} is a + * no-op so the SLL stage of two-stage parsing stays cheap. Diagnostics are + * emitted from {@link #recover}: after a failed SLL probe the strategy may still + * be in ANTLR's internal recovery mode, which suppresses + * {@link #reportError}, while {@code recover} still runs under LL. * </p> + * <p> + * For IDE multi-error collection use {@link #create(CharStream, boolean)} with + * {@code recover == true}, which selects + * {@link RecoveringDescriptiveErrorStrategy}. Hosts should treat + * {@link org.codehaus.groovy.control.ErrorCollector} as the multi-error source + * of truth: recovery may still produce a partial tree that fails later during + * AST building. + * </p> + * + * @see RecoveringDescriptiveErrorStrategy + * @see org.codehaus.groovy.control.CompilerConfiguration#ERROR_RECOVERY */ -public class DescriptiveErrorStrategy extends BailErrorStrategy { - private final CharStream charStream; +public class DescriptiveErrorStrategy extends AbstractFriendlyErrorStrategy { - public DescriptiveErrorStrategy(CharStream charStream) { - this.charStream = charStream; + /** + * Select fail-fast or recovering strategy. + * + * @param charStream source character stream used for snippet extraction + * @param recover {@code true} for multi-error resync; {@code false} for fail-fast + * @return an error strategy instance (never {@code null}) + */ + public static ANTLRErrorStrategy create(final CharStream charStream, final boolean recover) { + return recover + ? new RecoveringDescriptiveErrorStrategy(charStream) + : new DescriptiveErrorStrategy(charStream); + } + + /** + * Fail-fast strategy with friendly diagnostics. + * + * @param charStream source character stream used for snippet extraction + */ + public DescriptiveErrorStrategy(final CharStream charStream) { + super(charStream); } + /** + * {@inheritDoc} + * <p> + * Mark incomplete contexts, emit a friendly LL diagnostic, then cancel. + * </p> + */ @Override - public void recover(Parser recognizer, RecognitionException e) { + public void recover(final Parser recognizer, final RecognitionException e) { for (ParserRuleContext context = recognizer.getContext(); context != null; context = context.getParent()) { context.exception = e; } + // Report here (not only via reportError): after a failed SLL stage the + // shared strategy may still have errorRecoveryMode=true, which makes + // DefaultErrorStrategy.reportError a no-op on the LL retry. if (PredictionMode.LL.equals(recognizer.getInterpreter().getPredictionMode())) { if (e instanceof NoViableAltException) { - this.reportNoViableAlternative(recognizer, (NoViableAltException) e); + reportNoViableAlternative(recognizer, (NoViableAltException) e); } else if (e instanceof InputMismatchException) { - this.reportInputMismatch(recognizer, (InputMismatchException) e); + reportInputMismatch(recognizer, (InputMismatchException) e); } else if (e instanceof FailedPredicateException) { - this.reportFailedPredicate(recognizer, (FailedPredicateException) e); + reportFailedPredicate(recognizer, (FailedPredicateException) e); } } throw new ParseCancellationException(e); } + /** + * {@inheritDoc} + * <p> + * Route through {@link #recover} so diagnostics and cancel stay unified. + * </p> + */ @Override - public Token recoverInline(Parser recognizer) - throws RecognitionException { - - this.recover(recognizer, new InputMismatchException(recognizer)); // stop parsing + public Token recoverInline(final Parser recognizer) throws RecognitionException { + recover(recognizer, new InputMismatchException(recognizer)); // stop parsing return null; } /** - * Prefer a precise "Missing …" delimiter diagnostic when the token stream - * clearly indicates an unclosed / incomplete construct; otherwise fall - * back to the generic "Unexpected input" message. + * {@inheritDoc} + * <p> + * No-op (matches {@link org.antlr.v4.runtime.BailErrorStrategy}) so SLL stays cheap. + * </p> */ - private void reportError(Parser recognizer, RecognitionException e, String fallbackMessage) { - MissingDelimiterDiagnostic.Hit hit = - MissingDelimiterDiagnostic.locate(recognizer.getInputStream(), e); - if (hit != null) { - recognizer.notifyErrorListeners(hit.at, hit.message, e); - return; - } - notifyErrorListeners(recognizer, fallbackMessage, e); - } - - protected String createNoViableAlternativeErrorMessage(Parser recognizer, NoViableAltException e) { - TokenStream tokens = recognizer.getInputStream(); - String input; - if (tokens != null) { - if (e.getStartToken().getType() == Token.EOF) { - input = "<EOF>"; - } else { - input = charStream.getText(Interval.of(e.getStartToken().getStartIndex(), e.getOffendingToken().getStopIndex())); - } - } else { - input = "<unknown input>"; - } - - return "Unexpected input: " + escapeWSAndQuote(input); - } - - @Override - protected void reportNoViableAlternative(Parser recognizer, - NoViableAltException e) { - - reportError(recognizer, e, this.createNoViableAlternativeErrorMessage(recognizer, e)); - } - - protected String createInputMismatchErrorMessage(Parser recognizer, - InputMismatchException e) { - return "Unexpected input: " + getTokenErrorDisplay(e.getOffendingToken(recognizer)); - } - - @Override - protected void reportInputMismatch(Parser recognizer, - InputMismatchException e) { - - reportError(recognizer, e, this.createInputMismatchErrorMessage(recognizer, e)); - } - - protected String createFailedPredicateErrorMessage(Parser recognizer, - FailedPredicateException e) { - return e.getMessage(); - } - @Override - protected void reportFailedPredicate(Parser recognizer, - FailedPredicateException e) { - notifyErrorListeners(recognizer, this.createFailedPredicateErrorMessage(recognizer, e), e); + public void sync(final Parser recognizer) { + // intentionally empty } } diff --git a/src/main/java/org/apache/groovy/parser/antlr4/internal/RecoveringDescriptiveErrorStrategy.java b/src/main/java/org/apache/groovy/parser/antlr4/internal/RecoveringDescriptiveErrorStrategy.java new file mode 100644 index 0000000000..9db6a3d2d7 --- /dev/null +++ b/src/main/java/org/apache/groovy/parser/antlr4/internal/RecoveringDescriptiveErrorStrategy.java @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.groovy.parser.antlr4.internal; + +import org.antlr.v4.runtime.CharStream; + +/** + * Recovering parser error strategy with the same friendly diagnostics as + * {@link DescriptiveErrorStrategy}. + * <p> + * Uses ANTLR's default resync / single-token repair so multiple recognition + * errors can be reported in one pass (IDE editing). Prefer construction via + * {@link DescriptiveErrorStrategy#create(CharStream, boolean)}. + * </p> + * <p> + * Callers must parse with LL prediction and error listeners installed. Hosts + * should read diagnostics from + * {@link org.codehaus.groovy.control.ErrorCollector}: a partial tree may still + * fail during AST building after recovery. + * </p> + * + * @see DescriptiveErrorStrategy + * @see org.codehaus.groovy.control.CompilerConfiguration#ERROR_RECOVERY + */ +public final class RecoveringDescriptiveErrorStrategy extends AbstractFriendlyErrorStrategy { + + /** + * @param charStream source character stream used for snippet extraction + */ + public RecoveringDescriptiveErrorStrategy(final CharStream charStream) { + super(charStream); + } +} diff --git a/src/main/java/org/apache/groovy/parser/antlr4/internal/package-info.java b/src/main/java/org/apache/groovy/parser/antlr4/internal/package-info.java index f1e8439d37..adfc5e5381 100644 --- a/src/main/java/org/apache/groovy/parser/antlr4/internal/package-info.java +++ b/src/main/java/org/apache/groovy/parser/antlr4/internal/package-info.java @@ -18,6 +18,8 @@ */ /** - * Internal utilities and support classes for ANTLR4 parser operation. Handles parser configuration, error handling, and internal state management. + * Internal utilities for the ANTLR4 (Parrot) parser: ATN management, friendly + * error strategies ({@link DescriptiveErrorStrategy} fail-fast, + * {@link RecoveringDescriptiveErrorStrategy} multi-error), and related diagnostics. */ package org.apache.groovy.parser.antlr4.internal; diff --git a/src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java b/src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java index 95c2a98e19..71cf670b01 100644 --- a/src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java +++ b/src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java @@ -64,6 +64,21 @@ public class CompilerConfiguration { /** Optimization Option for enabling parallel parsing. */ public static final String PARALLEL_PARSE = "parallelParse"; + /** + * Option for enabling ANTLR parser error recovery on the Parrot parser. + * <p> + * Disabled by default. When enabled (typically for IDE editing), the parser + * resynchronizes after recognition errors so multiple diagnostics can be + * collected in one pass instead of failing fast on the first error. + * Stored in {@link #getOptimizationOptions()} for consistency with other + * compiler feature flags (e.g. {@link #GROOVYDOC}). + * </p> + * + * @see #isErrorRecoveryEnabled() + * @since 5.0.0 + */ + public static final String ERROR_RECOVERY = "errorRecovery"; + /** Joint Compilation Option for enabling generating stubs in memory. */ public static final String MEM_STUB = "memStub"; @@ -456,6 +471,7 @@ public class CompilerConfiguration { * <tr><td><code>groovy.parallel.parse</code></td><td>{@link #getOptimizationOptions}</td></tr> * <tr><td><code>groovy.attach.groovydoc</code></td><td>{@link #getOptimizationOptions}</td></tr> * <tr><td><code>groovy.attach.runtime.groovydoc</code></td><td>{@link #getOptimizationOptions}</td></tr> + * <tr><td><code>groovy.parser.error.recovery</code></td><td>{@link #getOptimizationOptions} / {@link #isErrorRecoveryEnabled}</td></tr> * </table> * </blockquote> */ @@ -475,11 +491,12 @@ public class CompilerConfiguration { setTargetBytecodeIfValid(getSystemPropertySafe("groovy.target.bytecode", DEFAULT_TARGET_BYTECODE)); defaultScriptExtension = getSystemPropertySafe("groovy.default.scriptExtension", ".groovy"); - optimizationOptions = new HashMap<>(4); + optimizationOptions = new HashMap<>(5); handleOptimizationOption(INVOKEDYNAMIC, getSystemPropertySafe("groovy.target.indy", "true")); handleOptimizationOption(GROOVYDOC, getSystemPropertySafe("groovy.attach.groovydoc")); handleOptimizationOption(RUNTIME_GROOVYDOC, getSystemPropertySafe("groovy.attach.runtime.groovydoc")); handleOptimizationOption(PARALLEL_PARSE, getSystemPropertySafe("groovy.parallel.parse", "true")); + handleOptimizationOption(ERROR_RECOVERY, getSystemPropertySafe("groovy.parser.error.recovery")); if (getBooleanSafe("groovy.mem.stub")) { jointCompilationOptions = new HashMap<>(2); @@ -1369,4 +1386,20 @@ public class CompilerConfiguration { public boolean isRuntimeGroovydocEnabled() { return Boolean.TRUE.equals(getOptimizationOptions().get(RUNTIME_GROOVYDOC)); } + + /** + * Checks if ANTLR parser error recovery is enabled. + * <p> + * Disabled by default. Enable via {@link #ERROR_RECOVERY} or the + * {@code groovy.parser.error.recovery} system property. Intended for IDE + * editing (multiple syntax diagnostics per pass). Leave off for production + * compilation: fail-fast is cheaper and avoids building partial trees. + * </p> + * + * @return {@code true} if parser error recovery is enabled + * @since 5.0.0 + */ + public boolean isErrorRecoveryEnabled() { + return Boolean.TRUE.equals(getOptimizationOptions().get(ERROR_RECOVERY)); + } } diff --git a/src/main/java/org/codehaus/groovy/control/SourceUnit.java b/src/main/java/org/codehaus/groovy/control/SourceUnit.java index fc78c9ad21..0685ac7654 100644 --- a/src/main/java/org/codehaus/groovy/control/SourceUnit.java +++ b/src/main/java/org/codehaus/groovy/control/SourceUnit.java @@ -254,6 +254,13 @@ public class SourceUnit extends ProcessingUnit { try { this.ast = parserPlugin.buildAST(this, this.classLoader, this.cst); this.ast.setDescription(this.name); + } catch (CompilationFailedException e) { + // Parser plugins (e.g. Parrot) report via ErrorCollector and may throw + // CompilationFailedException; keep an empty module so later phases can run + // and so multi-error recovery can finish collecting diagnostics. + if (this.ast == null) { + this.ast = new ModuleNode(this); + } } catch (SyntaxException e) { if (this.ast == null) { // create an empty ModuleNode to represent a failed parse, in case a later phase attempts to use the AST diff --git a/src/test/groovy/org/apache/groovy/parser/antlr4/Groovy9192.groovy b/src/test/groovy/org/apache/groovy/parser/antlr4/Groovy9192.groovy new file mode 100644 index 0000000000..14dbabd41f --- /dev/null +++ b/src/test/groovy/org/apache/groovy/parser/antlr4/Groovy9192.groovy @@ -0,0 +1,310 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.groovy.parser.antlr4 + +import org.antlr.v4.runtime.CharStreams +import org.antlr.v4.runtime.CommonTokenStream +import org.antlr.v4.runtime.InputMismatchException +import org.antlr.v4.runtime.ParserRuleContext +import org.antlr.v4.runtime.atn.PredictionMode +import org.antlr.v4.runtime.misc.ParseCancellationException +import org.apache.groovy.parser.antlr4.internal.DescriptiveErrorStrategy +import org.apache.groovy.parser.antlr4.internal.RecoveringDescriptiveErrorStrategy +import org.codehaus.groovy.control.CompilationFailedException +import org.codehaus.groovy.control.CompilationUnit +import org.codehaus.groovy.control.CompilerConfiguration +import org.codehaus.groovy.control.ErrorCollector +import org.codehaus.groovy.control.Phases +import org.codehaus.groovy.control.SourceUnit +import org.codehaus.groovy.control.io.StringReaderSource +import org.codehaus.groovy.control.messages.SyntaxErrorMessage +import org.junit.jupiter.api.Test + +import static org.codehaus.groovy.control.CompilerConfiguration.ERROR_RECOVERY +import static org.junit.jupiter.api.Assertions.assertEquals +import static org.junit.jupiter.api.Assertions.assertFalse +import static org.junit.jupiter.api.Assertions.assertInstanceOf +import static org.junit.jupiter.api.Assertions.assertNotNull +import static org.junit.jupiter.api.Assertions.assertThrows +import static org.junit.jupiter.api.Assertions.assertTrue + +/** + * GROOVY-9192: optional ANTLR error recovery for the Parrot parser. + * <p> + * Recovery is off by default (fail-fast). Enabling {@code errorRecovery} + * resynchronizes after recognition errors so multiple diagnostics can be + * collected in one pass (IDE editing). Hosts should trust + * {@link ErrorCollector} as the multi-error source of truth. + * </p> + */ +final class Groovy9192 { + + //-------------------------------------------------------------------------- + // Configuration + //-------------------------------------------------------------------------- + + @Test + void 'error recovery is disabled by default'() { + assertFalse new CompilerConfiguration().errorRecoveryEnabled + assertFalse CompilerConfiguration.DEFAULT.errorRecoveryEnabled + assertEquals 'errorRecovery', ERROR_RECOVERY + } + + @Test + void 'error recovery can be enabled via optimization option'() { + def config = new CompilerConfiguration() + config.optimizationOptions[ERROR_RECOVERY] = true + assertTrue config.errorRecoveryEnabled + + config.optimizationOptions[ERROR_RECOVERY] = false + assertFalse config.errorRecoveryEnabled + + config.optimizationOptions[ERROR_RECOVERY] = null + assertFalse config.errorRecoveryEnabled + } + + @Test + void 'copy constructor preserves error recovery option'() { + def source = new CompilerConfiguration() + source.optimizationOptions[ERROR_RECOVERY] = true + assertTrue new CompilerConfiguration(source).errorRecoveryEnabled + } + + @Test + void 'DescriptiveErrorStrategy create selects fail-fast or recovering strategy'() { + def stream = CharStreams.fromString('1') + assertInstanceOf DescriptiveErrorStrategy, DescriptiveErrorStrategy.create(stream, false) + assertInstanceOf RecoveringDescriptiveErrorStrategy, DescriptiveErrorStrategy.create(stream, true) + } + + //-------------------------------------------------------------------------- + // Fail-fast (default) — compatibility with existing contracts + //-------------------------------------------------------------------------- + + @Test + void 'default fail-fast still reports a friendly syntax error'() { + def errors = collectErrors('class C {\n def x = (\n', false) + assertTrue errors.size() >= 1, "expected at least one error, got: $errors" + assertTrue errors.any { it.contains("Missing ')'") || it.toLowerCase().contains('unexpected') }, + "unexpected diagnostics: $errors" + } + + @Test + void 'default fail-fast cancels parse via ParseCancellationException'() { + def charStream = CharStreams.fromString('class C {') + def strategy = new DescriptiveErrorStrategy(charStream) + + def lexer = new GroovyLangLexer(charStream) + def parser = new GroovyLangParser(new CommonTokenStream(lexer)) + parser.errorHandler = strategy + parser.interpreter.predictionMode = PredictionMode.LL + parser.removeErrorListeners() + + assertThrows(ParseCancellationException) { + parser.compilationUnit() + } + } + + @Test + void 'fail-fast recoverInline cancels on mismatched token'() { + // "class" alone: Identifier expected after CLASS → recoverInline bail path. + def errors = collectErrors('class', false) + assertTrue errors.size() >= 1, String.valueOf(errors) + } + + @Test + void 'fail-fast recover marks incomplete rule contexts before cancelling'() { + // Under SLL the strategy does not report (avoids needing a live ATN state); + // it only marks contexts and cancels — the BailErrorStrategy contract. + def charStream = CharStreams.fromString('}') + def strategy = new DescriptiveErrorStrategy(charStream) + def lexer = new GroovyLangLexer(charStream) + def parser = new GroovyLangParser(new CommonTokenStream(lexer)) + parser.errorHandler = strategy + parser.interpreter.predictionMode = PredictionMode.SLL + parser.removeErrorListeners() + + def parent = new ParserRuleContext() + def child = new ParserRuleContext(parent, 0) + parser.context = child + + def e = new InputMismatchException(parser) + def pce = assertThrows(ParseCancellationException) { + strategy.recover(parser, e) + } + assertTrue pce.cause instanceof InputMismatchException + assertEquals e, child.exception + assertEquals e, parent.exception + } + + @Test + void 'fail-fast recoverInline routes through recover and cancels'() { + def charStream = CharStreams.fromString('}') + def strategy = new DescriptiveErrorStrategy(charStream) + def lexer = new GroovyLangLexer(charStream) + def parser = new GroovyLangParser(new CommonTokenStream(lexer)) + parser.errorHandler = strategy + parser.interpreter.predictionMode = PredictionMode.SLL + parser.removeErrorListeners() + parser.context = new ParserRuleContext() + + assertThrows(ParseCancellationException) { + strategy.recoverInline(parser) + } + } + + @Test + void 'fail-fast sync is a no-op'() { + def charStream = CharStreams.fromString('1') + def strategy = new DescriptiveErrorStrategy(charStream) + def parser = new GroovyLangParser(new CommonTokenStream(new GroovyLangLexer(charStream))) + parser.errorHandler = strategy + def indexBefore = parser.inputStream.index() + strategy.sync(parser) + assertEquals indexBefore, parser.inputStream.index() + } + + //-------------------------------------------------------------------------- + // Recovery mode + //-------------------------------------------------------------------------- + + @Test + void 'recovery mode reports more diagnostics than fail-fast for multi-fault source'() { + // Two top-level broken classes so recovery can resync past the first fault. + String source = '''\ + |class A { + | def x = ( + |} + |class B { + | def y = [ + |} + |'''.stripMargin() + + def failFastErrors = collectErrors(source, false) + def recoveryErrors = collectErrors(source, true) + + assertTrue failFastErrors.size() >= 1, "fail-fast: $failFastErrors" + // Product claim: multi-error collection — recovery must surface strictly more + // diagnostics than fail-fast (which stops at the first fatal recognition error). + assertTrue recoveryErrors.size() > failFastErrors.size(), + "recovery (${recoveryErrors.size()}) must report more than fail-fast (${failFastErrors.size()})\nfail-fast: $failFastErrors\nrecovery: $recoveryErrors" + assertTrue recoveryErrors.size() >= 2, "recovery should collect multiple diagnostics: $recoveryErrors" + // Continued parse past the first fault should leave more than one unique message + // and/or include delimiter diagnostics from the recovering strategy. + assertTrue recoveryErrors.any { it.contains("Missing ')'") || it.contains("Missing ']'") || it.contains("Missing '}'") }, + "expected a Missing-delimiter diagnostic in: $recoveryErrors" + } + + @Test + void 'recovery mode still fails the compilation'() { + def config = recoveryConfig() + def cu = new CompilationUnit(config) + cu.addSource('Broken.groovy', 'class C {\n def x = (\n') + assertThrows(CompilationFailedException) { + cu.compile(Phases.CONVERSION) + } + assertTrue cu.errorCollector.hasErrors() + } + + @Test + void 'recovery mode keeps friendly Missing delimiter diagnostics'() { + def errors = collectErrors('class C {\n def x\n', true) + assertTrue errors.any { it.contains("Missing '}'") }, "expected Missing '}', got: $errors" + } + + @Test + void 'recovery strategy resynchronizes instead of cancelling'() { + def charStream = CharStreams.fromString('class C { def x = ( } class D {}') + def strategy = new RecoveringDescriptiveErrorStrategy(charStream) + + def lexer = new GroovyLangLexer(charStream) + def parser = new GroovyLangParser(new CommonTokenStream(lexer)) + parser.errorHandler = strategy + parser.interpreter.predictionMode = PredictionMode.LL + parser.removeErrorListeners() + + assertNotNull parser.compilationUnit() + } + + @Test + void 'recovery mode parse of mismatched tokens does not throw ParseCancellationException'() { + def charStream = CharStreams.fromString('def x = (]\ndef y = 1\n') + def strategy = DescriptiveErrorStrategy.create(charStream, true) + def lexer = new GroovyLangLexer(charStream) + def parser = new GroovyLangParser(new CommonTokenStream(lexer)) + parser.errorHandler = strategy + parser.interpreter.predictionMode = PredictionMode.LL + parser.removeErrorListeners() + + assertNotNull parser.compilationUnit() + } + + @Test + void 'AstBuilder wires recovery from CompilerConfiguration for valid source'() { + def config = recoveryConfig() + def sourceUnit = new SourceUnit( + 't.groovy', + new StringReaderSource('class C {}', config), + config, + null, + new ErrorCollector(config) + ) + def module = new AstBuilder(sourceUnit, false, false).buildAST() + assertNotNull module + assertFalse sourceUnit.errorCollector.hasErrors() + } + + @Test + void 'EOF unexpected input diagnostics still surface end-to-end'() { + // End-to-end path that exercises NVAE-style messaging without invokeMethod. + def errors = collectErrors('class', false) + assertTrue errors.size() >= 1, String.valueOf(errors) + } + + //-------------------------------------------------------------------------- + // helpers + //-------------------------------------------------------------------------- + + private static CompilerConfiguration recoveryConfig() { + def config = new CompilerConfiguration() + config.optimizationOptions[ERROR_RECOVERY] = true + config + } + + /** + * Compile {@code source} through CONVERSION and return syntax error messages + * (empty if compilation somehow succeeded). + */ + private static List<String> collectErrors(String source, boolean recovery) { + def config = new CompilerConfiguration() + if (recovery) { + config.optimizationOptions[ERROR_RECOVERY] = true + } + def cu = new CompilationUnit(config) + cu.addSource('test.groovy', source) + try { + cu.compile(Phases.CONVERSION) + return Collections.emptyList() + } catch (CompilationFailedException ignored) { + return cu.errorCollector.errors + .findAll { it instanceof SyntaxErrorMessage } + .collect { ((SyntaxErrorMessage) it).cause.message } + } + } +} diff --git a/src/test/groovy/org/codehaus/groovy/control/CompilerConfigurationTest.java b/src/test/groovy/org/codehaus/groovy/control/CompilerConfigurationTest.java index 4251d187c7..92a34633e3 100644 --- a/src/test/groovy/org/codehaus/groovy/control/CompilerConfigurationTest.java +++ b/src/test/groovy/org/codehaus/groovy/control/CompilerConfigurationTest.java @@ -58,6 +58,26 @@ public final class CompilerConfigurationTest { assertNotNull(config.getPluginFactory()); assertNull(config.getScriptBaseClass()); assertNull(config.getTargetDirectory()); + // GROOVY-9192: parser error recovery is opt-in + assertFalse(config.isErrorRecoveryEnabled()); + assertEquals("errorRecovery", CompilerConfiguration.ERROR_RECOVERY); + } + + @Test + public void testErrorRecoveryOptimizationOption() { + CompilerConfiguration config = new CompilerConfiguration(); + assertFalse(config.isErrorRecoveryEnabled()); + + config.getOptimizationOptions().put(CompilerConfiguration.ERROR_RECOVERY, Boolean.TRUE); + assertTrue(config.isErrorRecoveryEnabled()); + + config.getOptimizationOptions().put(CompilerConfiguration.ERROR_RECOVERY, Boolean.FALSE); + assertFalse(config.isErrorRecoveryEnabled()); + + CompilerConfiguration copy = new CompilerConfiguration(config); + assertFalse(copy.isErrorRecoveryEnabled()); + copy.getOptimizationOptions().put(CompilerConfiguration.ERROR_RECOVERY, Boolean.TRUE); + assertTrue(copy.isErrorRecoveryEnabled()); } @Test
