[
https://issues.apache.org/jira/browse/GROOVY-12306?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18108622#comment-18108622
]
ASF GitHub Bot commented on GROOVY-12306:
-----------------------------------------
codecov-commenter commented on PR #2835:
URL: https://github.com/apache/groovy/pull/2835#issuecomment-5433954940
##
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/2835?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
Report
:x: Patch coverage is `86.36364%` with `3 lines` in your changes missing
coverage. Please review.
:white_check_mark: Project coverage is 70.6608%. Comparing base
([`914da78`](https://app.codecov.io/gh/apache/groovy/commit/914da787bb357e4fcc952ebf33e4d34a6a9055c9?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`109fa29`](https://app.codecov.io/gh/apache/groovy/commit/109fa297c89c0de5e757808b337da40f8874fffd?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/2835?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Patch % | Lines |
|---|---|---|
|
[...va/org/codehaus/groovy/control/ErrorCollector.java](https://app.codecov.io/gh/apache/groovy/pull/2835?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fcontrol%2FErrorCollector.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NvbnRyb2wvRXJyb3JDb2xsZWN0b3IuamF2YQ==)
| 50.0000% | [0 Missing and 1 partial :warning:
](https://app.codecov.io/gh/apache/groovy/pull/2835?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
|
[...n/java/org/codehaus/groovy/control/SourceUnit.java](https://app.codecov.io/gh/apache/groovy/pull/2835?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fcontrol%2FSourceUnit.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NvbnRyb2wvU291cmNlVW5pdC5qYXZh)
| 0.0000% | [1 Missing :warning:
](https://app.codecov.io/gh/apache/groovy/pull/2835?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
|
[...src/main/java/org/codehaus/groovy/ant/Groovyc.java](https://app.codecov.io/gh/apache/groovy/pull/2835?src=pr&el=tree&filepath=subprojects%2Fgroovy-ant%2Fsrc%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fant%2FGroovyc.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LWFudC9zcmMvbWFpbi9qYXZhL29yZy9jb2RlaGF1cy9ncm9vdnkvYW50L0dyb292eWMuamF2YQ==)
| 83.3333% | [1 Missing :warning:
](https://app.codecov.io/gh/apache/groovy/pull/2835?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/groovy/pull/2835?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ 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
```
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/2835?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Coverage Δ | |
|---|---|---|
|
[...g/codehaus/groovy/ast/ClassCodeVisitorSupport.java](https://app.codecov.io/gh/apache/groovy/pull/2835?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fast%2FClassCodeVisitorSupport.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2FzdC9DbGFzc0NvZGVWaXNpdG9yU3VwcG9ydC5qYXZh)
| `100.0000% <100.0000%> (ø)` | |
|
[...codehaus/groovy/control/CompilerConfiguration.java](https://app.codecov.io/gh/apache/groovy/pull/2835?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fcontrol%2FCompilerConfiguration.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NvbnRyb2wvQ29tcGlsZXJDb25maWd1cmF0aW9uLmphdmE=)
| `75.1534% <100.0000%> (ø)` | |
|
[.../org/codehaus/groovy/tools/FileSystemCompiler.java](https://app.codecov.io/gh/apache/groovy/pull/2835?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Ftools%2FFileSystemCompiler.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3Rvb2xzL0ZpbGVTeXN0ZW1Db21waWxlci5qYXZh)
| `54.7619% <100.0000%> (+0.9524%)` | :arrow_up: |
|
[...roovy/transform/stc/StaticTypeCheckingVisitor.java](https://app.codecov.io/gh/apache/groovy/pull/2835?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Ftransform%2Fstc%2FStaticTypeCheckingVisitor.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3RyYW5zZm9ybS9zdGMvU3RhdGljVHlwZUNoZWNraW5nVmlzaXRvci5qYXZh)
| `87.2758% <100.0000%> (+0.0156%)` | :arrow_up: |
|
[...va/org/codehaus/groovy/control/ErrorCollector.java](https://app.codecov.io/gh/apache/groovy/pull/2835?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fcontrol%2FErrorCollector.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NvbnRyb2wvRXJyb3JDb2xsZWN0b3IuamF2YQ==)
| `63.5417% <50.0000%> (+0.3838%)` | :arrow_up: |
|
[...n/java/org/codehaus/groovy/control/SourceUnit.java](https://app.codecov.io/gh/apache/groovy/pull/2835?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fcontrol%2FSourceUnit.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NvbnRyb2wvU291cmNlVW5pdC5qYXZh)
| `71.9512% <0.0000%> (+5.2846%)` | :arrow_up: |
|
[...src/main/java/org/codehaus/groovy/ant/Groovyc.java](https://app.codecov.io/gh/apache/groovy/pull/2835?src=pr&el=tree&filepath=subprojects%2Fgroovy-ant%2Fsrc%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fant%2FGroovyc.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LWFudC9zcmMvbWFpbi9qYXZhL29yZy9jb2RlaGF1cy9ncm9vdnkvYW50L0dyb292eWMuamF2YQ==)
| `52.7132% <83.3333%> (+0.7524%)` | :arrow_up: |
... and [8 files with indirect coverage
changes](https://app.codecov.io/gh/apache/groovy/pull/2835/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test
Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests,
report on failures, and find test suite problems.
- :package: [JS Bundle
Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save
yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
> 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
> Assignee: 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. Three 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: only {{ErrorCollector.addError(Message)}} performs the {{errors.size()
> >= configuration.getTolerance()}} check. {{ClassCodeVisitorSupport.addError}}
> calls {{addErrorAndContinue}} instead, so every diagnostic raised through a
> visitor bypasses the threshold, and {{StaticTypeCheckingVisitor}} overrides
> {{addError}} to call the collector directly as well.
> Errors raised through {{SourceUnit.addError}} *are* capped correctly, which
> produces a confusing split: 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}} guards the assignment with {{if (tolerance > 0)}}, so
> {{-t 0}} 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. 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. Fix
> # Tolerance is applied uniformly. {{ClassCodeVisitorSupport.addError}} now
> routes through the tolerance-aware {{ErrorCollector.addError}}, and
> {{StaticTypeCheckingVisitor}} does the same for the source unit's own
> collector -- but not for the temporary collectors it pushes for speculative
> checks, whose errors are routinely discarded once a candidate is ruled in or
> out.
> # A tolerance of zero or less means unlimited. The command-line option is
> held in a boxed {{Integer}} so an explicit {{0}} is distinguishable from the
> option being absent, the default is named as
> {{CompilerConfiguration.DEFAULT_TOLERANCE}} rather than repeated as a
> literal, and {{--help}} states it.
> # The Ant {{<groovyc>}} task gains 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.
> The {{-t}} option has been undocumented since it was added in GROOVY-11194,
> so it is now in the groovyc option table, alongside the new Ant attribute in
> that task's table.
> h2. Behavioural change
> Type checking errors now count towards the tolerance, and the default of 10
> therefore caps them where they were previously unbounded. A compilation
> reporting 40 type checking errors will report 10 and stop. Use {{-t 0}} (or
> {{configuration.tolerance = 0}}) to restore full reporting. The default is
> deliberately left at 10 so that the documented contract, already honoured by
> parse and class generation errors, now holds for every error kind.
> This affects any caller reporting more than the tolerance through a
> {{ClassCodeVisitorSupport}} subclass, not only the compiler front ends. In
> particular {{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; the three-argument overload takes an explicit tolerance.
> 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 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}} uses {{new
> CompilerConfiguration(System.getProperties())}} and honours it, whereas
> {{FileSystemCompiler}} uses the no-arg constructor, which never consults the
> property.
> {noformat}
> groovy -Dgroovy.errors.tolerance=50 ... -> 14 errors
> groovyc -Dgroovy.errors.tolerance=50 ... -> 10 errors
> {noformat}
> With the fixes above every path has a first-class route to the setting, so
> 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.
> Documenting the current behaviour is preferred over changing it.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
