diff --git a/COMPATIBILITY.md b/COMPATIBILITY.md index b9905a42381..616d19e054a 100644 --- a/COMPATIBILITY.md +++ b/COMPATIBILITY.md @@ -368,6 +368,52 @@ compilation but does not exempt the method from an enclosing class's `@TypeChecked` checking; only `@TypeChecked(TypeCheckingMode.SKIP)` does that. +### Groovy 6 — error tolerance applies to type checking errors (GROOVY-12306) + +The compiler's error tolerance — the number of non-fatal errors collected +before compilation is abandoned, `CompilerConfiguration.getTolerance()`, +`groovyc -t` — is now enforced for every error kind. It previously covered +only errors reported through `SourceUnit#addError` (parse and class +generation); errors reported through `ClassCodeVisitorSupport#addError`, +which includes all static type checking errors, went straight to +`ErrorCollector#addErrorAndContinue` and were unbounded. + +The default is unchanged at 10 (now named +`CompilerConfiguration.DEFAULT_TOLERANCE`). A tolerance of zero or less +now means unlimited, which is the setting to reach for when every error +is wanted; previously zero was indistinguishable from the option being +absent on the command line, and would have bailed out on the first error +if set through the API. + +**Who is affected.** Any compilation reporting more than the tolerance in +type checking errors. Where 40 such errors were previously all reported, +10 are now reported and compilation stops. Pass `-t 0` on the groovyc +command line, set `tolerance="0"` on the Ant `` task, or set +`configuration.tolerance = 0` in a compiler configuration script (the +route Gradle users have, via `groovyOptions.configurationScript`) to +restore full reporting. + +This is not limited to the compiler front ends: it applies to any caller +driving a `ClassCodeVisitorSupport` subclass over a `SourceUnit` and +counting the errors it collects — static analysers, IDE integrations and +AST transformation test harnesses among them. Note in particular that +`SourceUnit.create(String, String)` selects a tolerance of **1**, so a +visitor driven over a source unit from that factory now stops at the +first error. Use the three-argument overload to state the tolerance the +caller actually wants. + +**What is unchanged.** The default of 10, so parse and class generation +errors behave exactly as before. The temporary error collectors that +`StaticTypeCheckingVisitor` pushes for speculative checks are also +unaffected — they must keep collecting without bailing out, because their +errors are routinely discarded once a candidate is ruled in or out, so +tolerance is enforced only against the source unit's own collector. + +Completeness remains per-phase, as it always has been: `failIfErrors()` +runs at the end of each phase, so a single type checking error anywhere +in the compilation still suppresses every class generation error, +whatever the tolerance. + ## The binary-compatibility check The [`subprojects/binary-compatibility/`](subprojects/binary-compatibility) diff --git a/src/main/java/org/codehaus/groovy/ast/ClassCodeVisitorSupport.java b/src/main/java/org/codehaus/groovy/ast/ClassCodeVisitorSupport.java index 82c0ccdde4a..7b17bec9318 100644 --- a/src/main/java/org/codehaus/groovy/ast/ClassCodeVisitorSupport.java +++ b/src/main/java/org/codehaus/groovy/ast/ClassCodeVisitorSupport.java @@ -40,6 +40,7 @@ import org.codehaus.groovy.ast.stmt.WhileStatement; import org.codehaus.groovy.ast.stmt.YieldStatement; import org.codehaus.groovy.control.SourceUnit; +import org.codehaus.groovy.control.messages.Message; import org.codehaus.groovy.syntax.SyntaxException; import org.codehaus.groovy.transform.ErrorCollecting; @@ -495,6 +496,12 @@ protected void visitStatementAnnotations(Statement statement) { /** * Adds an error message associated with an AST node to the source unit. * Errors are accumulated and reported after visitation completes. + *

