[
https://issues.apache.org/jira/browse/GROOVY-12476?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18130236#comment-18130236
]
ASF GitHub Bot commented on GROOVY-12476:
-----------------------------------------
codecov-commenter commented on PR #3005:
URL: https://github.com/apache/groovy/pull/3005#issuecomment-6338436776
##
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/3005?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 `88.00000%` with `3 lines` in your changes missing
coverage. Please review.
:white_check_mark: Project coverage is 72.4398%. Comparing base
([`434b75d`](https://app.codecov.io/gh/apache/groovy/commit/434b75d05fa8bb2628b8908ecac0abd8158474cd?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`491c056`](https://app.codecov.io/gh/apache/groovy/commit/491c05673db8463b4c5a4be4f672a681b446faea?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/3005?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Patch % | Lines |
|---|---|---|
|
[...java/org/codehaus/groovy/runtime/ConvertedMap.java](https://app.codecov.io/gh/apache/groovy/pull/3005?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fruntime%2FConvertedMap.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3J1bnRpbWUvQ29udmVydGVkTWFwLmphdmE=)
| 0.0000% | [1 Missing :warning:
](https://app.codecov.io/gh/apache/groovy/pull/3005?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
|
[.../codehaus/groovy/runtime/DefaultGroovyMethods.java](https://app.codecov.io/gh/apache/groovy/pull/3005?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fruntime%2FDefaultGroovyMethods.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3J1bnRpbWUvRGVmYXVsdEdyb292eU1ldGhvZHMuamF2YQ==)
| 92.8571% | [0 Missing and 1 partial :warning:
](https://app.codecov.io/gh/apache/groovy/pull/3005?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
|
[...roovy/transform/stc/StaticTypeCheckingVisitor.java](https://app.codecov.io/gh/apache/groovy/pull/3005?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)
| 85.7143% | [0 Missing and 1 partial :warning:
](https://app.codecov.io/gh/apache/groovy/pull/3005?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/3005?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #3005 +/- ##
==================================================
+ Coverage 72.4304% 72.4398% +0.0094%
- Complexity 40648 40663 +15
==================================================
Files 1643 1643
Lines 142795 142811 +16
Branches 26844 26852 +8
==================================================
+ Hits 103427 103452 +25
+ Misses 29826 29818 -8
+ Partials 9542 9541 -1
```
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/3005?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/runtime/StringGroovyMethods.java](https://app.codecov.io/gh/apache/groovy/pull/3005?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fruntime%2FStringGroovyMethods.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3J1bnRpbWUvU3RyaW5nR3Jvb3Z5TWV0aG9kcy5qYXZh)
| `82.6456% <100.0000%> (ø)` | |
|
[...java/org/codehaus/groovy/runtime/ConvertedMap.java](https://app.codecov.io/gh/apache/groovy/pull/3005?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fruntime%2FConvertedMap.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3J1bnRpbWUvQ29udmVydGVkTWFwLmphdmE=)
| `91.6667% <0.0000%> (ø)` | |
|
[.../codehaus/groovy/runtime/DefaultGroovyMethods.java](https://app.codecov.io/gh/apache/groovy/pull/3005?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fruntime%2FDefaultGroovyMethods.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3J1bnRpbWUvRGVmYXVsdEdyb292eU1ldGhvZHMuamF2YQ==)
| `75.7059% <92.8571%> (+0.0302%)` | :arrow_up: |
|
[...roovy/transform/stc/StaticTypeCheckingVisitor.java](https://app.codecov.io/gh/apache/groovy/pull/3005?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.1968% <85.7143%> (-0.0024%)` | :arrow_down: |
... and [12 files with indirect coverage
changes](https://app.codecov.io/gh/apache/groovy/pull/3005/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>
> STC: toString() on an interface-typed receiver selects DGM toString(Object)
> over the runtime override (regression from 4.x)
> ---------------------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12476
> URL: https://issues.apache.org/jira/browse/GROOVY-12476
> Project: Groovy
> Issue Type: Bug
> Components: Static compilation, Static Type Checker
> Affects Versions: 5.0.3, 5.0.8, 5.1.3, 6.0.0
> Environment: OpenJDK 25.0.4 (25.0.4+9-LTS), macOS. Groovy 4.0.33 is
> unaffected; 5.0.3, 5.0.8, 5.1.3, 6.0.0-RC-3 and 6.0.0 reproduce.
> Reporter: James Daugherty
> Priority: Major
> Attachments: InterfaceToString.groovy
>
>
> h3. Summary
> Under {{@CompileStatic}} (and {{@TypeChecked}}), a call to {{toString()}} on
> a receiver whose *static type is an interface* ({{List}}, {{Map}}, or any
> user-defined interface) now resolves to the extension method
> {{DefaultGroovyMethods#toString(Object)}} instead of
> {{java.lang.Object#toString()}}. The extension renders through
> {{InvokerHelper.format}}, which formats any {{Collection}} or {{Map}} itself
> and never calls the runtime class's {{toString()}} override.
> Groovy 4.0.33 resolved the same call to {{Object#toString()}} and emitted a
> virtual call, which reached the override. Dynamic dispatch selects the
> override in every version, so adding {{@CompileStatic}} now changes a
> program's output. Nothing fails at compile time.
> h3. Reproducer
> {code:groovy}
> import groovy.transform.CompileStatic
> interface Element {}
> class JsonList extends ArrayList<Object> implements Element {
> @Override
> String toString() { '[{"name":"name","value":"x"}]' }
> }
> @CompileStatic
> class Check {
> static String viaInterface(Element e) { e.toString() }
> static String viaList(List e) { e.toString() }
> static String viaClass(JsonList e) { e.toString() }
> static String viaObject(Object e) { e.toString() }
> }
> JsonList list = new JsonList()
> list << [name: 'name', value: 'x']
> println "Groovy ${GroovySystem.version}"
> println "static, receiver typed Element : ${Check.viaInterface(list)}"
> println "static, receiver typed List : ${Check.viaList(list)}"
> println "static, receiver typed JsonList: ${Check.viaClass(list)}"
> println "static, receiver typed Object : ${Check.viaObject(list)}"
> println "dynamic : ${list.toString()}"
> {code}
> Groovy 4.0.33:
> {noformat}
> static, receiver typed Element : [{"name":"name","value":"x"}]
> static, receiver typed List : [{"name":"name","value":"x"}]
> static, receiver typed JsonList: [{"name":"name","value":"x"}]
> static, receiver typed Object : [[name:name, value:x]]
> dynamic : [{"name":"name","value":"x"}]
> {noformat}
> Groovy 5.0.3, 5.0.8, 5.1.3, 6.0.0-RC-3 and 6.0.0 (identical output):
> {noformat}
> static, receiver typed Element : [[name:name, value:x]]
> static, receiver typed List : [[name:name, value:x]]
> static, receiver typed JsonList: [{"name":"name","value":"x"}]
> static, receiver typed Object : [[name:name, value:x]]
> dynamic : [{"name":"name","value":"x"}]
> {noformat}
> Run with {{java -cp groovy-<version>.jar groovy.ui.GroovyMain
> InterfaceToString.groovy}} on OpenJDK 25.0.4.
> h3. Analysis
> Both versions collect the same two candidates for the call:
> {{java.lang.Object#toString()}}, which
> {{StaticTypeCheckingVisitor#findMethodsWithGenerated}} adds for interface
> receivers, and the extension {{DefaultGroovyMethods#toString(Object)}}. They
> rank them differently:
> * 4.0.33 {{StaticTypeCheckingSupport#chooseBestMethod}} computes one summed
> distance per candidate: the parameter/argument distance (for an extension
> method the receiver is prepended as the self argument), plus the
> declaring-class distance, plus the extension bonus. The self-argument
> distance from the interface to {{Object}} outweighs the bonus, so
> {{Object#toString()}} wins.
> * 5.x and 6.x {{StaticTypeCheckingSupport#chooseBestMethods}} (introduced by
> the GROOVY-8788 fix, commit 14946ff27a60d74b5d07c1b9a63f02624283c863 in
> 5.0.0-alpha-1, which also removed {{getExtensionDistance}}) scores in phases.
> Phase 1 compares declared parameters only, so the extension's self parameter
> no longer counts and both candidates score 0. Phase 2 compares declaring
> class to receiver; both declare in {{Object}}, so they tie again. Phase 3
> prefers the extension method on a tie (the GROOVY-6849 rule), so the
> extension wins for every interface-typed receiver.
> A receiver typed as a class ({{JsonList}}, {{ArrayList}}) is unaffected
> because the class's own {{toString()}} has declaring-class distance 0 and
> wins phase 2. An {{Object}}-typed receiver already resolved to the extension
> in 4.x. The regression is that interface types, which per JLS 9.2 implicitly
> declare the public methods of {{Object}}, are now scored like {{Object}}
> instead of like a class, so {{toString()}} through an interface-typed
> variable, parameter or return type no longer reaches the override.
> h3. Where it bit
> Grails 8 (Groovy 5.1.3): a statically compiled service normalized JSON with
> {{grails.converters.JSON.parse(text).toString()}}. {{JSON.parse}} returns the
> interface {{JSONElement}}; the runtime {{JSONArray}} overrides {{toString()}}
> to render JSON. Under Groovy 5 the stored text silently became Groovy list
> notation ({{[[name:name, value:x]]}}) and failed to parse later. The same
> expression is correct in 4.0.33 and in dynamic Groovy.
> h3. Expected
> {{toString()}} on an interface-typed receiver should compile to a virtual
> {{Object#toString()}} call, as 4.0.33 did and as dynamic Groovy does, or at
> least agree with dynamic dispatch for a class that declares its own
> {{toString()}}.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)