GROOVY-12306: Error tolerance is not applied to type checking errors and has no unlimited setting

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 `<groovyc>` 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)

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,13 +496,21 @@ 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.
+     * <p>
+     * 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
      * @see ErrorCollecting
      */
     @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));
     }
 }

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}.
+     * <p>
+     * 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;

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.
+     * <p>
+     * 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();
         }
     }

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();

src/main/java/org/codehaus/groovy/tools/FileSystemCompiler.java

@@ -538,8 +538,9 @@ public static class CompilationOptions {
         @Option(names = {"-cf", "--configscript"}, paramLabel = "<script>", description = "A script for tweaking the configuration options")
         private String configScript;
 
-        @Option(names = {"-t", "--tolerance"}, description = "The number of non-fatal errors to allow before bailing")
-        private int tolerance;
+        @Option(names = {"-t", "--tolerance"}, paramLabel = "<count>",
+                description = "The number of non-fatal errors to allow before bailing (default: 10; 0 for unlimited)")
+        private Integer tolerance;
 
         @Option(names = {"-h", "--help"}, usageHelp = true, description = "Show this help message and exit")
         private boolean helpRequested;
@@ -585,7 +586,7 @@ public CompilerConfiguration toCompilerConfiguration() throws IOException {
             if (checkOnly) {
                 configuration.setTargetPhase(Phases.INSTRUCTION_SELECTION);
             }
-            if (tolerance > 0) {
+            if (tolerance != null) {
                 configuration.setTolerance(tolerance);
             }
 

src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java

@@ -117,8 +117,10 @@
 import org.codehaus.groovy.control.ErrorCollector;
 import org.codehaus.groovy.control.ResolveVisitor;
 import org.codehaus.groovy.control.SourceUnit;
+import org.codehaus.groovy.control.messages.Message;
 import org.codehaus.groovy.control.messages.WarningMessage;
 import org.codehaus.groovy.runtime.DefaultGroovyMethods;
+import org.codehaus.groovy.syntax.SyntaxException;
 import org.codehaus.groovy.syntax.Token;
 import org.codehaus.groovy.syntax.TokenUtil;
 import org.codehaus.groovy.transform.RecordTypeASTTransformation;
@@ -7123,7 +7125,17 @@ private static SetterInfo removeSetterInfo(final Expression exp) {
     public void addError(final String msg, final ASTNode node) {
         Long err = ((long) node.getLineNumber()) << 16 + node.getColumnNumber();
         if ((DEBUG_GENERATED_CODE && node.getLineNumber() < 0) || !typeCheckingContext.reportedErrors.contains(err)) {
-            typeCheckingContext.getErrorCollector().addErrorAndContinue(msg + '\n', node, getSourceUnit());
+            SourceUnit source = getSourceUnit();
+            ErrorCollector collector = typeCheckingContext.getErrorCollector();
+            Message message = Message.create(new SyntaxException(msg + '\n', node), source);
+            // GROOVY-12306: only the source unit's own collector enforces the error tolerance.
+            // The temporary collectors pushed for speculative checks must never bail out, as
+            // their errors are routinely discarded once a candidate is ruled in or out.
+            if (collector == source.getErrorCollector()) {
+                collector.addError(message);
+            } else {
+                collector.addErrorAndContinue(message);
+            }
             typeCheckingContext.reportedErrors.add(err);
         }
     }

src/spec/doc/tools-groovyc.adoc

@@ -53,6 +53,7 @@ a number of command line switches:
 | -j | --jointCompilation* | Enables joint compilation | groovyc -j A.groovy B.java
 | -b | --basescript | Base class name for scripts (must derive from Script)|
 | | --configscript | Advanced compiler configuration script | groovyc --configscript config/config.groovy src/Person.groovy
+| -t | --tolerance | The number of non-fatal errors to collect before compilation is abandoned. Defaults to `10`; use `0` for unlimited. | groovyc -t 0 Person.groovy
 | -Jproperty=value | | Properties to be passed to `javac` if joint compilation is enabled | groovyc -j -Jtarget=1.6 -Jsource=1.6 A.groovy B.java
 | -Fflag | | Flags to be passed to `javac` if joint compilation is enabled | groovyc -j -Fnowarn A.groovy B.java
 | -pa | --parameters | Generates metadata for reflection on method parameter names. Requires Java 8+. | groovyc --parameters Person.groovy

src/test/groovy/org/codehaus/groovy/classgen/ClassCompletionVerifierTest.java

@@ -48,7 +48,9 @@
 
 final class ClassCompletionVerifierTest {
 
-    private final SourceUnit sourceUnit = SourceUnit.create("dummy.groovy", "");
+    // GROOVY-12306: these tests collect and count every error the verifier reports, so they need
+    // unlimited tolerance; the two-argument SourceUnit.create factory asks for fail-fast (1)
+    private final SourceUnit sourceUnit = SourceUnit.create("dummy.groovy", "", 0);
     private final ClassCompletionVerifier verifier = new ClassCompletionVerifier(sourceUnit);
 
     @Test

src/test/groovy/org/codehaus/groovy/control/ErrorToleranceTest.groovy

@@ -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.codehaus.groovy.control
+
+import org.junit.jupiter.api.Test
+
+import static org.junit.jupiter.api.Assertions.assertEquals
+
+/**
+ * Tests {@link CompilerConfiguration#setTolerance(int)}, the number of non-fatal
+ * errors collected before compilation is abandoned.
+ */
+final class ErrorToleranceTest {
+
+    private static final int MANY = 14
+
+    /**
+     * Static type-checking errors, one per method. These are raised via
+     * {@code StaticTypeCheckingVisitor#addError}, which overrides the base
+     * {@code ClassCodeVisitorSupport} reporting used below.
+     */
+    private static String typeCheckingErrors(int count) {
+        def methods = (0..<count).collect { "    def m$it() { new Object().nope$it() }" }
+        "@groovy.transform.CompileStatic\nclass Subject {\n${methods.join('\n')}\n}\n"
+    }
+
+    /**
+     * Class resolution errors, one per field. These are raised by {@link ResolveVisitor}
+     * via {@code ClassCodeVisitorSupport#addError}, a different reporting path and a
+     * different compile phase to the type-checking errors above.
+     */
+    private static String resolutionErrors(int count) {
+        def fields = (0..<count).collect { "    NoSuchType$it field$it" }
+        "class Subject {\n${fields.join('\n')}\n}\n"
+    }
+
+    private static int errorCount(String source, Integer tolerance = null) {
+        def config = new CompilerConfiguration()
+        if (tolerance != null) config.tolerance = tolerance
+        def unit = new CompilationUnit(config, null, new GroovyClassLoader(ErrorToleranceTest.class.classLoader))
+        unit.addSource('Subject.groovy', source)
+        try {
+            unit.compile()
+            0
+        } catch (MultipleCompilationErrorsException e) {
+            e.errorCollector.errorCount
+        }
+    }
+
+    // GROOVY-12306: tolerance was ignored for type-checking errors, which reported in full
+    // however low it was set, so there was no way to ask the compiler to stop after the first
+    @Test
+    void testTypeCheckingErrorsHonourTolerance() {
+        assertEquals(1, errorCount(typeCheckingErrors(MANY), 1))
+        assertEquals(3, errorCount(typeCheckingErrors(MANY), 3))
+    }
+
+    // the same must hold for errors reported through the base ClassCodeVisitorSupport path
+    @Test
+    void testResolutionErrorsHonourTolerance() {
+        assertEquals(1, errorCount(resolutionErrors(MANY), 1))
+        assertEquals(3, errorCount(resolutionErrors(MANY), 3))
+    }
+
+    // GROOVY-12306: zero means unlimited, the setting to reach for when every error is wanted
+    @Test
+    void testZeroToleranceMeansUnlimited() {
+        assertEquals(MANY, errorCount(typeCheckingErrors(MANY), 0))
+        assertEquals(MANY, errorCount(resolutionErrors(MANY), 0))
+    }
+
+    // a negative tolerance is meaningless as a count, so it is treated as unlimited too
+    @Test
+    void testNegativeToleranceMeansUnlimited() {
+        assertEquals(MANY, errorCount(typeCheckingErrors(MANY), -1))
+    }
+
+    // the default is unchanged by GROOVY-12306, and now applies to both kinds of error
+    @Test
+    void testDefaultToleranceAppliesToBothErrorKinds() {
+        assertEquals(CompilerConfiguration.DEFAULT_TOLERANCE, new CompilerConfiguration().tolerance)
+        assertEquals(CompilerConfiguration.DEFAULT_TOLERANCE, errorCount(typeCheckingErrors(MANY)))
+        assertEquals(CompilerConfiguration.DEFAULT_TOLERANCE, errorCount(resolutionErrors(MANY)))
+    }
+
+    // fewer errors than the tolerance are all reported, whichever path raised them
+    @Test
+    void testAllErrorsReportedBelowTolerance() {
+        assertEquals(3, errorCount(typeCheckingErrors(3), 10))
+        assertEquals(3, errorCount(resolutionErrors(3), 10))
+    }
+}