From 49cb55535e72f8a6c02bdccc577218084159617e Mon Sep 17 00:00:00 2001 From: Damien Le Thiec Date: Sat, 26 Oct 2024 01:34:23 +0200 Subject: [PATCH 1/3] Add default and null info on hover --- lib/ruby_lsp/ruby_lsp_rails/hover.rb | 19 +++++++++++++++++-- lib/ruby_lsp/ruby_lsp_rails/server.rb | 2 +- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/lib/ruby_lsp/ruby_lsp_rails/hover.rb b/lib/ruby_lsp/ruby_lsp_rails/hover.rb index cdbad144..668c3fe4 100644 --- a/lib/ruby_lsp/ruby_lsp_rails/hover.rb +++ b/lib/ruby_lsp/ruby_lsp_rails/hover.rb @@ -67,13 +67,28 @@ def generate_column_content(name) ) if schema_file @response_builder.push( - model[:columns].map do |name, type| + model[:columns].map do |name, type, default_string, null| primary_key_suffix = " (PK)" if model[:primary_keys].include?(name) - "**#{name}**: #{type}#{primary_key_suffix}\n" + suffixes = [] + suffixes << "default: #{format_default(default_string, type)}" if default_string + suffixes << "not null" unless null + suffix_string = " - #{suffixes.join(" - ")}" if suffixes.any? + "**#{name}**: #{type}#{primary_key_suffix}#{suffix_string}\n" end.join("\n"), category: :documentation, ) end + + sig { params(default_string: String, type: String).returns(String) } + def format_default(default_string, type) + if type == "boolean" + default_string == "true" ? "TRUE" : "FALSE" + elsif type == "string" + default_string.inspect + else + default_string + end + end end end end diff --git a/lib/ruby_lsp/ruby_lsp_rails/server.rb b/lib/ruby_lsp/ruby_lsp_rails/server.rb index e48787db..7cad461c 100644 --- a/lib/ruby_lsp/ruby_lsp_rails/server.rb +++ b/lib/ruby_lsp/ruby_lsp_rails/server.rb @@ -200,7 +200,7 @@ def resolve_database_info_from_model(model_name) info = { result: { - columns: const.columns.map { |column| [column.name, column.type] }, + columns: const.columns.map { |column| [column.name, column.type, column.default, column.null] }, primary_keys: Array(const.primary_key), }, } From f2ac46448cdeca7c728249886d8fd92b6ade56cf Mon Sep 17 00:00:00 2001 From: Damien Le Thiec Date: Sat, 26 Oct 2024 01:34:42 +0200 Subject: [PATCH 2/3] Adapt tests to default and null info on hover --- .../20241025225348_add_defaults_to_user.rb | 7 ++ test/dummy/db/schema.rb | 7 +- test/ruby_lsp_rails/hover_test.rb | 66 +++++++++++-------- test/ruby_lsp_rails/runner_client_test.rb | 15 +++-- 4 files changed, 58 insertions(+), 37 deletions(-) create mode 100644 test/dummy/db/migrate/20241025225348_add_defaults_to_user.rb diff --git a/test/dummy/db/migrate/20241025225348_add_defaults_to_user.rb b/test/dummy/db/migrate/20241025225348_add_defaults_to_user.rb new file mode 100644 index 00000000..021037a1 --- /dev/null +++ b/test/dummy/db/migrate/20241025225348_add_defaults_to_user.rb @@ -0,0 +1,7 @@ +class AddDefaultsToUser < ActiveRecord::Migration[7.1] + def change + change_column_default :users, :age, from: nil, to: 0 + add_column :users, :active, :boolean, default: true, null: false + change_column_default :users, :first_name, from: nil, to: "" + end +end diff --git a/test/dummy/db/schema.rb b/test/dummy/db/schema.rb index 303d3d4a..d15d6c1f 100644 --- a/test/dummy/db/schema.rb +++ b/test/dummy/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_05_21_183200) do +ActiveRecord::Schema[7.1].define(version: 2024_10_25_225348) do create_table "composite_primary_keys", primary_key: ["order_id", "product_id"], force: :cascade do |t| t.integer "order_id" t.integer "product_id" @@ -48,12 +48,13 @@ end create_table "users", force: :cascade do |t| - t.string "first_name" + t.string "first_name", default: "" t.string "last_name" - t.integer "age" + t.integer "age", default: 0 t.datetime "created_at", null: false t.datetime "updated_at", null: false t.integer "country_id", null: false + t.boolean "active", default: true, null: false t.index ["country_id"], name: "index_users_on_country_id" end diff --git a/test/ruby_lsp_rails/hover_test.rb b/test/ruby_lsp_rails/hover_test.rb index f9bb3bad..d5816baf 100644 --- a/test/ruby_lsp_rails/hover_test.rb +++ b/test/ruby_lsp_rails/hover_test.rb @@ -10,12 +10,14 @@ class HoverTest < ActiveSupport::TestCase expected_response = { schema_file: "#{dummy_root}/db/schema.rb", columns: [ - ["id", "integer"], - ["first_name", "string"], - ["last_name", "string"], - ["age", "integer"], - ["created_at", "datetime"], - ["updated_at", "datetime"], + ["id", "integer", nil, false], + ["first_name", "string", "", true], + ["last_name", "string", nil, true], + ["age", "integer", "0", true], + ["created_at", "datetime", nil, false], + ["updated_at", "datetime", nil, false], + ["country_id", "integer", nil, false], + ["active", "boolean", "true", false], ], primary_keys: ["id"], } @@ -39,17 +41,21 @@ class User < ApplicationRecord [Schema](#{URI::Generic.from_path(path: dummy_root + "/db/schema.rb")}) - **id**: integer (PK) + **id**: integer (PK) - not null - **first_name**: string + **first_name**: string - default: "" **last_name**: string - **age**: integer + **age**: integer - default: 0 + + **created_at**: datetime - not null + + **updated_at**: datetime - not null - **created_at**: datetime + **country_id**: integer - not null - **updated_at**: datetime + **active**: boolean - default: TRUE - not null CONTENT end @@ -57,12 +63,14 @@ class User < ApplicationRecord expected_response = { schema_file: "#{dummy_root}/db/schema.rb", columns: [ - ["id", "integer"], - ["first_name", "string"], - ["last_name", "string"], - ["age", "integer"], - ["created_at", "datetime"], - ["updated_at", "datetime"], + ["id", "integer", nil, false], + ["first_name", "string", "", true], + ["last_name", "string", nil, true], + ["age", "integer", "0", true], + ["created_at", "datetime", nil, false], + ["updated_at", "datetime", nil, false], + ["country_id", "integer", nil, false], + ["active", "boolean", "true", false], ], primary_keys: ["id"], } @@ -88,17 +96,21 @@ class User < ApplicationRecord [Schema](#{URI::Generic.from_path(path: dummy_root + "/db/schema.rb")}) - **id**: integer (PK) + **id**: integer (PK) - not null - **first_name**: string + **first_name**: string - default: "" **last_name**: string - **age**: integer + **age**: integer - default: 0 + + **created_at**: datetime - not null + + **updated_at**: datetime - not null - **created_at**: datetime + **country_id**: integer - not null - **updated_at**: datetime + **active**: boolean - default: TRUE - not null CONTENT end @@ -134,15 +146,15 @@ class CompositePrimaryKey < ApplicationRecord [Schema](#{URI::Generic.from_path(path: dummy_root + "/db/schema.rb")}) - **order_id**: integer (PK) + **order_id**: integer (PK) - not null - **product_id**: integer (PK) + **product_id**: integer (PK) - not null - **note**: string + **note**: string - not null - **created_at**: datetime + **created_at**: datetime - not null - **updated_at**: datetime + **updated_at**: datetime - not null CONTENT end diff --git a/test/ruby_lsp_rails/runner_client_test.rb b/test/ruby_lsp_rails/runner_client_test.rb index 2bec27c4..217d1589 100644 --- a/test/ruby_lsp_rails/runner_client_test.rb +++ b/test/ruby_lsp_rails/runner_client_test.rb @@ -26,13 +26,14 @@ class RunnerClientTest < ActiveSupport::TestCase test "#model returns information for the requested model" do # These columns are from the schema in the dummy app: test/dummy/db/schema.rb columns = [ - ["id", "integer"], - ["first_name", "string"], - ["last_name", "string"], - ["age", "integer"], - ["created_at", "datetime"], - ["updated_at", "datetime"], - ["country_id", "integer"], + ["id", "integer", nil, false], + ["first_name", "string", "", true], + ["last_name", "string", nil, true], + ["age", "integer", "0", true], + ["created_at", "datetime", nil, false], + ["updated_at", "datetime", nil, false], + ["country_id", "integer", nil, false], + ["active", "boolean", "1", false], ] response = T.must(@client.model("User")) assert_equal(columns, response.fetch(:columns)) From d5e4cca31cee729771d0643494dae175a0542963 Mon Sep 17 00:00:00 2001 From: Damien Le Thiec Date: Tue, 29 Oct 2024 21:50:19 +0100 Subject: [PATCH 3/3] Improve lisibility post review --- lib/ruby_lsp/ruby_lsp_rails/hover.rb | 21 +++++++++++---------- test/ruby_lsp_rails/hover_test.rb | 12 ++++++------ 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/lib/ruby_lsp/ruby_lsp_rails/hover.rb b/lib/ruby_lsp/ruby_lsp_rails/hover.rb index 668c3fe4..002612b4 100644 --- a/lib/ruby_lsp/ruby_lsp_rails/hover.rb +++ b/lib/ruby_lsp/ruby_lsp_rails/hover.rb @@ -67,11 +67,11 @@ def generate_column_content(name) ) if schema_file @response_builder.push( - model[:columns].map do |name, type, default_string, null| + model[:columns].map do |name, type, default_value, nullable| primary_key_suffix = " (PK)" if model[:primary_keys].include?(name) suffixes = [] - suffixes << "default: #{format_default(default_string, type)}" if default_string - suffixes << "not null" unless null + suffixes << "default: #{format_default(default_value, type)}" if default_value + suffixes << "not null" unless nullable || primary_key_suffix.present? suffix_string = " - #{suffixes.join(" - ")}" if suffixes.any? "**#{name}**: #{type}#{primary_key_suffix}#{suffix_string}\n" end.join("\n"), @@ -79,14 +79,15 @@ def generate_column_content(name) ) end - sig { params(default_string: String, type: String).returns(String) } - def format_default(default_string, type) - if type == "boolean" - default_string == "true" ? "TRUE" : "FALSE" - elsif type == "string" - default_string.inspect + sig { params(default_value: String, type: String).returns(String) } + def format_default(default_value, type) + case type + when "boolean" + default_value == "true" ? "true" : "false" + when "string" + default_value.inspect else - default_string + default_value end end end diff --git a/test/ruby_lsp_rails/hover_test.rb b/test/ruby_lsp_rails/hover_test.rb index d5816baf..90861fec 100644 --- a/test/ruby_lsp_rails/hover_test.rb +++ b/test/ruby_lsp_rails/hover_test.rb @@ -41,7 +41,7 @@ class User < ApplicationRecord [Schema](#{URI::Generic.from_path(path: dummy_root + "/db/schema.rb")}) - **id**: integer (PK) - not null + **id**: integer (PK) **first_name**: string - default: "" @@ -55,7 +55,7 @@ class User < ApplicationRecord **country_id**: integer - not null - **active**: boolean - default: TRUE - not null + **active**: boolean - default: true - not null CONTENT end @@ -96,7 +96,7 @@ class User < ApplicationRecord [Schema](#{URI::Generic.from_path(path: dummy_root + "/db/schema.rb")}) - **id**: integer (PK) - not null + **id**: integer (PK) **first_name**: string - default: "" @@ -110,7 +110,7 @@ class User < ApplicationRecord **country_id**: integer - not null - **active**: boolean - default: TRUE - not null + **active**: boolean - default: true - not null CONTENT end @@ -146,9 +146,9 @@ class CompositePrimaryKey < ApplicationRecord [Schema](#{URI::Generic.from_path(path: dummy_root + "/db/schema.rb")}) - **order_id**: integer (PK) - not null + **order_id**: integer (PK) - **product_id**: integer (PK) - not null + **product_id**: integer (PK) **note**: string - not null