Skip to content

Add support for :through associations #630

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion lib/ruby_lsp/ruby_lsp_rails/definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,23 @@ def handle_association(node)
return unless first_argument.is_a?(Prism::SymbolNode)

association_name = first_argument.unescaped
handle_association_name(association_name)

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
end

#: (String association_name) -> void
def handle_association_name(association_name)
result = @client.association_target_location(
model_name: @nesting.join("::"),
association_name: association_name,
)

return unless result

@response_builder << Support::LocationBuilder.line_location_from_s(result.fetch(:location))
Expand Down
1 change: 1 addition & 0 deletions test/dummy/app/models/country.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# frozen_string_literal: true

class Country < ApplicationRecord
has_one :flag, dependent: :destroy
end
5 changes: 5 additions & 0 deletions test/dummy/app/models/flag.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# frozen_string_literal: true

class Flag < ApplicationRecord
belongs_to :country
end
3 changes: 2 additions & 1 deletion test/dummy/app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ class User < ApplicationRecord
validates :first_name, presence: true
has_one :profile
scope :adult, -> { where(age: 18..) }
has_one :location, class_name: "Country"
belongs_to :location, class_name: "Country"
Copy link
Author

Choose a reason for hiding this comment

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

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.

has_one :country_flag, through: :location, source: :flag

attr_readonly :last_name

Expand Down
9 changes: 9 additions & 0 deletions test/dummy/db/migrate/20250703132109_create_flags.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
class CreateFlags < ActiveRecord::Migration[8.0]
def change
create_table :flags do |t|
t.references :country, null: false, foreign_key: true

t.timestamps
end
end
end
10 changes: 9 additions & 1 deletion test/dummy/db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[8.0].define(version: 2024_10_25_225348) do
ActiveRecord::Schema[8.0].define(version: 2025_07_03_132109) do
create_table "composite_primary_keys", primary_key: ["order_id", "product_id"], force: :cascade do |t|
t.integer "order_id"
t.integer "product_id"
Expand All @@ -25,6 +25,13 @@
t.datetime "updated_at", null: false
end

create_table "flags", force: :cascade do |t|
t.integer "country_id", null: false
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.index ["country_id"], name: "index_flags_on_country_id"
end

create_table "memberships", force: :cascade do |t|
t.integer "user_id", null: false
t.integer "organization_id", null: false
Expand Down Expand Up @@ -58,6 +65,7 @@
t.index ["country_id"], name: "index_users_on_country_id"
end

add_foreign_key "flags", "countries"
add_foreign_key "memberships", "organizations"
add_foreign_key "memberships", "users"
add_foreign_key "users", "countries"
Expand Down
56 changes: 54 additions & 2 deletions test/ruby_lsp_rails/definition_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,32 @@ class Organization < ActiveRecord::Base
assert_equal(2, response[0].range.end.line)
end

test "recognizes has_many :through model associations" do
response = generate_definitions_for_source(<<~RUBY, { line: 4, character: 29 })
# typed: false

class Organization < ActiveRecord::Base
has_many :memberships
has_many :users, through: :memberships
end
RUBY

assert_equal(2, response.size)
Copy link
Author

Choose a reason for hiding this comment

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

Without patch:
Screenshot 2025-07-03 at 11 52 27 pm


assert_equal(
URI::Generic.from_path(path: File.join(dummy_root, "app", "models", "user.rb")).to_s,
response[0].uri,
)
assert_equal(
URI::Generic.from_path(path: File.join(dummy_root, "app", "models", "membership.rb")).to_s,
response[1].uri,
)
assert_equal(2, response[0].range.start.line)
assert_equal(2, response[0].range.end.line)
assert_equal(2, response[1].range.start.line)
assert_equal(2, response[1].range.end.line)
end

test "recognizes belongs_to model associations" do
response = generate_definitions_for_source(<<~RUBY, { line: 3, character: 14 })
# typed: false
Expand Down Expand Up @@ -90,6 +116,32 @@ class User < ActiveRecord::Base
assert_equal(2, response[0].range.end.line)
end

test "recognizes has_one :through model associations" do
response = generate_definitions_for_source(<<~RUBY, { line: 4, character: 35 })
# typed: false

class User < ActiveRecord::Base
belongs_to :location, class_name: "Country"
has_one :country_flag, through: :location, source: :flag
end
RUBY

assert_equal(2, response.size)
Copy link
Author

Choose a reason for hiding this comment

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

Without patch:
Screenshot 2025-07-03 at 11 53 04 pm


assert_equal(
URI::Generic.from_path(path: File.join(dummy_root, "app", "models", "flag.rb")).to_s,
response[0].uri,
)
assert_equal(
URI::Generic.from_path(path: File.join(dummy_root, "app", "models", "country.rb")).to_s,
response[1].uri,
)
assert_equal(2, response[0].range.start.line)
assert_equal(2, response[0].range.end.line)
assert_equal(2, response[1].range.start.line)
assert_equal(2, response[1].range.end.line)
end

test "recognizes has_and_belongs_to_many model associations" do
response = generate_definitions_for_source(<<~RUBY, { line: 3, character: 27 })
# typed: false
Expand All @@ -110,11 +162,11 @@ class Profile < ActiveRecord::Base
end

test "handles class_name argument for associations" do
response = generate_definitions_for_source(<<~RUBY, { line: 3, character: 11 })
response = generate_definitions_for_source(<<~RUBY, { line: 3, character: 14 })
# typed: false

class User < ActiveRecord::Base
has_one :location, class_name: "Country"
belongs_to :location, class_name: "Country"
end
RUBY

Expand Down