[
https://issues.apache.org/jira/browse/GROOVY-12630?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18135974#comment-18135974
]
ASF GitHub Bot commented on GROOVY-12630:
-----------------------------------------
codecov-commenter commented on PR #3160:
URL: https://github.com/apache/groovy/pull/3160#issuecomment-6552965810
##
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/3160?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 `92.30769%` with `7 lines` in your changes missing
coverage. Please review.
:white_check_mark: Project coverage is 74.1391%. Comparing base
([`e488e73`](https://app.codecov.io/gh/apache/groovy/commit/e488e73d89c2f679302969784581bb77f6061e48?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`2a1374e`](https://app.codecov.io/gh/apache/groovy/commit/2a1374e9e155921285c950c52a1f8ff1214b7ca0?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 9 commits behind head on master.
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/3160?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Patch % | Lines |
|---|---|---|
|
[...m/sc/transformers/BinaryExpressionTransformer.java](https://app.codecov.io/gh/apache/groovy/pull/3160?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Ftransform%2Fsc%2Ftransformers%2FBinaryExpressionTransformer.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3RyYW5zZm9ybS9zYy90cmFuc2Zvcm1lcnMvQmluYXJ5RXhwcmVzc2lvblRyYW5zZm9ybWVyLmphdmE=)
| 92.0454% | [1 Missing and 6 partials :warning:
](https://app.codecov.io/gh/apache/groovy/pull/3160?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/3160?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #3160 +/- ##
==================================================
+ Coverage 74.1257% 74.1391% +0.0134%
- Complexity 43650 43699 +49
==================================================
Files 1692 1692
Lines 149322 149465 +143
Branches 28306 28346 +40
==================================================
+ Hits 110686 110812 +126
- Misses 28448 28449 +1
- Partials 10188 10204 +16
```
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/3160?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Coverage Δ | |
|---|---|---|
|
[.../sc/transformers/StaticCompilationTransformer.java](https://app.codecov.io/gh/apache/groovy/pull/3160?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Ftransform%2Fsc%2Ftransformers%2FStaticCompilationTransformer.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3RyYW5zZm9ybS9zYy90cmFuc2Zvcm1lcnMvU3RhdGljQ29tcGlsYXRpb25UcmFuc2Zvcm1lci5qYXZh)
| `95.5056% <100.0000%> (+0.1568%)` | :arrow_up: |
|
[...m/sc/transformers/BinaryExpressionTransformer.java](https://app.codecov.io/gh/apache/groovy/pull/3160?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Ftransform%2Fsc%2Ftransformers%2FBinaryExpressionTransformer.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3RyYW5zZm9ybS9zYy90cmFuc2Zvcm1lcnMvQmluYXJ5RXhwcmVzc2lvblRyYW5zZm9ybWVyLmphdmE=)
| `93.5205% <92.0454%> (-0.4585%)` | :arrow_down: |
... and [19 files with indirect coverage
changes](https://app.codecov.io/gh/apache/groovy/pull/3160/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>
> static-compilation gaps (`++`/`--` and real setter with extension getter)
> -------------------------------------------------------------------------
>
> Key: GROOVY-12630
> URL: https://issues.apache.org/jira/browse/GROOVY-12630
> Project: Groovy
> Issue Type: Bug
> Components: Static compilation
> Affects Versions: 6.0.0
> Reporter: Paul King
> Priority: Major
>
> h2. Description
> Under {{@CompileStatic}}, GROOVY-12524 made compound assignment ({{+=}},
> {{-=}}, {{?=}}, ...) work on a property whose accessors are extension
> methods. Two related forms still fail to compile. Both work in dynamic and
> {{@TypeChecked}} code, and both failed the same way in 6.0.0.
> # Prefix and postfix increment and decrement ({{++}}, {{--}}) on an extension
> property.
> # Compound assignment to a property with a real setter and an extension
> getter.
> h2. Example
> An extension module declares:
> {code:java}
> public class Box { public int v; }
> public class Mix {
> public String v = "a";
> public void setRs(String x) { v = "RS:" + x; } // real setter
> }
> public class BoxExtension {
> public static int getCount(Box b) { return b.v; }
> public static void setCount(Box b, int x) { b.v = x; }
> public static String getRs(Mix m) { return m.v + "E"; } // extension
> getter
> }
> {code}
> Then:
> {code:groovy}
> @groovy.transform.CompileStatic
> void test(Box b, Mix m) {
> b.count += 1 // compiles since GROOVY-12524
> b.count++ // Access to Box#count is forbidden
> ++b.count // Access to Box#count is forbidden
> b.count-- // Access to Box#count is forbidden
> m.rs += 'x' // Access to Mix#rs is forbidden
> }
> {code}
> The same failure occurs for a static extension property ({{Box.sx++}}) and
> for extension properties supplied by Groovy itself.
> The long forms compile and are the workaround:
> {code:groovy}
> b.count = b.count + 1
> m.rs = m.rs + 'x'
> {code}
> A real getter with an extension setter ({{m.rg += 'x'}}) already works.
> h2. Cause
> * *Increment and decrement.* Static compilation has no rewrite for
> {{PrefixExpression}} or {{PostfixExpression}}. Codegen loads the operand as a
> property, and that load does not consider extension getters. A plain read
> works only because {{PropertyExpressionTransformer}} first turns it into a
> getter call.
> * *Mixed accessors.* The GROOVY-12524 rewrite in
> {{BinaryExpressionTransformer}} applies only when the setter is an
> {{ExtensionMethodNode}}. When the setter is a real method, the compound
> assignment takes the old path, which fails on the extension getter for the
> read.
> h2. Suggested fix
> * Let the compound-assignment rewrite apply when *either* accessor is an
> extension method. The type checker already records the extension getter for
> the read side.
> * Rewrite {{++p}} / {{--p}} on such a property to {{setP(getP().next())}} /
> {{setP(getP().previous())}}, whose value is the new value. Rewrite {{p++}} /
> {{p--}} the same way, keeping the old value in a temporary as the
> expression's value. Reuse the getter lookup and setter-call construction from
> GROOVY-12524.
> * The receiver can be evaluated twice, once for the read and once for the
> write. Dynamic, {{@TypeChecked}} and {{@CompileStatic}} code all do this
> today, for real and extension properties alike.
> h2. Test clean-up in Groovy12524
> Three {{shouldFail}} tests pass without reaching the guard they are named
> after, and none of them checks a message:
> * {{testAttributeCompoundAssignOfExtensionPropertyIsRejected}}: {{file.@text
> += 'b'}} fails with an internal compiler error
> ({{ArrayIndexOutOfBoundsException}}). Plain {{file.@text = 'b'}} fails the
> same way, which is a separate pre-existing bug.
> * {{testSpreadSafeCompoundAssignIsNotRewrittenAsExtensionProperty}}: fails on
> a generics error ({{Cannot assign
> java.util.List<java.util.List<java.lang.String>> to:
> java.util.List<java.lang.String>}}) before the spread-safe guard is reached.
> * {{testPrimitiveExtensionSetterWithoutGetterIsRejected}}: fails with
> {{Cannot find matching method java.lang.Object#plus(int)}}.
> Each test should assert the error it expects. If no spread-safe form gets
> past type checking, that guard is defensive only and its test can be dropped.
> The Javadoc of the private method
> {{transformExtensionPropertyCompoundAssignment}} also opens with the JIRA
> key, which CONTRIBUTING asks to keep out of doc comments.
> h2. Not in scope
> * {{b.count **= 2}} is rejected under {{@TypeChecked}} and
> {{@CompileStatic}}, with the same error as for a real {{int}} property ({{int
> ** int}} is a {{Number}}).
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
