Skip to content
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

Support inherited methods in definition, hover and completion #2028

Merged
merged 2 commits into from
May 30, 2024

Conversation

vinistock
Copy link
Member

@vinistock vinistock commented May 9, 2024

Motivation

Step towards #899

Use the linearized ancestor information from #2024 to support inherited methods for definition, hover and completion.

Implementation

  1. Changed resolve_method to linearize ancestors lazily. Then we search ancestors in order and return the first methods we return, which ensures we jump to the right method
  2. Adapted completion to consider all ancestors when filtering possible methods being invoked

Automated Tests

Added tests.

inheritance_resolution.mov

@vinistock vinistock added enhancement New feature or request server This pull request should be included in the server gem's release notes labels May 9, 2024
@vinistock vinistock self-assigned this May 9, 2024
@vinistock vinistock requested a review from a team as a code owner May 9, 2024 20:41
@vinistock vinistock requested review from andyw8 and st0012 May 9, 2024 20:41
@andyw8
Copy link
Contributor

andyw8 commented May 10, 2024

So this should also allow us to improve Definition in ruby-lsp-rails, e.g. so that a callback method can be found even if defined in a parent controller?

@vinistock
Copy link
Member Author

Yes. And this will happen automatically, by making resolve_method smarter, it will return more accurate results without any changes needed in the Rails addon.

lib/ruby_indexer/lib/ruby_indexer/index.rb Show resolved Hide resolved
lib/ruby_indexer/lib/ruby_indexer/index.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@Morriar Morriar left a comment

Choose a reason for hiding this comment

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

Can't we also handle go to and hover to super now?

lib/ruby_indexer/lib/ruby_indexer/index.rb Outdated Show resolved Hide resolved
@vinistock vinistock force-pushed the vs/add_ancestors_linearization branch 6 times, most recently from 401c668 to 9566119 Compare May 30, 2024 17:59
Base automatically changed from vs/add_ancestors_linearization to main May 30, 2024 18:12
@vinistock vinistock force-pushed the vs/resolve_inherited_methods branch from 3464078 to cb1afee Compare May 30, 2024 18:26
@vinistock
Copy link
Member Author

Can't we also handle go to and hover to super now?

We can, but we need the work that @andyw8 is doing here #2099 so that we know which method we're currently inside of.

@vinistock vinistock merged commit f159d4e into main May 30, 2024
34 checks passed
@vinistock vinistock deleted the vs/resolve_inherited_methods branch May 30, 2024 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request server This pull request should be included in the server gem's release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants