dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.2k stars 1.57k forks source link

Auto import (or quickfix?) for Extensions #38894

Closed shinayser closed 2 years ago

shinayser commented 5 years ago

I am opening this issue as @scheglov asked for at https://github.com/dart-lang/sdk/issues/25820#issuecomment-541972606

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

ThinkDigitalSoftware commented 4 years ago

Looks like this is fixed now in the latest master

DanTup commented 4 years ago

@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 commented 4 years ago

@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').

scheglov commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

Any update on this?

bwilkerson commented 4 years ago

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

vanlooverenkoen commented 4 years ago

Any idea of a timeframe?

devoncarew commented 4 years ago

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

vanlooverenkoen commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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.

JulianBissekkou commented 3 years ago

Any updates on this issue? 🧐

i-show commented 3 years ago

Following

pedromassango commented 3 years ago

Hi @JulianBissekkou @i-show and anyone seeing this.

Using "add reaction" on the initial comment would increase priority while +1 comments are rather pointless and cause lots of people to get notified for no reason.

To get notified yourself, use the [Subscribe] button to the top right instead.

vanlooverenkoen commented 3 years ago

@pedromassango Indeed +1 should be used increase priority.

that said. this ticket is in the top 10 issues on github. But there is no activity on this ticket. Is there a reason why this ticket is already open for more than a year? and still in the top 10?

shinayser commented 3 years ago

@pedromassango Indeed +1 should be used increase priority.

that said. this ticket is in the top 10 issues on github. But there is no activity on this ticket. Is there a reason why this ticket is already open for more than a year? and still in the top 10?

My I ask how can we see the top 10 most up voted?

vanlooverenkoen commented 3 years ago

@pedromassango Indeed +1 should be used increase priority. that said. this ticket is in the top 10 issues on github. But there is no activity on this ticket. Is there a reason why this ticket is already open for more than a year? and still in the top 10?

My I ask how can we see the top 10 most up voted?

https://github.com/dart-lang/sdk/issues?q=is%3Aissue+is%3Aopen+sort%3Areactions-%2B1-desc

image

pedromassango commented 3 years ago

@pedromassango Indeed +1 should be used increase priority.

that said. this ticket is in the top 10 issues on github. But there is no activity on this ticket. Is there a reason why this ticket is already open for more than a year? and still in the top 10?

The Dart team is focused on other important features such as Null-Safety, they may get back into this once they are done with other important tasks. I really want this as well because I own a package that is useless without this but all we can do is to wait.

hpoul commented 3 years ago

@pedromassango

but all we can do is to wait.

or create a PR 😅️ - but a stupid question: Which code is actually affected by this? I think the actual intellij plugin is provided by jetbrains? But this would probably be mostly in the analyzer (or lsp server?) to provide auto completion?

bwilkerson commented 3 years ago

Is there a reason why this ticket is already open for more than a year?

Primarily because of resource constraints. We appreciate both the importance of and demand for this feature, and yes, I too hope that we can address it after Null Safety has shipped.

... I own a package that is useless without this ...

I believe that you mean that the lack of this feature significantly reduces the discoverability of the extensions in your package, but if there's a different meaning that I missed, please let us know.

Which code is actually affected by this?

The fixes for both requests would be in the analysis_server package.

esDotDev commented 3 years ago

Since extensions tend to be used project-wide, what we do to make them 'usable' is group one file that is something like this:

export 'package:provider/provider.dart';
export 'package:sized_context/sized_context.dart';
export 'package:time/time.dart';

Name it something like 'extensions' and it's easy to manually import. You can go a step further and make a little mixin to do this import too, making it even easier:

export 'package:provider/provider.dart';
export 'package:sized_context/sized_context.dart';
export 'package:time/time.dart';

mixin ExtensionsMixin {}

Having first class support for this would be great. Seems like you should be able to use the type of the thing, to lookup any members/fields on the class, as well as any extensions that exist in the project?

For example, using context. why would auto-complete not be able to find the Context extensions from Provider?

MatrixDev commented 3 years ago

one and a half years have passed... and... still no progress/comments. nice work, Dart team.

devoncarew commented 3 years ago

@MatrixDev, the tone of your comment is not appropriate. Please review the Dart and Flutter team's code of conducts:

In particular, discussions here need to be respectful and courteous.

vishalrao8 commented 3 years ago

+1 This issue needs to be resolved.

shinayser commented 3 years ago

The MatrixDev comment was very indelicate, but I understand his deasapointment.

I know Dart Team is now 100% focused on solving the biggest update since 2.0, which is NNDB. But I hope that after that the developers could give some love to the completion area, which is by far, the worst area of the language.

vishnuagbly commented 3 years ago

really looking forward to this update, will be most helpful

dagba commented 3 years ago

omg, still no any steps to resolve this problem...

TheWCKD commented 3 years ago

With all due respect to Dart devs and community, I really love Dart and have a ton of appreciation for everyone involved in this.

However, this issue is the kind of issue that drags down every other amazing aspect of Dart, and to be honest, this is not the right way of doing it.

People should have questions and issues on more delicate problems for more advanced packages and not for a literally basic and critic feature like this one reported here.

What is this "importing the file in which the extension is located" feature so different from "importing the material.dart file in which the MaterialApp widget is located"?

Why is this so difficult to implement? Instead of focusing on huge breaking changes like null safety, you should've focused in the first place in getting the entire language stable and functional. People wouldn't leave the language if they had to double check nullability, but they could leave really fast if simple features like this one aren't available.

This is not an ordinary issue, with 1-2 likes. It's really, really requested and it is a really huge drawback of Dart language and yet not a single developer from your team didn't had the time to work on it. It simply defeats the purpose of extensions at all to he honest.

If someone could really point me the file in the static analyzer where Dart searches for a library or file containing an extension, I'd be happy to take a look and try to implement a fix and maybe make a PR. It shouldn't be so hart since all the analyzer has to do is to search for the right extension extending the right class.

EDIT: And for all of those disliking comments just because they think it's a lack of respect to point out some real facts should take a moment and consider their approach, as throwing hate is not going to help anyone.

bwilkerson commented 3 years ago

