[ 
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>
   
   
   
   [![Impacted file tree 
graph](https://app.codecov.io/gh/apache/groovy/pull/3005/graphs/tree.svg?width=650&height=150&src=pr&token=1r45138NfQ&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)](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)

Reply via email to