Skip to content

Commit 9a52310

Browse files
authored
Fix undefined method 'join' for an instance of String on hover (#645)
* create index to reproduce issue * handle when index[:columns] is Array or String * users table now has one more index, add that new index to the assertion * fix lint issues on modified files * fix another rubocop issue * sort indexes list when running assertion
1 parent f30f100 commit 9a52310

File tree

5 files changed

+84
-4
lines changed

5 files changed

+84
-4
lines changed

lib/ruby_lsp/ruby_lsp_rails/hover.rb

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,15 @@ def generate_column_content(name)
9797
@response_builder.push(
9898
model[:indexes].map do |index|
9999
uniqueness = index[:unique] ? " (unique)" : ""
100-
"- **#{index[:name]}** (#{index[:columns].join(",")})#{uniqueness}"
100+
columns = case index[:columns]
101+
when Array
102+
index[:columns].join(",")
103+
when String
104+
index[:columns]
105+
else
106+
index[:name]
107+
end
108+
"- **#{index[:name]}** (#{columns})#{uniqueness}"
101109
end.join("\n"),
102110
category: :documentation,
103111
)
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
class AddComplexIndexToUsers < ActiveRecord::Migration[8.0]
2+
def change
3+
execute "CREATE UNIQUE INDEX users_unique_complex ON users (COALESCE(country_id, 0), ltrim(first_name));"
4+
end
5+
end

test/dummy/db/schema.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
#
1111
# It's strongly recommended that you check this file into your version control system.
1212

13-
ActiveRecord::Schema[8.0].define(version: 2024_10_25_225348) do
13+
ActiveRecord::Schema[8.0].define(version: 2025_08_16_140957) do
1414
create_table "composite_primary_keys", primary_key: ["order_id", "product_id"], force: :cascade do |t|
1515
t.integer "order_id"
1616
t.integer "product_id"
@@ -55,6 +55,7 @@
5555
t.datetime "updated_at", null: false
5656
t.integer "country_id", null: false
5757
t.boolean "active", default: true, null: false
58+
t.index "COALESCE(country_id, 0), ltrim(first_name)", name: "users_unique_complex", unique: true
5859
t.index ["country_id"], name: "index_users_on_country_id"
5960
end
6061

test/ruby_lsp_rails/hover_test.rb

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,69 @@ class Bar < ApplicationRecord
321321
CONTENT
322322
end
323323

324+
test "handles complex indexes with expressions" do
325+
expected_response = {
326+
schema_file: "#{dummy_root}/db/schema.rb",
327+
columns: [
328+
["id", "integer", nil, false],
329+
["first_name", "string", "", true],
330+
["last_name", "string", nil, true],
331+
["age", "integer", "0", true],
332+
["created_at", "datetime", nil, false],
333+
["updated_at", "datetime", nil, false],
334+
["country_id", "integer", nil, false],
335+
["active", "boolean", "true", false],
336+
],
337+
primary_keys: ["id"],
338+
foreign_keys: ["country_id"],
339+
indexes: [
340+
{ name: "index_users_on_country_id", columns: ["country_id"], unique: false },
341+
{ name: "users_unique_complex", columns: "COALESCE(country_id, 0), ltrim(first_name)", unique: true },
342+
],
343+
}
344+
345+
RunnerClient.any_instance.stubs(model: expected_response)
346+
347+
response = hover_on_source(<<~RUBY, { line: 3, character: 0 })
348+
class User < ApplicationRecord
349+
end
350+
351+
User
352+
RUBY
353+
354+
assert_equal(<<~CONTENT.chomp, response.contents.value)
355+
```ruby
356+
User
357+
```
358+
359+
**Definitions**: [fake.rb](file:///fake.rb#L1,1-2,4)
360+
361+
362+
[Schema](#{URI::Generic.from_path(path: dummy_root + "/db/schema.rb")})
363+
364+
### Columns
365+
- **id**: integer (PK)
366+
367+
- **first_name**: string - default: ""
368+
369+
- **last_name**: string
370+
371+
- **age**: integer - default: 0
372+
373+
- **created_at**: datetime - not null
374+
375+
- **updated_at**: datetime - not null
376+
377+
- **country_id**: integer (FK) - not null
378+
379+
- **active**: boolean - default: true - not null
380+
381+
### Indexes
382+
- **index_users_on_country_id** (country_id)
383+
- **users_unique_complex** (COALESCE(country_id, 0), ltrim(first_name)) (unique)
384+
CONTENT
385+
end
386+
324387
private
325388

326389
def hover_on_source(source, position)

test/ruby_lsp_rails/runner_client_test.rb

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,14 @@ class RunnerClientTest < ActiveSupport::TestCase
5858
]
5959
end
6060
foreign_keys = ["country_id"]
61-
indexes = [{ name: "index_users_on_country_id", columns: ["country_id"], unique: false }]
61+
indexes = [
62+
{ name: "users_unique_complex", columns: "COALESCE(country_id, 0), ltrim(first_name)", unique: true },
63+
{ name: "index_users_on_country_id", columns: ["country_id"], unique: false },
64+
]
6265
response = @client.model("User") #: as !nil
6366
assert_equal(columns, response.fetch(:columns))
6467
assert_equal(foreign_keys, response.fetch(:foreign_keys))
65-
assert_equal(indexes, response.fetch(:indexes))
68+
assert_equal(indexes.sort_by { |i| i[:name] }, response.fetch(:indexes).sort_by { |i| i[:name] })
6669
assert_match(%r{db/schema\.rb$}, response.fetch(:schema_file))
6770
end
6871

0 commit comments

Comments
 (0)