Skip to content

IDE: Support jump to definition in imports #4199

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

Merged
merged 21 commits into from
Nov 16, 2018

Conversation

Duhemm
Copy link
Contributor

@Duhemm Duhemm commented Mar 27, 2018

No description provided.

@Duhemm
Copy link
Contributor Author

Duhemm commented Apr 11, 2018

This PR should probably wait for the IDE tests (#3766) to be merged.

@Duhemm Duhemm force-pushed the topic/go-to-definition-imports branch from 0182e4c to 4aa799e Compare April 12, 2018 17:49
@Duhemm Duhemm force-pushed the topic/go-to-definition-imports branch from 4aa799e to 421b6ce Compare April 23, 2018 12:06
@Duhemm Duhemm force-pushed the topic/go-to-definition-imports branch from 421b6ce to eab4b3b Compare May 1, 2018 13:39
@Duhemm Duhemm force-pushed the topic/go-to-definition-imports branch from eab4b3b to 9663752 Compare August 21, 2018 14:01
@Duhemm
Copy link
Contributor Author

Duhemm commented Aug 21, 2018

Rebased on latest master

@Duhemm Duhemm force-pushed the topic/go-to-definition-imports branch from e3d69c9 to 52e76f6 Compare September 4, 2018 09:18
@Duhemm Duhemm force-pushed the topic/go-to-definition-imports branch from 52e76f6 to fb2ac33 Compare October 15, 2018 08:24
@Duhemm
Copy link
Contributor Author

Duhemm commented Oct 15, 2018

@smarter I think I've addressed your first comments. My reply to your comment about the new AST node is hidden, but it's here

@Duhemm Duhemm force-pushed the topic/go-to-definition-imports branch 2 times, most recently from bee2d3f to b44e3b2 Compare October 19, 2018 11:21
@Duhemm
Copy link
Contributor Author

Duhemm commented Oct 29, 2018

@smarter I've added support for renaming renamed symbols. RenameTree is gone, too.

@Duhemm Duhemm force-pushed the topic/go-to-definition-imports branch 3 times, most recently from 8aa5176 to a74b52a Compare November 13, 2018 15:09
 This commit makes the renaming less precise for the (probably rare)
 case of an imports being imported multiple times with the same
 renaming.

 When doing a renaming of a symbol that is import-renamed, we would
 exclude subscope where the same symbol was imported again with the same
 name. This commit changes the logic so that we no longer exclude those
 subscope, and keep renaming.

 See RenameTest#renameRenamingImportNested for an example where the
 langauge server will now behave differently.
Renaming a symbol that has been import-renamed will now rename all
occurrences of that symbol with the new name in the whole file,
regardless of scope.
We keep only `SourceTree`, that we had before, but it now accepts any
`tpd.Tree`. After we bootstrap, this should become `tpd.Import |
tpd.NameTree`.
When renaming a symbol that overrides other symbols, the overridden
symbols and all their overrides will be renamed too.
When renaming a symbol that is overriding another symbol, we now ask the
user whether we should rename the base symbol or only this member.
@Duhemm Duhemm force-pushed the topic/go-to-definition-imports branch from a74b52a to ec6882c Compare November 13, 2018 15:12
When doing hover on an imported node, the window now shows the
documentation for all the imported symbols.
Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✨ 🌟 💞

@Duhemm Duhemm merged commit 614265d into scala:master Nov 16, 2018
@Duhemm Duhemm deleted the topic/go-to-definition-imports branch November 16, 2018 07:23
@Duhemm
Copy link
Contributor Author

Duhemm commented Nov 16, 2018

Thanks a lot for the numerous reviews @smarter !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants