Code assist completion with "{" and method with Closure as last argument

[mauromol]

Ensure the option Window | Preferences | Groovy | Editor | Content Assist | Use closure literals for closure arguments is UNCHECKED.

Try the following:

package test17
class Test17 {
	void foo() {
		new Object().wit|
	}
}

Invoke code assist at "|", ensure with(Closure) is highlighted and hit "{".
Result: new Object().with({closure) (with strange selection: {closur).
Expected result:

new Object().with {
  |
}

or at least: new Object().with { | }
(with cursor at "|").

If Window | Preferences | Groovy | Editor | Content Assist | Use closure literals for closure arguments is CHECKED, the result is different, but still arguable:

new Object().with({{ it }) (with selection: i). Please note the double "{" and the strange selection.

[eric-milles]

{ is a trigger character, meaning it will end the inline edit boxes and accept the selections. I think I need to re-evaluate if { should be a trigger character for the last argument/parameter.

[mauromol]

Indeed, the "closure as last argument" is a very special case for Groovy, because it supports a special syntax (i.e.: outside the round parenthesis used for standard parameter passing to method calls). Perhaps it may deserve some special handling.

[eric-milles]

If you have "Place trailing closure arguments after closing parenthesis" checked, I would expect .with { | } for triggering with {. If it is not checked, .with({ | }). The replacement and smart-trigger handling is implemented in GroovyJavaMethodCompletionProposal's apply and computeReplacementString.

[eric-milles]

The call to set cursor position was removed. So at least now the { should be inserted at the end in all cases. I am working on special casing to prevent duplicate braces and to substitute a brace when completing for both sort(Closure) and sort(Comparator).

[eric-milles]

I've got something workable for you to try out. It does not support the parameter guessing mode yet, but should give a good completions with any of the other Content Assist preferences in checked or unchecked. There are more details in the comment of the commit above.

[mauromol]

Updated to 3.0.0.xx-201801312125-e47, this is what I see:

• new List().wi| with "Place trailing closure..." checked: selecting with(Closure) I get with {, no closing bracket; the opening bracket is selected, so if I start typing the opening bracket gets deleted; you have to press Enter to accept proposal, then user cursor keys to move away from selection, not very comfortable; the closing bracket (either inline or on the next line) would also be useful
• new List().wi| with "Place trailing closure..." unchecked: selecting with(Closure) I get with({ }), which would be fine, except for the fact that once again the opening curly bracket is selected and if you start typing you'll delete it
• trying with the various new List().sort overridings seems to work well, except for the problem with the opening curly bracket being selected as above and the missing closing curly bracket if "Place trailing closure..." is checked

[mauromol]

And, as a further improvement: could "{" be considered a trigger character for just method calls accepting closures, with special handling to avoid a duplicate open curly bracket insertion?

[eric-milles]

I'm not sure if you caught my proviso. Changes so far are only available when "Use guessed arguments for method calls" is unchecked. There is a separate code path for the guess handling and I still need to deal with that.

[mauromol]

Hi Eric,
it was not evident to me that both the curly braces highlighting problem and the inability to use "{" as a trigger character were directly related to "use guessed arguments for method calls" being checked. I just thought that no argument guessing were applied in that case.
Indeed, with that option unchecked, the first problem disappears and "{" works as a trigger character as expected!

[eric-milles]

I'm working to bring together the handling of guessed arguments and the other mode. It will take some time to unwind some complicated code but I wanted to give a taste of what the behavior would be in the end.

[eric-milles]

Here is a little summary of where I'm at. I removed the Groovy preference for enabling guessing. The Java one is used now for consistent behavior between Java and Groovy editors.

For constructor completion, I'm almost done. 2 of the 3 possibilities for argument insertion are fully supported and the guessing case just requires some missing context information. Also, the 2 possibilities for import insertion are fully supported -- "Add import instead of qualified name" checked or unchecked. WRT generics, Java tries to fill in the <> and provide guesses for type parameters in some cases. Groovy does not bother with the generics, keeping things easier for the developer.

For method completion, I don't have a table worked up yet, but will soon. All 3 possibilities for argument insertion are fully supported. I have a couple edge cases for Closure where there is a different insertion depending on the preferences. All 3 possibilities for import insertion are fully supported for static methods -- "Add import instead of qualified name" and "Use static imports (only 1.5 or higher)". Only the "static imports" case works for guessed enum values at the moment (issue #367).

Using { as a trigger for insertion should work for both constructor and method completion. For constructor, it starts an anonymous inner class. For a method, it depends on whether or not the last parameter is Closure or a SAM type.

Also, I want to illustrate the differences when the 3 Groovy Content Assist preference are on or off. All should work fine, but testing them all out is time consuming. I did my best with the 3 x 3 x 2^3 different combinations.

[eric-milles]

Also the Java Formatter preferences for spaces before and after colons, commas, curly braces and parentheses should be respected. I used the array initializer prefs for inside closure literals.

[eric-milles]

Also, the context display (completion just inside ( or just after , of a ctor or method call expression) appears to be broken in the Groovy editor. I'll probably open a separate issue for that.

[eric-milles]

[mauromol]

Hi Eric, great work here! I upgraded to 3.0.0.xx-201802052313-e47 and I concentrate now on this bug report.

This is what I see when trying to use { as a trigger character on new Object().wi|:

• "Fill method arguments" unchecked, "trailing closure arguments after closing parenthesis" checked: new Object().with(|) { - this is fine, except that I would appreciate if Greclipse also added the closing curly bracket
• "Fill method arguments" unchecked, "trailing closure arguments after closing parenthesis" unchecked: same result as above, I would have expected this instead: new Object().with({ | })
• "Fill method arguments" checked, "Insert parameter names" checked, "trailing closure arguments after closing parenthesis" checked: new Object().with { it } - this is fine, except that the whole { it } is highlighted, which is not comfortable because you will always need to use cursor keys to get inside the closure to type something useful; I would prefer just it to be highlighted
• "Fill method arguments" checked, "Insert parameter names" checked, "trailing closure arguments after closing parenthesis" unchecked: new Object().with({ it }) - this is fine and highlighting is good (just it)
• "Fill method arguments" checked, "Insert best guessed arguments" checked, "trailing closure arguments after closing parenthesis" checked: new Object().with { } - fine, but the whole { } is highlighted, I would prefer the cursor to be inside { } with no highlighting, so I can start typing my closure implementation without having to move with the cursor keys
• "Fill method arguments" checked, "Insert best guessed arguments" checked, "trailing closure arguments after closing parenthesis" unchecked: new Object().with({ it }) - this is fine and highlighting is good (just it)

[eric-milles]

For the cases where you end up with a trailing { you also get a tab position just past that. Once you tab there (or type ) to jump there) you can type enter and the } will be inserted if you have auto-close on.

For the cases where the entire closure is highlighted, I'm aware of that and have it as something to look at outside of this issue.

[mauromol]

Regarding the closing bracket: ok, I just find it inconsistent with the other cases, where the closing bracket is always inserted beforehand.
Regarding the entire closure highlighted, do you prefer me to open a new issue?
Regarding the second point of my list, I think it's an issue. Do you want me to open a new report for that too?