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 5c00b9f882deb9591d3f21a3f2a49e5a148edd71 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 | 42 ++- .../internal/AbstractFriendlyErrorStrategy.java | 108 ++++++++ .../antlr4/internal/DescriptiveErrorStrategy.java | 163 ++++++----- .../RecoveringDescriptiveErrorStrategy.java | 49 ++++ .../parser/antlr4/internal/package-info.java | 4 +- .../groovy/control/CompilerConfiguration.java | 35 ++- .../apache/groovy/parser/antlr4/Groovy9192.groovy | 303 +++++++++++++++++++++ .../groovy/control/CompilerConfigurationTest.java | 20 ++ .../antlr4/internal/ErrorStrategyCoverageTest.java | 156 +++++++++++ 9 files changed, 791 insertions(+), 89 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..c0b016bb10 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. @@ -273,8 +283,22 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { public ModuleNode buildAST() { try { return (ModuleNode) this.visit(this.buildCST()); + } catch (CompilationFailedException e) { + // Recovery records diagnostics via addErrorAndContinue and must not depend on + // SourceUnit swallowing CFE. Return the partially built module; CompilationUnit + // fails the phase via ErrorCollector.failIfErrors() with all diagnostics. + if (errorRecovery) { + return this.moduleNode; + } + throw e; } catch (Throwable t) { - throw convertException(t); + // convertException may throw under fail-fast (addFatalError) or return a CFE + // after recording under recovery. + CompilationFailedException cfe = convertException(t); + if (errorRecovery) { + return this.moduleNode; + } + throw cfe; } } @@ -4874,7 +4898,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 +4915,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 +4998,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..e97e3e861b --- /dev/null +++ b/src/main/java/org/apache/groovy/parser/antlr4/internal/AbstractFriendlyErrorStrategy.java @@ -0,0 +1,108 @@ +/* + * 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 = null; + try { + // Incomplete / synthetic contexts can leave token indices out of range. + hit = MissingDelimiterDiagnostic.locate(recognizer.getInputStream(), e); + } catch (IndexOutOfBoundsException | IllegalArgumentException ignored) { + // Fall through to the generic message. Catch only locate()'s known + // defensive failures — never listener-side fatals (e.g. addFatalError). + } + 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..b23958546a 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,112 @@ 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 { + + /** + * 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); + } - public DescriptiveErrorStrategy(CharStream charStream) { - this.charStream = charStream; + /** + * {@inheritDoc} + * <p> + * Mark incomplete contexts, emit a friendly LL diagnostic, then cancel. + * </p> + */ + @Override + public void recover(final Parser recognizer, final RecognitionException e) { + throw cancel(recognizer, e); } + /** + * {@inheritDoc} + * <p> + * Bail-style: never return a synthetic token; report then cancel. + * </p> + */ @Override - public void recover(Parser recognizer, RecognitionException e) { + public Token recoverInline(final Parser recognizer) throws RecognitionException { + throw cancel(recognizer, new InputMismatchException(recognizer)); + } + + /** + * Shared fail-fast cancel path for {@link #recover} and {@link #recoverInline}. + * Marks incomplete contexts, emits an LL diagnostic when applicable, and + * returns a {@link ParseCancellationException} for the caller to throw + * (bail-style: control never resumes in the strategy after cancel). + */ + private ParseCancellationException cancel(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); - } - - @Override - public Token recoverInline(Parser recognizer) - throws RecognitionException { - - this.recover(recognizer, new InputMismatchException(recognizer)); // stop parsing - return null; + return new ParseCancellationException(e); } /** - * 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/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..83a088a7ed --- /dev/null +++ b/src/test/groovy/org/apache/groovy/parser/antlr4/Groovy9192.groovy @@ -0,0 +1,303 @@ +/* + * 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) + assertEquals 1, errors.size(), "fail-fast must stop at first fatal: $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 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 cancels with InputMismatchException cause'() { + 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() + + def pce = assertThrows(ParseCancellationException) { + strategy.recoverInline(parser) + } + assertTrue pce.cause instanceof InputMismatchException + } + + @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() + } + + @Test + void 'fail-fast collectSyntaxError path still aborts on first error'() { + def errors = collectErrors('class C { def x = ( }', false) + assertEquals 1, errors.size(), String.valueOf(errors) + } + + //-------------------------------------------------------------------------- + // 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) + + assertEquals 1, failFastErrors.size(), "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" + 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 '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 'recovery buildAST leaves module and collector for CompilationUnit to fail'() { + // Recovery must not require SourceUnit to swallow CompilationFailedException: + // AstBuilder returns a module with diagnostics already in the ErrorCollector. + def config = recoveryConfig() + def cu = new CompilationUnit(config) + cu.addSource('Broken.groovy', 'class C {\n def x = (\n') + assertThrows(CompilationFailedException) { + cu.compile(Phases.CONVERSION) + } + def unit = cu.sources.values().iterator().next() + assertNotNull unit.AST, 'recovery should materialize a module without SourceUnit CFE catch' + assertTrue unit.errorCollector.hasErrors() + } + + //-------------------------------------------------------------------------- + // 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 diff --git a/src/test/java/org/apache/groovy/parser/antlr4/internal/ErrorStrategyCoverageTest.java b/src/test/java/org/apache/groovy/parser/antlr4/internal/ErrorStrategyCoverageTest.java new file mode 100644 index 0000000000..c9b97e188f --- /dev/null +++ b/src/test/java/org/apache/groovy/parser/antlr4/internal/ErrorStrategyCoverageTest.java @@ -0,0 +1,156 @@ +/* + * 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.BaseErrorListener; +import org.antlr.v4.runtime.CharStreams; +import org.antlr.v4.runtime.CommonTokenStream; +import org.antlr.v4.runtime.InputMismatchException; +import org.antlr.v4.runtime.NoViableAltException; +import org.antlr.v4.runtime.Parser; +import org.antlr.v4.runtime.ParserRuleContext; +import org.antlr.v4.runtime.RecognitionException; +import org.antlr.v4.runtime.Recognizer; +import org.antlr.v4.runtime.Token; +import org.antlr.v4.runtime.atn.PredictionMode; +import org.antlr.v4.runtime.misc.ParseCancellationException; +import org.apache.groovy.parser.antlr4.GroovyLangLexer; +import org.apache.groovy.parser.antlr4.GroovyLangParser; +import org.apache.groovy.parser.antlr4.GroovyParser; +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.List; + +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; + +/** + * Behavioural contracts for GROOVY-9192 error strategies (package-local). + * <p> + * End-to-end multi-error collection lives in {@code Groovy9192}; this class + * locks strategy-level control flow that is awkward to assert through the + * full compilation pipeline alone. + * </p> + */ +final class ErrorStrategyCoverageTest { + + @Test + void failFastRecoverCancelsWithCause() { + var charStream = CharStreams.fromString("}"); + var strategy = new DescriptiveErrorStrategy(charStream); + var parser = parser(charStream, strategy, PredictionMode.SLL); + parser.setContext(new ParserRuleContext()); + + InputMismatchException cause = new InputMismatchException(parser); + ParseCancellationException pce = assertThrows(ParseCancellationException.class, + () -> strategy.recover(parser, cause)); + assertInstanceOf(InputMismatchException.class, pce.getCause()); + assertTrue(pce.getCause() == cause); + } + + @Test + void failFastRecoverInlineCancelsWithInputMismatch() { + var charStream = CharStreams.fromString("}"); + var strategy = new DescriptiveErrorStrategy(charStream); + var parser = parser(charStream, strategy, PredictionMode.SLL); + parser.setContext(new ParserRuleContext()); + + ParseCancellationException pce = assertThrows(ParseCancellationException.class, + () -> strategy.recoverInline(parser)); + assertInstanceOf(InputMismatchException.class, pce.getCause()); + } + + @Test + void failFastRecoverInlineViaParserMatchCancels() { + var charStream = CharStreams.fromString("}"); + var strategy = new DescriptiveErrorStrategy(charStream); + var parser = parser(charStream, strategy, PredictionMode.LL); + parser.setContext(new ParserRuleContext()); + assertThrows(ParseCancellationException.class, + () -> parser.match(GroovyParser.Identifier)); + } + + @Test + void failFastRecoverUnderLlReportsAndCancels() { + var charStream = CharStreams.fromString("}"); + var strategy = new DescriptiveErrorStrategy(charStream); + var messages = new ArrayList<String>(); + var tokens = new CommonTokenStream(new GroovyLangLexer(charStream)); + tokens.fill(); + var parser = new GroovyLangParser(tokens); + parser.setErrorHandler(strategy); + parser.getInterpreter().setPredictionMode(PredictionMode.LL); + attachListener(parser, messages); + parser.setContext(new ParserRuleContext()); + + Token tok = tokens.get(0); + var nvae = new NoViableAltException(parser, tokens, tok, tok, null, parser.getContext()); + ParseCancellationException pce = assertThrows(ParseCancellationException.class, + () -> strategy.recover(parser, nvae)); + assertInstanceOf(NoViableAltException.class, pce.getCause()); + assertFalse(messages.isEmpty(), "LL recover must report before cancel"); + assertTrue(messages.stream().anyMatch(m -> + m.contains("Unexpected input") || m.startsWith("Missing ")), + "friendly diagnostic expected, got: " + messages); + } + + @Test + void factorySelectsStrategies() { + var cs = CharStreams.fromString("1"); + assertInstanceOf(RecoveringDescriptiveErrorStrategy.class, + DescriptiveErrorStrategy.create(cs, true)); + assertInstanceOf(DescriptiveErrorStrategy.class, + DescriptiveErrorStrategy.create(cs, false)); + } + + @Test + void recoveringStrategyCompletesMultiFaultSnippetWithoutCancel() { + var charStream = CharStreams.fromString("class C { def x = ( } class D {}"); + var strategy = new RecoveringDescriptiveErrorStrategy(charStream); + var parser = parser(charStream, strategy, PredictionMode.LL); + assertNotNull(parser.compilationUnit()); + } + + // --- helpers ---------------------------------------------------------------- + + private static GroovyLangParser parser(org.antlr.v4.runtime.CharStream charStream, + org.antlr.v4.runtime.ANTLRErrorStrategy strategy, + PredictionMode mode) { + var parser = new GroovyLangParser(new CommonTokenStream(new GroovyLangLexer(charStream))); + parser.setErrorHandler(strategy); + parser.getInterpreter().setPredictionMode(mode); + parser.removeErrorListeners(); + return parser; + } + + private static void attachListener(Parser parser, List<String> messages) { + parser.removeErrorListeners(); + parser.addErrorListener(new BaseErrorListener() { + @Override + public <T extends Token> void syntaxError(Recognizer<T, ?> recognizer, T offendingSymbol, int line, + int charPositionInLine, String msg, RecognitionException e) { + messages.add(msg); + } + }); + } +}
