Wrong method call detected when chaining two calls

[mauromol]

This was the old GRECLIPSE-1783 (with a little variation), as well as part of GRECLIPSE-1702.

Consider the following Java class:

package a;
import java.util.List;
public class Utility {
	public static int remove(List<?> list, Object obj) {
		final int i = list.indexOf(obj);
		if (i != -1)
			list.remove(i);
		return i;
	}
}

And the following Groovy class:

package a 

class Test1 {
	void doSomething() {
		List a = new ArrayList() 
		List b = new ArrayList() 
		a.remove(Utility.remove(b, new String()))
	}
}

Press F2 over Utility.remove (or press F3 to navigate): when GRECLIPSE-1702 was opened, Greclipse was detecting the wrong return type (it thought it were returning Integer instead of int). This problem cascaded to the detection of which method of a is actually being called: it thought java.util.List.remove(Object) were called, while in fact it's java.util.List.remove(int).

Meanwhile the problem of Greclipse detecting the wrong return type was fixed, but Greclipse is still thinking, for some reason, that when you type a.remove, java.util.List.remove(Object) is called instead of java.util.List.remove(int). You can see that by pressing F2 or F3 over a.remove.
As a side note, when GRECLIPSE-1783 was opened, F2 was behaving wrong just like now, but F3 was unexpectedly working correctly.

[eric-milles]

Here is a little background info on this one. The type inferencing visitor visits the method before the arguments. To try and help out in these situations of method return type determining the overload, there is some pre-lookup that is patched together. It is not able to resolve Util.remove correctly and so there is not a correct match to List.remove(int).

    public void visitMethodCallExpression(MethodCallExpression node) {
        VariableScope scope = scopes.getLast();
        scope.setCurrentNode(node);
        if (isDependentExpression(node)) {
            primaryTypeStack.removeLast();
        }
        completeExpressionStack.add(node);
        node.getObjectExpression().visit(this);

        if (node.isSpreadSafe()) {
            // must find the component type of the object expression type
            ClassNode objType = primaryTypeStack.removeLast();
            primaryTypeStack.add(VariableScope.extractElementType(objType));
        }

        if (node.isUsingGenerics()) {
            visitGenericTypes(node.getGenericsTypes(), null);
        }

        node.getMethod().visit(this);

        // this is the inferred return type of this method
        // must pop now before visiting any other nodes
        ClassNode returnType = dependentTypeStack.removeLast();

        // this is the inferred declaring type of this method
        Tuple t = dependentDeclarationStack.removeLast();
        VariableScope.CallAndType call = new VariableScope.CallAndType(node, t.declaration, t.declaringType, scope.getEnclosingTypeDeclaration(), scope.isOwnerStatic());

        completeExpressionStack.removeLast();

        ClassNode catNode = isCategoryDeclaration(node);
        if (catNode != null) {
            scope.setCategoryBeingDeclared(catNode);
        }

        // remember that we are inside a method call while analyzing the arguments
        scope.addEnclosingMethodCall(call);
        node.getArguments().visit(this);
        scope.forgetEnclosingMethodCall();

Here is the sketchy method that was put in place to help out:

    private List<ClassNode> getMethodCallArgumentTypes(ASTNode node) {
        // TODO: Check for MethodCall once 2.1 is the minimum supported Groovy runtime
        Expression arguments = null;
        if (node instanceof MethodCallExpression) {
            arguments = ((MethodCallExpression) node).getArguments();
        } else if (node instanceof ConstructorCallExpression) {
            arguments = ((ConstructorCallExpression) node).getArguments();
        } else if (node instanceof StaticMethodCallExpression) {
            arguments = ((StaticMethodCallExpression) node).getArguments();
        }

        if (arguments != null) {
            if (arguments instanceof ArgumentListExpression) {
                List<Expression> expressions = ((ArgumentListExpression) arguments).getExpressions();
                if (!expressions.isEmpty()) {
                    List<ClassNode> types = new ArrayList<ClassNode>(expressions.size());
                    for (Expression expression : expressions) {
                        if (expression instanceof ConstantExpression &&
                            ((ConstantExpression) expression).isNullExpression()) {

                            types.add(VariableScope.NULL_TYPE); // sentinel value
                        } else {
                            scopes.getLast().setMethodCallArgumentTypes(getMethodCallArgumentTypes(expression));
                            Expression objExpr = expression instanceof MethodCallExpression ? ((MethodCallExpression) expression).getObjectExpression() : null;
                            TypeLookupResult tlr = lookupExpressionType(expression, objExpr != null ? objExpr.getType() : null, objExpr instanceof ClassExpression, scopes.getLast());

                            types.add(tlr.type);
                        }
                    }
                    return types;
                }
            }
            // TODO: Might be useful to look into TupleExpression
            return Collections.emptyList();
        }
        return null;
    }

[eric-milles]

There are 2 workarounds for this bug:

1. Assign Util.remove(...) to a local variable with the type specified (not def)
2. Add a static import for Utility.remove and replace Util.remove with remove -- sounds strange, but the lack of an object expression actually allows the return type to be inferred correctly

[eric-milles]

ready to test

[mauromol]

With 2.9.2.xx-201711080155-e46 this works fine now, thank you! 👍