[
https://issues.apache.org/jira/browse/GROOVY-12160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18095830#comment-18095830
]
ASF GitHub Bot commented on GROOVY-12160:
-----------------------------------------
codecov-commenter commented on PR #2705:
URL: https://github.com/apache/groovy/pull/2705#issuecomment-4956213370
##
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/2705?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 69.1132%. Comparing base
([`86c6648`](https://app.codecov.io/gh/apache/groovy/commit/86c6648d1e61436fe513afaa85d8d0c403de3cda?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`72d9bb9`](https://app.codecov.io/gh/apache/groovy/commit/72d9bb94b81cf85e085d6f181e4c773e045749ea?dropdown=coverage&el=desc&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/2705?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #2705 +/- ##
==================================================
+ Coverage 69.1077% 69.1132% +0.0054%
- Complexity 34235 34236 +1
==================================================
Files 1537 1537
Lines 129356 129356
Branches 23503 23503
==================================================
+ Hits 89395 89402 +7
+ Misses 31941 31933 -8
- Partials 8020 8021 +1
```
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/2705?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Coverage Δ | |
|---|---|---|
|
[...groovy/transform/AnnotationCollectorTransform.java](https://app.codecov.io/gh/apache/groovy/pull/2705?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Ftransform%2FAnnotationCollectorTransform.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3RyYW5zZm9ybS9Bbm5vdGF0aW9uQ29sbGVjdG9yVHJhbnNmb3JtLmphdmE=)
| `90.6404% <100.0000%> (ø)` | |
... and [8 files with indirect coverage
changes](https://app.codecov.io/gh/apache/groovy/pull/2705/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>
> @AnnotationCollector emits collected annotation members in hash order when
> the collector is precompiled
> -------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12160
> URL: https://issues.apache.org/jira/browse/GROOVY-12160
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
> h2. Description
> When a collector annotation is precompiled, {{AnnotationCollectorTransform}}
> reads its preset members back from the generated {{CollectorHelper}} and
> copies them onto the annotations the collector expands to. That copy goes
> through an unordered map:
> {code:java}
> // AnnotationCollectorTransform.java:316-337 (makeListOfAnnotations)
> for (Object[] inner : data) {
> Class<?> anno = (Class<?>) inner[0];
> AnnotationNode toAdd = new AnnotationNode(ClassHelper.make(anno));
> ret.add(toAdd);
> Map<String,Object> member = (Map<String, Object>) inner[1];
> if (member.isEmpty()) {
> continue;
> }
> Map<String, Expression> generated = new HashMap<>(member.size());
> for (Map.Entry<String, Object> entry : member.entrySet()) {
> generated.put(entry.getKey(), makeExpression(entry.getValue()));
> }
> copyMembers(generated, toAdd); // iterates `generated` ->
> toAdd.addMember(...)
> }
> {code}
> {{copyMembers}} ({{:274}}) iterates that map into
> {{AnnotationNode.addMember}}, and those members are written to the class
> file's annotation attribute in that order. So *the members of the expanded
> annotation are emitted in the hash order of their names*.
> Declaration order is intact everywhere else in the chain and is lost only
> here: {{serialize(AnnotationNode)}} ({{:184}}) builds the helper's map by
> iterating {{AnnotationNode.getMembers()}}, which is a {{LinkedHashMap}}, so
> the order the members were declared on the collector survives into the
> precompiled {{CollectorHelper}}.
> h3. Reachability
> Only when the collector is *precompiled* — i.e. resolved from the classpath,
> so the members are read reflectively from its {{CollectorHelper}} via
> {{getTargetListFromClass}} ({{:280-290}}). A collector declared in the same
> compilation unit takes the AST-based path, which preserves order. The
> precompiled case is the normal one for any collector shipped in a library,
> including Groovy's own {{@Immutable}} and {{@Canonical}}.
> h3. Demonstration
> Compilation unit 1 (the collector, presetting five members):
> {code:java}
> import groovy.transform.AnnotationCollector
> import java.lang.annotation.*
> @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.TYPE)
> @interface Marker {
> String alpha() default ''
> String beta() default ''
> String gamma() default ''
> String delta() default ''
> String epsilon() default ''
> }
> @Marker(alpha='a', beta='b', gamma='c', delta='d', epsilon='e')
> @AnnotationCollector
> @interface Coll {}
> {code}
> Compilation unit 2, compiled against the first:
> {code:java}
> @Coll
> class Holder {}
> {code}
> Reading the emitted {{Holder.class}} with an ASM {{ClassReader}}, the members
> of the {{@Marker}} annotation appear as:
> {noformat}
> delta, epsilon, beta, gamma, alpha
> {noformat}
> rather than the declared order {{alpha, beta, gamma, delta, epsilon}}.
> h3. Impact
> The keys are {{String}}s and {{String.hashCode}} is stable, so for a given
> key set the iteration order is reproducible across JVM runs — this is not by
> itself a live reproducible-builds failure. It still matters because the
> emitted order is arbitrary rather than meaningful, it lands in the
> {{RuntimeVisibleAnnotations}} attribute of the class file, and within a
> colliding bucket {{HashMap}} chain order follows insertion order. Of the
> related ordering issues below, this one has the most ordinary trigger: any
> classpath collector with more than one preset member.
> h3. Proposed fix
> Use an insertion-ordered map:
> {code:java}
> Map<String, Expression> generated = new LinkedHashMap<>(member.size());
> {code}
> The collected members are then emitted in the order they were declared on the
> collector. The member *set* and their values are unchanged — only the order.
> h3. Test
> {{src/test/groovy/org/codehaus/groovy/transform/AnnotationCollectorMemberOrderTest.groovy}}
> compiles the annotation and collector into a temp directory as one
> compilation unit, applies the collector from a second unit with that
> directory on the classpath (so the precompiled path is exercised), then reads
> the emitted bytes with an ASM {{ClassReader}} and asserts the annotation's
> members appear in declaration order. It fails on the current code with
> {{['delta', 'epsilon', 'beta', 'gamma', 'alpha']}} and passes with the fix.
> h3. Related
> GROOVY-12149 (nondeterministic reflection order flows into generated
> bytecode). Same class of defect: {{Verifier.addCovariantMethods}} (covariant
> bridge methods emitted in hash order),
> {{AutoImplementASTTransformation.getAllCorrectedMethodsMap}}
> ({{@AutoImplement}} methods emitted in hash order), {{AsmClassGenerator}}
> {{referencedClasses}} ({{$class$}} synthetic fields and accessors), and
> GROOVY-12156 ({{chooseBestMethod}} candidates in identity-hash order).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
