GROOVY-12306: Error tolerance is not applied to type checking errors and has no unlimited setting - #2835
Open
paulk-asert wants to merge 5 commits into
Open
GROOVY-12306: Error tolerance is not applied to type checking errors and has no unlimited setting#2835paulk-asert wants to merge 5 commits into
paulk-asert wants to merge 5 commits into
Conversation
The configured error tolerance was only enforced for errors reported through SourceUnit#addError. Errors reported through ClassCodeVisitorSupport#addError went straight to ErrorCollector#addErrorAndContinue and so were unbounded, which meant `groovyc -t 1` had no effect on the most common error class in @CompileStatic code: type checking errors reported in full however low the tolerance was set. Route the base ClassCodeVisitorSupport#addError through the tolerance-aware ErrorCollector#addError. StaticTypeCheckingVisitor overrides addError for its own error de-duplication and needs the same treatment, but only for the source unit's own collector: the temporary collectors it pushes for speculative checks must continue to collect without bailing out, since their errors are routinely discarded once a candidate is ruled in or out. ClassCompletionVerifierTest counts every error the verifier reports, so it now asks for the unlimited tolerance it always relied on rather than the fail-fast tolerance of 1 that SourceUnit#create(String,String) selects. Assisted-by: Claude Opus 5 (1M context) via Claude Code
There was no way to ask the compiler to report every error. `groovyc -t 0` was silently discarded, because CompilationOptions could not tell a supplied zero from the option being absent, leaving the default of 10 in place; and even when set, a tolerance of zero would have bailed out on the first error rather than collecting them all. Treat a tolerance of zero or less as unlimited in ErrorCollector, and hold the command-line option in a boxed Integer so that an explicit zero reaches the configuration. Name the default as CompilerConfiguration.DEFAULT_TOLERANCE rather than repeating the literal, and state it in the option help, which previously gave no hint that the option was bounded by default. The option itself has been undocumented since it was added in GROOVY-11194, so add it to the groovyc option table too. Assisted-by: Claude Opus 5 (1M context) via Claude Code
The error tolerance could be set on the groovyc command line and through the CompilerConfiguration API, but the <groovyc> Ant task offered no way to reach it short of a compiler configuration script. Add a tolerance attribute. Both the forked and in-process paths run the same assembled argument list through the FileSystemCompiler parser, so emitting the option once covers both. An unset attribute emits nothing and leaves the compiler default in place, while an explicit zero is passed through as the request for unlimited error reporting that it is. Assisted-by: Claude Opus 5 (1M context) via Claude Code
paulk-asert
changed the title
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #2835 +/- ##
==================================================
+ Coverage 70.6486% 70.6608% +0.0122%
- Complexity 36522 36539 +17
==================================================
Files 1571 1571
Lines 133963 133981 +18
Branches 24690 24692 +2
==================================================
+ Hits 94643 94672 +29
+ Misses 30802 30795 -7
+ Partials 8518 8514 -4
🚀 New features to boost your workflow:
|
✅ All tests passed ✅🏷️ Commit: 109fa29 Learn more about TestLens at testlens.app/docs. |
Contributor
There was a problem hiding this comment.
Pull request overview
Extends compiler error tolerance to static type-checking and visitor errors, adds unlimited reporting, and exposes tolerance through compiler front ends.
Changes:
- Enforces tolerance consistently and treats non-positive values as unlimited.
- Adds CLI and Ant task support.
- Adds regression tests and documentation.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
COMPATIBILITY.md |
Documents compatibility impact. |
src/main/java/org/codehaus/groovy/ast/ClassCodeVisitorSupport.java |
Applies tolerance to visitor errors. |
src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java |
Defines default and unlimited semantics. |
src/main/java/org/codehaus/groovy/control/ErrorCollector.java |
Implements unlimited tolerance. |
src/main/java/org/codehaus/groovy/control/SourceUnit.java |
Clarifies legacy and factory behavior. |
src/main/java/org/codehaus/groovy/tools/FileSystemCompiler.java |
Preserves explicit zero from CLI parsing. |
src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java |
Enforces tolerance for type-checking errors. |
src/spec/doc/tools-groovyc.adoc |
Documents the CLI option. |
src/test/groovy/org/codehaus/groovy/classgen/ClassCompletionVerifierTest.java |
Requests unlimited errors in verifier tests. |
src/test/groovy/org/codehaus/groovy/control/ErrorToleranceTest.groovy |
Tests tolerance across error paths. |
src/test/groovy/org/codehaus/groovy/tools/FileSystemCompilerTest.java |
Tests CLI tolerance parsing. |
subprojects/groovy-ant/src/main/java/org/codehaus/groovy/ant/Groovyc.java |
Adds the Ant task attribute. |
subprojects/groovy-ant/src/spec/doc/groovyc-ant-task.adoc |
Documents the Ant attribute. |
subprojects/groovy-ant/src/test/groovy/org/codehaus/groovy/ant/GroovycToleranceTest.java |
Tests Ant argument propagation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+373
to
+379
| 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.