Add definition and hover support for :through associations#630
Add definition and hover support for :through associations#630joshuay03 wants to merge 2 commits intoShopify:mainfrom
:through associations#630Conversation
joshuay03
force-pushed
the
through-association-support
branch
from
de3a801 to
62c54e5
Compare
joshuay03
changed the title
:through associations
| has_one :profile | ||
| scope :adult, -> { where(age: 18..) } | ||
| has_one :location, class_name: "Country" | ||
| belongs_to :location, class_name: "Country" |
There was a problem hiding this comment.
Going off of the schema I'm pretty sure this was incorrectly set up, as I don't see any tests that expect this to be invalid.
joshuay03
force-pushed
the
through-association-support
branch
from
62c54e5 to
631ee7e
Compare
| end | ||
| RUBY | ||
|
|
||
| assert_equal(2, response.size) |
| end | ||
| RUBY | ||
|
|
||
| assert_equal(2, response.size) |
vinistock
left a comment
vinistock
left a comment
There was a problem hiding this comment.
The Windows failures are unrelated. This makes sense, just added a question about the experience and the structure of the code
| through_association_name = node.arguments.arguments | ||
| .filter_map { |arg| arg.elements if arg.is_a?(Prism::KeywordHashNode) } | ||
| .flatten | ||
| .find { |elem| elem.key.value == "through" } | ||
| &.value | ||
| &.unescaped | ||
| handle_association_name(through_association_name) if through_association_name |
There was a problem hiding this comment.
Just to understand the intended experience, when going to definition you'd see both the association and the through association as options, correct?
Also, the node.arguments return is nilable for when there are no arguments. I think we can restructure this a bit.
- At the top of the
handle_associationmethod, we can get the arguments and return early if it's nil.
arguments = node.arguments&.arguments
return unless arguments- Then you can reuse the same arguments array for checking the first argument and the
through
There was a problem hiding this comment.
Just to understand the intended experience, when going to definition you'd see both the association and the
throughassociation as options, correct?
Yeah, that's what I initially implemented, but it's not what I actually wanted. I was showing both definitions regardless of which symbol was clicked. What I actually wanted was for each symbol to show only its specific definition.
I've updated both the implementation and tests to fix this.
Also, the
node.argumentsreturn is nilable for when there are no arguments. I think we can restructure this a bit.
- At the top of the
handle_associationmethod, we can get the arguments and return early if it's nil.arguments = node.arguments&.arguments return unless arguments
- Then you can reuse the same arguments array for checking the first argument and the
through
Good idea. I've implemented this.
I've also added hover support here as a follow-up to #616.
joshuay03
force-pushed
the
through-association-support
branch
3 times, most recently
from
93b5518 to
a09ac2d
Compare
joshuay03
changed the title
:through associations:through associations
joshuay03
force-pushed
the
through-association-support
branch
13 times, most recently
from
50f1527 to
098529a
Compare
joshuay03
force-pushed
the
through-association-support
branch
from
098529a to
1f9206b
Compare
2 participants
Currently, cmd + clicking on the through association name goes to the same file as the association name. I've added definition support to navigate to the correct through association model file.
I've also added hover support as a follow-up to #616.