Argument names missing in code assist proposals for implementing missing methods

[mauromol]

This was the old GRECLIPSE-1473.

Consider the following Java interface:

package test;

public interface I {
	void myMethod(String a, int b);
}

And the following Groovy class:

package test

class A implements I {
	|
}

Invoke code assist at "|": the following two proposals are shown:

• myMethod(String arg0, int arg1): void - Override method in 'I'
• equals(Object param0): boolean - Override method in 'Object'

I would expect to see, instead:

• myMethod(String a, int b): void - Override method in 'I' (actually, it should be "implement missing method", but it's not that important)
• equals(Object obj): boolean - Override method in 'Object'

If you accept one of those proposals, though, the result is correct (thanks to the old fix for GRECLIPSE-1347).

[eric-milles]

There is an efficiency concern here. Looking up all those parameter names is costly. The code currently builds names quickly based on position and even has caching for the first 10.

[mauromol]

I see... Doesn't JDT have the same problem, though?

[eric-milles]

Here is how it was done in DSLContributionGroup:

    Map<String, ClassNode> params(MethodNode node) {
        Parameter[] parameters = node.getParameters();
        if (parameters == null || parameters.length < 1) {
            return Collections.emptyMap();
        }

        String[] names = null;

        // adapted from GroovyMethodProposal.createAllParameterNames(ICompilationUnit)
        if (parameters[0].getName().equals("arg0") || parameters[0].getName().equals("param0")) {
            ClassNode declaringClass = node.getDeclaringClass();
            if (declaringClass instanceof org.codehaus.jdt.groovy.internal.compiler.ast.JDTClassNode) {
                try {
                    org.eclipse.jdt.core.IType declaringType = project.findType(declaringClass.getName());
                    if (declaringType != null && declaringType.exists()) {
                        String[] parameterTypeSignatures = new String[parameters.length];
                        for (int i = 0; i < parameters.length; i += 1) {
                            if (declaringType.isBinary()) {
                                parameterTypeSignatures[i] = ProposalUtils.createTypeSignatureStr(parameters[i].getType());
                            } else {
                                parameterTypeSignatures[i] = ProposalUtils.createUnresolvedTypeSignatureStr(parameters[i].getType());
                            }
                        }

                        String name = !node.getName().equals("<init>") ? node.getName() : declaringClass.getNameWithoutPackage();
                        org.eclipse.jdt.core.IMethod declaredMethod = declaringType.getMethod(name, parameterTypeSignatures);
                        if (declaredMethod != null && declaredMethod.exists()) {
                            names = declaredMethod.getParameterNames();
                        }
                    }
                } catch (Exception e) {
                    GroovyDSLCoreActivator.logException(e);
                }
            }
        }

        Map<String, ClassNode> params = new LinkedHashMap<>(parameters.length);
        for (int i = 0; i < parameters.length; i += 1) {
            params.put(names != null ? names[i] : parameters[i].getName(), parameters[i].getType());
        }
        return params;
    }

I think there is a more direct path in ClassScope/MethodScope to transfer the names from AbstractMethodDeclaration to MethodBinding.

[eric-milles]

Ready to test

[mauromol]

Verified in 3.1.0.xx-201809070102-e47, thank you! 👍