[feature] Improve functionality behind java.completion.guessMethodArguments

[gayanper]

When enabled java.completion.guessMethodArguments it will fill in variable names which are matching the arguments of the method. But when it doesn't find any variable names, instead of filing with argument names, it fills them with null.

We can improve this behavior as follows

• first try to find matching variable names and fill them
• For arguments which doesn't have a match, use the argument name as variable suggestion if the method has parameter names.

This new behavior will help the developer to create fields, local variable or parameters for the names that didn't match.

[gayanper]

I looked inside the code a bit. It seems we have two options

• Instead of using null for object type as default values, use the parameter name which go against completing with matching values
• Change setting to (Fill Parameter Names | Fill Parameter Names After Guessing | Guess Values | Do Nothing)

The second option give better choice which can simulate

• Old behavior we had
• New behavior where we use paramater names when there is no match instead of default literal values
• Can simulate IJ behavior where no paramater values are filled.

[rgrunber]

Having separate setting options to handle all of these makes sense. If a user doesn't enable parameter name code lens, they might find it useful to have values auto-filled, particularly for primitive default values. I agree null is pretty annoying.

I think you also mentioned that something like chain completion might also improve things down the road. Or even some heuristic like searching all method invocations, looking for what's used for that argument. Could be helpful where an argument is static.

[gayanper]

@rgrunber @jdneo @testforstephen @mickaelistria WDYT ?

[gayanper]

Having separate setting options to handle all of these makes sense. If a user doesn't enable parameter name code lens, they might find it useful to have values auto-filled, particularly for primitive default values. I agree null is pretty annoying.

I think you also mentioned that something like chain completion might also improve things down the road. Or even some heuristic like searching all method invocations, looking for what's used for that argument. Could be helpful where an argument is static.

So when you say a separate setting, should we go with keeping the current setting as it is and then have a new setting like
java.completion.guessMethodArgumentsStrategy where it has (Fill Parameter Names After Guessing | Guess Values) ?

Or should we keep it simple by just filling with parameter names instead of literals when there is no variable to fill in ? Because if there is no variable all arguments will be filled by parameter names and then when the java.completion.guessMethodArguments is switched off, we can just complete the method without filling in argument names. That will enable us to provide more smarter completions down the line.

For example I have a pending PR in jdt.core which change the relevance of methods and fields based on paramater name and also adding more smarter chain completion will improve the experience.

[rgrunber]

I meant java.completion.guessMethodArguments could become an enum with various strategies. It's better than having 2 different settings. On the other hand, If everyone used code lens for parameter hints, or knew about the signature hints, I don't see the value in filling in methods with the parameter names. It just seems like an attempt to inform the user about the nature of the parameters but we have better ways to do that now.

Also, I managed to find the old chain completion PR, https://github.com/rgrunber/eclipse.jdt.ls/tree/chain . I needs to be cleaned up but it should work, if there's interest in it.

[jdneo]

I meant java.completion.guessMethodArguments could become an enum with various strategies. It's better than having 2 different settings.

+1

There are some users asking about not filling anything: redhat-developer/vscode-java#2903.

[gayanper]

OK considering different perspectives, I would suggest to move forward as follows

java.completion.guessMethodArguments: None | FindBestMatchWithDefault | FillInArgumentNames

None - Do nothing, just complete the method as foo()
FindBestMatchWithDefault - Current behavior when true where we fill with best matching variables and rest is willed with defaults.
FillInArgumentNames - Current behavior when false where we fill with parameter names.

We start like this and later we can also add something like FindBestMatch which will avoid adding default literals and nulls for arguments which doesn't have a match.

@rgrunber @jdneo WDYT ?

[jdneo]

The three setting values looks good to me.

One concern of the value None. If we introduce it, should we only display one method item in the completion lists? Currently all the overload versions are displayed.

[jdneo]

I'll take a look at this.