Guessed enum values proposed without qualification when invoking a method

[mauromol]

This was the old GRECLIPSE-1705

Consider the following Java enumeration:

package b;
public enum MyEnum {
	VAL1, VAL2, VAL3;
}

The following Java class:

package b;
public class Utility {
	public static void doSomething(MyEnum enumValue) {
	}
}

And the following Groovy class:

package b 

class Test4 { 
	void doSomething() { 
		Utility.doSomet|
	} 
}

Ensure that the option Groovy | Editor | Content Assist | "Use guessed arguments for method calls" is checked.

Invoke code assist at "|". As you can see, MyEnum.VAL1, MyEnum.VAL2 and MyEnum.VAL3 are correctly proposed, however alongside them there are also the unqualified VAL1, VAL2 and VAL3. If you choose one of the unqualified choices, they are inserted "as-is", which is quite useless, unless you also add a corresponding static import to Test4 class.

IMHO, Greclipse should just propose the qualified enumeration values.

[mauromol]

I just saw #365, I don't know whether this might be somewhat related.

[eric-milles]

Here is the relevant code in ParameterGuesserDelegate. The block that sets useFull to false adds the qualifier to the replacement. The block that sets useFull to true should be adding an import but is not.

    public ICompletionProposal[] parameterProposals(String parameterType, String paramName, Position position, IJavaElement[] assignable, boolean fillBestGuess) {
        try {
            ICompletionProposal[] allCompletions = guesser.parameterProposals(parameterType, paramName, position, assignable, fillBestGuess, false);

            // ensure enum proposals insert the declaring type as part of the name
            if (allCompletions != null && allCompletions.length > 0 && assignable != null && assignable.length > 0) {
                IType declaring = (IType) assignable[0].getAncestor(IJavaElement.TYPE);
                if (declaring != null && declaring.isEnum()) {
                    // each enum is proposed twice; the first with the qualified name and the second with the simple name
                    boolean useFull = true;
                    for (int i = 0; i < assignable.length && i < allCompletions.length; i += 1) {
                        if (assignable[i].getElementType() == IJavaElement.FIELD) {
                            if (useFull) {
                                String newReplacement = declaring.getElementName() + '.' + assignable[i].getElementName();
                                ReflectionUtils.setPrivateField(PositionBasedCompletionProposal.class, "fDisplayString", allCompletions[i], newReplacement);
                                ReflectionUtils.setPrivateField(PositionBasedCompletionProposal.class, "fReplacementString", allCompletions[i], newReplacement);
                                useFull = false;
                            } else {
                                useFull = true;
                            }
                        }
                    }
                }
            }
            return addExtras(allCompletions, parameterType, position);

        } catch (Exception e) {
            GroovyContentAssist.logError("Exception trying to reflectively invoke 'parameterProposals' method.", e);

            return ProposalUtils.NO_COMPLETIONS;
        }
    }

[eric-milles]

Ready to test. I am also looking at supporting just one set of proposals contingent on these prefs:

[mauromol]

I'm testing with 3.0.0.xx-201801241910-e47.
First of all, now the list of suggestions is not duplicated with both qualified and unqualified entries, but I see some misbehaviour when the suggestion is chosen..

Case 1: "Use static imports" unchecked.
This is what I see when I choose each value:

• VAL3: MyEnum.VAL3 is correctly inserted
• VAL2: a mistyped MyEnum.VAL3VAL2 is inserted
• VAL1: a mistyped MyEnum.VAL3VAL1 is inserted

Case 2: "Use static imports" checked.
When I choose each value, the static import is always inserted correctly. But this is what gets inserted inline:

• VAL3: VAL3 is correctly inserted
• VAL2: a mistyped VAL3VAL2 is inserted
• VAL1: a mistyped VAL3VAL1 is inserted

On the other hand, please note that I observe the original problem if I am in this situation:

package test4

class Test4 { 
	void doSomething() { 
		Utility.doSomething(VA|)
	} 
}

Invoke code assist and choose VAL1: the text VAL1 is inserted without qualification and without any static import being added.

I have some concerns, though with regards to the use of "Use static imports" option. If you consider an equivalent Java Class:

package test4;

public class Test4J {

