[
https://issues.apache.org/jira/browse/GROOVY-12445?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18126032#comment-18126032
]
ASF GitHub Bot commented on GROOVY-12445:
-----------------------------------------
codecov-commenter commented on PR #2972:
URL: https://github.com/apache/groovy/pull/2972#issuecomment-6108299188
##
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/2972?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 `80.00000%` with `1 line` in your changes missing
coverage. Please review.
:white_check_mark: Project coverage is 71.6026%. Comparing base
([`5d37b46`](https://app.codecov.io/gh/apache/groovy/commit/5d37b46bbec1750910a9959bc155345f2e91d515?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`52196b5`](https://app.codecov.io/gh/apache/groovy/commit/52196b59b278cfd27a7b13bbe4b4288e86014980?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/2972?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Patch % | Lines |
|---|---|---|
|
[...roovy/transform/stc/StaticTypeCheckingVisitor.java](https://app.codecov.io/gh/apache/groovy/pull/2972?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)
| 80.0000% | [1 Missing :warning:
](https://app.codecov.io/gh/apache/groovy/pull/2972?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/2972?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #2972 +/- ##
==================================================
- Coverage 71.6089% 71.6026% -0.0063%
+ Complexity 38063 38057 -6
==================================================
Files 1588 1588
Lines 137078 137083 +5
Branches 25507 25509 +2
==================================================
- Hits 98160 98155 -5
- Misses 30014 30020 +6
- Partials 8904 8908 +4
```
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/2972?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Coverage Δ | |
|---|---|---|
|
[...roovy/transform/stc/StaticTypeCheckingVisitor.java](https://app.codecov.io/gh/apache/groovy/pull/2972?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.2350% <80.0000%> (+0.0386%)` | :arrow_up: |
... and [8 files with indirect coverage
changes](https://app.codecov.io/gh/apache/groovy/pull/2972/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: refuse unknown SignatureCodec versions without reading payload bytes
> -------------------------------------------------------------------------
>
> Key: GROOVY-12445
> URL: https://issues.apache.org/jira/browse/GROOVY-12445
> Project: Groovy
> Issue Type: Improvement
> Reporter: Daniel Sun
> Priority: Major
>
> h2. Context
> {{@TypeChecked}} stores an inferred method return type on bytecode as
> {{@TypeChecked.TypeCheckingInfo}}. The annotation has two members:
> * {{version}} — an {{int}}, default {{0}}
> * {{inferredType}} — a Base64 string
> The protocol version is that {{version}} member. The payload string has no
> version byte, magic number, or extra length prefix. This release implements
> version {{1}} only. {{SignatureCodecFactory.getCodec(int)}} returns
> {{SignatureCodecVersion1}} for {{1}}, and {{null}} for every other {{int}}:
> the annotation default {{0}}, any negative value, and {{2}} or higher.
> h2. Problem
> The {{SignatureCodec}} Javadoc said the encoded string itself must carry a
> version so later Groovy releases can decode older payloads. That puts the
> version in the wrong place and invites a reader to parse bytes whose layout
> it does not implement.
> On the write path, {{addTypeCheckingInfoAnnotation}} called
> {{codec.encode(...)}} with no null check. A {{null}} codec would throw
> {{NullPointerException}}.
> On the read path there was no helper that treated a {{null}} codec as "there
> is no inferred signature". Feeding an unknown version to the version-1 codec,
> or decoding anyway, would throw or invent a type from bytes that belong to
> another layout. Two experimental compilers may use the same unpublished
> version {{int}} for different layouts. A release compiler refuses those
> payloads the same way it refuses a higher version it does not implement.
> h2. Contract
> A {{null}} codec means there is no inferred signature. The caller uses the
> method's declared return type. The payload string is left unread.
> ||Version||getCodec||Read path||
> |{{1}}|{{SignatureCodecVersion1}}|decode {{inferredType}}|
> |{{0}}, negative, or {{2}} and higher|{{null}}|declared return type; payload
> unread|
> A version {{int}} that has shipped in a release is append-only: one {{int}}
> keeps one layout. A later codec that claims an older version still decodes
> that older version. The version stays a single {{int}} on the annotation.
> {{getCodec}} continues to return {{null}} for an unimplemented version. The
> write path returns before {{encode}} when the codec is {{null}}.
> {code:java}
> static ClassNode inferredReturnOrDeclared(int version, String signature,
> ClassLoader loader, ClassNode declared) {
> SignatureCodec codec = getCodec(version, loader);
> if (codec == null) {
> return declared;
> }
> return codec.decode(signature);
> }
> {code}
> {{addTypeCheckingInfoAnnotation}} currently has no production callers
> (disconnected under GROOVY-6401). There is also no production reader of
> {{@TypeCheckingInfo}} today. Locking the unknown-version rule now means a
> later reader uses the declared return type for any version this compiler does
> not implement.
> h2. Touch points
> * {{org.codehaus.groovy.transform.stc.SignatureCodec}} — protocol Javadoc
> * {{StaticTypeCheckingVisitor.addTypeCheckingInfoAnnotation}} — return when
> the codec is {{null}}
> * {{SignatureCodecFactory.inferredReturnOrDeclared}} — decode helper
> *
> {{src/test/groovy/org/codehaus/groovy/transform/stc/SignatureCodecTest.groovy}}
> h2. Scope
> This issue covers the unknown-version rule only. Union payload layout,
> rewiring {{addTypeCheckingInfoAnnotation}} onto {{startMethodInference}}, and
> a new compiler diagnostic are separate work.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
