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

Auto import (or quickfix?) for Extensions #38894

Closed
shinayser opened this issue Oct 15, 2019 · 81 comments
Closed

Auto import (or quickfix?) for Extensions #38894

shinayser opened this issue Oct 15, 2019 · 81 comments
Assignees
Labels
analyzer-completion Issues with the analysis server's code completion feature analyzer-quick-fix analyzer-server area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@shinayser
Copy link

shinayser commented Oct 15, 2019

I am opening this issue as @scheglov asked for at #25820 (comment)

I am having problem with auto completing the new Extension Functions from dart 2.6.0. I have added a new file called "time.dart" with some extensions, like this:

image

My main file just can't auto complete it. If I try to quickfix the error I get the following:

image

As soon as I move the extensions to the same file as main, everything works:

image

If I add the import manually, it also works:

image

My Dart for Intellij plugin version is 192.6817.14

@shinayser shinayser changed the title Auto import for Extensions Auto import (or quickfix?) for Extensions Oct 15, 2019
@scheglov scheglov added analyzer-server area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on labels Oct 15, 2019
@ThinkDigitalSoftware
Copy link

Looks like this is fixed now in the latest master

@DanTup
Copy link
Collaborator

DanTup commented May 6, 2020

@bwilkerson @ThinkDigitalSoftware as far as I can see, this isn't working (in the most recently nightly build):

Shows up if imported

Screenshot 2020-05-06 at 12 12 40

No completion if not imported

Screenshot 2020-05-06 at 12 12 20

No quickfix

Screenshot 2020-05-06 at 12 12 30

@bwilkerson
Copy link
Member

@jwren Using a build from yesterday morning (and IntelliJ), I'm seeing the completion when the library has been imported, but I am not seeing a completion when it has not been imported, nor any kind of quick fix (I'm not even getting 'Extract Method').

@bwilkerson bwilkerson reopened this May 6, 2020
@scheglov
Copy link
Contributor

scheglov commented May 6, 2020

I don't think this can work when the extension is not imported.

Available declarations don't have enough type information to support type matching in general case.

One thought that I had recently is that I like how completion in Cider works, with limiting the number of suggestions transferred, an ability to transfer more if necessary, server-side initial filtering, and re-request if the filter changes. This keep amount of data transferred limited, and still allows full power of types in the server.

@bwilkerson
Copy link
Member

I don't think this can work when the extension is not imported.

What if we cached (either in available declarations or elsewhere) a map from the names of extended types (without type arguments) to the (paths of) libraries containing extensions of those types. I'm guessing that for any given type name the list of library paths would be fairly small and that we could then access the elements for those libraries, checking that the actual types match the target type, and add suggestions for the members of the extension.

One thought that I had recently is that I like how completion in Cider works ...

That's valid, but I don't think it's necessary in order to make completion work in this case. Nor can we force our clients to adopt the Cider model.

If necessary I suppose we could implement this functionality only for Cider-like clients, but I'd rather try to find a more general solution first.

@scheglov
Copy link
Contributor

scheglov commented May 6, 2020

Ah, I'm sorry, I was thinking about free standing identifiers when stated that it will not work because of available declarations. Here we do have the target.

Yes, I think this could be done with caching of potentially applicable extensions.

@DanTup
Copy link
Collaborator

DanTup commented May 7, 2020

One thought that I had recently is that I like how completion in Cider works, with limiting the number of suggestions transferred, an ability to transfer more if necessary, server-side initial filtering, and re-request if the filter changes

Slightly off-topic, but this is something I often think about for VS Code/LSP (in the context of performance, esp. for LSP where we can't preload all the available suggestions onto the client). There's support for going back to the server as the user types in both LSP and VS Code's APIs (though not adding more to the list asynchronously if the user didn't type) but I worry a little about:

  1. Increased latency because the filtering now has round trips (including JSON serialisation/deserialisation of potentially large numbers of items)
  2. Increase resource usage because of transferring a lot of the same items repeatedly

It might be something worth testing out though (though VS Code currently does fuzzy matching and accounting for transposed letters, so to keep the same behaviour that would need to be done on the server too).

@vanlooverenkoen
Copy link

Any update on this?