	public void doSomething() { 
		Utility.doSomething(VA|
	} 
}

If you invoke code assist there, and choose, for instance, VAL1 suggestion, MyEnum.VAL1 is inserted even if "Use static imports" is checked. So, I suspect that option is used in other contexts (I wonder which ones...).

[mauromol]

Also consider this risk. Suppose you have the following enumeration besides MyEnum:

package b;
public enum MyEnum2 {
	VAL1, VAL2, VAL3;
}

And change Utility class to:

package test4;
public class Utility {
	public static void doSomething(MyEnum enumValue) {
	}
	public static void doOther(MyEnum2 enumValue) {
	}
}

Then repeat the original steps to repro to complete doSomething and doOther, ensure "use static imports" option is checked and choose for both an enum value with the same name (VAL3 for instance); this is the final result:

package b

import static b.MyEnum.VAL3
import static b.MyEnum2.VAL3

class Test4 { 
	void doSomething() { 
		Utility.doSomething(VAL3)
		Utility.doOther(VAL3)
	} 
}

which of course is wrong. In other words, if you plan to respect "Use static imports" option in this scenario, you should ensure that a static import for a class/enum with the same name does not already exist, otherwise you should fall back to qualified insertion anyway.

[eric-milles]

I can't seem to recreate the improper insertions you mentioned -- "a mistyped VAL3VAL2 is inserted". Supporting the qualifiers took a bunch of extra lifting due to the moving position of the arguments. I could easily get the static import case working at the expense of the 2 qualifier cases. Not sure which state is preferred until a more complete solution can be found.

[mauromol]

This is what I see now with 3.0.0.xx-201801252143-e47:

• the problem with VAL3VAL2 being inserted does not happen any more
• with "Use static imports" checked and code assist invoked as "doSom|" or "doOth|", all works fine except for the fact that when you have both Utility.doSomething() and Utility.doOther() calls the problem of duplicated static imports for enumeration values with the same name still exists (and leads to invalid code)
• with "Use static imports" checked and code assist invoked as "doSomething(VA|" or "doOther(VA|", the behaviour is the same as the previous point
• with "Use static imports" unchecked and code assist invoked as "doSom|" or "doOth|", values are inserted unqualified and no static import is added, so the result is invalid
• with "Use static imports" unchecked and code assist invoked as "doSomething(VA|" or "doOther(VA|", values are inserted correctly and qualified

[eric-milles]

I have opened a separate issue to resolve the introduction of conflicting imports. The qualifiers missing will be worked on as part of this issue, but are a lower priority to me as I use the staic imports and I think it is the default setting for a new workspace.

[eric-milles]

ready for retest against the 3 states of import/qualifier insertion

[mauromol]

Tested with 3.0.0.xx-201802242107-e47.

• Case 1: "Use static import" checked, "Fill method arguments and show guessed arguments" unchecked
  Utility.doSome| => completed as Utility.doSomething(|) => Ctrl+Space = no suggestion for MyEnum values => starting to type VA| => Ctrl+Space and choose VAL1 => static import added, VAL1 inserted 👍

• Case 2: "Use static import" checked, "Fill method arguments and show guessed arguments" checked, "Insert parameter names" checked:
  Utility.doSome| => completed as Utility.doSomething(enumValue) (enumValue highlighted) => Ctrl+Space = no suggestion for MyEnum values => starting to type VA| => Ctrl+Space and choose VAL1 => static import added, VAL1 inserted 👍

• Case 3: "Use static import" checked, "Fill method arguments and show guessed arguments" checked, "Insert best guessed arguments" checked:
  Utility.doSome| => completed as Utility.doSomething(VAL3) (VAL3 highlighted and suggestion list open) => choose VAL1 => static import added, VAL3VAL1 inserted 👎

• Case 4: "Use static import" unchecked, "Fill method arguments and show guessed arguments" unchecked
  Utility.doSome| => completed as Utility.doSomething(|) => Ctrl+Space = no suggestion for MyEnum values => starting to type VA| => Ctrl+Space and choose VAL1 => MyEnum.VAL1 inserted 👍

• Case 5: "Use static import" unchecked, "Fill method arguments and show guessed arguments" checked, "Insert parameter names" checked:
  Utility.doSome| => completed as Utility.doSomething(enumValue) (enumValue highlighted) => Ctrl+Space = no suggestion for MyEnum values => starting to type VA| => Ctrl+Space and choose VAL1 => MyEnum.VAL1 inserted 👍

• Case 6: "Use static import" unchecked, "Fill method arguments and show guessed arguments" checked, "Insert best guessed arguments" checked:
  Utility.doSome| => completed as Utility.doSomething(VAL3) (VAL3 highlighted and suggestion list open) => choose VAL1 => MyEnum.VAL3VAL1 inserted 👎

Conclusion
I think the point of this ticket is now fixed, however I see two possible new follow up tickets that can be created:

• when "Insert best guessed arguments" is checked, the guessed argument is inserted unqualified and it interferes with proper code assist completion when the user chooses any suggestion from the suggestion list
• when invoking code assist from Utility.doSomething(|), it is reasonable to expect suggestions from MyEnum values to be shown

What do you think?

[eric-milles]

when "Insert best guessed arguments" is checked, the guessed argument is inserted unqualified and it interferes with proper code assist completion when the user chooses any suggestion from the suggestion list

I tested these cases and everything was inserting correctly. I'm not sure what the difference is between our two editors.

when invoking code assist from Utility.doSomething(|), it is reasonable to expect suggestions from MyEnum values to be shown

Never gave this much thought because the ticket has always referred to argument guessing.

[mauromol]

• when invoking code assist from Utility.doSomething(|), it is reasonable to expect suggestions from MyEnum values to be shown

I just saw this is not the case for JDT either, so let's ignore this.

[mauromol]

• when "Insert best guessed arguments" is checked, the guessed argument is inserted unqualified and it interferes with proper code assist completion when the user chooses any suggestion from the suggestion list

Regarding this, I just found that the problem occurs only if "Completion overwrites" in Java | Content Assist settings is checked, instead of "Completion inserts".
The 6 cases above, in my today comment, were tested with "Completion overwrites", and bring the above results (so the only problem is about the VAL3VAL1 problem).
Repeating all those 6 cases with "Completion inserts" leads to correct results for all the cases.
So I think the only remaining issue is the one described in #500.