You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Feature request (analysis server): Import multiple symbols in show clause(s) at once #56608
Manually adding imports to show clauses is a very time-consuming process and disturbs focus. The show clauses are very useful for code reviews, reading code on GitHub and knowing which dependencies the library has. In Java, both IntelliJ and eclipse can fix all imports and this problem doesn't happen. For example: we can press CTRL + SHIFT + O to import required symbols, remove unused imports and order imports.
In Dart we need to import symbol by symbol when a show clause is present. In the below screenshot, JsonSerializable, FieldRename and JsonConverter are 3 types from the same library.
I would like an option to import all symbols in the show clause at once, instead of having to move the cursor to each unknown symbol to add each item in the show clause. IntelliJ and eclipse have a shortcut to add missing imports, remove unused imports and sort imports. When multiple symbols are found in multiple packages, the user is prompted to select the proper import if any. If no symbol is missing, then no import is added. If no import is found, the IDE can show a little hint above the cursor and no import is added. For this to work, support from the analysis server is required.
I considered not using show clauses in imports, because of this issue and Dart-Code/Dart-Code#5238 which is being addressed. I considered suggesting another feature to show import hints Dart-Code/Dart-Code#5240 instead, but these hints would not be visible on GitHub.
Summary: The user requests the ability to import multiple symbols in show clauses at once, similar to IntelliJ and Eclipse's import functionality. This would streamline the process of adding imports and improve code readability by reducing the need to manually add each symbol individually.
Summary: The user requests the ability to import multiple symbols in show clauses at once, similar to IntelliJ and Eclipse's import functionality. This would streamline the process of adding imports and improve code readability by automatically importing all necessary symbols from a library.
I find this issue to be confusing. There really should have been some discussion about what user journey(s) we wanted to support and the best way of doing that. I can't even tell, by reading this, what the current state is or whether we're any closer to meeting the user's need.
I got stuck trying to make this work.
Making what work? Adding support for multi-generators? And if that's the problem, why are you trying to add them? (There's nothing in this issue that makes me think that we need them.)
Making what work? Adding support for multi-generators? And if that's the problem, why are you trying to add them? (There's nothing in this issue that makes me think that we need them.)
Yes. The current "Update library import" implementation is added on ImportLibrary (analysis_server/lib/src/services/correction/dart/import_library.dart) which is a MultiCorrectionProducer (same for "Use imported library with prefix"). It considers every possible way that the declaration can be added to this library with an import. So to avoid duplicating logic I was trying to make this work.
My take on what this issue is asking: If there are multiple top-level declarations that come from the same import, add an assist option to add them all.
So for this example, on Random and e there would be an option to "Update library import with all" or something similar so then both eandRandom would be added.
If there are multiple declarations with the same name from different imports on scope, we'd assume the user knows what it is doing and show the one that triggered the assist and not try to guess anything here. If something was added erroneously then I'd expect the user to fix it (probably activated the fix in the wrong order).
The class MultiCorrectionProducer is used in cases where there are (at least potentially) multiple ways to fix the same diagnostic, all of which are equally valid. For example, ImportLibrary is a subclass because there might be multiple libraries that could be imported in order to resolve an unresolved reference. The user is required to choose the desired import because there's no way for the server to know which import it should choose.
It doesn't really make sense to allow that class to be used to fix all similar diagnostics in the same file.
So, I don't think we want to support using a MultiCorrectionProducer in the FixInFileProcessor.
My take on what this issue is asking: If there are multiple top-level declarations that come from the same import, add an assist option to add them all.
The good news is, that doesn't sound like a fix-all-in-file style fix. The existing fix is worded poorly for this purpose. I'd rewrite "Update library '$0' import" to "Import '$1' from '$0'", and then I'd add a second fix titled something like "Import '$1' and $2 others from '$0'". In the context of your example, those would be "Import 'Random' from 'dart:math'" and "Import 'Random' and 1 other from 'dart:math".
The new "and $2 other" variants could probably be generated by ImportLibrary (though I haven't looked at the implementation recently, so I don't know what would be involved in order to make that happen).
There is still some work to be done, but the outline is there. There is still an issue with detecting other extensions and I'd like to think of a way to make the name detection not repeat all the current patterns since that would make future fixes to anything else on this file probably not fix this as well.
I've managed to refactor things to make this change also work with extensions. Now the only basic work left is to create tests for all cases here but I think this might be as easy as copying the existing ones and changing a few things.
Last update (I hope). Everything is passing now and all the tests are in place. Just waiting for reviews on this. I've set you @bwilkerson since you were talking about this here and @srawlins. Thanks a lot!
Activity
dart-github-bot commentedon Aug 30, 2024
Summary: The user requests the ability to import multiple symbols in show clauses at once, similar to IntelliJ and Eclipse's import functionality. This would streamline the process of adding imports and improve code readability by reducing the need to manually add each symbol individually.
dart-github-bot commentedon Aug 30, 2024
Summary: The user requests the ability to import multiple symbols in show clauses at once, similar to IntelliJ and Eclipse's import functionality. This would streamline the process of adding imports and improve code readability by automatically importing all necessary symbols from a library.
FMorschel commentedon Oct 30, 2024
Related to #32234 and #55842.
FMorschel commentedon Oct 31, 2024
I've closed #30503 and #54786 that might help here.
Also, if you have a code like this:
There is already an assist to add all used declarations to a
show:Maybe removing everything from
showand reading it all with this might be faster than what you'd need to do today.FMorschel commentedon Dec 2, 2024
Now both #32234 and #55842 were closed. I started taking a look at this but over at:
sdk/pkg/analysis_server_plugin/lib/src/correction/fix_in_file_processor.dart
Lines 127 to 135 in 06caa3f
I got stuck trying to make this work. @pq, is there an issue specifically for this change mentioned on the comment?
Going back I found that this was added on this CL https://dart-review.googlesource.com/c/sdk/+/183700/6/pkg/analysis_server/lib/src/services/correction/fix_internal.dart#379 over three years ago.
bwilkerson commentedon Dec 2, 2024
I find this issue to be confusing. There really should have been some discussion about what user journey(s) we wanted to support and the best way of doing that. I can't even tell, by reading this, what the current state is or whether we're any closer to meeting the user's need.
Making what work? Adding support for multi-generators? And if that's the problem, why are you trying to add them? (There's nothing in this issue that makes me think that we need them.)
FMorschel commentedon Dec 2, 2024
Yes. The current "Update library import" implementation is added on
ImportLibrary(analysis_server/lib/src/services/correction/dart/import_library.dart) which is aMultiCorrectionProducer(same for "Use imported library with prefix"). It considers every possible way that the declaration can be added to this library with an import. So to avoid duplicating logic I was trying to make this work.My take on what this issue is asking: If there are multiple top-level declarations that come from the same import, add an assist option to add them all.
So for this example, on
Randomandethere would be an option to "Update library import with all" or something similar so then botheandRandomwould be added.If there are multiple declarations with the same name from different imports on scope, we'd assume the user knows what it is doing and
showthe one that triggered the assist and not try to guess anything here. If something was added erroneously then I'd expect the user to fix it (probably activated the fix in the wrong order).bwilkerson commentedon Dec 2, 2024
Ok. I don't understand the TODO comment.
The class
MultiCorrectionProduceris used in cases where there are (at least potentially) multiple ways to fix the same diagnostic, all of which are equally valid. For example,ImportLibraryis a subclass because there might be multiple libraries that could be imported in order to resolve an unresolved reference. The user is required to choose the desired import because there's no way for the server to know which import it should choose.It doesn't really make sense to allow that class to be used to fix all similar diagnostics in the same file.
So, I don't think we want to support using a
MultiCorrectionProducerin theFixInFileProcessor.The good news is, that doesn't sound like a fix-all-in-file style fix. The existing fix is worded poorly for this purpose. I'd rewrite "Update library '$0' import" to "Import '$1' from '$0'", and then I'd add a second fix titled something like "Import '$1' and $2 others from '$0'". In the context of your example, those would be "Import 'Random' from 'dart:math'" and "Import 'Random' and 1 other from 'dart:math".
The new "and $2 other" variants could probably be generated by
ImportLibrary(though I haven't looked at the implementation recently, so I don't know what would be involved in order to make that happen).FMorschel commentedon Dec 2, 2024
I see what you mean. Agreed. I'll take a look again with this in mind and see if I can come up with something. Once I do I'll post the CL link here.
FMorschel commentedon Dec 5, 2024
Here is the CL for this https://dart-review.googlesource.com/c/sdk/+/399021.
Edit
There is still some work to be done, but the outline is there. There is still an issue with detecting other extensions and I'd like to think of a way to make the name detection not repeat all the current patterns since that would make future fixes to anything else on this file probably not fix this as well.
FMorschel commentedon Dec 5, 2024
I've managed to refactor things to make this change also work with extensions. Now the only basic work left is to create tests for all cases here but I think this might be as easy as copying the existing ones and changing a few things.
FMorschel commentedon Dec 5, 2024
Last update (I hope). Everything is passing now and all the tests are in place. Just waiting for reviews on this. I've set you @bwilkerson since you were talking about this here and @srawlins. Thanks a lot!
[DAS] Adds new fix for importing multiple names in combinantors
FMorschel commentedon Feb 4, 2025
Can I get one more review, @srawlins or @bwilkerson? Pq has already reviewed so only one more review is needed. Thanks a lot!
[DAS] Adds new fix for importing multiple names in combinantors
FMorschel commentedon Feb 11, 2025
This should be working on the latest master, @Wdestroier! It will be out on Dart 3.8.0 but you can download it here https://gsdview.appspot.com/dart-archive/channels/main/raw/latest/sdk/ to try it out.
Wdestroier commentedon Feb 11, 2025
Amazing 🔥🔥, thank you!