java-language-server icon indicating copy to clipboard operation
java-language-server copied to clipboard

Add-import on completion

Open georgewfraser opened this issue 6 years ago • 3 comments

Right now, we automatically add imports on save: https://github.com/georgewfraser/java-language-server/blob/incremental/src/main/java/org/javacs/CompileBatch.java#L291

This works well for scenarios like copy-pasting large blocks of code, but it doesn't work great when you autocomplete:

  • User starts typing the name of an unimported class
  • User selects a particular unimported class and tab completes
  • This class is still not imported, so subsequent autocomplete is broken until the user saves

For example, new SomeNotImportedClass()._ will not autocomplete.

The Language Server Protocol provides for this exact scenario with CompletionItem#additionalTextEdits: https://microsoft.github.io/language-server-protocol/specification#textDocument_completion

Related: #95, #44

@albfan

georgewfraser avatar May 05 '19 21:05 georgewfraser

Following fixImports() I see it is called on formatting()

https://github.com/georgewfraser/java-language-server/blob/incremental/src/main/java/org/javacs/JavaLanguageServer.java#L1173

so I guess you suppose it is triggered on save because some IDEs autoformat on save, am I right? We attend textDocument/formatting so I can try that, but It might not be called on save.

The additionalEdits makes sense, let me check if we are taking care of that when we receive the completionItems. Seems we need to call completionItem/resolve if isImcomplete=true?

albfan avatar May 06 '19 02:05 albfan

Confirmed: After call textDocument/formatting:

autoimport-on-formatting

{"jsonrpc":"2.0","id":22,"result":[{"range":{"start":{"line":1,"character":0},"end":{"line":1,"character":0}},"newText":"\nimport java.util.Vector;\n"}]}ava/org/albfan/App.java","version":13},"contentChanges":[{"range":{"start":{"line":1,"character":0},"end":{"line":1,"character":0}},"rangeLength":0,"text":"\nimport java.util.Vector;\n"}]}}Content-Length: 178

We are still not taking care of additionalTextEdits, but as seen it's just plumbings

albfan avatar May 06 '19 06:05 albfan

It's a bit trickier than just connecting additionalTextEdits <--> existing formatting code. Imports are sometimes ambiguous, but when completing you have additional context: the user has selected a specific not-yet-imported class, so you know which import needs to be added. Same is true for code actions.

So, you will probably need to refactor the formatting code a bit to make it reusable for codeActions and completions, but it shouldn't be too difficult.

georgewfraser avatar May 06 '19 21:05 georgewfraser