[ 
https://issues.apache.org/jira/browse/GROOVY-12550?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18134339#comment-18134339
 ] 

ASF GitHub Bot commented on GROOVY-12550:
-----------------------------------------

codecov-commenter commented on PR #3079:
URL: https://github.com/apache/groovy/pull/3079#issuecomment-6504482082

   ## 
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/3079?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 `50.00000%` with `1 line` in your changes missing 
coverage. Please review.
   :white_check_mark: Project coverage is 73.6759%. Comparing base 
([`5b077f9`](https://app.codecov.io/gh/apache/groovy/commit/5b077f9dcf610898e3415f1f1cebbaab8eecab61?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
 to head 
([`ae7cd17`](https://app.codecov.io/gh/apache/groovy/commit/ae7cd17d8968eeb013e6d96f7f5cde70596b26cb?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/3079?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | Patch % | Lines |
   |---|---|---|
   | 
[...rg/codehaus/groovy/classgen/asm/ClosureWriter.java](https://app.codecov.io/gh/apache/groovy/pull/3079?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2Fasm%2FClosureWriter.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL2FzbS9DbG9zdXJlV3JpdGVyLmphdmE=)
 | 50.0000% | [0 Missing and 1 partial :warning: 
](https://app.codecov.io/gh/apache/groovy/pull/3079?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/3079/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/3079?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   
   ```diff
   @@                Coverage Diff                 @@
   ##               master      #3079        +/-   ##
   ==================================================
   + Coverage     73.6711%   73.6759%   +0.0048%     
   - Complexity      42584      42587         +3     
   ==================================================
     Files            1678       1678                
     Lines          146907     146907                
     Branches        27790      27790                
   ==================================================
   + Hits           108228     108235         +7     
   + Misses          28528      28523         -5     
   + Partials        10151      10149         -2     
   ```
   
   | [Files with missing 
lines](https://app.codecov.io/gh/apache/groovy/pull/3079?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | Coverage Δ | |
   |---|---|---|
   | 
[...rg/codehaus/groovy/classgen/asm/ClosureWriter.java](https://app.codecov.io/gh/apache/groovy/pull/3079?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2Fasm%2FClosureWriter.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL2FzbS9DbG9zdXJlV3JpdGVyLmphdmE=)
 | `90.0404% <50.0000%> (ø)` | |
   
   ... and [10 files with indirect coverage 
changes](https://app.codecov.io/gh/apache/groovy/pull/3079/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: a closure's doCall drops the type arguments of a return type whose class 
> is in the same compilation unit
> -------------------------------------------------------------------------------------------------------------
>
>                 Key: GROOVY-12550
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12550
>             Project: Groovy
>          Issue Type: Bug
>          Components: Static compilation
>    Affects Versions: 4.0.33, 5.1.3
>            Reporter: James Daugherty
>            Priority: Major
>
> With static type checking, the {{doCall}} method of a closure is declared 
> with the closure's inferred return type. When that type is a parameterized 
> type whose class is in the same compilation unit, its type arguments are 
> dropped. When the class is read from the class path, they are kept. The same 
> source therefore compiles to a closure class with a different signature 
> depending on what else is compiled, which incremental builds and build caches 
> see as a change.
> {code:groovy}
> interface Up {}
> class Res<T> { List<T> items = [] }
> @groovy.transform.CompileStatic
> class Svc {
>     static <V> V timeIt(Closure<V> body) { body.call() }
>     def <U extends Up> Res<U> read(Class<U> type) { new Res<U>() }
>     Res<Up> load(Class<Up> type) { timeIt { -> read(type) } }
> }
> {code}
> {noformat}
> Svc compiled together with Res:     public Res doCall()
> Svc compiled against a compiled Res: public Res<Up> doCall()
> {noformat}
> Reproduced with 4.0.33, 5.1.3 and master. A reproducer with a groovyc script 
> and a Gradle build: 
> https://github.com/jdaugherty/groovy-closure-return-type-signature
> Grails applications hit this in incremental builds: {{@Transactional}} and 
> similar transformations move a method body into a new method without the 
> method's type parameters, so a closure there returns a concrete parameterized 
> type.
> *Cause*
> {{ClosureWriter.createClosureClass}} normalises the inferred return type with:
> {code:java}
> else if (returnType.isPrimaryClassNode()) returnType = 
> returnType.getPlainNodeReference();
> {code}
> {{isPrimaryClassNode()}} is true for the class node of a class in the 
> compilation unit and for every reference to it, including a parameterized one 
> such as {{Res<Up>}}, and {{getPlainNodeReference()}} drops the type 
> arguments. A reference to a class read from bytecode is not primary, so it 
> keeps them unless they are unresolved. On master, 
> {{ClosureWriter.inferredClosureReturnType}} does the same for packed closures.
> *Proposed fix*
> Only replace the class node itself, which carries the declared type 
> parameters, by its plain reference: {{returnType.isPrimaryClassNode() && 
> !returnType.isRedirectNode()}}. A parameterized reference then goes through 
> the same check for unresolved generics as any other type, and both builds 
> produce {{Res<Up> doCall()}}, the signature a closure already gets for a 
> parameterized library type such as {{List<String>}}.
> Branches with the fix and a test, {{ClosureReturnTypeSignatureTest}}, that 
> compiles the example both ways and compares the {{doCall}} signatures. The 
> full core test suite passes on both:
> * master: 
> https://github.com/jdaugherty/groovy/tree/closure-return-type-signature
> * GROOVY_5_1_X: 
> https://github.com/jdaugherty/groovy/tree/closure-return-type-signature-5_1_X
> Related: GROOVY-10932 and GROOVY-12544, other differences between classes of 
> the compilation unit and pre-compiled classes that affect incremental builds.



--
This message was sent by Atlassian Jira
(v10.3.26#10030026)

Reply via email to