[jira] [Commented] (GROOVY-8788) Inconsistency in extension method selection with @CompileStatic
[ https://issues.apache.org/jira/browse/GROOVY-8788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17583845#comment-17583845 ] ASF GitHub Bot commented on GROOVY-8788: eric-milles merged PR #1768: URL: https://github.com/apache/groovy/pull/1768 > 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)
[jira] [Commented] (GROOVY-8788) Inconsistency in extension method selection with @CompileStatic
[ https://issues.apache.org/jira/browse/GROOVY-8788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17583097#comment-17583097 ] Eric Milles commented on GROOVY-8788: - The pull request has been updated with mitigation for type checking of map value read and write. The one item that suffered is key type checking, as noted above. > 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)
[jira] [Commented] (GROOVY-8788) Inconsistency in extension method selection with @CompileStatic
[ https://issues.apache.org/jira/browse/GROOVY-8788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17583059#comment-17583059 ] Eric Milles commented on GROOVY-8788: - Since basically everything allows get/put property using string for property name via {{getAt(Object,String)}} and {{putAt(Object,String,Object)}} maybe an optional type checking extension should be created, much like was done for regular expressions. > 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)
[jira] [Commented] (GROOVY-8788) Inconsistency in extension method selection with @CompileStatic
[ https://issues.apache.org/jira/browse/GROOVY-8788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17583055#comment-17583055 ] Eric Milles commented on GROOVY-8788: - It is this excerpt of [StaticTypeCheckingVisitor#getResultType|https://github.com/apache/groovy/blob/7f6fcb2f81b58a54bc7beef33ac382cb50a79cb4/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java#L4522] that determines the type of "x = map['key']" and "map['key'] = x". Yes, it only looks for getAt... {code:java} if (isArrayOp(op)) { Expression copy = binX(leftExpression, expr.getOperation(), rightExpression); copy.setSourcePosition(expr); // do not propagate BINARY_EXP_TARGET, etc. MethodNode method = findMethodOrFail(copy, left, "getAt", rightRedirect); if (method != null && !isNumberCategory(getWrapper(rightRedirect))) { return inferReturnTypeGenerics(left, method, rightExpression); } return inferComponentType(left, right); } {code} When method selection changes, it determines Object not Type for Map. I can add a special case for Map left expression and String right expression. For put, type checking is done via this block in [StaticTypeCheckingVisitor#visitBinaryExpression|https://github.com/apache/groovy/blob/7f6fcb2f81b58a54bc7beef33ac382cb50a79cb4/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java#L820]: {code:java} if (isArrayOp(op)) { ... if (!lType.isArray() && enclosingBinaryExpression != null && enclosingBinaryExpression.getLeftExpression() == expression && isAssignment(enclosingBinaryExpression.getOperation().getType())) { // left hand side of a subscript assignment: map['foo'] = ... Expression enclosingExpressionRHS = enclosingBinaryExpression.getRightExpression(); if (!(enclosingExpressionRHS instanceof ClosureExpression)) { enclosingExpressionRHS.visit(this); } ClassNode[] arguments = {rType, getType(enclosingExpressionRHS)}; List methods = findMethod(lType, "putAt", arguments); if (methods.size() == 1) { typeCheckMethodsWithGenericsOrFail(lType, arguments, methods.get(0), enclosingExpressionRHS); } else if (methods.isEmpty()) { addNoMatchingMethodError(lType, "putAt", arguments, enclosingBinaryExpression); } } } {code} It relies on method selection to produce "Cannot call #putAt(Map,K,V) with arguments ..." errors. Since {{putAt(Object,String,Object)}} is now selected for string keys, this block checks nothing. However, the result type allows "Cannot assign value of type X to variable of type Y" to be produced. > 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)
[jira] [Commented] (GROOVY-8788) Inconsistency in extension method selection with @CompileStatic
[ https://issues.apache.org/jira/browse/GROOVY-8788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17583001#comment-17583001 ] Eric Milles commented on GROOVY-8788: - So I can think of a few options to move forward from this point: 1. Add {{getAt(Map,String}} and {{putAt(Map,String,V}} extension methods. This gets in front of the getProperty/setProperty behavior. But it means "map['key']" diverges from "map.key". I tried this option in the [previous PR|https://github.com/apache/groovy/pull/1766] 2. Add special-case logic in STC to select {{getAt(Map,Object)}} for string argument and map receiver. (same for putAt). This lets type-checker work like before. But the public property case described above will still be a cast exception for SC at least. 3. Add type inference logic to STC for left-square-bracket binary expression with map receiver and String key. This overrides the type information provided by method selection. > 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)
[jira] [Commented] (GROOVY-8788) Inconsistency in extension method selection with @CompileStatic
[ https://issues.apache.org/jira/browse/GROOVY-8788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17582993#comment-17582993 ] Eric Milles commented on GROOVY-8788: - First of all, thank you for taking the time to read through all this and provide feedback. Now we come to the hard decisions. {{getAt(Object,String)}} is chosen over {{getAt(Map,Object)}} for dynamic runtime and now STC for String input. As you can see from all the test failures, this goes against expectations because the inferred return type is Object not the map's value type. There is an edge case where this bears out now: {code:groovy} class A { } class B { } class HM extends HashMap { B b = new B() } map = new HM() map['a'] = new A() A value = map['a'] value = map['b'] // Groovy 5 returns the property declared by HM (GROOVY-5001, GROOVY-5491, GROOVY-6144) {code} https://github.com/apache/groovy/blame/c18411fe233bd60d4d045d65d9bb04b459f95e12/src/main/java/groovy/lang/MetaClassImpl.java#L1966 > 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)
[jira] [Commented] (GROOVY-8788) Inconsistency in extension method selection with @CompileStatic
[ 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() 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#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#getAt(Object):V with Map#getAt(K):V. This raises of course the question of if this even shadows (and even should do so) the Object method. A Map 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. 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 would then be no problem. And as in (1) Map 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#getAt(String). This would mean to have it additionally to Map#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 cor
[jira] [Commented] (GROOVY-8788) Inconsistency in extension method selection with @CompileStatic
[ https://issues.apache.org/jira/browse/GROOVY-8788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17582677#comment-17582677 ] ASF GitHub Bot commented on GROOVY-8788: eric-milles closed pull request #1766: GROOVY-8788: STC: prefer closer parameter match over self-type match URL: https://github.com/apache/groovy/pull/1766 > 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)
[jira] [Commented] (GROOVY-8788) Inconsistency in extension method selection with @CompileStatic
[ https://issues.apache.org/jira/browse/GROOVY-8788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17582672#comment-17582672 ] ASF GitHub Bot commented on GROOVY-8788: eric-milles opened a new pull request, #1768: URL: https://github.com/apache/groovy/pull/1768 Given choice between methods m(String,Object) and m(Object,String), prefer closer parameter matching. This aligns with the method selection of the dynamic runtime. There is much more discussion of the various test cases in the ticket. With proper selection of `getAt(Object,String)` over `getAt(Map,Object)` and `putAt(Object,String,Object)` over `putAt(Map,K,V)` there are quite a few STC test issues. Not sure if I should separate them out to a test script just for 8788 or mitigate them with some minor changes or something else. I'd be interested to hear how this sits now. @blackdrag @paulk-asert https://issues.apache.org/jira/browse/GROOVY-8788 https://issues.apache.org/jira/browse/GROOVY-6504 https://issues.apache.org/jira/browse/GROOVY-6849 https://issues.apache.org/jira/browse/GROOVY-6970 https://issues.apache.org/jira/browse/GROOVY-8787 https://issues.apache.org/jira/browse/GROOVY-9069 https://issues.apache.org/jira/browse/GROOVY-9420 > 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)
[jira] [Commented] (GROOVY-8788) Inconsistency in extension method selection with @CompileStatic
[ https://issues.apache.org/jira/browse/GROOVY-8788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17582550#comment-17582550 ] Eric Milles commented on GROOVY-8788: - 6. mixed declaration and extension (pt.2) {code:groovy} class A { int m(String s) {4} } class B extends A { } class C extends B { } class Ext { static m(B self, Object o) {2} } use (Ext) { def a = new A() assert a.m(new String()) == 4 def b = new B() assert b.m(new Object()) == 2 assert b.m(new String()) == 4 def c = new C() assert c.m(new Object()) == 2 assert c.m(new String()) == 4 } {code} This case behaves under dynamic and static compilation. So no change required. :) > 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)
[jira] [Commented] (GROOVY-8788) Inconsistency in extension method selection with @CompileStatic
[ https://issues.apache.org/jira/browse/GROOVY-8788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17582543#comment-17582543 ] Eric Milles commented on GROOVY-8788: - If my understanding of #1-#5 is consistent, this commit does work to fix the self-type weighting of STC/SC: https://github.com/apache/groovy/pull/1766/commits/488f8a8f336efae32349317f219a8f8df8994114 I can separate out the getAt change if that helps. There will be some fallout to having {{getAt(Object,String)}} selected over {{getAt(Map,Object)}}. And based on all this, I actually noticed that the javadoc assertions for {{getAt(Map,Object)}} are not even targeting that method. They go to {{getAt(Object,String)}} because they use string keys. Further, because {{getAt(Object,String)}} is implemented via getProperty, there is an edge case where you can extend map and declare a property, which is what this refers to (edited to reflect current behavior): {code:groovy} class HM extends HashMap { String a = 'x' } map = new HM() map.put("a",1) assert map["a"] == 'x' {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)
[jira] [Commented] (GROOVY-8788) Inconsistency in extension method selection with @CompileStatic
[ https://issues.apache.org/jira/browse/GROOVY-8788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17582540#comment-17582540 ] Eric Milles commented on GROOVY-8788: - 4. overload and override -- I think we all expect {{b.m(new String())}} to return 3 now and Groovy follow suit {code:groovy} class A { } class B extends A { } class Cat { static m(A self, String s) {1} static m(B self, Object o) {2} static m(B self, String s) {3} } use (Cat) { def b = new B() assert b.m(new Object()) == 2 assert b.m(new String()) == 3 } {code} 5. mixed declaration and extension {code:groovy} class A { int m(String s) {4} } class B extends A { } class Cat { static m(A self, String s) {1} static m(B self, Object o) {2} } use (Cat) { def a = new A() assert a.m(new String()) == 1 def b = new B() assert b.m(new Object()) == 2 assert b.m(new String()) == 1 } {code} Here we have something interesting and new. Groovy chooses extension method {{m(String)}} over declared method {{m(String)}}. I think this is how {{String toString(Object self)}} works. Even in this case for {{b.m(new String())}}, the static compiler selects {{m(Object)}} given two {{m(String)}} options. Further confirmation for me that SC is improperly weighting the self type B. > 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)
[jira] [Commented] (GROOVY-8788) Inconsistency in extension method selection with @CompileStatic
[ https://issues.apache.org/jira/browse/GROOVY-8788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17582532#comment-17582532 ] Eric Milles commented on GROOVY-8788: - Let me see if I can break this down from your examples, with some small edits of mine. 1. overload is more specific -- nothing surprising here {code:groovy} class A { def m(Object o) {1} } class B extends A { def m(String s) {2} } def a = new A() assert a.m(new Object()) == 1 assert a.m(new String()) == 1 def b = new B() assert b.m(new Object()) == 1 assert b.m(new String()) == 2 {code} 2. overload is more general -- selection is based on argument type {code:groovy} import static groovy.test.GroovyAssert.shouldFail class A { def m(String s) {1} } class B extends A { def m(Object o) {2} } def a = new A() shouldFail(MissingMethodException) { a.m(new Object()) } assert a.m(new String()) == 1 def b = new B() assert b.m(new Object()) == 2 assert b.m(new String()) == 1 {code} 3. category/extension overloads -- same behavior as declared methods; OP expects this behavior {code:groovy} class A { } class B extends A { } class Cat { static m(A self, String s) {1} static m(B self, Object o) {2} } use (Cat) { def a = new A() assert a.m(new String()) == 1 def b = new B() assert b.m(new Object()) == 2 assert b.m(new String()) == 1 } {code} Hopefully nothing surprising so far. #1 and #2 run the same under static compilation. #3 requires a little more work since {{use}} cannot be used under static compilation. > 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)
[jira] [Commented] (GROOVY-8788) Inconsistency in extension method selection with @CompileStatic
[ https://issues.apache.org/jira/browse/GROOVY-8788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17582251#comment-17582251 ] Jochen Theodorou commented on GROOVY-8788: -- Point A is that consistency. The static compiler should behave the same for extension methods and non extension methods in terms of what extension methods mean (I made examples above). This is not specific to map of course. But the trouble here already starts with Point B. From what I gathered Map would convert GString keys to String. But then in case of Map I got from your comment that this is not the case. If GString would really extend String, then this would not matter, but since it does not extend it we get a problem that does not exist for example in Java. GString->String is no cast, it is a conversion. Such exist in Java as widening and as boxing. But since you cannot use a different key for Map than for Map boxing does not matter. Potentially there would be a problem with trying to use an int for Map and Map, but I have not checked if that is actually legal without using any cast. there are also Lambda expressions... but since they have no specific type on their own the problem does not exist here as well. I think you can do Point A, Point B+ is a can of worms. > 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)
[jira] [Commented] (GROOVY-8788) Inconsistency in extension method selection with @CompileStatic
[ https://issues.apache.org/jira/browse/GROOVY-8788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17581945#comment-17581945 ] Eric Milles commented on GROOVY-8788: - I did look at putAt. It is declared as "public V putAt(Map map, K key, V val)". So there is some difference as far as the static compiler is concerned. For "Map" it says that GString is not compatible with String. GROOVY-9420 changed this same error for getAt, which now accepts Object like Map#get does. Is there a way to break all this down and proceed with stepwise refinements? 8788 is about inconsistent selection of extension methods. I can make the static compiler select like the dynamic compiler does (the PR) but there would be consequences for map access and some other things like set intersect. It sounds like you explicitly wanted the static compiler to work differently in this space. So is there a play to bring the dynamic compiler in line or should we just close this out and accept the difference in bahavior? > 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)
[jira] [Commented] (GROOVY-8788) Inconsistency in extension method selection with @CompileStatic
[ https://issues.apache.org/jira/browse/GROOVY-8788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17581874#comment-17581874 ] Jochen Theodorou commented on GROOVY-8788: -- >From the comments I am not sure the fix goes in the right direction - of >course I could be totally wrong here. {code:Java} class A { def m(o){1} } class B extends A { def m(String s) {2} } def a = new A() assert a.m(new Object()) == 1 def b = new B() assert b.m(new Object()) == 1 assert b.m("") == 2 {code} in this case it is obvious that the assert should be fulfilled this way. Extension methods are kind of overlays for the class. {code:Java} class MExt { static m(A self, String s) {1} static m(B self, Object o) {2} } class A {} class B extends A {} @groovy.transform.CompileStatic void usageExtStatic() { def b = new B() assert b.m(new Object()) == 2 assert b.m("") == 1 } usageExtStatic() {code} m(Object) is to be seen as if it is a method of B. m(String) is to be seen as if a method of A. Since B extends A and since B does not define a new m(String), the extension method for A has to be chosen here. {code:Java} class MExt { static m(A self, String s) {1} static m(B self, Object o) {2} } class A {} class B extends A { def m(String s){3} } @groovy.transform.CompileStatic void usageExtStatic() { def b = new B() assert b.m(new Object()) == 2 assert b.m("") == 3 } usageExtStatic() {code} In this case m(String) exists on A as extension method, but B defined its own m(String), which "overwrites" m(String) from A. Neither overwrite nor overload are the right terms for this I guess. class MExt { static m(A self, String s) {1} static m(B self, Object o) {2} } class A { def m(String s) {4} } class B extends A {} class C extends B {} @groovy.transform.CompileStatic void usageExtStatic() { def b = new B() assert b.m(new Object()) == 2 assert b.m("") == 1 def c = new C() assert c.m(new Object()) == 2 assert c.m("") == 1 } usageExtStatic() {code} Here, even though A defines m(String), the extension method "shadows" it and inherits it to B. For C again it is the extension methods only. I know very long comment, but well... the above was for the issue described here, to show how it is supposed to work - and does in the dynamic case. The fix though involves getAt(Map,String) and GString: {code:Java} class Container { public someObject = "" String toString(){ return "Container_" +someObject} int hashCode() { return 42 + someObject.hashCode() } boolean equals(o) { return o!=null && o.class==Container && o.someObject == someObject } void newObject(o) {this.someObject = o} } def m = new HashMap() def container = new Container(someObject: "1") m.put(container, 1) assert m.get(container) == 1 container.newObject("2") assert m.get(container) == null {code} Here the second get fails even though container itself did not change really. But since the contained Object did, you cannot access the map value using that same reference. This is given by Java. {code:Java} class Container { public someObject = "" String toString(){ return "Container_" +someObject} int hashCode() { return 42 + someObject.hashCode() } boolean equals(o) { return o!=null && o.class==Container && o.someObject == someObject } void newObject(o) {this.someObject = o} } def m = new HashMap() def container = new Container(someObject: "1") m.put("$container", 1) assert m.get("$container") == 1 container.newObject("2") assert m.get("$container") == null {code} In this version it is a similar situation to before, the only change is that we create now a ton of GString objects. GString though has the same problem that Container has, hashcode and equals depend on the contained object, which is subject to change. Examples are GStrings made from Closures and GStrings made of containers like here. the more idiomatic usage would be: {code:Java} // Container like before def m = [:] def container = new Container(someObject: "1") m["$container"] = 1 //putAt assert m["$container"] == 1 //getAt container.newObject("2") assert m["$container"] == null //getAt {code} This proofed to be difficult to understand for the people, which is why we said we do not allow that for the static compiler. I noticed you added getAt(Map,String), to allow assert map["a"] == 1 // not 'x' But what about the putAt? This will go through InvokerHelper, thus map["a"] = 1 will set the property. I think one is not complete without the other. Right now{code:Java} class HM extends HashMap { String a = 'x' } def map = new HM() map["a"] = 1 assert map["a"] == 1 {code} will fail Groovy because map["a"] sets the property, while map["a"] gets the map value. {code:Java} class HM extends HashMap { String a = 'x' } def map = new HM() def x = "a" map["$x"] = 1 assert map["$x"] == 1 {code} did behave just the same. The reason is that in dynamic Gro
[jira] [Commented] (GROOVY-8788) Inconsistency in extension method selection with @CompileStatic
[ https://issues.apache.org/jira/browse/GROOVY-8788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17581698#comment-17581698 ] ASF GitHub Bot commented on GROOVY-8788: daniellansun commented on PR #1766: URL: https://github.com/apache/groovy/pull/1766#issuecomment-1220335936 I am keen to vote +1 > 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)
[jira] [Commented] (GROOVY-8788) Inconsistency in extension method selection with @CompileStatic
[ https://issues.apache.org/jira/browse/GROOVY-8788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17581448#comment-17581448 ] ASF GitHub Bot commented on GROOVY-8788: eric-milles commented on PR #1766: URL: https://github.com/apache/groovy/pull/1766#issuecomment-1219738720 With the addition of `getAt(Map,String)` a small change in `StaticTypeCheckingSupport#getDistance` can resolve the `map["$key"]` issues. I know Cedric stated in 6668 that this should be an STC error, but if there is just one method and it accepts String, it will be chosen for a GString argument. This changes just closes the distance between GString and String so that `m(String)` is selected over `m(Object)`. https://issues.apache.org/jira/browse/GROOVY-6668 https://issues.apache.org/jira/browse/GROOVY-8212 > 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)
[jira] [Commented] (GROOVY-8788) Inconsistency in extension method selection with @CompileStatic
[ https://issues.apache.org/jira/browse/GROOVY-8788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17581022#comment-17581022 ] ASF GitHub Bot commented on GROOVY-8788: sonatype-lift[bot] commented on code in PR #1766: URL: https://github.com/apache/groovy/pull/1766#discussion_r948409220 ## src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java: ## @@ -1103,6 +1104,21 @@ Person foo(B b) {} onlyExtensionMethods.add(choice); } } +if (onlyExtensionMethods.size() > 1) { +// GROOVY-8788: prefer closer parameter match over closer self-type match +bestDist = Integer.MAX_VALUE; List bestExtensions = new LinkedList<>(); Review Comment: *[JdkObsolete](https://errorprone.info/bugpattern/JdkObsolete):* It is very rare for LinkedList to out-perform ArrayList or ArrayDeque. Avoid it unless you're willing to invest a lot of time into benchmarking. Caveat: LinkedList supports null elements, but ArrayDeque does not. --- ```suggestion bestDist = Integer.MAX_VALUE; List bestExtensions = new ArrayList<>(); ``` --- Reply with *"**@sonatype-lift help**"* for info about LiftBot commands. Reply with *"**@sonatype-lift ignore**"* to tell LiftBot to leave out the above finding from this PR. Reply with *"**@sonatype-lift ignoreall**"* to tell LiftBot to leave out all the findings from this PR and from the status bar in Github. When talking to LiftBot, you need to **refresh** the page to see its response. [Click here](https://help.sonatype.com/lift/talking-to-lift) to get to know more about LiftBot commands. --- Was this a good recommendation? [ [🙁 Not relevant](https://www.sonatype.com/lift-comment-rating?comment=317131720&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=317131720&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=317131720&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=317131720&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=317131720&lift_comment_rating=5) ] > 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)
[jira] [Commented] (GROOVY-8788) Inconsistency in extension method selection with @CompileStatic
[ https://issues.apache.org/jira/browse/GROOVY-8788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17580975#comment-17580975 ] ASF GitHub Bot commented on GROOVY-8788: eric-milles opened a new pull request, #1766: URL: https://github.com/apache/groovy/pull/1766 Given choice between extension methods `m(String,Object)` and `m(Object,String)`, prefer closer parameter matching. This aligns with the method selection of the dynamic runtime. So there are some edge cases that were giving false STC errors or missing them. This is a breaking change! `getAt(Map,String)` was added to prevent STC errors for `Type obj = map[str]` expressions. https://issues.apache.org/jira/browse/GROOVY-8788 https://issues.apache.org/jira/browse/GROOVY-6504 https://issues.apache.org/jira/browse/GROOVY-6849 https://issues.apache.org/jira/browse/GROOVY-6970 https://issues.apache.org/jira/browse/GROOVY-8787 https://issues.apache.org/jira/browse/GROOVY-9069 https://issues.apache.org/jira/browse/GROOVY-9420 > 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)
[jira] [Commented] (GROOVY-8788) Inconsistency in extension method selection with @CompileStatic
[ https://issues.apache.org/jira/browse/GROOVY-8788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17580956#comment-17580956 ] Eric Milles commented on GROOVY-8788: - One thing that would reduce the impact of this change is to add {{getAt(Map,String)}} extension method to step in front of {{getAt(Object,String)}}. > 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)
[jira] [Commented] (GROOVY-8788) Inconsistency in extension method selection with @CompileStatic
[ https://issues.apache.org/jira/browse/GROOVY-8788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17580951#comment-17580951 ] Eric Milles commented on GROOVY-8788: - I can fix this by updating {{StaticTypeCheckingSupport#chooseBestMethod}} (see below). There are a number of cases that stretch intuition and may result in breaking existing code. In both cases the method on the left is chosen by the static compiler and the method on the right is chosen by the dynamic runtime and the new static compiler (with chooseBestMethod below). The dynamic runtime has preference for parameter matching and the static compiler prefers receiver matching. This is the source of the problem described in the original post. Changing this preference will bring closer alignment for the two modes but has some consequences. {code:groovy} @groovy.transform.CompileStatic void test(Set one, Set two) { one.intersect(two) // Set#intersect(Iterable) vs. Collection#intersect(Collection) } @groovy.transform.CompileStatic void test(Map map, String key) { Number num = map[key] // Map#getAt(Object) vs. Object#getAt(String) } {code} [~paulk] [~blackdrag] {code:java} /** * Returns the method(s) which best fit the argument types. * * @return zero or more results */ public static List chooseBestMethod(final ClassNode receiver, final Collection methods, final ClassNode... argumentTypes) { if (!asBoolean(methods)) { return Collections.emptyList(); } int bestDist = Integer.MAX_VALUE; List bestChoices = new LinkedList<>(); boolean noCulling = methods.size() <= 1 || "".equals(methods.iterator().next().getName()); Iterable candidates = noCulling ? methods : removeCovariantsAndInterfaceEquivalents(methods); for (MethodNode candidate : candidates) { MethodNode safeNode = candidate; ClassNode[] safeArgs = argumentTypes; boolean isExtensionMethod = candidate instanceof ExtensionMethodNode; if (isExtensionMethod) { int nArgs = argumentTypes.length; safeArgs = new ClassNode[nArgs + 1]; System.arraycopy(argumentTypes, 0, safeArgs, 1, nArgs); safeArgs[0] = receiver; // prepend self-type as first argument safeNode = ((ExtensionMethodNode) candidate).getExtensionMethodNode(); } /* TODO: corner case class B extends A {} Animal foo(A a) {} Person foo(B b) {} B b = new B() Person p = foo(b) */ ClassNode declaringClass = candidate.getDeclaringClass(); ClassNode actualReceiver = receiver != null ? receiver : declaringClass; Map spec; if (/*!isExtensionMethod && */candidate.isStatic()) { spec = Collections.emptyMap(); // none visible } else { spec = GenericsUtils.makeDeclaringAndActualGenericsTypeMapOfExactType(declaringClass, actualReceiver); GenericsType[] methodGenerics = candidate.getGenericsTypes(); if (/*!isExtensionMethod && */methodGenerics != null) { // GROOVY-10322: remove hidden type parameters for (int i = 0, n = methodGenerics.length; i < n && !spec.isEmpty(); i += 1) { for (Iterator it = spec.keySet().iterator(); it.hasNext(); ) { if (it.next().getName().equals(methodGenerics[i].getName())) it.remove(); } } } } Parameter[] params = makeRawTypes(safeNode.getParameters(), spec); int dist = measureParametersAndArgumentsDistance(params, safeArgs); if (dist >= 0) { if (!isExtensionMethod) { // GROOVY-6849, GROOVY-8788, et al. dist += (1 + getClassDistance(declaringClass, actualReceiver)); } if (dist < bestDist) { bestDist = dist; bestChoices.clear(); bestChoices.add(candidate); } else if (dist == bestDist) { bestChoices.add(candidate); } } } if (bestChoices.size() > 1) { // GROOVY-6849: prefer extension method in case of ambiguity List onlyExtensionMethods = new LinkedList<>(); for (MethodNode choice : bestChoices) { if (choice instanceof ExtensionMethodNode) { onlyExtensionMethods.add(choice); } } if (onlyExtensionMethods.size() > 1) { // GROOVY-8788: prefer closer parameter match over closer self-type match bestDist = Integer.MAX_VALUE; List bestExtension
[jira] [Commented] (GROOVY-8788) Inconsistency in extension method selection with @CompileStatic
[ https://issues.apache.org/jira/browse/GROOVY-8788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16613377#comment-16613377 ] Daniil Ovchinnikov commented on GROOVY-8788: Possibly the same cause. > 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 >Priority: Major > > 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 (v7.6.3#76005)