[
https://issues.apache.org/jira/browse/GROOVY-12585?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18136334#comment-18136334
]
ASF GitHub Bot commented on GROOVY-12585:
-----------------------------------------
codecov-commenter commented on PR #3165:
URL: https://github.com/apache/groovy/pull/3165#issuecomment-6572193692
##
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/3165?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.70833%` with `7 lines` in your changes missing
coverage. Please review.
:white_check_mark: Project coverage is 74.1730%. Comparing base
([`4603713`](https://app.codecov.io/gh/apache/groovy/commit/46037138d7dad29fc51d2cdd6a828f7b7ad6bf38?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`fd35106`](https://app.codecov.io/gh/apache/groovy/commit/fd35106cd010326108a94f315d0bf0743feca494?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/3165?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/3165?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)
| 93.7500% | [0 Missing and 3 partials :warning:
](https://app.codecov.io/gh/apache/groovy/pull/3165?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
|
[.../codehaus/groovy/runtime/DefaultGroovyMethods.java](https://app.codecov.io/gh/apache/groovy/pull/3165?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fruntime%2FDefaultGroovyMethods.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3J1bnRpbWUvRGVmYXVsdEdyb292eU1ldGhvZHMuamF2YQ==)
| 66.6667% | [1 Missing and 1 partial :warning:
](https://app.codecov.io/gh/apache/groovy/pull/3165?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
|
[...untime/typehandling/DefaultTypeTransformation.java](https://app.codecov.io/gh/apache/groovy/pull/3165?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fruntime%2Ftypehandling%2FDefaultTypeTransformation.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3J1bnRpbWUvdHlwZWhhbmRsaW5nL0RlZmF1bHRUeXBlVHJhbnNmb3JtYXRpb24uamF2YQ==)
| 95.2381% | [0 Missing and 2 partials :warning:
](https://app.codecov.io/gh/apache/groovy/pull/3165?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/3165?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #3165 +/- ##
==================================================
+ Coverage 74.1568% 74.1730% +0.0163%
- Complexity 43757 43792 +35
==================================================
Files 1692 1692
Lines 149517 149584 +67
Branches 28366 28383 +17
==================================================
+ Hits 110877 110951 +74
+ Misses 28446 28440 -6
+ Partials 10194 10193 -1
```
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/3165?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Coverage Δ | |
|---|---|---|
|
[...rg/codehaus/groovy/runtime/ArrayGroovyMethods.java](https://app.codecov.io/gh/apache/groovy/pull/3165?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fruntime%2FArrayGroovyMethods.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3J1bnRpbWUvQXJyYXlHcm9vdnlNZXRob2RzLmphdmE=)
| `87.3918% <ø> (+0.0671%)` | :arrow_up: |
|
[.../codehaus/groovy/runtime/DefaultGroovyMethods.java](https://app.codecov.io/gh/apache/groovy/pull/3165?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fruntime%2FDefaultGroovyMethods.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3J1bnRpbWUvRGVmYXVsdEdyb292eU1ldGhvZHMuamF2YQ==)
| `75.7645% <66.6667%> (+0.0401%)` | :arrow_up: |
|
[...untime/typehandling/DefaultTypeTransformation.java](https://app.codecov.io/gh/apache/groovy/pull/3165?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fruntime%2Ftypehandling%2FDefaultTypeTransformation.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3J1bnRpbWUvdHlwZWhhbmRsaW5nL0RlZmF1bHRUeXBlVHJhbnNmb3JtYXRpb24uamF2YQ==)
| `66.6667% <95.2381%> (+1.7812%)` | :arrow_up: |
|
[...roovy/transform/stc/StaticTypeCheckingVisitor.java](https://app.codecov.io/gh/apache/groovy/pull/3165?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)
| `88.2452% <93.7500%> (+0.0273%)` | :arrow_up: |
... and [18 files with indirect coverage
changes](https://app.codecov.io/gh/apache/groovy/pull/3165/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>
> Casting a collection to SortedSet, Queue, Deque and similar interfaces fails,
> or with "as" gives a broken proxy
> ---------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12585
> URL: https://issues.apache.org/jira/browse/GROOVY-12585
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Priority: Major
> Fix For: 7.x
>
>
> h3. Problem
> Groovy turns a collection into another collection type when it is assigned or
> cast ({{T x = value}}, {{(T) value}}, through
> {{DefaultTypeTransformation.castToType}}), and when it is coerced with {{as}}
> (through {{DefaultGroovyMethods.asType(Collection, Class)}}). The two paths
> know different default implementations, and neither covers the common
> collection interfaces. So:
> * some assignments fail where the equivalent {{as}} works;
> * some {{as}} coercions return a proxy that does not behave like the
> requested type;
> * an array source and a collection source give different results for the same
> target;
> * a single-element list assigned to some collection classes gives an empty
> collection.
> Results below are the same with indy and classic call sites, on JDK 17 and
> 21. Groups 1 and 2 fail the same way in every release checked, from 2.4.12 to
> 6.0.0.
> h4. 1. Assignment fails where {{as}} works
> ||Target||{{T x = [3, 1, 2, 1]}}||{{[3, 1, 2, 1] as T}}||
> |{{SortedSet}}|{{GroovyCastException}}|{{TreeSet [1, 2, 3]}}|
> |{{Queue}}|{{GroovyCastException}}|{{LinkedList [3, 1, 2, 1]}}|
> |{{Stack}}|{{GroovyCastException}}|{{Stack [3, 1, 2, 1]}}|
> |{{List}} (source {{[3, 1, 2] as
> LinkedHashSet}})|{{GroovyCastException}}|{{ArrayList [3, 1, 2]}}|
> {{asType}} has a table of defaults for {{List}}, {{Set}}, {{SortedSet}},
> {{Queue}} and {{Stack}}. {{castToType}} has none for a {{Collection}} source:
> it gives a {{LinkedHashSet}} when the target accepts one. Otherwise it tries
> the target's constructor with the elements, then with the collection, which
> fails for an interface and for a class without a copy constructor such as
> {{Stack}}.
> An array source goes through a different branch: a no-arg constructor plus
> {{addAll}}. So {{Stack s = new Integer[]\{3, 1, 2\}}} works while {{Stack s =
> [3, 1, 2]}} fails.
> h4. 2. Assignment fails, and {{as}} returns a proxy that does not behave like
> the type
> ||Target||{{T x = value}}||{{value as T}}||
> |{{NavigableSet}}|{{GroovyCastException}}|proxy over the list: {{([3, 1, 2,
> 1] as NavigableSet).size() == 4}}, {{first() == 3}}|
> |{{Deque}}|{{GroovyCastException}}|proxy over the list: {{peekFirst()}}
> throws {{MissingMethodException}}|
> |{{BlockingQueue}}|{{GroovyCastException}}|proxy over the list|
> |{{SortedMap}}, {{NavigableMap}}|{{GroovyCastException}}|dynamic proxy over
> the map: {{firstKey()}} throws {{UnsupportedOperationException}}|
> |{{ConcurrentMap}}|{{GroovyCastException}}|dynamic proxy over the map|
> |{{Iterable}} (array source)|{{GroovyCastException}}|proxy over the array|
> h4. 3. Array and collection sources give different result types
> ||Expression||Result||
> |{{Set s = new Integer[]\{3, 1, 2\}}}|{{LinkedHashSet [3, 1, 2]}}|
> |{{new Integer[]\{3, 1, 2\} as Set}}|{{HashSet [1, 2, 3]}} (order lost)|
> |{{[3, 1, 2] as Set}}|{{LinkedHashSet [3, 1, 2]}}|
> |{{HashSet h = [3, 1, 2]}}|{{LinkedHashSet [3, 1, 2]}}|
> |{{[3, 1, 2] as HashSet}}|{{HashSet [1, 2, 3]}}|
> h4. 4. A single element is passed to a capacity constructor
> Because {{castToType}} tries the target's constructor with the elements
> first, a one-element list picks a capacity constructor:
> ||Expression||Result||
> |{{Vector v = [5]}}|empty {{Vector}} (capacity 5)|
> |{{PriorityQueue q = [5]}}|empty {{PriorityQueue}}|
> |{{ArrayDeque d = [5]}}|empty {{ArrayDeque}}|
> |{{[5] as Vector}}|{{Vector [5]}}|
> h3. Already working (no change intended)
> {{Collection}}, {{Iterable}} (collection source), {{Set}}, {{AbstractSet}},
> {{List}} (list or array source), {{HashSet}}, {{LinkedHashSet}}, {{TreeSet}},
> {{ArrayList}}, {{LinkedList}}, {{Vector}} (more than one element),
> {{ArrayDeque}}, {{PriorityQueue}}, {{ConcurrentLinkedQueue}},
> {{CopyOnWriteArrayList}}, {{Map}}, {{HashMap}}, {{LinkedHashMap}},
> {{AbstractMap}}, {{TreeMap}}, {{ConcurrentHashMap}},
> {{ConcurrentSkipListMap}}. On JDK 21, also {{SequencedCollection}},
> {{SequencedSet}} and {{SequencedMap}}.
> h3. Proposal
> One table of default implementations, used by both {{castToType}} and
> {{asType}}. A value that is already an instance of the target is returned
> unchanged, as now.
> ||Target||Default||
> |{{List}}, {{Collection}}, {{Iterable}}|{{ArrayList}}|
> |{{Set}}|{{LinkedHashSet}}|
> |{{SortedSet}}, {{NavigableSet}}|{{TreeSet}}|
> |{{Queue}}, {{Deque}}|{{LinkedList}} (accepts {{null}} elements, unlike
> {{ArrayDeque}}; matches today's {{as Queue}})|
> |{{BlockingQueue}}|{{LinkedBlockingQueue}}|
> |{{SortedMap}}, {{NavigableMap}}|{{TreeMap}}|
> |{{ConcurrentMap}}|{{ConcurrentHashMap}}|
> For a concrete collection class, assignment and cast try its copy
> constructor, then its no-arg constructor followed by {{addAll}}, and only
> then the elements as constructor arguments. This is the order {{as}} already
> uses, and it fixes group 4. As with {{as}} today (GROOVY-7867), a constructor
> that throws is skipped in favour of the next step, and its exception is kept
> as a suppressed exception if every step fails.
> {{as}} with a concrete class keeps its current path. So the open question is
> answered by keeping today's behaviour: {{HashSet h = list}} still gives a
> {{LinkedHashSet}} (a subclass, so order is kept), and {{list as HashSet}}
> still gives a {{HashSet}}.
> h3. Behaviour changes
> * Assignments in groups 1 and 2 that now fail would succeed.
> * {{as NavigableSet}}, {{as Deque}}, {{as BlockingQueue}}, {{as SortedMap}},
> {{as NavigableMap}} and {{as ConcurrentMap}} would return a real collection
> instead of a proxy, as would {{as Iterable}} and {{as Deque}} with an array
> source. Code relying on the proxy keeping a reference to the original
> collection would see a copy.
> * {{array as Set}} would keep element order ({{LinkedHashSet}} instead of
> {{HashSet}}).
> * A single-element list assigned to a collection class would hold the element
> instead of giving an empty collection (group 4).
> * {{TreeSet}}/{{TreeMap}} targets throw {{ClassCastException}} or
> {{NullPointerException}} for non-{{Comparable}} or {{null}} elements and
> keys, as {{as SortedSet}} and {{TreeSet x = ...}} already do.
> * A map coerced with {{as}} to an interface outside the table, such as
> {{Runnable}} or {{Comparator}}, is still a proxy over the map's closures.
> h3. Out of scope
> The type checker has its own rules for list and map literals, and they are to
> be raised in their own issues:
> * GROOVY-12643: under {{@TypeChecked}} and {{@CompileStatic}}, a literal
> assigned to the interfaces in the table is a compile error, for example
> {{SortedSet s = [3, 1]}}. A literal assigned to a concrete collection class
> is matched against a constructor taking the elements, so {{LinkedList l = [3,
> 4, 5]}} fails with "Cannot find matching constructor
> java.util.LinkedList(int, int, int)". Under {{@CompileStatic}}, {{Vector v =
> [5]}} calls {{Vector(int)}} and stays empty. Until that issue is fixed,
> {{Vector v = [5]}} gives {{[5]}} in dynamic and {{@TypeChecked}} code but
> {{[]}} under {{@CompileStatic}}.
> * GROOVY-12644: under static checking, a literal returned from a method is
> rejected unless its own class is assignable to the return type. {{Set m() \{
> [3, 1] \}}} and {{TreeMap m() \{ [b: 2, a: 1] \}}} fail, although the same
> literal assigned to a {{Set}} or {{TreeMap}} variable compiles.
> * A list or map argument is not converted to a method's parameter type, in
> any mode, by design.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