@bwilkerson
Copy link
Member

Not yet, but it's definitely something we would like to have working.

@vanlooverenkoen
Copy link

Any idea of a timeframe?

@devoncarew
Copy link
Member

@vanlooverenkoen, thanks for the ping; watching this issue is the best way to know about any progress.

@vanlooverenkoen
Copy link

Yes indeed. Because we are using a lot of extensions in our projects. But it also takes a lot of time to manually import them every time you need them. :( it would be nice to have a timeframe.

@bnvmxm
Copy link

bnvmxm commented Aug 3, 2020

As a dirty workaround, you can declare a fake type in the extensions' library. Like

const ext = 0;

Then in your target file, start typing "ext" - to auto-import the library needed.
All its extensions are visible then in suggestions.

At least, this saves a lot of time now.

@m-skolnick
Copy link

m-skolnick commented Aug 3, 2020

As a dirty workaround, you can declare a fake type in the extensions' library. Like

const ext = 0;

Then in your target file, start typing "ext" - to auto-import the library needed.
All its extensions are visible then in suggestions.

At least, this saves a lot of time now.

This same effect can be achieved by using a named extension.

extension StringExtensions on String {
    // Your extensions
}

Typing the name of the extension will show the import suggestion.

This is still not a solid solution. I'm personally sticking with Utils classes until Extensions are usable without manually adding the imports.

@gbaldeck
Copy link

gbaldeck commented Sep 18, 2020

Guys, it's been almost a year now and this is still not working in IDE's. Autocomplete is what makes extension functions useful. Without that you might as well use a Util class since you will at least get some autocompletion.

Not having tooling support around a major feature like this cripples the feature. Might as well have not added it to the language.

@shinayser
Copy link
Author

Hey @bwilkerson this sounds a very important feature to add to the language, but considering the time I've opened the issue and it isn't fixed, I can assume that definitely is not an easy task.

Perhaps you can explain to us the difficulty of that issue? Maybe we can try to find a workaround and open an PR :)

@bwilkerson
Copy link
Member

It's definitely an important feature. The delay is primarily a result of (a) the current architecture of the code completion support and (b) limited resources.

When we first decided to start suggesting completions from libraries that weren't imported (about three years ago), we first tried the simple tactic of just adding the suggestions to the completion results. Unfortunately, this had some performance issues due to (a) having to process all of the elements from these un-imported libraries in order to create suggestions, and (b) the large amount of data that was being sent from the the server to the client (the IDE). These issues led to an unusable code completion experience.

So we designed a new protocol. The new protocol, which we're still using today, allowed the server to send all of the suggestions from un-imported libraries during the initial analysis and to keep it updated if the user changed which packages were available to a given package. That protocol was designed before extensions were introduced into the language and isn't flexible enough to support extension members.

We have considered changes to the protocol, and that's still an option that we're discussing, but we're currently looking at other approaches.

We changed our use of the protocol slightly, which is what allows us to suggest extension members if the package defining the extension is already imported. Initially we decided to use the same ahead-of-time computation of suggestions for imported libraries as for un-imported libraries. Now we're back to computing and sending suggestions from imported libraries at the time that completion suggestions are requested.

Recently we've seen some performance problems in code completion, and after some investigation we're now looking into a more radical change to the protocol. We're basically investigating using a protocol similar to the one LSP defines. The new protocol would allow the server to send back a small portion of the total list, allowing the client to control how much of the list should be returned.

If that work is successful, not only should it improve performance, but it should also allow us to compute suggestions from un-imported libraries when completions are requested, which should then allow us to include suggestions for extension members.

Thanks for the offer to help. I'm not the primary developer working on the new protocol, so I'm not sure how you could best contribute. I think that most of the work in the analysis server is completed and now we're working on finishing the work on the client side so that we can start testing it end-to-end.

@shinayser
Copy link
Author

shinayser commented Feb 5, 2022

Those are amazing news @bwilkerson. We're very excited to see the result of all your efforts!

I am a completion maniac myself, so if you folks need some guy like me to test that stuff, just call me :)

@srawlins
Copy link
Member

@scheglov can you confirm you implemented this?

@scheglov
Copy link
Contributor

