scalameta / metals-feature-requests

Issue tracker for Metals feature requests
37 stars 4 forks source link

Offer to 'Find all references' on overridden method #94

Open raboof opened 5 years ago

raboof commented 5 years ago

When doing a 'find all references' (in vscode) on a method that overrides a method from a superclass, only references to the method on the subclass are returned. It would be useful to have the option to also see references to the method on the superclass. Also easily jumping to the method in the superclass would be helpful.

olafurpg commented 5 years ago

Thanks for reporting! This feature will be possible to implement once "rename symbol" is done. Currently, we are missing indexes to find all the overriden methods.

raboof commented 5 years ago

:+1: - linking to https://github.com/scalameta/metals/issues/679

tgodzik commented 4 years ago

It is finally possible to find all references on an overridden method now, everything that we need is implemented and it's used in the RenameProvider, however we need to figure out how to offer that option to user.

Perhaps it would make sense to offer it via code actions ?

gabro commented 4 years ago

@tgodzik what so you think about making it the default?

tgodzik commented 4 years ago

@tgodzik what so you think about making it the default?

I think that would be fine, but I would like to hear some more opinions.

Overall, it would be also nice to be able to customize the options for finding references, so that we can do it both ways.

olafurpg commented 4 years ago

I think it's fine to default to always finding references to all overridden and supermethods. I don't think we need to add options to begin with.

olafurpg commented 4 years ago

Moving to the main repo since this issue is unblocked by textDocument/rename.

raboof commented 4 years ago

I think it's fine to default to always finding references to all overridden and supermethods.

yeah, only for toString and similar generic methods it can be helpful to only search for the specific one, for now I agree it makes sense to search for the supermethods

olafurpg commented 4 years ago

Good point. We may want to special case toString, hashCode and equals to not include supermethods and overridden methods.

kubukoz commented 4 years ago

Hi, is anyone currently working on this? If it's not too hard for a newcomer to the project, I'd like to give it a try :)

tgodzik commented 4 years ago

Hi, is anyone currently working on this? If it's not too hard for a newcomer to the project, I'd like to give it a try :)

Not currently, you're welcome to give it a try! We already try to find all references in the RenameProvider, which is done by going to the top symbol and searching for references of all implementations. This might probably be done better and we can move that logic to the ReferenceProvider.

tgodzik commented 4 years ago

@kubukoz Are you working on this? Or is ok for someone else to pick it up?

kubukoz commented 4 years ago

I'm not, got swamped with other commitments, sorry.

On Mon, Feb 3, 2020, 10:59 Tomasz Godzik notifications@github.com wrote:

@kubukoz https://github.com/kubukoz Are you working on this? Or is ok for someone else to pick it up?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/scalameta/metals/issues/1063?email_source=notifications&email_token=AAG2PJA7UGXVUCKPE3GKXGTRA7THHA5CNFSM4JMH4L52YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKTGRZQ#issuecomment-581331174, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG2PJAQJATYM4OAFVIMB43RA7THHANCNFSM4JMH4L5Q .

tgodzik commented 4 years ago

We'll work on it with @kbochenek to get him introduced into Metals and overall Scalameta environment.

tgodzik commented 4 years ago

This is blocked until we have a faster way to extract info about class hierarchy in dependencies.

Looking into ways to do it here: https://github.com/scalameta/metals/issues/1287

tgodzik commented 4 years ago

Moved it back to feature requests, since it will not be as easy to implement until we improve the efficiency of finding inheritance hierarchy in dependencies.

gabro commented 4 years ago

Also easily jumping to the method in the superclass would be helpful.

By the way, this part of the OP has been implemented in https://github.com/scalameta/metals/pull/1487 🎉

kubukoz commented 3 years ago

Is "find references for implementations of method" also in scope for this?

tgodzik commented 3 years ago

@kubukoz that's the plan, however we still need a way to get class hierarchy from dependencies, which is not trivial to do effectively.