[
https://issues.apache.org/jira/browse/GROOVY-12225?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18101191#comment-18101191
]
ASF GitHub Bot commented on GROOVY-12225:
-----------------------------------------
codecov-commenter commented on PR #2756:
URL: https://github.com/apache/groovy/pull/2756#issuecomment-5162120250
##
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/2756?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 `57.14286%` with `3 lines` in your changes missing
coverage. Please review.
:white_check_mark: Project coverage is 69.9753%. Comparing base
([`3b82bb1`](https://app.codecov.io/gh/apache/groovy/commit/3b82bb18b0751e22cfe2758848c8a85a3b6bd582?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`6191cd0`](https://app.codecov.io/gh/apache/groovy/commit/6191cd0a0a7c9464e589af2b637eee2cac52593b?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/2756?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Patch % | Lines |
|---|---|---|
|
[...ovy/classgen/asm/sc/StaticTypesCallSiteWriter.java](https://app.codecov.io/gh/apache/groovy/pull/2756?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2Fasm%2Fsc%2FStaticTypesCallSiteWriter.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL2FzbS9zYy9TdGF0aWNUeXBlc0NhbGxTaXRlV3JpdGVyLmphdmE=)
| 57.1429% | [1 Missing and 2 partials :warning:
](https://app.codecov.io/gh/apache/groovy/pull/2756?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/2756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #2756 +/- ##
==================================================
- Coverage 69.9806% 69.9753% -0.0052%
- Complexity 35503 35504 +1
==================================================
Files 1557 1557
Lines 131668 131675 +7
Branches 24166 24169 +3
==================================================
- Hits 92142 92140 -2
- Misses 31191 31194 +3
- Partials 8335 8341 +6
```
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/2756?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Coverage Δ | |
|---|---|---|
|
[...ovy/classgen/asm/sc/StaticTypesCallSiteWriter.java](https://app.codecov.io/gh/apache/groovy/pull/2756?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2Fasm%2Fsc%2FStaticTypesCallSiteWriter.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL2FzbS9zYy9TdGF0aWNUeXBlc0NhbGxTaXRlV3JpdGVyLmphdmE=)
| `77.5378% <57.1429%> (-0.3131%)` | :arrow_down: |
... and [5 files with indirect coverage
changes](https://app.codecov.io/gh/apache/groovy/pull/2756/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: property access to a record component fails for precompiled records
> -------------------------------------------------------------------------
>
> Key: GROOVY-12225
> URL: https://issues.apache.org/jira/browse/GROOVY-12225
> Project: Groovy
> Issue Type: Bug
> Affects Versions: 4.0.33, 5.0.8, 6.0.0-beta-1
> Reporter: Paul King
> Priority: Major
>
> Property access to a record component ({{p.x}}) should resolve to the
> component accessor {{x()}}. Under {{@CompileStatic}} it instead resolves to
> the private backing field and fails at bytecode generation:
> {noformat}
> Access to demo.Point#x is forbidden
> {noformat}
> This only happens when the record is *precompiled* (read from a class file).
> A record declared in the same compilation unit works.
> h2. Reproducing
> Compile these to a directory first, then compile the consumer against it:
> {code:java}
> // demo/Point.groovy -- compiled separately
> package demo
> record Point(int x, int y) { }
> {code}
> {code:java}
> import groovy.transform.CompileStatic
> import groovy.transform.TypeChecked
> import demo.Point
> @TypeChecked
> static int typeChecked(Point p) { p.x } // OK
> @CompileStatic
> static int accessorForm(Point p) { p.x() } // OK
> @CompileStatic
> static int compileStatic(Point p) { p.x } // FAILS: Access to demo.Point#x
> is forbidden
> {code}
> h2. What works and what doesn't
> ||Form||Result||
> |{{p.x}} -- dynamic|works|
> |{{p.x}} -- {{@TypeChecked}}|works|
> |{{p.x}} -- {{@CompileStatic}}, record in the *same* compilation unit|works|
> |{{p.x}} -- {{@CompileStatic}}, record *precompiled* (Groovy)|*fails*|
> |{{p.x}} -- {{@CompileStatic}}, record *precompiled* (Java)|*fails*|
> |{{p.x()}} -- {{@CompileStatic}}|works (workaround)|
> Two discriminators worth noting:
> * {{@TypeChecked}} passes and {{@CompileStatic}} fails, so the type checker
> resolves the property correctly and the failure is in the call-site writer.
> * Java records fail identically, so this is about how the record is _read_,
> not how it was produced.
> h2. Root cause
> {{StaticTypesCallSiteWriter.makeGetPropertyWithGetter}} looks for a getter in
> this order:
> {code:java}
> String getterName = "is" + capitalize(propertyName);
> MethodNode getterNode = receiverType.getGetterMethod(getterName);
> if (getterNode == null) {
> getterName = "get" + capitalize(propertyName);
> getterNode = receiverType.getGetterMethod(getterName);
> }
> // GROOVY-5561: same-source-unit fallback
> PropertyNode propertyNode = receiverType.getProperty(propertyName);
> {code}
> A record component accessor is {{x()}}, not {{getX()}}/{{isX()}}, so both
> name-based lookups miss. The third lookup -- the GROOVY-5561 same-source-unit
> fallback -- is what rescues the same-unit case, because the record transform
> has already attached a {{PropertyNode}}. For a record read from a class file
> there is no {{PropertyNode}}, so resolution falls through to field access and
> {{addPropertyAccessError}} fires.
> Inspecting the decompiled {{ClassNode}} shows everything needed is already
> present, it just isn't consulted:
> {noformat}
> precompiled Groovy record demo.Point precompiled Java record demo.JPoint
> isRecord() : true isRecord() : true
> recordComponents : [x, y] recordComponents : [x, y]
> getProperty('x') : null getProperty('x') : null
> getField('x') : private int x getField('x') : private
> int x
> getDeclaredMethod x() : int x() getDeclaredMethod x() : int x()
> getGetterMethod('getX') : null getGetterMethod('getX') : null
> {noformat}
> h2. Suggested fix
> Add a record-component branch to {{makeGetPropertyWithGetter}}, before the
> field fallback: if {{receiverType.isRecord()}} and a record component matches
> {{propertyName}}, use the {{propertyName()}} accessor as the getter.
> {{ClassNode}} already exposes {{isRecord()}} and {{getRecordComponents()}} on
> decompiled types.
> h2. Impact
> Any {{@CompileStatic}} code consuming records from a jar hits this.
> Concretely it affects Groovy's own modules: {{groovy.http.HttpResult}} in
> {{groovy-http-builder}} is a record, so {{result.status}} and {{result.body}}
> cannot be used from statically compiled code without the {{()}} workaround.
> h2. Affects
> Reproduced on 6.0.0-beta-1 and a current master build, JDK 21.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