Works for me.

  1. Completion.

image

  1. Quick fix.

image

@JulianBissekkou
Copy link

Thanks to everybody that was involved in this feature. It works really smooth 👍🏽

@naamapps
Copy link

naamapps commented Jul 22, 2022

Hi,
How can I get to use this feature?
I'm using vscode with latest flutter and Dart plug-in versions but this doesn't seem to work for me.
Do I need to set a setting or update something?
Thanks

@scheglov
Copy link
Contributor

As you can see, I use IntelliJ, but at least quick fix should work in VS Code as well.
Does it work for you?

@naamapps
Copy link

Sorry, I must have missed that.
Yes the quick fix is working, but autocomplete doesn't work.
Can we expect autocomplete to arrive to vscode soon?

@scheglov
Copy link
Contributor

@DanTup for VS Code completion insides :-)

@shinayser
Copy link
Author

@scheglov do we need to update dart to some specific version to get this working?

@scheglov
Copy link
Contributor

The changes, at least for quick fixes, were done in 979f3ea.
And for code completions about the same time.
So, I expect that these are available in 2.17 Dart.

@shinayser
Copy link
Author

Ok I tested and it worked on some scenarios.

I believe I know why I haven't realized that change before and that is because our dependencies are transient.

I have a module that imports a dependencies module, and because of that the extensions doesn't work. Was it an intended behavior?

@DanTup
Copy link
Collaborator

DanTup commented Jul 25, 2022

@shinayser my understanding is that this is intended. Any packages you directly depend on should be listed in your pubspec.yaml (so you can set constraints and ensure they're always available even if other dependencies stop depending on them). As a result, completion/fixes/etc. will only suggest from those packages (if it didn't, code completion would contain a large number of classes from transient dependencies that not be wanted).

There's a related lint at https://dart-lang.github.io/linter/lints/depend_on_referenced_packages.html.

@naamapps
Copy link

@DanTup Any news about this feature arriving to vscode? :)

@DanTup
Copy link
Collaborator

DanTup commented Jul 25, 2022

@naamapps the quick-fix should already work today. Showing up in the completion list and auto-importing requires using a new completion API so isn't available in stable yet, although it is working behind a flag in the latest bleeding-edge/master SDK builds:

Screenshot 2022-07-25 at 11 30 30

I'm not ready to make it default/remove the flag yet (I've still been fixing some issues fairly recently), but it's getting there.

@naamapps
Copy link

Thanks for the quick reply @DanTup.
Looking forward to use this feature 👍

@Adrian-Samoticha
Copy link

@naamapps the quick-fix should already work today. Showing up in the completion list and auto-importing requires using a new completion API so isn't available in stable yet, although it is working behind a flag in the latest bleeding-edge/master SDK builds:


I'm not ready to make it default/remove the flag yet (I've still been fixing some issues fairly recently), but it's getting there.

This is great to hear. I'm looking forward to using this feature as well.

@shinayser
Copy link
Author

@shinayser my understanding is that this is intended. Any packages you directly depend on should be listed in your pubspec.yaml (so you can set constraints and ensure they're always available even if other dependencies stop depending on them). As a result, completion/fixes/etc. will only suggest from those packages (if it didn't, code completion would contain a large number of classes from transient dependencies that not be wanted).

There's a related lint at https://dart-lang.github.io/linter/lints/depend_on_referenced_packages.html.

Thanks for your reply!
I understand your point but it is a common pattern on some monorepo architectures to concentrate all the dependencies on a common "dependencies" module. So we would never get code completion on larger products because of this limitation.

You think I should open another issue to report that? Maybe we could think on a different solution. Maybe adding s tag to the pubspec to include those dependencies to the auto complete algorithm?

@DanTup
Copy link
Collaborator

DanTup commented Jul 25, 2022

@shinayser there's some similar discussion at #43678 (comment). The suggestion was allowing the "common dependencies module" to record which of its dependencies it exposes to packages that depend on it. The response was that this should be filed against Pub, although I can't see any obvious issue that was raised about this, so perhaps it's worth filing one there to start a discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-completion Issues with the analysis server's code completion feature analyzer-quick-fix analyzer-server area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Development

No branches or pull requests