I understand your frustration and appreciate the fact that you're still talking to us. I can't address the resource allocation question (because I don't have any information about it other to say that this support is something we hope to address soon), but I can take a stab at discussing the technical side.

Why is this so difficult to implement?

It isn't difficult in the sense of being technically challenging. We know what needs to be done, but it's a non-trivial amount of work to make the necessary changes.

It boils down to the fact that portions of our code completion support were designed before there were extension members and it was designed in a way that doesn't support the requirements we have when supporting extension members. We just need to update the underlying code so that it does support those requirement. Not an uncommon experience in software development.

For more details, read on.

When the analysis server computes completion suggestions it does so by looking in the current library, the type hierarchy of the target (if there is a target) and the libraries that are imported by the current library. That gets most of the suggestions, but doesn't help with declarations that have not yet been imported.

Awhile back (before extension methods) we added support for declarations that could be imported but are not yet imported. Our first attempt was to look at all of the libraries that could be imported in the same way that we look at the already imported libraries. Unfortunately, that made code completion too slow, which significantly degraded the user experience. There were multiple reasons for the poor performance, but I'll skip over those details for now.

Our solution was to cache information about those libraries up-front. We're limited in the kind of information we can cache because of memory constraints. So what we cache is the information that a given name is defined in a given library and what kind of thing it's declared to be (class, top-level function, etc.). The cache only includes the names of top-level and static declarations. Because we look at the type hierarchy when there's a target there was no need to store instance members; we find them through the type hierarchy.

Unfortunately, that structure doesn't provide enough information to allow us to know which extension methods can be applied to a given target. The same limitations on the kind of information we can cache make it impossible for us to cache the data we really need, so we're going to have to cache enough information to tell us when there might be extension members to suggest and we'll have to do some extra work when computing the actual suggestions to determine whether the suggestions are valid.

TheWCKD commented 3 years ago

I understand your frustration and appreciate the fact that you're still talking to us. I can't address the resource allocation question (because I don't have any information about it other to say that this support is something we hope to address soon), but I can take a stab at discussing the technical side.

Why is this so difficult to implement?

It isn't difficult in the sense of being technically challenging. We know what needs to be done, but it's a non-trivial amount of work to make the necessary changes.

It boils down to the fact that portions of our code completion support were designed before there were extension members and it was designed in a way that doesn't support the requirements we have when supporting extension members. We just need to update the underlying code so that it does support those requirement. Not an uncommon experience in software development.

For more details, read on.

When the analysis server computes completion suggestions it does so by looking in the current library, the type hierarchy of the target (if there is a target) and the libraries that are imported by the current library. That gets most of the suggestions, but doesn't help with declarations that have not yet been imported.

Awhile back (before extension methods) we added support for declarations that could be imported but are not yet imported. Our first attempt was to look at all of the libraries that could be imported in the same way that we look at the already imported libraries. Unfortunately, that made code completion too slow, which significantly degraded the user experience. There were multiple reasons for the poor performance, but I'll skip over those details for now.

Our solution was to cache information about those libraries up-front. We're limited in the kind of information we can cache because of memory constraints. So what we cache is the information that a given name is defined in a given library and what kind of thing it's declared to be (class, top-level function, etc.). The cache only includes the names of top-level and static declarations. Because we look at the type hierarchy when there's a target there was no need to store instance members; we find them through the type hierarchy.

Unfortunately, that structure doesn't provide enough information to allow us to know which extension methods can be applied to a given target. The same limitations on the kind of information we can cache make it impossible for us to cache the data we really need, so we're going to have to cache enough information to tell us when there might be extension members to suggest and we'll have to do some extra work when computing the actual suggestions to determine whether the suggestions are valid.

Thank you so much for your kind words and really, really detailed explanation. I understood why this is a limitation currently, I don't know why I believed that the analyzer works by scanning all libraries, the performance issue would be indeed a huge drawback.

Anyways, I hope you'll make progress on this issue soon, of course, when you'll have more time for it. Cheers!

webstoreportal commented 3 years ago

+1 for explicit show/hide combinator support

context: coming from

import 'package:flutter_bloc/flutter_bloc.dart' 
    show ReadContext, SelectContext;

BuildContext context = ...;
context.select()
context.read()

/// provider-5.0.0 provider.dart
extension ReadContext on BuildContext {
...
}
/// provider-5.0.0 inherited_provider.dart
extension SelectContext on BuildContext {
...
}

tangential: exacerbates issues when using dart library part / part of importing explicit show/hide combinators for extensions https://pub.dev/documentation/analyzer/latest/dart_ast_ast/Combinator-class.html

atreeon commented 3 years ago

Possible Intellij workaround: for extension String_Extension on String

type String_Ext and when autocomplete offers the full extension name press enter; that will include the extension reference at the top of your code.

(this is a workaround and not a proper solution but I think this is the best work around I've found and I hope it helps you a little bit)

https://user-images.githubusercontent.com/4139336/120333071-3eb1b680-c2e7-11eb-8790-a9daed5606cd.mov

kuhnroyal commented 3 years ago

type String_Ext and when autocomplete offers the full extension name press enter; that will include the extension reference at the top of your code.

Yea sure, when you know that String_Extension exists and that it contains the function you are looking for. But the main idea for an IDE autocomplete is more like: "Hey IntelliJ, I have a property here, show me what I can do with this!".

firatcetiner commented 3 years ago

type String_Ext and when autocomplete offers the full extension name press enter; that will include the extension reference at the top of your code.

Yea sure, when you know that String_Extension exists and that it contains the function you are looking for. But the main idea for an IDE autocomplete is more like: "Hey IntelliJ, I have a property here, show me what I can do with this!".

Well, it's a workaround, not a fix. It's also better than manually importing the file.

jodinathan commented 3 years ago

@kuhnroyal if your extension names follow a good naming conventions then it should be fairly easy to identify the name of the extension.

what if I added some extension packages to my project and would like that the autocomplete show me the methods that the packages add to my variables and functions?

atreeon commented 3 years ago

@jodinathan as I say, it is just a work around to import your extension more easily. It is not a solution, just a way to make the import of the extensions easier (although maybe I should stay quiet in future!)

bwilkerson commented 3 years ago

I should have been a bit more diligent about providing updates, but there are a couple of improvements that I can share. I don't know when these changes will make it into a release, but they are on master and will roll out as soon as possible.

The first was in https://dart-review.googlesource.com/c/sdk/+/201021. This CL causes server to stop using the available suggestion support for already imported libraries. The impact of this is that server should now suggest extension methods if they are defined in libraries that are already being imported. It obviously won't help with discovering extension methods that are not yet imported, but it's at least one part of the issue.

The second was in https://dart-review.googlesource.com/c/sdk/+/201800 in which we added support for a quick fix to add an import when there's a reference to an extension method in an extension that isn't yet imported. If you type (or paste) code that references Iterable.lastWhereOrNull and it isn't found, we can fix the reference by adding an import of package:collection/collection.dart. Again, it doesn't help with discovery, but hopefully it will occasionally be useful.

kasperpeulen commented 3 years ago

@bwilkerson

If you type (or paste) code that references Iterable.lastWhereOrNull and it isn't found, we can fix the reference by adding an import of package:collection/collection.dart.

If which version of Dart should this work?

I tried typing, 2.seconds, but it didn't suggest an import, while I have the time package in my pub installed.

image

bwilkerson commented 3 years ago

If which version of Dart should this work?

I don't know. I used to know a way to figure which release a CL was first shipped in, but it doesn't appear to work anymore. @devoncarew How would I (or anyone) figure this out?

I tried typing, 2.seconds, but it didn't suggest an import, while I have the time package in my pub installed.

Is the library that defines seconds imported into any library? It seems plausible that the analyzer might not have seen (and hence indexed) it if not, but I'm just making a guess at this point as I haven't had time yet to try to reproduce the issue.

DanTup commented 3 years ago

I used to know a way to figure which release a CL was first shipped in, but it doesn't appear to work anymore. @devoncarew How would I (or anyone) figure this out?

The way I usually check this is to grab the git hashes shown in Gerrit ("Change has been successfully rebased and submitted as xyz") and then find them on GitHub:

3cc518b3cce843be20a76111151a63c8d224b05e 25b71e07e7c724933e6bbcec7ca0a5050ed57da3

That shows the tags the change is in (I think it shows the earliest and latest tags by default, though you can expand to see them all):

Screenshot 2021-09-08 at 09 29 22

If there's a vX.Y.Z-some-release-version listed (and the change wasn't reverted), I think the change usually ships in the following version. Both the CLs above are in v2.14.0-x.dev tags, so I think it's likely v2.14.0 of the Dart SDK would include them.

Is the library that defines seconds imported into any library? It seems plausible that the analyzer might not have seen (and hence indexed) it if not, but I'm just making a guess at this point as I haven't had time yet to try to reproduce the issue.

I just tried this, and this does seem to be the case. If I have the package in pubspec.yaml, have run pub get and restart the anlaysis server, the fix still isn't present. If I import anything from that package into another file, then the fix to import it shows up.

bwilkerson commented 3 years ago

The way I usually check this is to grab the git hashes shown in Gerrit ... and then find them on GitHub ...

Yep, that's what I used to do too. But the lists now only contain dev versions. I just didn't think about making the assumption that that would imply release versions. That probably holds true often enough to make it valid.

It seems plausible that the analyzer might not have seen (and hence indexed) it if not ...

I just tried this, and this does seem to be the case.

@scheglov Do we have the information needed to find declarations in libraries in packages that are not yet imported anywhere, or would that require additional indexing? If the latter, I assume there would be a performance hit by doing so, but I don't know what the magnitude of the hit would be.

scheglov commented 3 years ago

We can "discover" all available files from all packages, and then resolve / index them as necessary. See test_subtypes_discover, which does not import 'package:bbb', but its subtype results become available because there is a dependency on this package. Note that we resolve in Search.subtypes() after doing pre-filtering by name, using getFilesSubtypingName (which is based on parsed / unresolved AST).

Tasen-pro commented 3 years ago

Two years have passed

Bringoff commented 3 years ago

Intellij supports "extended" auto-completion on second ctrl+space press for some languages. Dart plugin doesn't seem to leverage this. If scanning extensions is too extensive (pun intended), why not to hide it behind this "extended" autocomplete?

qbait commented 3 years ago

That's the biggest productivity killer in Dart / Flutter. (BTW I love Flutter ❤️🤫)

Andreigr0 commented 3 years ago

Hm, strange, a couple days ago I've noticed that IntelliJ suggests to import extensions, but now it doesn't work🤔