+ * The error counts towards the configured + * {@link org.codehaus.groovy.control.CompilerConfiguration#getTolerance() error tolerance}, + * so visitation may be cut short once that many errors have been collected (GROOVY-12306). + * Use {@link org.codehaus.groovy.control.ErrorCollector#addErrorAndContinue(Message)} + * directly to report an error which must never bail out. * * @param error the error message to report * @param node the AST node associated with the error location @@ -502,6 +509,8 @@ protected void visitStatementAnnotations(Statement statement) { */ @Override public void addError(final String error, final ASTNode node) { - getSourceUnit().addErrorAndContinue(new SyntaxException(error + '\n', node)); + SourceUnit source = getSourceUnit(); + source.getErrorCollector().addError( + Message.create(new SyntaxException(error + '\n', node), source)); } } diff --git a/src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java b/src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java index 31f89f89303..ca8977e671b 100644 --- a/src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java +++ b/src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java @@ -188,6 +188,14 @@ public class CompilerConfiguration { */ public static final String DEFAULT_SOURCE_ENCODING = "UTF-8"; + /** + * The default number of non-fatal errors tolerated before compilation is aborted. + * + * @see #setTolerance(int) + * @since 6.0.0 + */ + public static final int DEFAULT_TOLERANCE = 10; + /** * A convenience for getting a default configuration. Do not modify it! * See {@link #CompilerConfiguration(Properties)} for an example on how to @@ -561,7 +569,7 @@ public void setLogClassgenStackTraceMaxDepth(int logClassgenStackTraceMaxDepth) public CompilerConfiguration() { classpath = new LinkedList<>(); - tolerance = 10; + tolerance = DEFAULT_TOLERANCE; minimumRecompilationInterval = 100; warningLevel = WarningMessage.LIKELY_ERRORS; parameters = getBooleanSafe("groovy.parameters"); @@ -935,8 +943,8 @@ public void configure(final Properties configuration) throws ConfigurationExcept text = configuration.getProperty("groovy.output.debug"); if (text != null) setDebug("true".equalsIgnoreCase(text)); - numeric = 10; - text = configuration.getProperty("groovy.errors.tolerance", "10"); + numeric = DEFAULT_TOLERANCE; + text = configuration.getProperty("groovy.errors.tolerance", Integer.toString(DEFAULT_TOLERANCE)); try { numeric = Integer.parseInt(text); } catch (NumberFormatException e) { @@ -1122,7 +1130,9 @@ public void setParameters(final boolean parameters) { } /** - * Returns the requested error tolerance. + * Returns the requested error tolerance. Zero or less means unlimited. + * + * @see #setTolerance(int) */ public int getTolerance() { return this.tolerance; @@ -1131,7 +1141,11 @@ public int getTolerance() { /** * Sets the error tolerance, which is the number of * non-fatal errors (per unit) that should be tolerated before - * compilation is aborted. + * compilation is aborted. Defaults to {@value #DEFAULT_TOLERANCE}. + *

+ * A value of zero or less means unlimited: every error is collected and + * reported, and compilation is never cut short by the error count alone + * (GROOVY-12306). */ public void setTolerance(final int tolerance) { this.tolerance = tolerance; diff --git a/src/main/java/org/codehaus/groovy/control/ErrorCollector.java b/src/main/java/org/codehaus/groovy/control/ErrorCollector.java index 8229da6cdf1..74e74162af1 100644 --- a/src/main/java/org/codehaus/groovy/control/ErrorCollector.java +++ b/src/main/java/org/codehaus/groovy/control/ErrorCollector.java @@ -122,11 +122,15 @@ public void addErrorAndContinue(final Message message) { * Adds a non-fatal error to the message set, which may cause a failure if the error threshold is exceeded. * The message is not required to have a source line and column specified, but it is best practice to try * and include that information. + *

+ * A {@link CompilerConfiguration#getTolerance() tolerance} of zero or less means unlimited: + * every error is collected and the threshold never triggers a failure (GROOVY-12306). */ public void addError(final Message message) throws CompilationFailedException { addErrorAndContinue(message); - if (errors != null && errors.size() >= configuration.getTolerance()) { + int tolerance = configuration.getTolerance(); + if (tolerance > 0 && errors != null && errors.size() >= tolerance) { failIfErrors(); } } diff --git a/src/main/java/org/codehaus/groovy/control/SourceUnit.java b/src/main/java/org/codehaus/groovy/control/SourceUnit.java index fc78c9ad217..b7016f0ad81 100644 --- a/src/main/java/org/codehaus/groovy/control/SourceUnit.java +++ b/src/main/java/org/codehaus/groovy/control/SourceUnit.java @@ -133,49 +133,24 @@ public ModuleNode getAST() { } /** - * Convenience routine, primarily for use by the InteractiveShell, - * that returns true if parse() failed with an unexpected EOF. + * Legacy Antlr 2 error reporting method no longer in use. + * Similar in intent to {@code getErrorCollector().hasErrors()}, which should be used instead. + * + * @return true if any error was collected while parsing + * @deprecated since 6.0.0, use {@link #getErrorCollector()} and + * {@link ErrorCollector#hasErrors()} instead. */ + @Deprecated(since = "6.0.0") public boolean failedWithUnexpectedEOF() { - // Implementation note - there are several ways for the Groovy compiler - // to report an unexpected EOF. Perhaps this implementation misses some. - // If you find another way, please add it. - if (getErrorCollector().hasErrors()) { - /* - Message last = (Message) getErrorCollector().getLastError(); - Throwable cause = null; - if (last instanceof SyntaxErrorMessage) { - cause = ((SyntaxErrorMessage) last).getCause().getCause(); - } - if (cause != null) { - if (cause instanceof groovyjarjarantlr.NoViableAltException) { - return isEofToken(((groovyjarjarantlr.NoViableAltException) cause).token); - } else if (cause instanceof groovyjarjarantlr.NoViableAltForCharException) { - char badChar = ((groovyjarjarantlr.NoViableAltForCharException) cause).foundChar; - return badChar == groovyjarjarantlr.CharScanner.EOF_CHAR; - } else if (cause instanceof groovyjarjarantlr.MismatchedCharException) { - char badChar = (char) ((groovyjarjarantlr.MismatchedCharException) cause).foundChar; - return badChar == groovyjarjarantlr.CharScanner.EOF_CHAR; - } else if (cause instanceof groovyjarjarantlr.MismatchedTokenException) { - return isEofToken(((groovyjarjarantlr.MismatchedTokenException) cause).token); - } - } - */ - return true; - } - return false; + return getErrorCollector().hasErrors(); } - /*protected boolean isEofToken(groovyjarjarantlr.Token token) { - return token.getType() == groovyjarjarantlr.Token.EOF_TYPE; - }*/ - //--------------------------------------------------------------------------- // FACTORIES /** * A convenience routine to create a standalone SourceUnit on a String - * with defaults for almost everything that is configurable. + * with defaults for almost everything that is configurable but with tolerance set to 1. */ public static SourceUnit create(String name, String source) { CompilerConfiguration configuration = new CompilerConfiguration(); diff --git a/src/main/java/org/codehaus/groovy/tools/FileSystemCompiler.java b/src/main/java/org/codehaus/groovy/tools/FileSystemCompiler.java index ba6da860a15..161e3459a7d 100644 --- a/src/main/java/org/codehaus/groovy/tools/FileSystemCompiler.java +++ b/src/main/java/org/codehaus/groovy/tools/FileSystemCompiler.java @@ -538,8 +538,9 @@ public static class CompilationOptions { @Option(names = {"-cf", "--configscript"}, paramLabel = "