[
https://issues.apache.org/jira/browse/GROOVY-12205?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Paul King updated GROOVY-12205:
-------------------------------
Description:
GROOVY-12054, GROOVY-12034 and GROOVY-12043 introduced "fat-free" variants of
common DGM methods accepting {{java.util.function}} types ({{Function}},
{{Predicate}}, {{BiFunction}}, ...) alongside the classic {{Closure}}
overloads, so lambdas and functional values obtained from Java APIs can be used
with Groovy's collection methods without wrapping. Coverage is currently
partial, which makes the surface irregular: a user (or an AI coding assistant)
who learns that {{collect}} accepts a {{Function}} will reasonably expect
{{collectMany}} to do the same, and currently gets a {{MissingMethodException}}.
This umbrella issue proposes completing the surface to a predictable rule:
{quote}Every Closure-taking collection/map DGM method has a
functional-interface twin, unless the JDK already provides the
equivalent.{quote}
Methods currently without any functional variant (Closure-overload count in
parens): {{collectMany}} (8), {{sum}} (6), {{takeWhile}} (6), {{dropWhile}}
(6), {{countBy}} (4), {{split}} (5), {{flatten}} (4, the {{flattenUsing}}
forms), {{injectAll}} (4), {{collectingMany}} (1). Partial coverage to round
out: {{inject}} (2 of 9), {{groupBy}} (1 of 6), and the key-extractor forms of
{{min}}/{{max}}.
Deliberate exception to confirm rather than implement:
{{removeAll}}/{{retainAll}}, where JDK {{Collection#removeIf(Predicate)}}
already provides the functional spelling.
The work partitions naturally into a few grouped commits or sub-tasks:
transforming ({{collectMany}}/{{collectingMany}}/{{flatten}}), reducing
({{inject}}/{{sum}}/{{countBy}}/{{groupBy}}/{{min}}/{{max}}), slicing
({{takeWhile}}/{{dropWhile}}/{{split}}), running-fold ({{injectAll}}). Each
addition follows the conventions established by GROOVY-12054: matching generics
wildcards, inline javadoc test blocks, and where useful the
{{BiFunction}}/{{BiPredicate}} + trailing-param variants mirroring
{{Lambdas.curryWith}}.
h3. Method-ambiguity assessment
*The load-bearing invariant holds:* {{groovy.lang.Closure}} implements none of
the {{java.util.function}} interfaces, so a Closure argument is never
assignable to a {{Function}}/{{Predicate}}/{{Consumer}}/{{Comparator}}
parameter. Any real closure, lambda, or method reference disambiguates by type
and arity. This is the invariant GROOVY-12054 relied on, and the remaining
methods are structurally identical.
Verified empirically against {{max}}/{{min}}/{{sort}}, which have carried
{{Closure}} + {{Comparator}} overloads for years (the exact pattern fat-free
work creates):
||Call||Dynamic dispatch||@CompileStatic||
|{{max(null)}} (Closure + Comparator)|Resolves _deterministically_ to
{{Comparator}}, then NPEs — *no ambiguity error*|*Hard compile error*:
"Reference to method is ambiguous. Cannot choose between \[max(Closure),
max(Comparator)\]"|
|{{max(s -> s.length())}} — 1-arg lambda|Fine|Fine — arity disambiguates|
|Closure literal (block form)|Fine|Fine — selects the Closure overload|
|Method reference {{String::length}}|Fine|Fine — selects the functional twin|
h3. Residual risks
# *Literal {{null}} argument — the one real case, and narrower than it first
appears.*
#* Dynamically it is _not_ a crash-ambiguity: Groovy resolves {{foo(null)}}
deterministically to one overload (for {{max}}/{{min}} it picks
{{Comparator}}). The hazard is a _silent behaviour change_ — the twin may win
where the Closure used to — and only ever for an untyped literal {{null}}.
#* The hard ambiguity _error_ appears only under {{@CompileStatic}}. This is
long-standing Groovy behaviour (reproducible on 4.0.27, predating all fat-free
work); GROOVY-12054 did not introduce it and this work does not worsen it in
kind — each twin just extends it to one more method.
#* Concrete regression: methods that today have a _single_ Closure overload
({{collectMany}}, {{sum}}, {{split}}, {{countBy}}, {{takeWhile}},
{{dropWhile}}, {{flatten}}, {{injectAll}}, and the remaining
{{inject}}/{{groupBy}} forms) would flip a {{@CompileStatic}} {{foo(null)}}
call from _compiles_ to _compile error_. In practice nobody writes
{{collectMany(null)}}; GROOVY-12054 proved the blast radius is nil (no call
site in the tree needed a cast). Treat it as a grep, not a blocker.
# *{{sum}} — semantic (not compile) shift.* {{sum(Iterable, Object
initialValue)}} already exists; a new {{sum(Iterable, Function)}} is _more
specific_ than {{Object}}, so real lambdas/method-refs resolve correctly — but
an object that happens to implement {{Function}} and was being passed as
{{initialValue}} would silently switch meaning. Low probability.
# *{{min}}/{{max}} — do not add key-extractor {{Function}} twins.* They would
create the only mixed-SAM site ({{Comparator}} arity-2 + {{Function}} arity-1),
and are redundant with the existing {{Comparator}} overload + JDK
{{Comparator.comparing}} — the "JDK already provides it" carve-out. Still add
the {{Comparator}} twin for the *Map* receiver, which genuinely lacks one.
# *Bare lambda literals under {{@CompileStatic}}* are already settled ({{\{
\}}} -> Closure, {{::}}/typed SAM -> twin). Extend {{LambdasTest}} with an STC
case per new method group so a future STC change cannot regress it silently.
*Bottom line:* no broad ambiguity hazard. The only concrete regression is
{{@CompileStatic}} + literal {{null}} on single-overload methods, which is
rare, pre-existing in kind, and grep-checkable.
was:
GROOVY-12054, GROOVY-12034 and GROOVY-12043 introduced "fat-free" variants of
common DGM methods accepting {{java.util.function}} types ({{Function}},
{{Predicate}}, {{BiFunction}}, ...) alongside the classic {{Closure}}
overloads, so lambdas and functional values obtained from Java APIs can be used
with Groovy's collection methods without wrapping. Coverage is currently
partial, which makes the surface irregular: a user (or an AI coding assistant)
who learns that {{collect}} accepts a {{Function}} will reasonably expect
{{collectMany}} to do the same, and currently gets a {{MissingMethodException}}.
This umbrella issue proposes completing the surface to a predictable rule:
{quote}Every Closure-taking collection/map DGM method has a
functional-interface twin, unless the JDK already provides the
equivalent.{quote}
Methods currently without any functional variant (Closure-overload count in
parens): {{collectMany}} (8), {{sum}} (6), {{takeWhile}} (6), {{dropWhile}}
(6), {{countBy}} (4), {{split}} (5), {{flatten}} (4, the {{flattenUsing}}
forms), {{injectAll}} (4), {{collectingMany}} (1). Partial coverage to round
out: {{inject}} (2 of 9), {{groupBy}} (1 of 6), and the key-extractor forms of
{{min}}/{{max}}.
Deliberate exception to confirm rather than implement:
{{removeAll}}/{{retainAll}}, where JDK {{Collection#removeIf(Predicate)}}
already provides the functional spelling.
The work partitions naturally into a few grouped commits or sub-tasks:
transforming ({{collectMany}}/{{collectingMany}}/{{flatten}}), reducing
({{inject}}/{{sum}}/{{countBy}}/{{groupBy}}/{{min}}/{{max}}), slicing
({{takeWhile}}/{{dropWhile}}/{{split}}), running-fold ({{injectAll}}). Each
addition follows the conventions established by GROOVY-12054: matching generics
wildcards, inline javadoc test blocks, and where useful the
{{BiFunction}}/{{BiPredicate}} + trailing-param variants mirroring
{{Lambdas.curryWith}}.
> Complete the functional-interface (fat-free) variants of Closure-taking DGM
> methods
> -----------------------------------------------------------------------------------
>
> Key: GROOVY-12205
> URL: https://issues.apache.org/jira/browse/GROOVY-12205
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Priority: Major
>
> GROOVY-12054, GROOVY-12034 and GROOVY-12043 introduced "fat-free" variants of
> common DGM methods accepting {{java.util.function}} types ({{Function}},
> {{Predicate}}, {{BiFunction}}, ...) alongside the classic {{Closure}}
> overloads, so lambdas and functional values obtained from Java APIs can be
> used with Groovy's collection methods without wrapping. Coverage is currently
> partial, which makes the surface irregular: a user (or an AI coding
> assistant) who learns that {{collect}} accepts a {{Function}} will reasonably
> expect {{collectMany}} to do the same, and currently gets a
> {{MissingMethodException}}.
> This umbrella issue proposes completing the surface to a predictable rule:
> {quote}Every Closure-taking collection/map DGM method has a
> functional-interface twin, unless the JDK already provides the
> equivalent.{quote}
> Methods currently without any functional variant (Closure-overload count in
> parens): {{collectMany}} (8), {{sum}} (6), {{takeWhile}} (6), {{dropWhile}}
> (6), {{countBy}} (4), {{split}} (5), {{flatten}} (4, the {{flattenUsing}}
> forms), {{injectAll}} (4), {{collectingMany}} (1). Partial coverage to round
> out: {{inject}} (2 of 9), {{groupBy}} (1 of 6), and the key-extractor forms
> of {{min}}/{{max}}.
> Deliberate exception to confirm rather than implement:
> {{removeAll}}/{{retainAll}}, where JDK {{Collection#removeIf(Predicate)}}
> already provides the functional spelling.
> The work partitions naturally into a few grouped commits or sub-tasks:
> transforming ({{collectMany}}/{{collectingMany}}/{{flatten}}), reducing
> ({{inject}}/{{sum}}/{{countBy}}/{{groupBy}}/{{min}}/{{max}}), slicing
> ({{takeWhile}}/{{dropWhile}}/{{split}}), running-fold ({{injectAll}}). Each
> addition follows the conventions established by GROOVY-12054: matching
> generics wildcards, inline javadoc test blocks, and where useful the
> {{BiFunction}}/{{BiPredicate}} + trailing-param variants mirroring
> {{Lambdas.curryWith}}.
> h3. Method-ambiguity assessment
> *The load-bearing invariant holds:* {{groovy.lang.Closure}} implements none
> of the {{java.util.function}} interfaces, so a Closure argument is never
> assignable to a {{Function}}/{{Predicate}}/{{Consumer}}/{{Comparator}}
> parameter. Any real closure, lambda, or method reference disambiguates by
> type and arity. This is the invariant GROOVY-12054 relied on, and the
> remaining methods are structurally identical.
> Verified empirically against {{max}}/{{min}}/{{sort}}, which have carried
> {{Closure}} + {{Comparator}} overloads for years (the exact pattern fat-free
> work creates):
> ||Call||Dynamic dispatch||@CompileStatic||
> |{{max(null)}} (Closure + Comparator)|Resolves _deterministically_ to
> {{Comparator}}, then NPEs — *no ambiguity error*|*Hard compile error*:
> "Reference to method is ambiguous. Cannot choose between \[max(Closure),
> max(Comparator)\]"|
> |{{max(s -> s.length())}} — 1-arg lambda|Fine|Fine — arity disambiguates|
> |Closure literal (block form)|Fine|Fine — selects the Closure overload|
> |Method reference {{String::length}}|Fine|Fine — selects the functional twin|
> h3. Residual risks
> # *Literal {{null}} argument — the one real case, and narrower than it first
> appears.*
> #* Dynamically it is _not_ a crash-ambiguity: Groovy resolves {{foo(null)}}
> deterministically to one overload (for {{max}}/{{min}} it picks
> {{Comparator}}). The hazard is a _silent behaviour change_ — the twin may win
> where the Closure used to — and only ever for an untyped literal {{null}}.
> #* The hard ambiguity _error_ appears only under {{@CompileStatic}}. This is
> long-standing Groovy behaviour (reproducible on 4.0.27, predating all
> fat-free work); GROOVY-12054 did not introduce it and this work does not
> worsen it in kind — each twin just extends it to one more method.
> #* Concrete regression: methods that today have a _single_ Closure overload
> ({{collectMany}}, {{sum}}, {{split}}, {{countBy}}, {{takeWhile}},
> {{dropWhile}}, {{flatten}}, {{injectAll}}, and the remaining
> {{inject}}/{{groupBy}} forms) would flip a {{@CompileStatic}} {{foo(null)}}
> call from _compiles_ to _compile error_. In practice nobody writes
> {{collectMany(null)}}; GROOVY-12054 proved the blast radius is nil (no call
> site in the tree needed a cast). Treat it as a grep, not a blocker.
> # *{{sum}} — semantic (not compile) shift.* {{sum(Iterable, Object
> initialValue)}} already exists; a new {{sum(Iterable, Function)}} is _more
> specific_ than {{Object}}, so real lambdas/method-refs resolve correctly —
> but an object that happens to implement {{Function}} and was being passed as
> {{initialValue}} would silently switch meaning. Low probability.
> # *{{min}}/{{max}} — do not add key-extractor {{Function}} twins.* They would
> create the only mixed-SAM site ({{Comparator}} arity-2 + {{Function}}
> arity-1), and are redundant with the existing {{Comparator}} overload + JDK
> {{Comparator.comparing}} — the "JDK already provides it" carve-out. Still add
> the {{Comparator}} twin for the *Map* receiver, which genuinely lacks one.
> # *Bare lambda literals under {{@CompileStatic}}* are already settled ({{\{
> \}}} -> Closure, {{::}}/typed SAM -> twin). Extend {{LambdasTest}} with an
> STC case per new method group so a future STC change cannot regress it
> silently.
> *Bottom line:* no broad ambiguity hazard. The only concrete regression is
> {{@CompileStatic}} + literal {{null}} on single-overload methods, which is
> rare, pre-existing in kind, and grep-checkable.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
