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

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

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

   ## 
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/3221?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 `75.00000%` with `4 lines` in your changes missing 
coverage. Please review.
   :white_check_mark: Project coverage is 74.1676%. Comparing base 
([`49860af`](https://app.codecov.io/gh/apache/groovy/commit/49860af6b85a8c8708463b223b3dc83e1ff2c269?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
 to head 
([`b9bc0ec`](https://app.codecov.io/gh/apache/groovy/commit/b9bc0ec6d162520b65f0b31f7b5bed9aec888134?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/3221?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | Patch % | Lines |
   |---|---|---|
   | 
[...haus/groovy/transform/ASTTestTransformation.groovy](https://app.codecov.io/gh/apache/groovy/pull/3221?src=pr&el=tree&filepath=src%2Fmain%2Fgroovy%2Forg%2Fcodehaus%2Fgroovy%2Ftransform%2FASTTestTransformation.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vZ3Jvb3Z5L29yZy9jb2RlaGF1cy9ncm9vdnkvdHJhbnNmb3JtL0FTVFRlc3RUcmFuc2Zvcm1hdGlvbi5ncm9vdnk=)
 | 75.0000% | [2 Missing and 2 partials :warning: 
](https://app.codecov.io/gh/apache/groovy/pull/3221?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/3221/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/3221?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   
   ```diff
   @@                Coverage Diff                 @@
   ##               master      #3221        +/-   ##
   ==================================================
   - Coverage     74.1685%   74.1676%   -0.0010%     
   - Complexity      43750      43753         +3     
   ==================================================
     Files            1692       1692                
     Lines          149585     149591         +6     
     Branches        28380      28382         +2     
   ==================================================
   + Hits           110945     110948         +3     
   - Misses          28430      28432         +2     
   - Partials        10210      10211         +1     
   ```
   
   | [Files with missing 
lines](https://app.codecov.io/gh/apache/groovy/pull/3221?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | Coverage Δ | |
   |---|---|---|
   | 
[...haus/groovy/transform/ASTTestTransformation.groovy](https://app.codecov.io/gh/apache/groovy/pull/3221?src=pr&el=tree&filepath=src%2Fmain%2Fgroovy%2Forg%2Fcodehaus%2Fgroovy%2Ftransform%2FASTTestTransformation.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vZ3Jvb3Z5L29yZy9jb2RlaGF1cy9ncm9vdnkvdHJhbnNmb3JtL0FTVFRlc3RUcmFuc2Zvcm1hdGlvbi5ncm9vdnk=)
 | `87.3563% <75.0000%> (+0.9366%)` | :arrow_up: |
   
   ... and [8 files with indirect coverage 
changes](https://app.codecov.io/gh/apache/groovy/pull/3221/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>




> @ASTTest on a declaration with several declarators fails with "BUG! ... Tried 
> to overwrite existing meta data"
> --------------------------------------------------------------------------------------------------------------
>
>                 Key: GROOVY-12701
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12701
>             Project: Groovy
>          Issue Type: Bug
>          Components: Compiler
>    Affects Versions: 4.0.32, 5.1.3, 6.0.0
>            Reporter: Paul King
>            Priority: Major
>
> h3. Description
> {{@ASTTest}} on a local variable declaration or field declaration with more 
> than one declarator makes the compiler fail with a {{GroovyBugError}}, 
> whatever the test closure does:
> {code:groovy}
> import groovy.transform.ASTTest
> import org.codehaus.groovy.ast.expr.DeclarationExpression
> def m() {
>     @ASTTest(value={
>         assert node instanceof DeclarationExpression
>     })
>     def a = 1, b = 2
>     a + b
> }
> println m()
> {code}
> {noformat}
> BUG! exception in phase 'semantic analysis' in source unit '...' Tried to 
> overwrite existing meta data 
> org.codehaus.groovy.ast.AnnotationNode@...[@groovy.transform.ASTTest(phase=...,
>  value={ ... })].
> {noformat}
> The same happens for fields:
> {code:groovy}
> import groovy.transform.ASTTest
> class F {
>     @ASTTest(value={
>         assert node.name in ['x', 'y']
>     })
>     int x = 1, y = 2
> }
> {code}
> and for any {{phase}}. A single declarator ({{def a = 1}}) and a multiple 
> assignment ({{def (a, b) = [1, 2]}}) work.
> Expected: the test closure runs once for each declared variable or field, 
> with {{node}} bound to that declaration, as other local AST transformations 
> are invoked for each declarator.
> Reproduced with 4.0.32, 5.1.3, 6.0.0 and master.
> h3. Cause
> The parser attaches the same {{AnnotationNode}} instance to every declarator 
> of the declaration ({{def a = 1, b = 2}} becomes two 
> {{DeclarationExpression}}s, {{int x = 1, y = 2}} two {{FieldNode}}s, sharing 
> the annotation). {{ASTTransformationVisitor}} therefore calls 
> {{ASTTestTransformation#visit}} once per declarator with the same annotation 
> node.
> On the first call, {{ASTTestTransformation}} moves the test closure out of 
> the annotation into node metadata and replaces the {{value}} member with an 
> empty closure:
> {code:groovy}
> annotationNode.setNodeMetaData(ASTTestTransformation, member)
> annotationNode.setMember('value', new 
> ClosureExpression(Parameter.EMPTY_ARRAY, EmptyStatement.INSTANCE))
> {code}
> On the second call {{setNodeMetaData}} finds the key already set and throws. 
> Even if it did not, it would store the empty replacement closure as the test.
> h3. Suggested fix
> In {{ASTTestTransformation#visit}}, do the conversion only the first time an 
> annotation node is seen: if the node metadata is already set, reuse the 
> stored test closure, skip the {{value}} checks and the replacement, and 
> register an {{ASTTester}} for the current node ({{nodes[1]}}). The {{phase}} 
> member is already rewritten into a form that the second call reads correctly.
> This is a general hazard for local transformations that keep state on the 
> annotation node: with several declarators, the same annotation node is passed 
> for each annotated node.



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

Reply via email to