Skip to content

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

Closed

Description

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

image

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.

Activity

  1. dart-github-bot commented on Aug 30, 2024

    @dart-github-bot
    Collaborator

    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.

  2. dart-github-bot commented on Aug 30, 2024

    @dart-github-bot
    Collaborator

    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.

  3. added
    devexp-serverIssues related to some aspect of the analysis server
    on Aug 30, 2024
  4. added
    P3A lower priority bug or feature request
    devexp-quick-fixIssues with analysis server (quick) fixes
    on Aug 30, 2024
  5. removed
    triage-automationSee https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot.
    on Aug 30, 2024
  6. FMorschel commented on Oct 30, 2024

    @FMorschel
    Contributor

    Related to #32234 and #55842.

  7. FMorschel commented on Oct 31, 2024

    @FMorschel
    Contributor

    I've closed #30503 and #54786 that might help here.

    Also, if you have a code like this:

    image

    There is already an assist to add all used declarations to a show:

    image

    Maybe removing everything from show and reading it all with this might be faster than what you'd need to do today.

  8. FMorschel commented on Dec 2, 2024

    @FMorschel
    Contributor

    Now both #32234 and #55842 were closed. I started taking a look at this but over at:

    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.

  9. bwilkerson commented on Dec 2, 2024

    @bwilkerson
    Member

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

  10. FMorschel commented on Dec 2, 2024

    @FMorschel
    Contributor

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

  11. bwilkerson commented on Dec 2, 2024

    @bwilkerson
    Member

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

  12. FMorschel commented on Dec 2, 2024

    @FMorschel
    Contributor

    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.

  13. FMorschel commented on Dec 5, 2024

    @FMorschel
    Contributor

    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.

  14. FMorschel commented on Dec 5, 2024

    @FMorschel
    Contributor

    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.

  15. FMorschel commented on Dec 5, 2024

    @FMorschel
    Contributor

    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!

  16. FMorschel commented on Feb 4, 2025

    @FMorschel
    Contributor

    Can I get one more review, @srawlins or @bwilkerson? Pq has already reviewed so only one more review is needed. Thanks a lot!

  17. FMorschel commented on Feb 11, 2025

    @FMorschel
    Contributor

    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.

    Image

  18. Wdestroier commented on Feb 11, 2025

    @Wdestroier
    Author

    Amazing 🔥🔥, thank you!

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    P3A lower priority bug or feature requestdevexp-quick-fixIssues with analysis server (quick) fixesdevexp-serverIssues related to some aspect of the analysis serverlegacy-area-analyzerUse area-devexp instead.type-enhancementA request for a change that isn't a bug

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions