Auto complete after new keyword

[jdneo]

This PR adds ability to trigger completion when user types "new |".

This can be a handy feature especially when the expected type is an object.

If the expected type is interface, currently JDT will only propose an anonymous class creation, which is not ideal and needs improve from upstream. For such case I've filed an issue there: https://bugs.eclipse.org/bugs/show_bug.cgi?id=578815

fix redhat-developer/vscode-java#1666

Signed-off-by: Sheng Chen sheche@microsoft.com

[jdneo]

test this please

[jdneo]

BTW there is a discussion about supporting multi-character match at LSP side: microsoft/language-server-protocol#1413

And there are already several extensions use similar solution in this PR to resolve this requirement.

Once this new capability is supported in LSP in the future, we can migrate the implementation to the LSP side.

[rgrunber]

This works pretty well for me, and we should probably merge. For the sake of discussion, how do we feel about completion being unintentionally triggered for things like : new String(val_new<space>); (where val_new is defined somewhere) ? Note that although completion comes up, selecting any item, doesn't do anything, which was a bit weird.

I think using SharedASTProviderCore to get the ASTNode and then maybe using that to write the isCompletionForConstructor logic, could probably polish up the correctness but at the risk of a more expensive operation (fetch the ASTNode (ideally should be faster), and traverse it). Only other added benefit to this approach is you could also handle things like new\n\n or some odd combination of whitespace.

[jdneo]

@rgrunber Thank you for the suggestions, leverage AST can definitely make the completion trigger more context-aware! Sharing some thought behind the appended commit:

I think using SharedASTProviderCore to get the ASTNode and then maybe using that to write the isCompletionForConstructor logic, could probably polish up the correctness but at the risk of a more expensive operation

Checking the response time in the output channel. I didn't see a significant time increase after using SharedASTProviderCore. The performance is acceptable. Meanwhile, StringLiteral and SimpleName is ignored now to improve the correctness.

handle things like new\n\n or some odd combination of whitespace.

After some attempts, I finally decide to not support this case. Because it needs to add more logic to find a valid offset(skip all the whitespaces), and thus find a valid node via NodeFinder. (if the offset denotes to a whitespace, then the node type of the found AST node could be varied depending on what kind of content is followed in the code, which makes the check logic so complex). Given that it's not a common code style, I think we can first ignore this scenario. (IDEA does not support it as well 😀 )

One more thing I found: Currently JDT won't give a right completion proposal when the expected type is an array, which we might need to improve it in upstream.

[rgrunber]

Feel free to merge after taking care of that minor issue listed.