Shopify / ruby-lsp-rails

A Ruby LSP addon for Rails
https://shopify.github.io/ruby-lsp-rails/
MIT License
515 stars 21 forks source link

Add support for Go To Definition for associations #373

Closed maxveldink closed 1 month ago

maxveldink commented 1 month ago

Closes #358

Adds support for jump to definition for belongs_to, has_one, has_many and has_and_belongs_to_many associations on models.

maxveldink commented 1 month ago

I have signed the CLA!

andyw8 commented 1 month ago

:wave: Looking into the on_class_node_enter problem.

andyw8 commented 1 month ago

Ah, we don't need to listen for class node events, we can use the nesting:

model_name: @nesting.join("::")

andyw8 commented 1 month ago

I notice the Builder classes (example) are marked :nodoc: which usually means they should treated as internal, so we may need to add a public interface for that in Rails.

maxveldink commented 1 month ago

Thanks @andyw8! I'm flipping this to ready since it's about good from my end. Ready to review whenever!

How do I go about upstreaming the behavior we need for the Builder classes? Can we merge this with the private interfaces if we have the next steps for a better interface?

andyw8 commented 1 month ago

Nice!

I found there's a simpler way than using the Builder classes - reflect_on_association It will also work with options, e.g. class_name, and it's documented so there's no concerns about using private APIs.

I branched off your PR and added a couple of commits in this branch:

https://github.com/Shopify/ruby-lsp-rails/compare/andyw8/go-to-def-assoc

If you want to update your branch then go ahead, otherwise I can things over from here.

maxveldink commented 1 month ago

@andyw8 Thanks! I was looking for a more stable way too and this works great.

I pulled in your commits and cleaned up the PR some more. Should be good for another look

maxveldink commented 1 month ago

@vinistock Ahh yes, GH PR additions always get me. Should be good to go 👍🏻

andyw8 commented 1 month ago

btw we're starting on fixing the target behaviour so you'll be to right click on the symbol rather than the has_many (etc.): https://github.com/Shopify/ruby-lsp/pull/2099