[
https://issues.apache.org/jira/browse/GROOVY-12306?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Paul King updated GROOVY-12306:
-------------------------------
Description:
The compiler's error tolerance -- the number of non-fatal errors accepted
before compilation bails out -- is only partially wired up. Two defects plus
one gap, all verified against 6.0.0-beta-2:
h2. 1. Tolerance is not applied to static type checking errors
{{-t}} / {{--tolerance}} has no effect whatsoever on type checking errors, so
there is no way to ask for fail-fast behaviour on the most common error class
in {{@CompileStatic}} code.
{code:java}
@groovy.transform.CompileStatic
class Z {
def m0() { new Object().nope0() }
def m1() { new Object().nope1() }
// ... 14 such methods
}
{code}
||Command||Expected||Actual||
|{{groovyc -t 1 Z.groovy}}|1 error|14 errors|
|{{groovyc -t 3 Z.groovy}}|3 errors|14 errors|
Cause: {{StaticTypeCheckingVisitor.addStaticTypeError}} (line 6855) routes
through {{ClassCodeVisitorSupport.addError}} (line 490), which calls
{{ErrorCollector.addErrorAndContinue}} directly. Only
{{ErrorCollector.addError(Message)}} (line 126) performs the {{errors.size() >=
configuration.getTolerance()}} check, so every diagnostic raised via a
{{ClassCodeVisitorSupport}} subclass bypasses the threshold.
Errors raised through {{SourceUnit.addError}} *are* capped correctly, which
produces a confusing split. Using the {{Access to P#M is forbidden}} error from
{{StaticTypesCallSiteWriter}} as a contrasting example, with 14 offending
accesses in a single file:
||Command||Result||
|{{groovyc}} (default)|10 errors|
|{{groovyc -t 50}}|14 errors|
|{{groovyc -t 1}}|1 error|
So the same flag governs class generation errors but silently does nothing for
type checking errors.
h2. 2. {{-t 0}} is a silent no-op, and there is no "unlimited" setting
{{FileSystemCompiler}} line 588 guards the assignment:
{code:java}
if (tolerance > 0) {
configuration.setTolerance(tolerance);
}
{code}
{{-t 0}} therefore leaves the default of 10 in place with no diagnostic. There
is also no spelling for "report everything" -- a caller wanting all errors has
to guess a sufficiently large number. {{0}} = unlimited would be the natural
convention.
Related: {{--help}} does not state the default, so the option reads as
unbounded-by-default when it is in fact 10.
h2. 3. Tolerance is not exposed by the Ant task
The Ant {{<groovyc>}} task has no tolerance attribute, so build-tool users have
no direct route to the setting. (Gradle users can already reach it through
{{groovyOptions.configurationScript}} with {{configuration.tolerance = 100}},
and embedded callers have {{setTolerance()}}.)
h2. Suggested fixes
# Apply tolerance uniformly across error kinds, so {{-t 1}} genuinely means
"stop after the first error" regardless of which phase raised it. This is the
highest-value part.
# Treat {{-t 0}} as unlimited rather than silently ignoring it, and document
the default of 10 in {{--help}}.
# Add a tolerance attribute to the Ant {{<groovyc>}} task, once the semantics
above are settled.
h2. Notes
*Completeness is per-phase.* Even with unlimited tolerance, a single type
checking error anywhere in the compilation suppresses every class generation
error, because {{failIfErrors()}} runs at the end of each phase. Worth being
aware of when reasoning about "report all errors", but a separate concern from
this issue.
*The {{groovy.errors.tolerance}} system property behaves inconsistently, but is
probably best left alone.* It has existed since the original 2004 commit
(a0a831f4e3) as a key of the {{CompilerConfiguration(Properties)}} bag, so
whether it is reachable as a system property depends purely on the entry point:
{{GroovyMain}} (line 110) uses {{new
CompilerConfiguration(System.getProperties())}} and honours it, whereas
{{FileSystemCompiler}} (line 571) uses the no-arg constructor, which hardcodes
{{tolerance = 10}} (line 564) and never consults the property.
{noformat}
groovy -Dgroovy.errors.tolerance=50 ... -> 14 errors
groovyc -Dgroovy.errors.tolerance=50 ... -> 10 errors
{noformat}
Once the fixes above land, every path has a first-class route to the setting
and the property adds little. Promoting it to the no-arg constructor would also
freeze it into {{CompilerConfiguration.DEFAULT}} at class-init, making it
JVM-global and sticky across every compilation in a Gradle daemon. Suggest
documenting the current behaviour rather than changing it.
was:
The compiler's error tolerance (the number of non-fatal errors accepted before
compilation bails out) is only partially wired up. Three related defects, all
verified against 6.0.0-beta-2:
h2. 1. Tolerance is not applied to static type checking errors
{{-t}} / {{--tolerance}} has no effect whatsoever on type checking errors, so
there is no way to ask for fail-fast behaviour on the most common error class
in {{@CompileStatic}} code.
{code:java}
@groovy.transform.CompileStatic
class Z {
def m0() { new Object().nope0() }
def m1() { new Object().nope1() }
// ... 14 such methods
}
{code}
||Command||Expected||Actual||
|{{groovyc -t 1 Z.groovy}}|1 error|14 errors|
|{{groovyc -t 3 Z.groovy}}|3 errors|14 errors|
Cause: {{StaticTypeCheckingVisitor.addStaticTypeError}} (line 6855) routes
through {{ClassCodeVisitorSupport.addError}} (line 490), which calls
{{ErrorCollector.addErrorAndContinue}} directly. Only
{{ErrorCollector.addError(Message)}} (line 126) performs the {{errors.size() >=
configuration.getTolerance()}} check, so every diagnostic raised via a
{{ClassCodeVisitorSupport}} subclass bypasses the threshold.
Errors raised through {{SourceUnit.addError}} are capped correctly, which
produces a confusing split. Using the {{Access to P#M is forbidden}} error from
{{StaticTypesCallSiteWriter}} as a contrasting example, with 14 offending
accesses in one file:
||Command||Result||
|{{groovyc}} (default)|10 errors|
|{{groovyc -t 50}}|14 errors|
|{{groovyc -t 1}}|1 error|
So the same flag governs codegen errors but silently does nothing for type
checking errors.
h2. 2. {{-t 0}} is a silent no-op, and there is no "unlimited" setting
{{FileSystemCompiler}} line 588 guards the assignment:
{code:java}
if (tolerance > 0) {
configuration.setTolerance(tolerance);
}
{code}
{{-t 0}} therefore leaves the default of 10 in place with no diagnostic. There
is also no spelling for "report everything" — a caller wanting all errors has
to guess a sufficiently large number. {{0}} = unlimited would be the natural
convention.
Related: {{--help}} does not state the default, so the option reads as
unbounded-by-default when it is in fact 10.
h2. 3. {{groovy.errors.tolerance}} is honoured by {{groovy}} but ignored by
{{groovyc}}
The property has existed since the original 2004 commit (a0a831f4e3) as a key
of the {{CompilerConfiguration(Properties)}} bag. Whether it is reachable as a
system property depends purely on the entry point:
* {{groovy.ui.GroovyMain}} line 110 uses {{new
CompilerConfiguration(System.getProperties())}}, so
{{-Dgroovy.errors.tolerance=50}} works.
* {{FileSystemCompiler}} line 571 uses {{new CompilerConfiguration()}}, whose
no-arg constructor hardcodes {{tolerance = 10}} (line 564) and never consults
{{groovy.errors.tolerance}}, so the same {{-D}} is silently ignored.
Verified:
{noformat}
groovy -Dgroovy.errors.tolerance=50 ... -> 14 errors
groovyc -Dgroovy.errors.tolerance=50 ... -> 10 errors
{noformat}
The property is thus supported by the entry point where batch error reporting
matters least, and ignored by the compiler.
h2. Suggested fixes
# Apply tolerance uniformly across error kinds, so {{-t 1}} genuinely means
"stop after the first error" regardless of which phase raised it. This is the
highest-value part.
# Treat {{\-t 0}} as unlimited rather than silently ignoring it, and document
the default of 10 in {{--help}}.
# Resolve the {{groovy}} / {{groovyc}} system-property asymmetry in one
direction or the other.
# Expose tolerance in the Ant {{<groovyc>}} task, which currently has no such
attribute. Probably a follow-up once the semantics above are settled.
h2. Notes
Even with unlimited tolerance, a single type checking error anywhere in the
compilation suppresses every class generation error, because {{failIfErrors()}}
runs at the end of each phase. That is a separate concern, but it is worth
being aware of when reasoning about "report all errors": completeness is
per-phase.
The current workaround for raising the limit under Gradle or Ant is a compiler
configuration script containing {{configuration.tolerance = 100}}.
> Error tolerance is not applied to type checking errors and has no unlimited
> setting
> -----------------------------------------------------------------------------------
>
> Key: GROOVY-12306
> URL: https://issues.apache.org/jira/browse/GROOVY-12306
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Priority: Major
>
> The compiler's error tolerance -- the number of non-fatal errors accepted
> before compilation bails out -- is only partially wired up. Two defects plus
> one gap, all verified against 6.0.0-beta-2:
> h2. 1. Tolerance is not applied to static type checking errors
> {{-t}} / {{--tolerance}} has no effect whatsoever on type checking errors, so
> there is no way to ask for fail-fast behaviour on the most common error class
> in {{@CompileStatic}} code.
> {code:java}
> @groovy.transform.CompileStatic
> class Z {
> def m0() { new Object().nope0() }
> def m1() { new Object().nope1() }
> // ... 14 such methods
> }
> {code}
> ||Command||Expected||Actual||
> |{{groovyc -t 1 Z.groovy}}|1 error|14 errors|
> |{{groovyc -t 3 Z.groovy}}|3 errors|14 errors|
> Cause: {{StaticTypeCheckingVisitor.addStaticTypeError}} (line 6855) routes
> through {{ClassCodeVisitorSupport.addError}} (line 490), which calls
> {{ErrorCollector.addErrorAndContinue}} directly. Only
> {{ErrorCollector.addError(Message)}} (line 126) performs the {{errors.size()
> >= configuration.getTolerance()}} check, so every diagnostic raised via a
> {{ClassCodeVisitorSupport}} subclass bypasses the threshold.
> Errors raised through {{SourceUnit.addError}} *are* capped correctly, which
> produces a confusing split. Using the {{Access to P#M is forbidden}} error
> from {{StaticTypesCallSiteWriter}} as a contrasting example, with 14
> offending accesses in a single file:
> ||Command||Result||
> |{{groovyc}} (default)|10 errors|
> |{{groovyc -t 50}}|14 errors|
> |{{groovyc -t 1}}|1 error|
> So the same flag governs class generation errors but silently does nothing
> for type checking errors.
> h2. 2. {{-t 0}} is a silent no-op, and there is no "unlimited" setting
> {{FileSystemCompiler}} line 588 guards the assignment:
> {code:java}
> if (tolerance > 0) {
> configuration.setTolerance(tolerance);
> }
> {code}
> {{-t 0}} therefore leaves the default of 10 in place with no diagnostic.
> There is also no spelling for "report everything" -- a caller wanting all
> errors has to guess a sufficiently large number. {{0}} = unlimited would be
> the natural convention.
> Related: {{--help}} does not state the default, so the option reads as
> unbounded-by-default when it is in fact 10.
> h2. 3. Tolerance is not exposed by the Ant task
> The Ant {{<groovyc>}} task has no tolerance attribute, so build-tool users
> have no direct route to the setting. (Gradle users can already reach it
> through {{groovyOptions.configurationScript}} with {{configuration.tolerance
> = 100}}, and embedded callers have {{setTolerance()}}.)
> h2. Suggested fixes
> # Apply tolerance uniformly across error kinds, so {{-t 1}} genuinely means
> "stop after the first error" regardless of which phase raised it. This is the
> highest-value part.
> # Treat {{-t 0}} as unlimited rather than silently ignoring it, and document
> the default of 10 in {{--help}}.
> # Add a tolerance attribute to the Ant {{<groovyc>}} task, once the semantics
> above are settled.
> h2. Notes
> *Completeness is per-phase.* Even with unlimited tolerance, a single type
> checking error anywhere in the compilation suppresses every class generation
> error, because {{failIfErrors()}} runs at the end of each phase. Worth being
> aware of when reasoning about "report all errors", but a separate concern
> from this issue.
> *The {{groovy.errors.tolerance}} system property behaves inconsistently, but
> is probably best left alone.* It has existed since the original 2004 commit
> (a0a831f4e3) as a key of the {{CompilerConfiguration(Properties)}} bag, so
> whether it is reachable as a system property depends purely on the entry
> point: {{GroovyMain}} (line 110) uses {{new
> CompilerConfiguration(System.getProperties())}} and honours it, whereas
> {{FileSystemCompiler}} (line 571) uses the no-arg constructor, which
> hardcodes {{tolerance = 10}} (line 564) and never consults the property.
> {noformat}
> groovy -Dgroovy.errors.tolerance=50 ... -> 14 errors
> groovyc -Dgroovy.errors.tolerance=50 ... -> 10 errors
> {noformat}
> Once the fixes above land, every path has a first-class route to the setting
> and the property adds little. Promoting it to the no-arg constructor would
> also freeze it into {{CompilerConfiguration.DEFAULT}} at class-init, making
> it JVM-global and sticky across every compilation in a Gradle daemon. Suggest
> documenting the current behaviour rather than changing it.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
