[
https://issues.apache.org/jira/browse/GROOVY-9848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18092240#comment-18092240
]
Paul King edited comment on GROOVY-9848 at 6/29/26 6:38 AM:
------------------------------------------------------------
h1. Path forward: decouple {{in}} membership from {{isCase}}
h2. Current behaviour (Groovy 6.0 / master)
* {{isCase(Map, Object)}} returns {{castToBoolean(map.get(switchValue))}} --
i.e. "the
key is present *and* its value is Groovy-truthy".
* The {{in}}/{{!in}} operators compile to
{{ScriptBytecodeAdapter.isCase}}/{{isNotCase}}
({{BinaryExpressionHelper}}, {{KEYWORD_IN}}/{{COMPARE_NOT_IN}}). {{switch}}
uses the same
{{isCase}} (via {{getIsCaseMethod}}); {{grep}} uses {{isCase}} internally in
DGM.
* There is no {{isCase(CharSequence, ...)}}, so {{'ell' in 'hello'}} is
{{false}} (falls to
{{isCase(Object,Object)}} == equality). This is the open GROOVY-2456.
* PR #1904 only added a test pinning the *current* behaviour; semantics are
unchanged.
Consequences users keep hitting:
* {{'b' in [a:1, b:0]}} is {{false}} (value {{0}} is falsy) -- surprising;
people expect
{{containsKey}} (JS/Python {{in}}).
* A membership test on a {{withDefault\{\}}} (auto-grow) map *mutates* it (the
get inserts
the key).
h2. Proposal
Decouple the {{in}} operator from {{isCase}} *minimally*: change only {{Map}}
and
{{CharSequence}} membership; leave {{switch}}/{{grep}} classification (and
{{in}} on
collections, ranges, numbers, and user-defined {{isCase}}) exactly as today.
# {{in}}/{{!in}} dispatch to new {{ScriptBytecodeAdapter.isIn}}/{{isNotIn}}:
{{Map}} -> {{containsKey}}; {{CharSequence}} -> {{contains}} (fixes
GROOVY-2456);
everything else -> existing {{isCase}} (so collections, custom {{isCase}}, etc.
are
unchanged, with no dynamic-dispatch ambiguity).
# {{isCase(Map, Object)}} gains a mutation guard:
{{containsKey(k) && castToBoolean(map.get(k))}} -- so
{{switch}}/{{grep}}/{{in}} never
auto-grow a {{withDefault}} map.
This preserves the genuinely-useful value-based {{grep}} filter
({{fruit.grep([apple:true, lemon:false])}}) and keeps the {{switch}}
classification
semantics, while making {{in}} mean intuitive membership. The
{{a in b <=> switch(a)\{case b\}}} identity is intentionally broken (membership
!=
classification) -- aligned with GEP-19, which reframes {{switch}} anyway.
h3. Deliberate divergence (new, by design)
{code:groovy}
'b' in [a:1, b:0] // true (key present)
switch('b') { case [a:1,b:0]: } // 'no' (value 0 falsy -- isCase, unchanged)
{code}
h2. Extent of the breaking change
||Surface||Before||After||Breaking?||
|{{k in map}}|key present *and* truthy value|{{map.containsKey(k)}}|*Yes*|
|{{s in string}}|equality ({{'ell' in 'hello'}} == false)|substring
({{contains}})|*Yes* (GROOVY-2456)|
|{{in}} on List/Set/Range/array|{{contains}}|{{contains}}|No|
|{{in}} on class with user {{isCase}}|{{obj.isCase(a)}}|{{obj.isCase(a)}}|No|
|{{switch(a)\{case map:\}}}|value-truthy|value-truthy|No (semantics same)|
|{{grep(map)}} value filter|value-truthy filter|value-truthy filter|No
(semantics same)|
|membership/classification on {{withDefault\{\}}} map|*inserts* the key|does
*not* insert|Yes (bug-fix direction)|
Concrete flips:
* {{'b' in [a:1, b:0]}} / {{[a:true, b:false]}}: {{false}} -> {{true}}
* {{'absent' in [:].withDefault\{true\}}}: {{true}}(+mutation) -> {{false}}(no
mutation)
* {{'ell' in 'hello'}}: {{false}} -> {{true}}
Migration for affected code: use {{map[k]}}/{{map.get(k)}} for the old
value-truthy meaning;
{{==}} for old string equality.
h2. Implementation (prototyped, branch {{groovy9848}})
Main (4 files): new {{ScriptBytecodeAdapter.isIn}}/{{isNotIn}}
(Map/CharSequence inline,
else delegate to {{isCase}}); {{BinaryExpressionHelper}} emits
{{isIn}}/{{isNotIn}};
{{isCase(Map)}} mutation guard; {{StaticTypeCheckingVisitor}} keeps the
*direct* {{isCase}}
target for concrete non-Map/non-CharSequence receivers (no perf regression) and
routes
{{Map}}/{{CharSequence}}/{{Object}} receivers through
{{ScriptBytecodeAdapter.isIn}}.
Validation: full STC + static-compilation suites +
switch/grep/operator/map/methods/nary +
a new {{bugs.Groovy9848}} (10 cases) -- *3835 passed, 0 failed*. Only two
existing tests
needed updating, both pinning the old {{in}} semantics
({{GroovyMethodsTest.testInForMaps}},
{{testInForStrings}}). {{switch}}/{{grep}} unaffected; user-defined {{isCase}}
for {{in}}
still works.
h2. Porting vehicle: {{@OperatorRename}} membership support (extends
GROOVY-10980)
{{@OperatorRename}} could not handle {{in}} (no membership member; and it
rewrites
{{left.m(right)}} whereas {{in}} dispatches on the *right* operand). Prototyped
extension:
new {{isIn}}/{{isNotIn}} members + a self-contained membership block that
reverses operands
({{a in b}} -> {{b.name(a)}}). Naming follows the existing method-name
convention ({{plus}},
{{compareTo}}, ...), keyed on the operator's default method {{isIn}}.
{code:groovy}
@OperatorRename(isIn='isCase', isNotIn='isNotCase') // restore pre-9848
value-based `in` in this scope
@OperatorRename(isIn='containsKey') // force key membership
(maps-only scope)
@OperatorRename(isIn='isIn') // full new semantics
(needs the isIn helper present)
{code}
Works in both dynamic and {{@CompileStatic}} (transform runs at
{{SEMANTIC_ANALYSIS}}, before
type checking). 7 {{OperatorRenameTransformTest}} cases + 1212-test regression
pass, including
the shared GEP-15 compound-assign path. Caveats: per-scope (renames *all*
{{in}} in scope, so
{{containsKey}} is unsafe with mixed container types -- {{isIn}} is the safe
target);
operator-only (doesn't touch {{grep}}/{{switch}}, which don't change semantics).
h2. Cross-version (binary) compatibility
Principle: the method a class *calls* for {{in}} is frozen at compile time;
what it *does*
is the runtime's copy; and the called method must exist on the runtime. The
only new symbols
are {{ScriptBytecodeAdapter.isIn}}/{{isNotIn}}; {{isCase}}/{{isNotCase}} exist
in both lines.
* *Groovy 5 artifact on Groovy 6 runtime*: emits {{isCase}} (present in 6) ->
keeps old
value/equality {{in}} semantics; the *only* difference is that {{withDefault}}
maps no longer
auto-grow (the guard lives in 6's {{isCase(Map)}}). Backward-compatible.
* *Groovy 6 artifact on Groovy 5 runtime*: emits {{isIn}} -> *absent in stock
5* ->
{{NoSuchMethodError}} (except static {{in}} on a concrete non-Map/String
receiver, which links
to {{isCase}}). Resolved by the backport below.
* *{{switch}}/{{grep}}/explicit {{isCase}}*: link both directions; the
mutation-guard detail
follows the runtime.
||Artifact \ Runtime||Groovy 5 (no isIn backport)||Groovy 5 (isIn
backported)||Groovy 6||
|G5-compiled {{in}} (emits {{isCase}})|old semantics, mutates
withDefault|same|old semantics, no mutation|
|G6-compiled {{in}} (emits {{isIn}})|{{NoSuchMethodError}}|links -> new
semantics|new semantics|
|{{switch}}/{{grep}}/{{isCase}}|links; mutates withDefault|links; mutates
withDefault|links; no mutation|
h2. Planned Groovy 5 backports (point release)
Two independently-shippable pieces; *neither changes Groovy 5's default {{in}}
semantics*
(5.x keeps emitting {{isCase}}). They only add an opt-in knob and forward
binary compat.
# *{{@OperatorRename}} membership members* ({{isIn}}/{{isNotIn}}) --
compile-time only, no
runtime dependency. Enables source-level opt-in
{{@OperatorRename(isIn='containsKey')}}
(maps-only) in 5.x, portable verbatim to 6.x (where it becomes the backward
opt-out).
# *{{ScriptBytecodeAdapter.isIn}}/{{isNotIn}} helpers* -- added as *available*
methods only
(5.x {{in}} codegen unchanged). Serves double duty: (a) the full-fidelity
opt-in target
{{@OperatorRename(isIn='isIn')}} (all types), and (b) makes Groovy-6-compiled
{{in}}
artifacts *link and run* on a 5.x runtime (closing the {{NoSuchMethodError}}
gap), with
behaviour consistent with 6.0.
Explicitly *not* backported: rewiring 5.x's {{in}} codegen to {{isIn}} -- that
*is* the
breaking change and belongs only in 6.0.
The same {{@OperatorRename(isIn=...)}} annotation is then portable across lines:
||Annotation||Groovy 5 (default value-based)||Groovy 6 (default key-based)||
|{{@OperatorRename(isIn='isIn')}}|opt *in* to new behaviour|no-op (already
default)|
|{{@OperatorRename(isIn='isCase')}}|no-op (already default)|opt *out* to legacy|
h2. Timing
Recommend landing the {{in}} change in *Groovy 6.0* (a major version = the
right window for
a breaking change; the bug is actively biting; independent of GEP-19, which
only reframes
list/map *literals* in {{switch}} and does not touch {{in}} or map-variable
receivers). Ship
the two opt-in/compat backports in a Groovy 5.x point release so teams can
pre-migrate and so
6.0 {{in}}-artifacts can run on 5.x.
h2. Open questions / decisions
* Confirm the direction ({{in}} == {{containsKey}}); there is prior support on
this issue for
JS/Python alignment.
* Ship the {{@OperatorRename}} membership extension and the two 5.x backports?
(This is one way
to address the earlier GROOVY-10980 "isCase on OperatorRename" suggestion.)
* Final member names ({{isIn}}/{{isNotIn}}).
* Relationship to GROOVY-11970 / GEP-15 -- shared operator machinery, but the
{{in}} change
should be decided on its own merits.
h2. Related issues
* GROOVY-2456 -- string {{in}} as substring (fixed by the same
{{isIn(CharSequence)}}).
* GROOVY-10980 -- {{@OperatorRename}} (extended here for the porting vehicle).
* GROOVY-7802 -- {{withDefault(autoGrow, autoShrink)}} (the partial mutation
mitigation).
* GROOVY-11970 / GEP-15 -- compound-assignment overloading (shared transform).
* GEP-19 -- structural pattern matching in {{switch}} (Groovy 7).
was (Author: paulk):
h1. Path forward: decouple {{in}} membership from {{isCase}}
h2. Current behaviour (Groovy 6.0 / master)
* {{isCase(Map, Object)}} returns {{castToBoolean(map.get(switchValue))}} --
i.e. "the
key is present *and* its value is Groovy-truthy".
* The {{in}}/{{!in}} operators compile to
{{ScriptBytecodeAdapter.isCase}}/{{isNotCase}}
({{BinaryExpressionHelper}}, {{KEYWORD_IN}}/{{COMPARE_NOT_IN}}). {{switch}}
uses the same
{{isCase}} (via {{getIsCaseMethod}}); {{grep}} uses {{isCase}} internally in
DGM.
* There is no {{isCase(CharSequence, ...)}}, so {{'ell' in 'hello'}} is
{{false}} (falls to
{{isCase(Object,Object)}} == equality). This is the open GROOVY-2456.
* PR #1904 only added a test pinning the *current* behaviour; semantics are
unchanged.
Consequences users keep hitting:
* {{'b' in [a:1, b:0]}} is {{false}} (value {{0}} is falsy) -- surprising;
people expect
{{containsKey}} (JS/Python {{in}}).
* A membership test on a {{withDefault\{\}}} (auto-grow) map *mutates* it (the
get inserts
the key).
h2. Proposal
Decouple the {{in}} operator from {{isCase}} *minimally*: change only {{Map}}
and
{{CharSequence}} membership; leave {{switch}}/{{grep}} classification (and
{{in}} on
collections, ranges, numbers, and user-defined {{isCase}}) exactly as today.
# {{in}}/{{!in}} dispatch to new {{ScriptBytecodeAdapter.isIn}}/{{isNotIn}}:
{{Map}} -> {{containsKey}}; {{CharSequence}} -> {{contains}} (fixes
GROOVY-2456);
everything else -> existing {{isCase}} (so collections, custom {{isCase}}, etc.
are
unchanged, with no dynamic-dispatch ambiguity).
# {{isCase(Map, Object)}} gains a mutation guard:
{{containsKey(k) && castToBoolean(map.get(k))}} -- so
{{switch}}/{{grep}}/{{in}} never
auto-grow a {{withDefault}} map.
This preserves the genuinely-useful value-based {{grep}} filter
({{fruit.grep([apple:true, lemon:false])}}) and keeps the {{switch}}
classification
semantics, while making {{in}} mean intuitive membership. The
{{x in y <=> switch(x)\{case y\}}} identity is intentionally broken (membership
!=
classification) -- aligned with GEP-19, which reframes {{switch}} anyway.
h3. Deliberate divergence (new, by design)
{code:groovy}
'b' in [a:1, b:0] // true (key present)
switch('b') { case [a:1,b:0]: } // 'no' (value 0 falsy -- isCase, unchanged)
{code}
h2. Extent of the breaking change
||Surface||Before||After||Breaking?||
|{{k in map}}|key present *and* truthy value|{{map.containsKey(k)}}|*Yes*|
|{{s in string}}|equality ({{'ell' in 'hello'}} == false)|substring
({{contains}})|*Yes* (GROOVY-2456)|
|{{in}} on List/Set/Range/array|{{contains}}|{{contains}}|No|
|{{in}} on class with user {{isCase}}|{{obj.isCase(x)}}|{{obj.isCase(x)}}|No|
|{{switch(x)\{case map:\}}}|value-truthy|value-truthy|No (semantics same)|
|{{grep(map)}} value filter|value-truthy filter|value-truthy filter|No
(semantics same)|
|membership/classification on {{withDefault\{\}}} map|*inserts* the key|does
*not* insert|Yes (bug-fix direction)|
Concrete flips:
* {{'b' in [a:1, b:0]}} / {{[a:true, b:false]}}: {{false}} -> {{true}}
* {{'absent' in [:].withDefault\{true\}}}: {{true}}(+mutation) -> {{false}}(no
mutation)
* {{'ell' in 'hello'}}: {{false}} -> {{true}}
Migration for affected code: use {{map[k]}}/{{map.get(k)}} for the old
value-truthy meaning;
{{==}} for old string equality.
h2. Implementation (prototyped, branch {{groovy9848}})
Main (4 files): new {{ScriptBytecodeAdapter.isIn}}/{{isNotIn}}
(Map/CharSequence inline,
else delegate to {{isCase}}); {{BinaryExpressionHelper}} emits
{{isIn}}/{{isNotIn}};
{{isCase(Map)}} mutation guard; {{StaticTypeCheckingVisitor}} keeps the
*direct* {{isCase}}
target for concrete non-Map/non-CharSequence receivers (no perf regression) and
routes
{{Map}}/{{CharSequence}}/{{Object}} receivers through
{{ScriptBytecodeAdapter.isIn}}.
Validation: full STC + static-compilation suites +
switch/grep/operator/map/methods/nary +
a new {{bugs.Groovy9848}} (10 cases) -- *3835 passed, 0 failed*. Only two
existing tests
needed updating, both pinning the old {{in}} semantics
({{GroovyMethodsTest.testInForMaps}},
{{testInForStrings}}). {{switch}}/{{grep}} unaffected; user-defined {{isCase}}
for {{in}}
still works.
h2. Porting vehicle: {{@OperatorRename}} membership support (extends
GROOVY-10980)
{{@OperatorRename}} could not handle {{in}} (no membership member; and it
rewrites
{{left.m(right)}} whereas {{in}} dispatches on the *right* operand). Prototyped
extension:
new {{isIn}}/{{isNotIn}} members + a self-contained membership block that
reverses operands
({{a in b}} -> {{b.name(a)}}). Naming follows the existing method-name
convention ({{plus}},
{{compareTo}}, ...), keyed on the operator's default method {{isIn}}.
{code:groovy}
@OperatorRename(isIn='isCase', isNotIn='isNotCase') // restore pre-9848
value-based `in` in this scope
@OperatorRename(isIn='containsKey') // force key membership
(maps-only scope)
@OperatorRename(isIn='isIn') // full new semantics
(needs the isIn helper present)
{code}
Works in both dynamic and {{@CompileStatic}} (transform runs at
{{SEMANTIC_ANALYSIS}}, before
type checking). 7 {{OperatorRenameTransformTest}} cases + 1212-test regression
pass, including
the shared GEP-15 compound-assign path. Caveats: per-scope (renames *all*
{{in}} in scope, so
{{containsKey}} is unsafe with mixed container types -- {{isIn}} is the safe
target);
operator-only (doesn't touch {{grep}}/{{switch}}, which don't change semantics).
h2. Cross-version (binary) compatibility
Principle: the method a class *calls* for {{in}} is frozen at compile time;
what it *does*
is the runtime's copy; and the called method must exist on the runtime. The
only new symbols
are {{ScriptBytecodeAdapter.isIn}}/{{isNotIn}}; {{isCase}}/{{isNotCase}} exist
in both lines.
* *Groovy 5 artifact on Groovy 6 runtime*: emits {{isCase}} (present in 6) ->
keeps old
value/equality {{in}} semantics; the *only* difference is that {{withDefault}}
maps no longer
auto-grow (the guard lives in 6's {{isCase(Map)}}). Backward-compatible.
* *Groovy 6 artifact on Groovy 5 runtime*: emits {{isIn}} -> *absent in stock
5* ->
{{NoSuchMethodError}} (except static {{in}} on a concrete non-Map/String
receiver, which links
to {{isCase}}). Resolved by the backport below.
* *{{switch}}/{{grep}}/explicit {{isCase}}*: link both directions; the
mutation-guard detail
follows the runtime.
||Artifact \ Runtime||Groovy 5 (no isIn backport)||Groovy 5 (isIn
backported)||Groovy 6||
|G5-compiled {{in}} (emits {{isCase}})|old semantics, mutates
withDefault|same|old semantics, no mutation|
|G6-compiled {{in}} (emits {{isIn}})|{{NoSuchMethodError}}|links -> new
semantics|new semantics|
|{{switch}}/{{grep}}/{{isCase}}|links; mutates withDefault|links; mutates
withDefault|links; no mutation|
h2. Planned Groovy 5 backports (point release)
Two independently-shippable pieces; *neither changes Groovy 5's default {{in}}
semantics*
(5.x keeps emitting {{isCase}}). They only add an opt-in knob and forward
binary compat.
# *{{@OperatorRename}} membership members* ({{isIn}}/{{isNotIn}}) --
compile-time only, no
runtime dependency. Enables source-level opt-in
{{@OperatorRename(isIn='containsKey')}}
(maps-only) in 5.x, portable verbatim to 6.x (where it becomes the backward
opt-out).
# *{{ScriptBytecodeAdapter.isIn}}/{{isNotIn}} helpers* -- added as *available*
methods only
(5.x {{in}} codegen unchanged). Serves double duty: (a) the full-fidelity
opt-in target
{{@OperatorRename(isIn='isIn')}} (all types), and (b) makes Groovy-6-compiled
{{in}}
artifacts *link and run* on a 5.x runtime (closing the {{NoSuchMethodError}}
gap), with
behaviour consistent with 6.0.
Explicitly *not* backported: rewiring 5.x's {{in}} codegen to {{isIn}} -- that
*is* the
breaking change and belongs only in 6.0.
The same {{@OperatorRename(isIn=...)}} annotation is then portable across lines:
||Annotation||Groovy 5 (default value-based)||Groovy 6 (default key-based)||
|{{@OperatorRename(isIn='isIn')}}|opt *in* to new behaviour|no-op (already
default)|
|{{@OperatorRename(isIn='isCase')}}|no-op (already default)|opt *out* to legacy|
h2. Timing
Recommend landing the {{in}} change in *Groovy 6.0* (a major version = the
right window for
a breaking change; the bug is actively biting; independent of GEP-19, which
only reframes
list/map *literals* in {{switch}} and does not touch {{in}} or map-variable
receivers). Ship
the two opt-in/compat backports in a Groovy 5.x point release so teams can
pre-migrate and so
6.0 {{in}}-artifacts can run on 5.x.
h2. Open questions / decisions
* Confirm the direction ({{in}} == {{containsKey}}); there is prior support on
this issue for
JS/Python alignment.
* Ship the {{@OperatorRename}} membership extension and the two 5.x backports?
(This is one way
to address the earlier GROOVY-10980 "isCase on OperatorRename" suggestion.)
* Final member names ({{isIn}}/{{isNotIn}}).
* Relationship to GROOVY-11970 / GEP-15 -- shared operator machinery, but the
{{in}} change
should be decided on its own merits.
h2. Related issues
* GROOVY-2456 -- string {{in}} as substring (fixed by the same
{{isIn(CharSequence)}}).
* GROOVY-10980 -- {{@OperatorRename}} (extended here for the porting vehicle).
* GROOVY-7802 -- {{withDefault(autoGrow, autoShrink)}} (the partial mutation
mitigation).
* GROOVY-11970 / GEP-15 -- compound-assignment overloading (shared transform).
* GEP-19 -- structural pattern matching in {{switch}} (Groovy 7).
> Allow membership operator to work on maps
> -----------------------------------------
>
> Key: GROOVY-9848
> URL: https://issues.apache.org/jira/browse/GROOVY-9848
> Project: Groovy
> Issue Type: Improvement
> Reporter: Keegan Witt
> Assignee: Eric Milles
> Priority: Major
> Labels: breaking, breaking_change
>
> {code:groovy}def map = [a:1,b:2,z:0].withDefault{3}{code}
> Options:
> # do nothing
> {code:groovy}
> assert 'a' in map
> assert 'b' in map
> assert 'x' in map // mutates!
> assert 'z' !in map
> assert 4 === map.size()
> {code}
> # change mutation of {{isCase}} -- guard with {{containsKey}}:
> {code:groovy}
> assert 'a' in map
> assert 'b' in map
> assert 'x' !in map
> assert 'z' !in map
> assert 3 === map.size()
> {code}
> # change {{isCase}} to {{containsKey}}:
> {code:groovy}
> assert 'a' in map
> assert 'b' in map
> assert 'x' !in map
> assert 'z' in map
> assert 3 === map.size()
> // grep and switch behavior change (see below)
> {code}
> # ask user to test key set
> {code:groovy}
> assert 'a' in map.keySet()
> assert 'b' in map.keySet()
> assert 'x' !in map.keySet()
> assert 'z' !in map.keySet()
> assert 3 === map.size()
> {code}
> # provide {{@OperatorRename}} support:
> {code:groovy}
> @OperatorRename(isCase='containsKey')
> void test() {
> assert 'a' in map
> assert 'b' in map
> assert 'x' !in map
> assert 'z' !in map
> assert 3 === map.size()
> }
> {code}
> # remap operator {{in}} to {{isIn}} (or some such):
> {code:groovy}
> // DGMs:
> boolean isIn(Map map, Object key) {
> return map != null && map.containsKey(key);
> }
> boolean isIn(Object obj, Object value) {
> return InvokerHelper.invokeMethod(obj, "isCase", new Object[]{value});
> }
> assert 'a' in map
> assert 'b' in map
> assert 'x' !in map
> assert 'z' in map
> assert 3 === map.size()
> // behavior change for any type that provides "isIn"
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
