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

Accept types if they are imported when completion #2467

Merged
merged 2 commits into from
Feb 15, 2023

Conversation

jdneo
Copy link
Contributor

@jdneo jdneo commented Feb 14, 2023

If types already exist in import declarations, then the completion handler will accept them even if they've been configured in the types filter.

This change should improve the experience for users who develop AWT applications, since the whole AWT package is set in the types filter by default now.

For example, if the compilation unit contains any of the below import declarations, the java.awt.List will appear in the completion list:

  • import java.awt.*;
  • import java.awt.List;
  • import static java.awt.List.*;
  • import static java.awt.List.ERROR; (or any other static members from java.awt.List)

fix redhat-developer/vscode-java#1147
fix redhat-developer/vscode-java#1966

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

- if types already exist in import declarations, then the completion
  handler will accept them even if they've been configured in the types
  filter.

Signed-off-by: Sheng Chen <sheche@microsoft.com>
Copy link
Contributor

@rgrunber rgrunber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Just one small thing that could probably be improved.

Signed-off-by: sheche <sheche@microsoft.com>
@rgrunber rgrunber mentioned this pull request Feb 15, 2023
@testforstephen
Copy link
Contributor

Current implementation is good for single file. I'm thinking of a different approach to improve it in future. Since the culprit is the typefilter blacklist, we can use existing import declarations of current file to remove the unexpected filter from the blacklist dynamically. This blacklist is a singleton in language server, so it ensures the refreshed blacklist also applies to other files in current project.

I also found unexpected type filters in some other use cases:

  • Manually typing import java.awt.|, no result. This is not a good behavior since I'm explicitly asking for a type in package java.awt.
  • Hover for quick fix on ActionListener listener;, it suggests to create a class instead of import java.awt.event.ActionListener. Applying typefilter is not a right choice for quickfix.

I'm OK to merge current PR first, and find time to improve other use cases in future PR.

@jdneo jdneo merged commit afc956e into eclipse-jdtls:master Feb 15, 2023
@jdneo jdneo deleted the cs/completion-for-import branch February 15, 2023 08:23
@jdneo
Copy link
Contributor Author

jdneo commented Feb 15, 2023

@testforstephen Thank you for the input. The suggestions make sense to me. I can take some further look in next iteration.

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