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 EP to customize "Add Import" weights #2853

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add EP to customize "Add Import" weights #2853

wants to merge 2 commits into from
+341 −21

Conversation

andymagee
Copy link
Contributor

@abelkov abelkov added the kotlin Pull requests for Kotlin IntelliJ plugin label Oct 7, 2024
@andymagee
Copy link
Contributor Author

Friendly ping on this - any updates?

@fedochet
Copy link
Member

Hi @andymagee !

We actually already have a simlar EP for callables - KotlinAutoImportCallableWeigher

Could you please see if it is possible to extend it or introduce a new EP which would look similar to it?

@andymagee
Allow KotlinAutoImportCallableWeigher to weigh type names
c3b4bf4 
Fix for https://youtrack.jetbrains.com/issue/KTIJ-31471

KotlinAutoImportCallableWeigher allows customization of the order within
the auto import list. This change updates it to more accurately handle
imports for type names.
@andymagee
Copy link
Contributor Author

andymagee commented Oct 24, 2024 •
edited
Loading

Hi @andymagee !

We actually already have a simlar EP for callables - KotlinAutoImportCallableWeigher

Could you please see if it is possible to extend it or introduce a new EP which would look similar to it?

Thank you, I had missed that existing EP!

The existing weigher uses CallExpressionWeigher/CallExpressionImportWeigher for all name references, even though the name references are not always call expressions (eg, in the case of type name references like val foo: SomeUnimportedType). I've updated the logic to more accurately handle type names that aren't callables, and added a number of test cases to validate those.

I'm also removing the @Internal markings for the EP, since the goal would be to use this in the Android Studio codebase.

Let me know what you think, thanks.

@andymagee
Fix import order for TypeAlias elements
dab3dbf 
There's an existing test for TypeAlias import ordering, but it's only
passing by accident. Adding a single parameter to the annotation that's
being aliased causes the order to get swapped, which doesn't match the
expected behavior. (I'm assuming the expected behavior is that the
typealias should always be lower on the list; this is based on the
existing test case.)

The existing test case worked because the annotation had a default
constructor with 0 arguments, and the referencing location that needs an
import also has 0 arguments. But any time that argument count doesn't
match, then the order is no longer determined by this weigher.

This fix simply demotes any TypeAlias elements slightly.
@andymagee
Copy link
Contributor Author

Hi there, any comments on the updated changes?

@fedochet
Copy link
Member

Sorry for the delay, I will take a look at it this week

@andymagee
Copy link
Contributor Author

Hi @fedochet , have you had a chance to take a look at this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@fedochet fedochet

Labels
kotlin Pull requests for Kotlin IntelliJ plugin
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@andymagee @fedochet @abelkov