Type inference problem on sort { }.each { }

[mauromol]

This seems to be some sort of regression, because it was fixed with GRECLIPSE-1695.

Consider the following Groovy class:

package test10

class Bar {
	void bar() {
		List<String> myList = new ArrayList<String>()
		myList.each {  
			it.trim() // it detected as String correctly here
		}
		myList.sort {  a, b ->
			a.trim() <=> b.trim() // a and b detected as String correctly here
		}

		myList.sort {  a, b ->
			a.trim() <=> b.trim() // a and b detected as String correctly here
		}.each {   
			it.trim() // it not detected as String here!
		}
	}
}

As you can see, the last it.trim() is underlined because it is not recognized as String. Probably this is because the call to myList.sort { } is wrongly recognized as a call to Java 8 java.util.List.sort(Comparator<? super E>), which returns void.

[eric-milles]

The addition of List.sort to the JDK caused a lot of trouble. If you use toSorted(), there should be no problem flow typing this out.

[eric-milles]

Which of these is more applicable? Can you see why there is confusion for the tool?
default void sort(Comparator<? super E> c) from List
static <T> List<T> sort(Collection<T> self, Comparator<T> c) from DefaultGroovyMethods

[mauromol]

I think Greclipse should choose what Groovy runtime does... I don't know how it behaves in this case.
However my call should not be ambiguous, I'm passing a closure to sort.

[eric-milles]

In both cases there is a closure that is being coerced to the SAM type Comparator. Do what Groovy does is not that easy when it is not stated clearly in a specification document exactly what that is. Groovy-Eclipse has preference for type's own member over category method. Here are some test cases that had to be written for this when Java 8 came around: @test // GRECLIPSE-1695 redux public void testDGM45() throws Throwable { // Java 8 adds default method sort(Comparator) to List interface... // TypeInferencingVisitorWithRequestor.lookupExpressionType replaces DGM (from CategoryTypeLookup) with JDK (from SimpleTypeLookup) String contents = "List<String> myList = new ArrayList<String>()\n" + "myList.sort { a, b ->\n" + " a.trim() <=> b.trim()\n" + "}.each {\n" + " it\n" + "}\n"; int start = contents.lastIndexOf("it"); int end = start + "it".length(); assertTypeOneOf(contents, start, end, "java.lang.Void", "java.lang.String"); } @test public void testListSort1() { String contents = "def list = []; list.sort()"; int start = contents.lastIndexOf("sort"); int end = start + "sort".length(); assertType(contents, start, end, "java.util.List<java.lang.Object>"); assertDeclaringType(contents, start, end, "org.codehaus.groovy.runtime.DefaultGroovyMethods"); } @test public void testListSort2() { // Java 8 added sort(Comparator) to the List interface boolean jdkListSort; try { List.class.getDeclaredMethod("sort", Comparator.class); jdkListSort = true; } catch (Exception e) { jdkListSort = false; } String contents = "def list = []; list.sort({ o1, o2 -> o1 <=> o2 } as Comparator)"; int start = contents.lastIndexOf("sort"); int end = start + "sort".length(); assertType(contents, start, end, jdkListSort ? "java.lang.Void" : "java.util.List<java.lang.Object>"); assertDeclaringType(contents, start, end, jdkListSort ? "java.util.List<java.lang.Object>" : "org.codehaus.groovy.runtime.DefaultGroovyMethods"); }

[eric-milles]

Here is the selection algorithm for multiple matches. List.sort has a parameter distance of 276824064 and DGM.sort has a distance of 201326592 which is lower and so it's preferred.

    private Object chooseMostSpecificParams(String name, List matchingMethods, Class[] arguments) {
        long matchesDistance = -1;
        LinkedList matches = new LinkedList();
        for (Object method : matchingMethods) {
            ParameterTypes paramTypes = (ParameterTypes) method;
            long dist = MetaClassHelper.calculateParameterDistance(arguments, paramTypes);
            if (dist == 0) return method;
            if (matches.isEmpty()) {
                matches.add(method);
                matchesDistance = dist;
            } else if (dist < matchesDistance) {
                matchesDistance = dist;
                matches.clear();
                matches.add(method);
            } else if (dist == matchesDistance) {
                matches.add(method);
            }
        }

[eric-milles]

So it's actually this DGM that satisfies: static <T> List<T> sort(Iterable<T> self, Closure closure). Closure a closer match than Comparator.

[eric-milles]

It is this block in SimpleTypeLookup that lets member matches defer to category method matches. All the hard work of comparing arguments to parameters happened within org.eclipse.jdt.groovy.search.SimpleTypeLookup.findDeclaration and very, very little information about those comparisons is returned.

    // check if the arguments and parameters are mismatched; a category method may make a better match
    if (((MethodNode) declaration).getParameters().length != scope.getMethodCallNumberOfArguments()) {
        confidence = TypeConfidence.LOOSELY_INFERRED;
    }

[mauromol]

So it's actually this DGM that satisfies: static <T> List<T> sort(Iterable<T> self, Closure closure). Closure a closer match than Comparator.

Exactly what I expect: in that code I actually want to invoke the sort variant that takes a Closure and returns a List<String>, but Greclipse thinks I'm invoking Java 8 sort (try to hit F2 or F3 over the last sort call), and hence the subsequent each call fails at determining the correct it type.

[eric-milles]

ready to test

[mauromol]

Tested with 2.9.2.xx-201711191708-e46, works fine, thank you! 👍