eclipse-jdtls
Use textEdit instead of insertText to avoid ambiguous
Consider the following code:
package temp;
public class T {
public void name() {
String s = null;
s.nnull|
}
}
Trigger completion at |. This will result in the following exchange:
[Trace - 7:58:43 AM] Sending request 'completionItem/resolve - (688)'.
Params: {
"label": "nnull",
"labelDetails": {
"description": "Creates an if statement and checks if the expression does not resolve to null"
},
"insertTextFormat": 2,
"insertText": "if (${i:inner_expression(java.lang.Object,array)} != null) {\n\t${0}\n}",
"kind": 15,
"sortText": "999999999",
"command": {
"title": "",
"command": "java.completion.onDidSelect",
"arguments": [
"74",
"0"
]
},
"data": {
"rid": "74",
"COMPLETION_EXECUTION_TIME": "4",
"pid": "0"
}
}
[Trace - 7:58:43 AM] Received response 'completionItem/resolve - (688)' in 1ms.
Result: {
"label": "nnull",
"labelDetails": {
"description": "Creates an if statement and checks if the expression does not resolve to null"
},
"kind": 15,
"detail": "Creates an if statement and checks if the expression does not resolve to null",
"documentation": {
"kind": "markdown",
"value": "```java\nif (s != null) {\n\t\n}\n```"
},
"sortText": "999999999",
"insertText": "if (${i:inner_expression(java.lang.Object,array)} != null) {\n\t${0}\n}",
"insertTextFormat": 2,
"textEdit": {
"range": {
"start": {
"line": 5,
"character": 0
},
"end": {
"line": 5,
"character": 7
}
},
"newText": "if (s != null) {\n\t${0}\n}"
},
"additionalTextEdits": [
{
"range": {
"start": {
"line": 5,
"character": 0
},
"end": {
"line": 5,
"character": 7
}
},
"newText": ""
}
],
"command": {
"title": "",
"command": "java.completion.onDidSelect",
"arguments": [
"74",
"0"
]
}
}
Issues:
- The spec says that textEdit should not be changed in the resolve call:
All other properties (usually sortText, filterText, insertText and textEdit) must be provided in the textDocument/completion response and must not be changed during resolve.
This allows the clients to apply the text edit synchronously and resolve additionalTextEdits in async manner.
-
additionalTextEditis bad and can be removed from the response: it will actually delete the stuff to be expanded. I guess vscode for some reason does not apply it, but this seems like some odd side effect/bug.
The spec says that textEdit should not be changed in the resolve call
Make sure you turned java.completion.lazyResolveTextEdit.enabled off. I think by default it's off at the LS side. Unless the client declares it's enabled.
When I do this, the issue with textEdit goes away but then the server sends bad additionalTextEdit.
[Trace - 08:30:09 AM] Received response 'completionItem/resolve - (5922)' in 0ms.
Result: {
"label": "nnull",
"kind": 15,
"detail": "Creates an if statement and checks if the expression does not resolve to null",
"documentation": {
"kind": "markdown",
"value": "```java\nif (s != null) {\n\t\n}\n```"
},
"sortText": "999999999",
"insertText": "if (s != null) {\n\t${0}\n}",
"insertTextFormat": 2,
"additionalTextEdits": [
{
"range": {
"start": {
"line": 5,
"character": 0
},
"end": {
"line": 5,
"character": 7
}
},
"newText": ""
}
],
"command": {
"title": "",
"command": "java.completion.onDidSelect",
"arguments": [
"44",
"0"
]
}
}
Note that the additional text edit does includes also nnull in its range but it is already deleted during the snippet expansion. Here it is the equivalent from rust analyzer expanding from a.letm:
{
"label": "letm",
"kind": 15,
"detail": "let mut",
"deprecated": null,
"preselect": true,
"sortText": "ffffff8e",
"filterText": "letm",
"insertTextFormat": 2,
"textEdit": {
"newText": "let mut $0 = a;",
"insert": {
"start": {
"line": 3,
"character": 2
},
"end": {
"line": 3,
"character": 6
}
},
"replace": {
"start": {
"line": 3,
"character": 2
},
"end": {
"line": 3,
"character": 6
}
}
},
"additionalTextEdits": [
{
"range": {
"start": {
"line": 3,
"character": 0
},
"end": {
"line": 3,
"character": 2
}
},
"newText": ""
}
]
}
To elaborate a bit more: note that RA range includes a. without letm while the jdtls includes s.nnull. For both the completion is requested after s.nnull and a.letm respectively.
[Trace - 08:30:09 AM] Received response 'completionItem/resolve - (5922)' in 0ms.
Result: {
"label": "nnull",
"kind": 15,
"detail": "Creates an if statement and checks if the expression does not resolve to null",
"documentation": {
"kind": "markdown",
"value": "```java\nif (s != null) {\n\t\n}\n```"
},
"sortText": "999999999",
"insertText": "if (s != null) {\n\t${0}\n}",
"insertTextFormat": 2,
"additionalTextEdits": [
{
"range": {
"start": {
"line": 5,
"character": 0
},
"end": {
"line": 5,
"character": 7
}
},
"newText": ""
}
],
"command": {
"title": "",
"command": "java.completion.onDidSelect",
"arguments": [
"44",
"0"
]
}
}
In this case, the completion item contains insert text only (not textEdit), additional text edit is used to remove the character. I think it should be fine.
Here it is experiment with lua servers:
b = "test"
b.du|
When I request completions, I get the following:
[Trace - 10:32:53 AM] Sending request 'textDocument/completion - (6710)'.
Params: {
"textDocument": {
"uri": "file:///home/yyoncho/Sources/DemoProjects/lua/lua.lua"
},
"position": {
"line": 2,
"character": 4
},
"context": {
"triggerKind": 1
}
}
[Trace - 10:32:53 AM] Received response 'textDocument/completion - (6710)' in 32ms.
Result: {
"isIncomplete": null,
"items": [
{
"data": {
"id": 32,
"uri": "file:///home/yyoncho/Sources/DemoProjects/lua/lua.lua"
},
"insertText": "dump",
"insertTextFormat": 2,
"kind": 3,
"label": "dump(f, strip)",
"sortText": "0001"
}
]
}
Note that the insert text is supposed to override the prefix as defined by the language boundaries. While in your example, it seems that it is not supposed to do that. Apparently, vscode does some heuristics that are not part of the spec to determine whether to replace prefix or not. Instead of forcing the other clients to reverse engineer vscode can you replace that insertText with text edit with the proper characteristics?
Also the spec says:
Completion item provides an insertText / label without a text edit: in the model the client should filter against what the user has already typed using the word boundary rules of the language (e.g. resolving the word under the cursor position). The reason for this mode is that it makes it extremely easy for a server to implement a basic completion list and get it filtered on the client.
It is not directly related to the issue but are we supposed to filter against the prefix but then keep it in some cases and drop it in others when inserting?!
Calm down, man.
Note that the insert text is supposed to override the prefix as defined by the language boundaries.
are we supposed to filter against the prefix but then keep it in some cases and drop it in others when inserting?!
My understanding is that: the LSP does not force the insert text to override the prefix. The behavior is determined by client implementation.
In the examples mentioned in our discussion:
-
dumpmatchesdu, so onlympin inserted. -
if (s != null) {\n\t${0}\n}does not matchnnull, so the entire string is inserted.
But, again, the behavior is subject to interpretation by the client side. In other clients, it's also possible to always do override. Based on that, I agree that we should use textEdit as much as possible to avoid ambiguous.
I would suggest renaming the issue title to Use textEdit instead of insertText to avoid ambiguous. Instead of ... not following.... WDYT?
Calm down, man.
Sorry, I am very calm. I guess this impression comes from English not being my native language.
I would suggest renaming the issue title to Use textEdit instead of insertText to avoid ambiguous. Instead of ... not following.... WDYT?
Sounds good. Done.
if (s != null) {\n\t${0}\n} does not match nnull, so the entire string is inserted.
FWIW this wont work as well because you may have fuzzy completion and complete mp to dump. Similarly, you can have the prefix in the insertText of the snippet for nnull