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.

Sign up for GitHub

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

Jump to bottom

Add-import on completion #96

Open
georgewfraser opened this issue May 5, 2019 · 3 comments · May be fixed by #299
Open

Add-import on completion #96

georgewfraser opened this issue May 5, 2019 · 3 comments · May be fixed by #299

Comments

@georgewfraser
Copy link
Owner

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
@albfan
Copy link

albfan commented May 6, 2019

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
Copy link

albfan commented May 6, 2019

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

@georgewfraser
Copy link
Owner Author

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.

@rafaeldeoliveira rafaeldeoliveira mentioned this issue Sep 4, 2020
Closed
nya3jp added a commit to nya3jp/java-language-server that referenced this issue Dec 18, 2024
@nya3jp
Autoimport completed class names
767bf1e 
This patch introduces a new interface to represent the import order. We
also provide its two implementations: the simple one (all imports in a
single section with lexicographical order), and the one conforming to
the Chromium Java style guide (which I use for my project).

The interface is used in two places: the existing logic to add an import
as a quick fix, and the new logic to add an import on completing class
names.

The style can be configured by user settings.

Fixes georgewfraser#96
nya3jp added a commit to nya3jp/java-language-server that referenced this issue Dec 18, 2024
@nya3jp
Autoimport completed class names
200d23d 
This patch introduces a new interface to represent the import order. We
also provide its two implementations: the simple one (all imports in a
single section with lexicographical order), and the one conforming to
the Chromium Java style guide (which I use for my project).

The interface is used in two places: the existing logic to add an import
as a quick fix, and the new logic to add an import on completing class
names.

The style can be configured by user settings.

Fixes georgewfraser#96
@nya3jp nya3jp linked a pull request Dec 18, 2024 that will close this issue
Open
nya3jp added a commit to nya3jp/java-language-server that referenced this issue Dec 18, 2024
@nya3jp
Autoimport completed class names
60032ee 
This patch introduces a new interface to represent the import order. We
also provide its two implementations: the simple one (all imports in a
single section with lexicographical order), and the one conforming to
the Chromium Java style guide (which I use for my project).

The interface is used in two places: the existing logic to add an import
as a quick fix, and the new logic to add an import on completing class
names.

The style can be configured by user settings.

Fixes georgewfraser#96
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@albfan @georgewfraser