Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

class snippet completion returns both insertText and editText #3341

Open
valentinegb opened this issue Dec 6, 2024 · 3 comments
Open

class snippet completion returns both insertText and editText #3341

valentinegb opened this issue Dec 6, 2024 · 3 comments

Comments

@valentinegb
Copy link

The class snippet completion returns both insertText and editText. The LSP spec says that if editText is given, insertText should be ignored, which I find unlikely to be the intentional behavior on behalf of JDTLS in this case.

/**
 * An edit which is applied to a document when selecting this completion.
 * When an edit is provided the value of `insertText` is ignored.
 *
 * *Note:* The range of the edit must be a single line range and it must
 * contain the position at which completion has been requested.
 *
 * Most editors support two different operations when accepting a completion
 * item. One is to insert a completion text and the other is to replace an
 * existing text with a completion text. Since this can usually not be
 * predetermined by a server it can report both ranges. Clients need to
 * signal support for `InsertReplaceEdit`s via the
 * `textDocument.completion.completionItem.insertReplaceSupport` client
 * capability property.
 *
 * *Note 1:* The text edit's range as well as both ranges from an insert
 * replace edit must be a [single line] and they must contain the position
 * at which completion has been requested.
 * *Note 2:* If an `InsertReplaceEdit` is returned the edit's insert range
 * must be a prefix of the edit's replace range, that means it must be
 * contained and starting at the same position.
 *
 * @since 3.16.0 additional type `InsertReplaceEdit`
 */
textEdit?: [TextEdit](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textEdit) | [InsertReplaceEdit](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#insertReplaceEdit);

Here is an example RPC message I received:

{"jsonrpc":"2.0","id":16,"result":{"label":"class","kind":15,"documentation":"/**\n * DriveSubsystem\n */\npublic class DriveSubsystem {\n\n\t\n}","sortText":"999999212","filterText":"class","insertText":"/**\n * DriveSubsystem\n */\npublic class DriveSubsystem {\n\n\t${0}\n}","insertTextFormat":2,"insertTextMode":2,"textEdit":{"newText":"class","insert":{"start":{"line":1,"character":0},"end":{"line":1,"character":3}},"replace":{"start":{"line":1,"character":0},"end":{"line":1,"character":3}}}}}

The effect of this is that instead of inserting a new public class with the same name as the current file, you just get the word class and that’s it.

@rgrunber
Copy link
Contributor

Is this the latest release of JDT-LS ? If not, does the problem still exist when trying https://download.eclipse.org/jdtls/milestones/1.42.0/ ? When I tried using the class snippet, and checking the response, both properties had the same value.

Looking at

CodeGenerationTemplate template = (scc.needsPublic(monitor)) ? CodeGenerationTemplate.CLASSSNIPPET_PUBLIC : CodeGenerationTemplate.CLASSSNIPPET_DEFAULT;
classSnippetItem.setInsertText(getSnippetContent(scc, template, true));
if (isCompletionListItemDefaultsSupport()) {
classSnippetItem.setTextEditText(getSnippetContent(scc, template, true));
}
, there should be no difference between the 2 fields. They definitely look like they could be simplified but I'm not seeing a case where it'd return different values for the textEditText and insertText.

@valentinegb
Copy link
Author

Originally, no, I think I used 1.39.0 or 1.40.0, but I've just now tested with 1.42.0 and it seems to be the same:

// Send:
{"jsonrpc":"2.0","id":38,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///Users/valentinebriese/Developer/SwerveDrive2025/src/main/java/frc/robot/subsystems/Test.java"},"position":{"line":0,"character":3},"context":{"triggerKind":1}}}
// Receive:
{"jsonrpc":"2.0","id":38,"result":{"isIncomplete":false,"items":[{"label":"class","kind":15,"documentation":"package frc.robot.subsystems;\n\npublic class Test {\n\n\t\n}","sortText":"999999212","filterText":"class","insertText":"package frc.robot.subsystems;\n\npublic class Test {\n\n\t${0}\n}","insertTextMode":2,"textEditText":"package frc.robot.subsystems;\n\npublic class Test {\n\n\t${0}\n}"},{"label":"class","kind":14,"sortText":"999999213","filterText":"class","textEditText":"class","command":{"title":"","command":"java.completion.onDidSelect","arguments":["10","0"]},"data":{"pid":"0","rid":"10"}}],"itemDefaults":{"editRange":{"insert":{"start":{"line":0,"character":0},"end":{"line":0,"character":3}},"replace":{"start":{"line":0,"character":0},"end":{"line":0,"character":3}}},"insertTextFormat":2,"data":{"completionKinds":[3]}}}}
// Send:
{"jsonrpc":"2.0","id":39,"method":"completionItem/resolve","params":{"label":"class","kind":15,"documentation":"package frc.robot.subsystems;\n\npublic class Test {\n\n\t\n}","sortText":"999999212","filterText":"class","insertText":"package frc.robot.subsystems;\n\npublic class Test {\n\n\t${0}\n}","insertTextFormat":2,"insertTextMode":2,"textEdit":{"newText":"class","insert":{"start":{"line":0,"character":0},"end":{"line":0,"character":3}},"replace":{"start":{"line":0,"character":0},"end":{"line":0,"character":3}}},"data":{"completionKinds":[3]}}}
// Receive:
{"jsonrpc":"2.0","id":39,"result":{"label":"class","kind":15,"documentation":"package frc.robot.subsystems;\n\npublic class Test {\n\n\t\n}","sortText":"999999212","filterText":"class","insertText":"package frc.robot.subsystems;\n\npublic class Test {\n\n\t${0}\n}","insertTextFormat":2,"insertTextMode":2,"textEdit":{"newText":"class","insert":{"start":{"line":0,"character":0},"end":{"line":0,"character":3}},"replace":{"start":{"line":0,"character":0},"end":{"line":0,"character":3}}}}}

I'm noticing what I think is a discrepancy here though, between the 2nd message and the 3rd message. That's probably something to bring up with the devs of my editor, right?

@rgrunber
Copy link
Contributor

rgrunber commented Dec 10, 2024

Yeah, I think something odd may be happening client-side. As an explanation :

The first message is the completion request, and the language server responds by returning the list of completions (2nd message). The 3rd message is the client taking the a given item that was selected (eg. users selected it) (3rd message) and requesting it be resolved. The server sends the resolved item as the 4th message. This resolution process can be useful if computing the text/documentation/some property for each completion item is very expensive. The resolve only gets called when the item is selected so it allows the language server to be more responsive and defer certain computations.

Back to your example :

  • The 1st request seems fine.
  • The response contains 2 items. The class snippet (which generates the type declaration), and the class keyword (literally just "class"). The default values are things that all items share in common, so rather than repeating them per-item, we hold them separately. I suspect the client knows to add them on to subsequent requests that involve those items.
  • Your 2nd request is asking to resolve the class snippet, by sending the entire thing back to the server. The client seems to correctly take the list of item defaults, and apply it to the individual item, but if you look closer, it definitely isn't setting textEdit.newText correctly. Any logic happening between the response from textDocument/completion and the completionItem/resolve request is on the client side, and that's probably where the issue is.

On a side note, I'm not entirely sure what triggers a client performing a resolve on an item, or whether we could potentially avoid the extra call. Might be worth investigating if possible to avoid the extra message. Update: Based on my reading of https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_completion , I don't think there's a way for the server to signal that a completion item is "resolved" in the response from textDocument/completion and that the client should refrain from calling completionItem/resolve on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants