Feature request (analysis server): Import multiple symbols in show clause(s) at once

[Wdestroier]

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.

[dart-github-bot]

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]

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]

Related to #32234 and #55842.

[FMorschel]

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 show and reading it all with this might be faster than what you'd need to do today.

[FMorschel]

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

	List<ProducerGenerator> _getGenerators(ErrorCode errorCode) {
	if (errorCode is LintCode) {
	return registeredFixGenerators.lintProducers[errorCode] ?? [];
	} else {
	// TODO(pq): consider support for multi-generators.
	return registeredFixGenerators.nonLintProducers[errorCode] ?? [];
	}
	}
	}

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]

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

[FMorschel]

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.

import 'dart:math' show pi;

void foo(Random r) {
  print(e);
}

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 e and Random 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).

[bwilkerson]

Adding support for multi-generators?

Yes.

Ok. I don't understand the TODO comment.

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

[FMorschel]

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]

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]

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]

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!