[
https://issues.apache.org/jira/browse/GROOVY-8788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17582810#comment-17582810
]
Jochen Theodorou commented on GROOVY-8788:
------------------------------------------
[~emilles] I think your analysis so far is very spot on. Sorry for writing so
much text all the time, but I think it is important that I explain you a bit my
thinking, not only the result.
{code:Java}
void testPutWithWrongValueType() {
shouldFailWithMessages '''
def map = new HashMap<String, Integer>()
map.put('hello', new Object())
''',
'Cannot find matching method java.util.HashMap#put(java.lang.String,
java.lang.Object). Please check if the declared type is correct and if the
method exists.'
}
{code}
This test passes now, but should have failed. It is not about getAt though.
Map#put(K,V) is then not the selected method here - which makes sense, since it
is not fitting. But there is no DGM for this either. So what method is actually
selected here? There might be more like this.
Of course the majority is, as you mentioned because of
Object#getAt(String):Object being selected over Map<K,V>#getAt(Object):V
To solve this I see a couple of possibilities which I all don't like all that
much. The problem is that the tests IMHO show the desired behavior. Changing
them would not only mean breaking existing code most likely, but also - to
avoid that - further work. So I am not really for this, unless we make that a
blocking issue for the next release. So my take on this is, fix the tests if
you want this PR going through, but document all the failing tests with a new
blocking issue and there is the danger of having to revert more than just the
test changes. What I would prefer is adding commits to this PR to try find a
solution. but I think some more discussion is required. So maybe may thoughts
on possible solutions:
(1) replace Map<K,V>#getAt(Object):V with Map<K,V>#getAt(K):V. This raises of
course the question of if this even shadows (and even should do so) the Object
method. A Map<?,V> could be super troublesome here. And of course this
signature is not aligned with the normal get-method Map has. I personally think
that having getAt more specific would be no problem, you still have get for the
strange things where you use a key from a different class with same hashcode
and accordingly behaving equals method - which should be super rare. And I do
not see a requirement to honor the old contract of Map here. But as I said..
should this really be preferred over the Object variant in case of Map<?,V>. Or
maybe it would be worth it, even if this fails in method selection? I think it
would. It is kind of my preferred solution without knowing all the implications
of course
(2) change Object#getAt(String) to Object#getAt(Object). This change would
require that we try to convert the argument to String in the getAt method. As
in (1) Map<String,V> would then be no problem. And as in (1) Map<?,V> could be
a problem. Actually again we have to ask what method is preferred by the static
compiler and which should be. And again I do not care about a method selection
error during compile time here. But this would be a breaking change whenever
somebody made at getAt(String) method with @Override - unless our
implementation of this does not check for DGM. though the question here is then
if that is correct.
(3) add Map<K, V>#getAt(String). This would mean to have it additionally to
Map<K,V>#getAt(Object). Here we would have to clear if there is a case of
Map#get(String) being called instead of Object#getAt(String) as part of the MOP
is possible. It is not get, but getAt, so there might be no conflict. But there
are direct calls to getAt as part of the meta class to map property/attribute
access I think. And is it then right to call Object#getAt(String) instead of
Map#getAt(String)?
For me all 3 of them have potential of breaking code.
> Inconsistency in extension method selection with @CompileStatic
> ---------------------------------------------------------------
>
> Key: GROOVY-8788
> URL: https://issues.apache.org/jira/browse/GROOVY-8788
> Project: Groovy
> Issue Type: Bug
> Components: Static compilation, Static Type Checker
> Affects Versions: 2.4.15, 2.5.2
> Reporter: Daniil Ovchinnikov
> Assignee: Eric Milles
> Priority: Major
> Labels: breaking
>
> Given properly registered extension class:
> {code:java|title=MyExtensions.java}
> public class MyExtensions {
> public static void foo(Object self, String s) {
> System.out.println("Object#foo(String)");
> }
> public static void foo(String self, Object o) {
> System.out.println("String#foo(Object)");
> }
> }
> {code}
> Run
> {code:java|title=playground.groovy}
> void usageExt() {
> "".foo("") // prints "Object#foo(String)" which is correct
> }
> @groovy.transform.CompileStatic
> void usageExtStatic() {
> "".foo("") // prints "String#foo(Object)" which is questionable
> }
> usageExt()
> usageExtStatic()
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
