[
https://issues.apache.org/jira/browse/GROOVY-12535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18138419#comment-18138419
]
ASF GitHub Bot commented on GROOVY-12535:
-----------------------------------------
paulk-asert commented on PR #3154:
URL: https://github.com/apache/groovy/pull/3154#issuecomment-6646349211
### What this PR achieves
With this PR, under `@TypeChecked` and `@CompileStatic`, a value written to
a `Map<K,V>` is converted to `V` whenever `V` is a concrete type. The rules are
the same ones an assignment uses, so `String s = "$n"` gives a String and `Long
l = 1` gives a Long. The conversion goes through `putAt(Map<K,V>, K, Object,
@ClassTag(preempt=true) Class<V>)`, with the compiler supplying the token. That
is the one sanctioned way for statically checked code to behave differently
from dynamic code, and `classTagPreemptionDisabled` turns it off. Dynamic code
is unchanged.
In practice:
- **Every write form behaves the same.** Before, `m.k = "$n"` and `m['k'] =
"$n"` gave different results. 5.1.3 stored the GString for both. 6.0.0 stored
it for the property write and rejected the subscript write. Master converts the
property write and still rejects the subscript. Now all of these convert:
- property writes, including inside `with { }` and with `?.`;
- subscript writes and `putAt`;
- `?=` and `??=`;
- nested maps;
- non-String keys (Integer, enum).
- **Writes rejected since 6.0.0 compile again.** They now store a value of
the declared type, rather than the wrong type 5.1.3 stored.
- **Conversion isn't limited to GString.** It applies wherever assignment
would convert: `Map<String,Long>`: `m.a = 1` stores a Long,
`Map<String,BigDecimal>`: `m.a = 1` stores a BigDecimal, and
`Map<String,String>`: `m.k = 123` stores `"123"`.
- **The value read back now matches the declared type.** A leftover GString
in a `Map<String,String>` gives false from `equals`, `containsValue` and hash
lookups against a String. It also throws ClassCastException in Java code
reading the map, and in 6.0.0 `@CompileStatic` String method calls.
Unchanged, by design:
- **Map methods:** `put`, `putAll`, `merge`, `<<`, map literals and the like
are still checked like any other call, so a GString argument is still a compile
error.
- **Value types with no single target:** wildcards, raw `Map`, `Object` and
`CharSequence` get no `@ClassTag` token, so their values are stored unchanged.
Two differences from master, both deliberate:
- master converts a property write to a `? extends String` map, and this PR
doesn't, because no token is supplied;
- master ignores `classTagPreemptionDisabled`, and this PR honours it.
Master's conversion predates the `@ClassTag` approach.
### What a write stores under static type checking
Each cell is the class of the value stored, with `def n = 'ab'`, under
`@TypeChecked` and `@CompileStatic`. The two modes agree in every cell except
where a cell shows *TC / CS*. ❌ is a compile error. Dynamic code stores the
value unchanged in every version, so it is shown once.
- † marks rows that `Groovy12535` didn't cover before this PR.
- ‡ marks writes that type check but still store a value that isn't of the
declared type. These are outside this PR; see the end of this comment.
| Write |
Dynamic (all) | 5.1.3 | 6.0.0 |
master | this PR |
| ----------------------------------------------------------------------
> STC: property-style assignment to a Map<String,String> accepts a GString
> value and stores it unconverted
> --------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12535
> URL: https://issues.apache.org/jira/browse/GROOVY-12535
> Project: Groovy
> Issue Type: Bug
> Reporter: Paul King
> Assignee: Daniel Sun
> Priority: Major
> Fix For: 7.0.0-alpha-1, 6.0.1
>
>
> Under static type checking, a {{GString}} can't be put into a
> {{Map<String,String>}} through {{put}} or the subscript operator; both are
> rejected. Assigning through property notation is accepted, though, and the
> {{GString}} is stored as is:
> {code:groovy}
> @groovy.transform.CompileStatic
> def go() {
> def n = 'ab'
> Map<String,String> m = [:]
> m.k = "$n"
> ((Map) m).values()*.getClass()*.simpleName
> }
> assert go() == ['GStringImpl'] // expected ['String']
> {code}
> ||Form||Result (4.0.29, 6.0.0, master)||
> |{{m.k = "$n"}}|compiles, stores a {{GStringImpl}}|
> |m.with { k = "$n" }|compiles, stores a {{GStringImpl}}|
> |{{m['k'] = "$n"}}|STC error: Cannot call ... putAt ...|
> |{{m.put('k', "$n")}}|STC error: Cannot call ... put(String, String) ...|
> Statically compiled code can't otherwise add a {{GString}} to a collection
> typed for Strings ({{<<}}, {{add}}, {{set}}, {{addAll}}, {{+=}}, {{putAt}}
> and {{put}} are all rejected), so this is the one hole on the write side.
> Later reads that expect a {{String}} can then fail. For example, Java code
> doing {{String s = m.get('k')}} throws {{ClassCastException}}, and
> {{m.get('k').equals('ab')}} is false.
> Expected: either convert the value to a {{String}} at the assignment, as
> Groovy does for {{String s = "$n"}}, or report the same error as {{put}} and
> {{putAt}}. Converting matches the general rule that a {{GString}} becomes a
> {{String}} where one is required, and wouldn't break existing code that
> compiles today.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
