Skip to content

Hovers should show what imports are adding that declaration #56935

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

Open
FMorschel opened this issue Oct 21, 2024 · 5 comments
Open

Hovers should show what imports are adding that declaration #56935

FMorschel opened this issue Oct 21, 2024 · 5 comments
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-hover Issues related to hovers devexp-ux P4 type-enhancement A request for a change that isn't a bug

Comments

@FMorschel
Copy link
Contributor

FMorschel commented Oct 21, 2024

Inspired by #32735.

When you have hovers today they show the origin of a given type at the end like the following:

import 'b.dart' as lib;

void main() {
  lib.MyClass();
}

image

But the URI placed there is not consistent with the current imported URI from the import directive.


Edit

I'm asking for the hover to also show exactly which import directives (may be more than one) are exporting that declaration. This would help identifying where are you getting it from wich may be helpful when you're unfamiliar with the current project/package.


CC: @DanTup

@dart-github-bot
Copy link
Collaborator

Summary: The hover tooltip for a type displays a URI that doesn't match the URI in the corresponding import directive. This inconsistency should be addressed to ensure accurate information is presented to the user.

@dart-github-bot dart-github-bot added legacy-area-analyzer Use area-devexp instead. triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. type-enhancement A request for a change that isn't a bug labels Oct 21, 2024
@lrhn
Copy link
Member

lrhn commented Oct 21, 2024

Do you mean the URL should be displayed as b.dart and not package:bug/b.dart?

The latter is the URL of the library. The former is a relative URI reference that is resolved relative to the importing library's URL. Presumably the importing library's URL is package:bug/me.dart.

The URI of the declaring library is the information being shown, not how it was imported. The latter is not always useful.

That is, the question being answered is not "how was this declaration imported?", but "where is this declaration declared?".
If an imported library contains an export, the URI of the import URI, and the imported library's URI, is not where the declaration really comes from.
Same if a declaration is available though more than one import (which is valid).

@FMorschel
Copy link
Contributor Author

I forgot about exports, you're right.

I'll change the description and title to ask for the hovers to display what imports are adding this declaration then. That would also help identifying where are you really getting a class/function/etc from, inside the current library and not where is the declaration for it, which may not be helpful in some cases as well (mainly when your unfamiliar with the current project/packages).

@FMorschel FMorschel changed the title Hovers show type origin uri differently from the file import Hovers should show what imports are adding that declaration Oct 21, 2024
@lrhn lrhn removed the triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. label Oct 22, 2024
@DanTup
Copy link
Collaborator

DanTup commented Oct 22, 2024

I'm not sure I agree with changing the standard hovers to show which import is contributing a declaration comes from. The hover is showing you information about this element and that includes "the library that defines this element". If you import flutter/widgets or flutter/material, the library that contains Widget is the same and I think we should continue to show that.

I think the new "Go to Import"s command you've started on is probably the better way to show the specific imports that are contributing a declaration rather than include it in every hover. I suspect in a Flutter project many symbols are contributed by many imports (because there's quite a lot of exporting in them). If we did include it, I think it should be at the bottom, after the documentation, to avoid pushing the docs down (which are probably one of the main reasons to use the hover) if there are multiple imports.

@FMorschel
Copy link
Contributor Author

If you import flutter/widgets or flutter/material, the library that contains Widget is the same and I think we should continue to show that.

To be clear, I'm not asking to replace this anymore. I think the "what lines are importing this" info could be added to the hover. Above or under the current "the library that defines this element".

If you all disagree that's fine by me as well, as you said, we are already working on #56584 so this would simply be a way of looking what is(are) the import(s) without directly going there.

If we did include it, I think it should be at the bottom, after the documentation, to avoid pushing the docs down (which are probably one of the main reasons to use the hover) if there are multiple imports.

Agreed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-hover Issues related to hovers devexp-ux P4 type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

6 participants