From 9f7f5e9d2ff8b4bf48e228f911cac30a46fd3be2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janko=20Marohni=C4=87?= Date: Mon, 27 Oct 2025 08:19:20 +0100 Subject: [PATCH 1/4] Implement "go to definition" for render calls in ERB templates It's common to want to traverse through several partials while updating HTML that a controller action renders. Rails.vim has a neat `gf` shortcut for this, though it probably doesn't have the precision that Prism would provide. This brings the same functionality to Ruby LSP Rails, by implementing "go to definition" support for render calls inside ERB templates. It supports partial name passed as positional argument, or via `:partial`, `:layout`, and `:spacer_template` keyword arguments. It even handles `:variants`, `:formats`, and `:handlers` options, as well as `:template` for rendering non-partial templates. Relative lookup will also check in view directories of controller ancestors. For the latter, I considered doing a call to the Rails process that will return `ActionController::Base._prefixes`. However, I couldn't think of a good enough interface, and that method ableit public is undocumented, so it seems like we shouldn't rely on it. Given that this ancestry lookup is non-configurable anyway, I chose to implement it in Ruby LSP land based on indexed controller files. To avoid the overhead of booting the Rails process too many times in tests, I updated the test helpers to allow sending multiple `textDocument/definition` requests to the same server. --- lib/ruby_lsp/ruby_lsp_rails/addon.rb | 3 +- lib/ruby_lsp/ruby_lsp_rails/code_lens.rb | 9 +- lib/ruby_lsp/ruby_lsp_rails/definition.rb | 92 +++++++++++- lib/ruby_lsp/ruby_lsp_rails/runner_client.rb | 4 + .../ruby_lsp_rails/support/inflections.rb | 24 ++++ test/ruby_lsp_rails/definition_test.rb | 134 ++++++++++++++++-- 6 files changed, 248 insertions(+), 18 deletions(-) create mode 100644 lib/ruby_lsp/ruby_lsp_rails/support/inflections.rb diff --git a/lib/ruby_lsp/ruby_lsp_rails/addon.rb b/lib/ruby_lsp/ruby_lsp_rails/addon.rb index a751708e..c3b3b912 100644 --- a/lib/ruby_lsp/ruby_lsp_rails/addon.rb +++ b/lib/ruby_lsp/ruby_lsp_rails/addon.rb @@ -9,6 +9,7 @@ require_relative "support/callbacks" require_relative "support/validations" require_relative "support/location_builder" +require_relative "support/inflections" require_relative "runner_client" require_relative "hover" require_relative "code_lens" @@ -129,7 +130,7 @@ def create_document_symbol_listener(response_builder, dispatcher) def create_definition_listener(response_builder, uri, node_context, dispatcher) return unless @global_state - Definition.new(@rails_runner_client, response_builder, node_context, @global_state.index, dispatcher) + Definition.new(@rails_runner_client, response_builder, uri, node_context, @global_state.index, dispatcher) end # @override diff --git a/lib/ruby_lsp/ruby_lsp_rails/code_lens.rb b/lib/ruby_lsp/ruby_lsp_rails/code_lens.rb index 5ce4e13a..bb25fcb4 100644 --- a/lib/ruby_lsp/ruby_lsp_rails/code_lens.rb +++ b/lib/ruby_lsp/ruby_lsp_rails/code_lens.rb @@ -73,6 +73,7 @@ module Rails # class CodeLens include Requests::Support::Common + include Inflections include ActiveSupportTestCaseHelper #: (RunnerClient, GlobalState, ResponseBuilders::CollectionResponseBuilder[Interface::CodeLens], URI::Generic, Prism::Dispatcher) -> void @@ -191,13 +192,9 @@ def controller? def add_jump_to_view(node) class_name = @constant_name_stack.map(&:first).join("::") action_name = node.name - controller_name = class_name - .delete_suffix("Controller") - .gsub(/([a-z])([A-Z])/, "\\1_\\2") - .gsub("::", "/") - .downcase + controller_name = underscore(class_name.delete_suffix("Controller")) - view_uris = Dir.glob("#{@client.rails_root}/app/views/#{controller_name}/#{action_name}*").filter_map do |path| + view_uris = Dir.glob("#{@client.views_dir}/#{controller_name}/#{action_name}*").filter_map do |path| # it's possible we could have a directory with the same name as the action, so we need to skip those next if File.directory?(path) diff --git a/lib/ruby_lsp/ruby_lsp_rails/definition.rb b/lib/ruby_lsp/ruby_lsp_rails/definition.rb index 133d7760..cc353ebf 100644 --- a/lib/ruby_lsp/ruby_lsp_rails/definition.rb +++ b/lib/ruby_lsp/ruby_lsp_rails/definition.rb @@ -1,6 +1,8 @@ # typed: strict # frozen_string_literal: true +require "pathname" + module RubyLsp module Rails # ![Definition demo](../../definition.gif) @@ -29,11 +31,13 @@ module Rails # - Changes to routes won't be picked up until the server is restarted. class Definition include Requests::Support::Common + include Inflections #: (RunnerClient client, RubyLsp::ResponseBuilders::CollectionResponseBuilder[(Interface::Location | Interface::LocationLink)] response_builder, NodeContext node_context, RubyIndexer::Index index, Prism::Dispatcher dispatcher) -> void - def initialize(client, response_builder, node_context, index, dispatcher) + def initialize(client, response_builder, uri, node_context, index, dispatcher) @client = client @response_builder = response_builder + @path = uri.to_standardized_path #: String? @node_context = node_context @nesting = node_context.nesting #: Array[String] @index = index @@ -49,6 +53,7 @@ def on_symbol_node_enter(node) #: (Prism::StringNode node) -> void def on_string_node_enter(node) handle_possible_dsl(node) + handle_possible_render(node) end #: (Prism::CallNode node) -> void @@ -142,6 +147,47 @@ def handle_association(node) @response_builder << Support::LocationBuilder.line_location_from_s(result.fetch(:location)) end + def handle_possible_render(node) + return unless @path&.end_with?(".html.erb") + + call_node = @node_context.call_node + return unless call_node + return unless self_receiver?(call_node) + + message = call_node.message + return unless message == "render" + + arguments = call_node.arguments&.arguments + return unless arguments + + argument = view_template_argument(arguments, node) + return unless argument + + template = node.content + template_options = view_template_options(arguments) + + formats_pattern = template_options[:formats] ? "{#{template_options[:formats].join(",")}}" : "html" + variants_pattern = "{#{template_options[:variants].map { |variant| "+#{variant}" }.join(",")},}" if template_options[:variants] + handlers_pattern = template_options[:handlers] ? "{#{template_options[:handlers].join(",")}}" : "*" + + extension_pattern = "#{formats_pattern}#{variants_pattern}.#{handlers_pattern}" + + template_pattern = if argument == "template" + File.join(@client.views_dir, "#{template}.#{extension_pattern}") + elsif template.include?("/") + *partial_dir, partial_name = template.split("/") + + File.join(@client.views_dir, *partial_dir, "_#{partial_name}.#{extension_pattern}") + else + File.join(@client.views_dir, "{#{view_prefixes.join(",")}}", "_#{template}.#{extension_pattern}") + end + + template_path = Dir.glob(template_pattern).first + return unless template_path + + @response_builder << Support::LocationBuilder.line_location_from_s("#{template_path}:1") + end + #: (Prism::CallNode node) -> void def handle_route(node) result = @client.route_location( @@ -194,6 +240,50 @@ def handle_if_unless_conditional(node, call_node, arguments) collect_definitions(method_name) end + + def view_template_argument(arguments, node) + return "partial" if arguments.first == node + + kwargs = arguments.find { |argument| argument.is_a?(Prism::KeywordHashNode) } + return unless kwargs + + kwarg = kwargs.elements.find do |pair| + ["partial", "layout", "spacer_template", "template"].include?(pair.key.value) && pair.value == node + end + + kwarg&.key&.value + end + + def view_template_options(arguments) + kwargs = arguments.find { |argument| argument.is_a?(Prism::KeywordHashNode) } + return {} unless kwargs + + kwargs.elements.each_with_object({}) do |pair, options| + next unless ["formats", "variants", "handlers"].include?(pair.key.value) + + value = [pair.value.value] if pair.value.is_a?(Prism::SymbolNode) + value = pair.value.elements.map(&:value) if pair.value.is_a?(Prism::ArrayNode) + + options[pair.key.value.to_sym] = value + end + end + + # Resolve available directories from which the controller can render relative + # partials based on its ancestry chain. + def view_prefixes + controller_dir = Pathname(@path).dirname.relative_path_from(@client.views_dir).to_s + controller_class = "#{camelize(controller_dir)}Controller" + controller_ancestors = [controller_class] + + controller_entry = @index.resolve(controller_class, [])&.find(&:parent_class) + while controller_entry + controller_entry = @index.resolve(controller_entry.parent_class, controller_entry.nesting)&.find(&:parent_class) + break unless controller_entry && not_in_dependencies?(controller_entry.file_path) + controller_ancestors << controller_entry.name + end + + controller_ancestors.map { |ancestor| underscore(ancestor.delete_suffix("Controller")) } + end end end end diff --git a/lib/ruby_lsp/ruby_lsp_rails/runner_client.rb b/lib/ruby_lsp/ruby_lsp_rails/runner_client.rb index dc284cb5..73fe8559 100644 --- a/lib/ruby_lsp/ruby_lsp_rails/runner_client.rb +++ b/lib/ruby_lsp/ruby_lsp_rails/runner_client.rb @@ -261,6 +261,10 @@ def connected? true end + def views_dir + File.join(@rails_root, "app/views") + end + private #: (String request, **untyped params) -> Hash[Symbol, untyped]? diff --git a/lib/ruby_lsp/ruby_lsp_rails/support/inflections.rb b/lib/ruby_lsp/ruby_lsp_rails/support/inflections.rb new file mode 100644 index 00000000..9e3d15e9 --- /dev/null +++ b/lib/ruby_lsp/ruby_lsp_rails/support/inflections.rb @@ -0,0 +1,24 @@ +# typed: strict +# frozen_string_literal: true + +module RubyLsp + module Rails + module Inflections + #: String -> String + def camelize(string) + string + .gsub(/_([a-z])/) { $1.upcase } + .gsub(/(^|\/)[a-z]/) { $&.upcase } + .gsub("/", "::") + end + + #: String -> String + def underscore(string) + string + .gsub(/([a-z])([A-Z])/, "\\1_\\2") + .gsub("::", "/") + .downcase + end + end + end +end diff --git a/test/ruby_lsp_rails/definition_test.rb b/test/ruby_lsp_rails/definition_test.rb index bc8c03e0..ec602301 100644 --- a/test/ruby_lsp_rails/definition_test.rb +++ b/test/ruby_lsp_rails/definition_test.rb @@ -467,20 +467,134 @@ def name; end assert_equal(15, response.range.end.character) end + test "recognizes render calls" do + FileUtils.touch("#{dummy_root}/app/views/users/_partial.html.erb") + + uri = Kernel.URI("file://#{dummy_root}/app/views/users/render.html.erb") + source = <<~ERB + <%= render "partial" %> + <%= render "users/partial" %> + <%= render partial: "partial" %> + <%= render layout: "partial" %> + <%= render spacer_template: "partial" %> + <%= render template: "users/index" %> + ERB + + with_ready_server(source, uri) do |server| + response = text_document_definition(server, { line: 0, character: 12 }, uri) + assert_equal("file://#{dummy_root}/app/views/users/_partial.html.erb", response.first.uri) + + response = text_document_definition(server, { line: 1, character: 12 }, uri) + assert_equal("file://#{dummy_root}/app/views/users/_partial.html.erb", response.first.uri) + + response = text_document_definition(server, { line: 2, character: 21 }, uri) + assert_equal("file://#{dummy_root}/app/views/users/_partial.html.erb", response.first.uri) + + response = text_document_definition(server, { line: 3, character: 20 }, uri) + assert_equal("file://#{dummy_root}/app/views/users/_partial.html.erb", response.first.uri) + + response = text_document_definition(server, { line: 4, character: 31 }, uri) + assert_equal("file://#{dummy_root}/app/views/users/_partial.html.erb", response.first.uri) + + response = text_document_definition(server, { line: 5, character: 23 }, uri) + assert_equal("file://#{dummy_root}/app/views/users/index.html.erb", response.first.uri) + end + ensure + FileUtils.rm("#{dummy_root}/app/views/users/_partial.html.erb") + end + + test "searches template directories of controller ancestors" do + FileUtils.mkdir_p("#{dummy_root}/app/views/application") + FileUtils.touch("#{dummy_root}/app/views/application/_partial.html.erb") + + uri = Kernel.URI("file://#{dummy_root}/app/views/users/render.html.erb") + source = <<~ERB + <%= render "partial" %> + ERB + + response = with_ready_server(source, uri) do |server| + server.global_state.index.index_file(URI::Generic.from_path(path: "#{dummy_root}/app/controllers/users_controller.rb")) + server.global_state.index.index_file(URI::Generic.from_path(path: "#{dummy_root}/app/controllers/application_controller.rb")) + + text_document_definition(server, { line: 0, character: 12 }, uri) + end + + assert_equal("file://#{dummy_root}/app/views/application/_partial.html.erb", response.first.uri) + ensure + FileUtils.rm_r("#{dummy_root}/app/views/application") + end + + test "handles template formats, variants and handlers" do + FileUtils.touch("#{dummy_root}/app/views/users/_partial.html.slim") + FileUtils.touch("#{dummy_root}/app/views/users/_partial.html+tablet.slim") + FileUtils.touch("#{dummy_root}/app/views/users/_partial.html+mobile.slim") + FileUtils.touch("#{dummy_root}/app/views/users/_partial.text.erb") + + uri = Kernel.URI("file://#{dummy_root}/app/views/users/render.html.erb") + source = <<~ERB + <%= render "partial" %> + <%= render "partial", formats: :html %> + <%= render "partial", formats: [:text] %> + <%= render "partial", handlers: :slim %> + <%= render "partial", handlers: [:erb] %> + <%= render "partial", variants: :mobile %> + <%= render "partial", variants: [:tablet, :mobile] %> + <%= render "partial", variants: :missing %> + ERB + + with_ready_server(source, uri) do |server| + response = text_document_definition(server, { line: 0, character: 12 }, uri) + assert_equal("file://#{dummy_root}/app/views/users/_partial.html.slim", response.first.uri) + + response = text_document_definition(server, { line: 1, character: 12 }, uri) + assert_equal("file://#{dummy_root}/app/views/users/_partial.html.slim", response.first.uri) + + response = text_document_definition(server, { line: 2, character: 12 }, uri) + assert_equal("file://#{dummy_root}/app/views/users/_partial.text.erb", response.first.uri) + + response = text_document_definition(server, { line: 3, character: 12 }, uri) + assert_equal("file://#{dummy_root}/app/views/users/_partial.html.slim", response.first.uri) + + response = text_document_definition(server, { line: 4, character: 12 }, uri) + assert_equal([], response) + + response = text_document_definition(server, { line: 5, character: 12 }, uri) + assert_equal("file://#{dummy_root}/app/views/users/_partial.html+mobile.slim", response.first.uri) + + response = text_document_definition(server, { line: 6, character: 12 }, uri) + assert_equal("file://#{dummy_root}/app/views/users/_partial.html+tablet.slim", response.first.uri) + + response = text_document_definition(server, { line: 7, character: 12 }, uri) + assert_equal("file://#{dummy_root}/app/views/users/_partial.html.slim", response.first.uri) + end + ensure + FileUtils.rm Dir["#{dummy_root}/app/views/users/_partial.*"] + end + private - def generate_definitions_for_source(source, position) - with_server(source) do |server, uri| - sleep(0.1) while RubyLsp::Addon.addons.first.instance_variable_get(:@rails_runner_client).is_a?(NullClient) + def generate_definitions_for_source(source, position, uri = Kernel.URI("file:///fake.rb")) + with_ready_server(source, uri) do |server| + text_document_definition(server, position, uri) + end + end - server.process_message( - id: 1, - method: "textDocument/definition", - params: { textDocument: { uri: uri }, position: position }, - ) + def text_document_definition(server, position, uri) + server.process_message( + id: 1, + method: "textDocument/definition", + params: { textDocument: { uri: uri }, position: position }, + ) + + result = pop_result(server) + result.response + end + + def with_ready_server(source, uri) + with_server(source, uri) do |server, uri| + sleep(0.1) while RubyLsp::Addon.addons.first.instance_variable_get(:@rails_runner_client).is_a?(NullClient) - result = pop_result(server) - result.response + yield server end end end From 1dd93704ea19bbb948c133b72c61565694f5503b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janko=20Marohni=C4=87?= Date: Fri, 31 Oct 2025 22:22:18 +0100 Subject: [PATCH 2/4] Make template lookup more robust by calling server Also add missing type annotations. --- lib/ruby_lsp/ruby_lsp_rails/code_lens.rb | 9 +- lib/ruby_lsp/ruby_lsp_rails/definition.rb | 119 ++++++++++-------- lib/ruby_lsp/ruby_lsp_rails/runner_client.rb | 32 ++++- lib/ruby_lsp/ruby_lsp_rails/server.rb | 48 +++++++ .../ruby_lsp_rails/support/inflections.rb | 8 +- test/ruby_lsp_rails/definition_test.rb | 50 ++------ test/ruby_lsp_rails/runner_client_test.rb | 15 +++ test/ruby_lsp_rails/server_test.rb | 85 +++++++++++++ 8 files changed, 266 insertions(+), 100 deletions(-) diff --git a/lib/ruby_lsp/ruby_lsp_rails/code_lens.rb b/lib/ruby_lsp/ruby_lsp_rails/code_lens.rb index bb25fcb4..a3a85bbb 100644 --- a/lib/ruby_lsp/ruby_lsp_rails/code_lens.rb +++ b/lib/ruby_lsp/ruby_lsp_rails/code_lens.rb @@ -194,7 +194,14 @@ def add_jump_to_view(node) action_name = node.name controller_name = underscore(class_name.delete_suffix("Controller")) - view_uris = Dir.glob("#{@client.views_dir}/#{controller_name}/#{action_name}*").filter_map do |path| + controller_info = @client.controller(class_name) + return unless controller_info + + view_paths = controller_info[:view_paths].select do |path| + path.start_with?(@client.rails_root) + end + + view_uris = Dir.glob("{#{view_paths.join(",")}}/#{controller_name}/#{action_name}*").filter_map do |path| # it's possible we could have a directory with the same name as the action, so we need to skip those next if File.directory?(path) diff --git a/lib/ruby_lsp/ruby_lsp_rails/definition.rb b/lib/ruby_lsp/ruby_lsp_rails/definition.rb index cc353ebf..52d31d25 100644 --- a/lib/ruby_lsp/ruby_lsp_rails/definition.rb +++ b/lib/ruby_lsp/ruby_lsp_rails/definition.rb @@ -1,8 +1,6 @@ # typed: strict # frozen_string_literal: true -require "pathname" - module RubyLsp module Rails # ![Definition demo](../../definition.gif) @@ -33,8 +31,8 @@ class Definition include Requests::Support::Common include Inflections - #: (RunnerClient client, RubyLsp::ResponseBuilders::CollectionResponseBuilder[(Interface::Location | Interface::LocationLink)] response_builder, NodeContext node_context, RubyIndexer::Index index, Prism::Dispatcher dispatcher) -> void - def initialize(client, response_builder, uri, node_context, index, dispatcher) + #: (RunnerClient client, RubyLsp::ResponseBuilders::CollectionResponseBuilder[(Interface::Location | Interface::LocationLink)] response_builder, URI::Generic uri, NodeContext node_context, RubyIndexer::Index index, Prism::Dispatcher dispatcher) -> void + def initialize(client, response_builder, uri, node_context, index, dispatcher) # rubocop:disable Metrics/ParameterLists @client = client @response_builder = response_builder @path = uri.to_standardized_path #: String? @@ -147,6 +145,7 @@ def handle_association(node) @response_builder << Support::LocationBuilder.line_location_from_s(result.fetch(:location)) end + #: (Prism::StringNode node) -> void def handle_possible_render(node) return unless @path&.end_with?(".html.erb") @@ -154,8 +153,7 @@ def handle_possible_render(node) return unless call_node return unless self_receiver?(call_node) - message = call_node.message - return unless message == "render" + return unless call_node.message == "render" arguments = call_node.arguments&.arguments return unless arguments @@ -163,29 +161,21 @@ def handle_possible_render(node) argument = view_template_argument(arguments, node) return unless argument - template = node.content - template_options = view_template_options(arguments) - - formats_pattern = template_options[:formats] ? "{#{template_options[:formats].join(",")}}" : "html" - variants_pattern = "{#{template_options[:variants].map { |variant| "+#{variant}" }.join(",")},}" if template_options[:variants] - handlers_pattern = template_options[:handlers] ? "{#{template_options[:handlers].join(",")}}" : "*" + controller_name = controller_for_template(@path) + return unless controller_name - extension_pattern = "#{formats_pattern}#{variants_pattern}.#{handlers_pattern}" + template_name = node.content + template_details = view_template_details(arguments) - template_pattern = if argument == "template" - File.join(@client.views_dir, "#{template}.#{extension_pattern}") - elsif template.include?("/") - *partial_dir, partial_name = template.split("/") - - File.join(@client.views_dir, *partial_dir, "_#{partial_name}.#{extension_pattern}") - else - File.join(@client.views_dir, "{#{view_prefixes.join(",")}}", "_#{template}.#{extension_pattern}") - end - - template_path = Dir.glob(template_pattern).first - return unless template_path + template = @client.find_template( + controller_name: controller_name, + template_name: template_name, + partial: argument != "template", + details: template_details, + ) + return unless template - @response_builder << Support::LocationBuilder.line_location_from_s("#{template_path}:1") + @response_builder << Support::LocationBuilder.line_location_from_s("#{template[:path]}:1") end #: (Prism::CallNode node) -> void @@ -241,48 +231,69 @@ def handle_if_unless_conditional(node, call_node, arguments) collect_definitions(method_name) end + #: (Array[Prism::Node] arguments, Prism::StringNode node) -> String? def view_template_argument(arguments, node) return "partial" if arguments.first == node - kwargs = arguments.find { |argument| argument.is_a?(Prism::KeywordHashNode) } - return unless kwargs + keyword_arguments = arguments.find { |argument| argument.is_a?(Prism::KeywordHashNode) } #: as Prism::KeywordHashNode? + return unless keyword_arguments - kwarg = kwargs.elements.find do |pair| - ["partial", "layout", "spacer_template", "template"].include?(pair.key.value) && pair.value == node - end + element = keyword_arguments.elements.find do |element| + next unless element.is_a?(Prism::AssocNode) + + key = element.key + next unless key.is_a?(Prism::SymbolNode) + + next unless element.value == node - kwarg&.key&.value + ["partial", "layout", "spacer_template", "template"].include?(key.value) + end #: as Prism::AssocNode? + + return unless element + + key = element.key #: as Prism::SymbolNode + key.value end - def view_template_options(arguments) - kwargs = arguments.find { |argument| argument.is_a?(Prism::KeywordHashNode) } - return {} unless kwargs + #: (Array[Prism::Node] arguments) -> Hash[String, (String | Array[String])] + def view_template_details(arguments) + keyword_arguments = arguments.find { |argument| argument.is_a?(Prism::KeywordHashNode) } #: as Prism::KeywordHashNode? + return {} unless keyword_arguments + + keyword_arguments.elements.each_with_object({}) do |element, options| + next unless element.is_a?(Prism::AssocNode) + + key = element.key + next unless key.is_a?(Prism::SymbolNode) - kwargs.elements.each_with_object({}) do |pair, options| - next unless ["formats", "variants", "handlers"].include?(pair.key.value) + key_value = key.value + next unless ["formats", "variants", "handlers"].include?(key_value) - value = [pair.value.value] if pair.value.is_a?(Prism::SymbolNode) - value = pair.value.elements.map(&:value) if pair.value.is_a?(Prism::ArrayNode) + value = element.value - options[pair.key.value.to_sym] = value + if value.is_a?(Prism::SymbolNode) + options[key_value] = value.value + elsif value.is_a?(Prism::ArrayNode) && value.elements.all?(Prism::SymbolNode) + elements = value.elements #: as Array[Prism::SymbolNode] + options[key_value] = elements.map(&:value) + end end end - # Resolve available directories from which the controller can render relative - # partials based on its ancestry chain. - def view_prefixes - controller_dir = Pathname(@path).dirname.relative_path_from(@client.views_dir).to_s - controller_class = "#{camelize(controller_dir)}Controller" - controller_ancestors = [controller_class] - - controller_entry = @index.resolve(controller_class, [])&.find(&:parent_class) - while controller_entry - controller_entry = @index.resolve(controller_entry.parent_class, controller_entry.nesting)&.find(&:parent_class) - break unless controller_entry && not_in_dependencies?(controller_entry.file_path) - controller_ancestors << controller_entry.name - end + #: (String template_path) -> String? + def controller_for_template(template_path) + controller_info = @client.controller("ActionController::Base") + return unless controller_info + + view_paths = controller_info[:view_paths] + template_directory = File.dirname(template_path) + + view_path = view_paths.find { |path| template_directory.start_with?(path + "/") } + return unless view_path + + controller_path = template_directory.delete_prefix(view_path + "/") - controller_ancestors.map { |ancestor| underscore(ancestor.delete_suffix("Controller")) } + camelize(controller_path) + "Controller" end end end diff --git a/lib/ruby_lsp/ruby_lsp_rails/runner_client.rb b/lib/ruby_lsp/ruby_lsp_rails/runner_client.rb index 73fe8559..5b73b887 100644 --- a/lib/ruby_lsp/ruby_lsp_rails/runner_client.rb +++ b/lib/ruby_lsp/ruby_lsp_rails/runner_client.rb @@ -159,6 +159,34 @@ def association_target(model_name:, association_name:) nil end + #: (String name) -> Hash[Symbol, untyped]? + def controller(name) + make_request("controller", name: name) + rescue MessageError + log_message( + "Ruby LSP Rails failed to get controller view paths", + type: RubyLsp::Constant::MessageType::ERROR, + ) + nil + end + + #: (controller_name: String, template_name: String, partial: bool, details: Hash[String, String | Array[String]]) -> Hash[Symbol, untyped]? + def find_template(controller_name:, template_name:, partial:, details:) + make_request( + "find_template", + controller_name: controller_name, + template_name: template_name, + partial: partial, + details: details, + ) + rescue MessageError + log_message( + "Ruby LSP Rails failed to find view template for controller", + type: RubyLsp::Constant::MessageType::ERROR, + ) + nil + end + #: (String name) -> Hash[Symbol, untyped]? def route_location(name) make_request("route_location", name: name) @@ -261,10 +289,6 @@ def connected? true end - def views_dir - File.join(@rails_root, "app/views") - end - private #: (String request, **untyped params) -> Hash[Symbol, untyped]? diff --git a/lib/ruby_lsp/ruby_lsp_rails/server.rb b/lib/ruby_lsp/ruby_lsp_rails/server.rb index 1fc3b9f4..3428e8ee 100644 --- a/lib/ruby_lsp/ruby_lsp_rails/server.rb +++ b/lib/ruby_lsp/ruby_lsp_rails/server.rb @@ -310,6 +310,14 @@ def execute(request, params) with_request_error_handling(request) do send_result(resolve_association_target(params)) end + when "controller" + with_request_error_handling(request) do + send_result(controller_info(params.fetch(:name))) + end + when "find_template" + with_request_error_handling(request) do + send_result(lookup_view_template(params)) + end when "pending_migrations_message" with_request_error_handling(request) do send_result({ pending_migrations_message: pending_migrations_message }) @@ -436,6 +444,36 @@ def resolve_association_target(params) nil end + #: (String controller_name) -> Hash[Symbol | String, untyped]? + def controller_info(controller_name) + const = ActiveSupport::Inflector.safe_constantize(controller_name) # rubocop:disable Sorbet/ConstantsFromStrings + return unless controller?(const) + + { view_paths: const.view_paths.map(&:to_s) } + end + + #: (Hash[Symbol | String, untyped]) -> Hash[Symbol | String, untyped]? + def lookup_view_template(params) + const = ActiveSupport::Inflector.safe_constantize(params[:controller_name]) # rubocop:disable Sorbet/ConstantsFromStrings + return unless controller?(const) + + path = params[:template_name] + partial = params[:partial] + details = params[:details].transform_values { |value| Array(value).map(&:to_sym) } + + lookup_context = const.new.lookup_context + prefixes = path.include?("/") || !partial ? [] : lookup_context.prefixes + + # Disable the lookup cache to ensure template changes are reflected + template = lookup_context.disable_cache do + lookup_context.find_template(path, prefixes, partial, [], details) + end + + { path: template.identifier } + rescue ActionView::MissingTemplate + nil + end + #: (Module?) -> bool def active_record_model?(const) !!( @@ -448,6 +486,16 @@ def active_record_model?(const) ) end + #: (Module?) -> bool + def controller?(const) + !!( + const && + defined?(ActionController) && + const.is_a?(Class) && + ActionController::Base >= const + ) + end + #: -> String? def pending_migrations_message # `check_all_pending!` is only available since Rails 7.1 diff --git a/lib/ruby_lsp/ruby_lsp_rails/support/inflections.rb b/lib/ruby_lsp/ruby_lsp_rails/support/inflections.rb index 9e3d15e9..12d76f47 100644 --- a/lib/ruby_lsp/ruby_lsp_rails/support/inflections.rb +++ b/lib/ruby_lsp/ruby_lsp_rails/support/inflections.rb @@ -4,15 +4,15 @@ module RubyLsp module Rails module Inflections - #: String -> String + #: (String) -> String def camelize(string) string - .gsub(/_([a-z])/) { $1.upcase } - .gsub(/(^|\/)[a-z]/) { $&.upcase } + .gsub(%r{(?:^|_|/)[a-z]}, &:upcase) + .tr("_", "") .gsub("/", "::") end - #: String -> String + #: (String) -> String def underscore(string) string .gsub(/([a-z])([A-Z])/, "\\1_\\2") diff --git a/test/ruby_lsp_rails/definition_test.rb b/test/ruby_lsp_rails/definition_test.rb index ec602301..d2f0d2d1 100644 --- a/test/ruby_lsp_rails/definition_test.rb +++ b/test/ruby_lsp_rails/definition_test.rb @@ -503,72 +503,48 @@ def name; end FileUtils.rm("#{dummy_root}/app/views/users/_partial.html.erb") end - test "searches template directories of controller ancestors" do - FileUtils.mkdir_p("#{dummy_root}/app/views/application") - FileUtils.touch("#{dummy_root}/app/views/application/_partial.html.erb") - - uri = Kernel.URI("file://#{dummy_root}/app/views/users/render.html.erb") - source = <<~ERB - <%= render "partial" %> - ERB - - response = with_ready_server(source, uri) do |server| - server.global_state.index.index_file(URI::Generic.from_path(path: "#{dummy_root}/app/controllers/users_controller.rb")) - server.global_state.index.index_file(URI::Generic.from_path(path: "#{dummy_root}/app/controllers/application_controller.rb")) - - text_document_definition(server, { line: 0, character: 12 }, uri) - end - - assert_equal("file://#{dummy_root}/app/views/application/_partial.html.erb", response.first.uri) - ensure - FileUtils.rm_r("#{dummy_root}/app/views/application") - end - test "handles template formats, variants and handlers" do - FileUtils.touch("#{dummy_root}/app/views/users/_partial.html.slim") - FileUtils.touch("#{dummy_root}/app/views/users/_partial.html+tablet.slim") - FileUtils.touch("#{dummy_root}/app/views/users/_partial.html+mobile.slim") + FileUtils.touch("#{dummy_root}/app/views/users/_partial.html.erb") FileUtils.touch("#{dummy_root}/app/views/users/_partial.text.erb") + FileUtils.touch("#{dummy_root}/app/views/users/_partial.html.ruby") + FileUtils.touch("#{dummy_root}/app/views/users/_partial.html+tablet.erb") + FileUtils.touch("#{dummy_root}/app/views/users/_partial.html+mobile.erb") uri = Kernel.URI("file://#{dummy_root}/app/views/users/render.html.erb") source = <<~ERB <%= render "partial" %> <%= render "partial", formats: :html %> <%= render "partial", formats: [:text] %> - <%= render "partial", handlers: :slim %> + <%= render "partial", handlers: :ruby %> <%= render "partial", handlers: [:erb] %> <%= render "partial", variants: :mobile %> <%= render "partial", variants: [:tablet, :mobile] %> - <%= render "partial", variants: :missing %> ERB with_ready_server(source, uri) do |server| response = text_document_definition(server, { line: 0, character: 12 }, uri) - assert_equal("file://#{dummy_root}/app/views/users/_partial.html.slim", response.first.uri) + assert_equal("file://#{dummy_root}/app/views/users/_partial.html.erb", response.first.uri) response = text_document_definition(server, { line: 1, character: 12 }, uri) - assert_equal("file://#{dummy_root}/app/views/users/_partial.html.slim", response.first.uri) + assert_equal("file://#{dummy_root}/app/views/users/_partial.html.erb", response.first.uri) response = text_document_definition(server, { line: 2, character: 12 }, uri) assert_equal("file://#{dummy_root}/app/views/users/_partial.text.erb", response.first.uri) response = text_document_definition(server, { line: 3, character: 12 }, uri) - assert_equal("file://#{dummy_root}/app/views/users/_partial.html.slim", response.first.uri) + assert_equal("file://#{dummy_root}/app/views/users/_partial.html.ruby", response.first.uri) response = text_document_definition(server, { line: 4, character: 12 }, uri) - assert_equal([], response) + assert_equal("file://#{dummy_root}/app/views/users/_partial.html.erb", response.first.uri) response = text_document_definition(server, { line: 5, character: 12 }, uri) - assert_equal("file://#{dummy_root}/app/views/users/_partial.html+mobile.slim", response.first.uri) + assert_equal("file://#{dummy_root}/app/views/users/_partial.html+mobile.erb", response.first.uri) response = text_document_definition(server, { line: 6, character: 12 }, uri) - assert_equal("file://#{dummy_root}/app/views/users/_partial.html+tablet.slim", response.first.uri) - - response = text_document_definition(server, { line: 7, character: 12 }, uri) - assert_equal("file://#{dummy_root}/app/views/users/_partial.html.slim", response.first.uri) + assert_equal("file://#{dummy_root}/app/views/users/_partial.html+tablet.erb", response.first.uri) end ensure - FileUtils.rm Dir["#{dummy_root}/app/views/users/_partial.*"] + FileUtils.rm(Dir["#{dummy_root}/app/views/users/_partial.*"]) end private @@ -591,7 +567,7 @@ def text_document_definition(server, position, uri) end def with_ready_server(source, uri) - with_server(source, uri) do |server, uri| + with_server(source, uri) do |server| sleep(0.1) while RubyLsp::Addon.addons.first.instance_variable_get(:@rails_runner_client).is_a?(NullClient) yield server diff --git a/test/ruby_lsp_rails/runner_client_test.rb b/test/ruby_lsp_rails/runner_client_test.rb index e9533e62..86b39014 100644 --- a/test/ruby_lsp_rails/runner_client_test.rb +++ b/test/ruby_lsp_rails/runner_client_test.rb @@ -66,6 +66,21 @@ class RunnerClientTest < ActiveSupport::TestCase assert_match(%r{db/schema\.rb$}, response.fetch(:schema_file)) end + test "#controller returns information for requested controller" do + response = @client.controller("ActionController::Base") #: as !nil + assert_equal("#{dummy_root}/app/views", response.fetch(:view_paths).first) + end + + test "#find_template returns resolved template path" do + response = @client.find_template( + controller_name: "UsersController", + template_name: "users/index", + partial: false, + details: {}, + ) #: as !nil + assert_equal("#{dummy_root}/app/views/users/index.html.erb", response.fetch(:path)) + end + test "returns nil if the request returns a nil response" do assert_nil @client.model("ApplicationRecord") # ApplicationRecord is abstract end diff --git a/test/ruby_lsp_rails/server_test.rb b/test/ruby_lsp_rails/server_test.rb index ecd2e02b..439d9ebb 100644 --- a/test/ruby_lsp_rails/server_test.rb +++ b/test/ruby_lsp_rails/server_test.rb @@ -53,6 +53,91 @@ def <(other) ActiveRecord::Tasks::DatabaseTasks.send(:alias_method, :schema_dump_path, :old_schema_dump_path) end + test "returns views paths for requested controller" do + @server.execute("controller", { name: "ActionController::Base" }) + assert_equal("#{dummy_root}/app/views", response.fetch(:result)[:view_paths].first) + end + + test "returns nil for API controller" do + @server.execute("controller", { name: "ActionController::API" }) + assert_nil(response.fetch(:result)) + end + + test "resolves relative view partial path" do + FileUtils.touch("#{dummy_root}/app/views/users/_user.html.erb") + @server.execute("find_template", { + controller_name: "UsersController", + template_name: "user", + partial: true, + details: {}, + }) + assert_equal("#{dummy_root}/app/views/users/_user.html.erb", response.fetch(:result)[:path]) + ensure + FileUtils.rm("#{dummy_root}/app/views/users/_user.html.erb") + end + + test "resolves absolute view partial path" do + FileUtils.touch("#{dummy_root}/app/views/users/_user.html.erb") + @server.execute("find_template", { + controller_name: "UsersController", + template_name: "users/user", + partial: true, + details: {}, + }) + assert_equal("#{dummy_root}/app/views/users/_user.html.erb", response.fetch(:result)[:path]) + ensure + FileUtils.rm("#{dummy_root}/app/views/users/_user.html.erb") + end + + test "resolves view template path" do + @server.execute("find_template", { + controller_name: "UsersController", + template_name: "users/index", + partial: false, + details: {}, + }) + assert_equal("#{dummy_root}/app/views/users/index.html.erb", response.fetch(:result)[:path]) + end + + test "searches template directories of controller ancestors" do + FileUtils.mkdir_p("#{dummy_root}/app/views/application") + FileUtils.touch("#{dummy_root}/app/views/application/_partial.html.erb") + @server.execute("find_template", { + controller_name: "UsersController", + template_name: "partial", + partial: true, + details: {}, + }) + assert_equal("#{dummy_root}/app/views/application/_partial.html.erb", response.fetch(:result)[:path]) + ensure + FileUtils.rm_r("#{dummy_root}/app/views/application") + end + + test "applies view template details" do + FileUtils.touch("#{dummy_root}/app/views/users/_user.html.erb") + FileUtils.touch("#{dummy_root}/app/views/users/_user.text.erb") + @server.execute("find_template", { + controller_name: "UsersController", + template_name: "user", + partial: true, + details: { formats: "text" }, + }) + assert_equal("#{dummy_root}/app/views/users/_user.text.erb", response.fetch(:result)[:path]) + ensure + FileUtils.rm("#{dummy_root}/app/views/users/_user.html.erb") + FileUtils.rm("#{dummy_root}/app/views/users/_user.text.erb") + end + + test "returns nil for missing view template" do + @server.execute("find_template", { + controller_name: "UsersController", + template_name: "users/missing", + partial: false, + details: {}, + }) + assert_nil(response.fetch(:result)) + end + test "resolve association returns the location of the target class of a has_many association" do @server.execute( "association_target", From 79213e302306d0fa0d86fec6f4f1063adb53eecd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janko=20Marohni=C4=87?= Date: Sun, 2 Nov 2025 20:59:27 +0100 Subject: [PATCH 3/4] Handle custom view paths of controller subclasses While here, we undo changes in code lens to handle custom view paths. If this approach is accepted, we can always update the code lens later. --- lib/ruby_lsp/ruby_lsp_rails/code_lens.rb | 9 +------ lib/ruby_lsp/ruby_lsp_rails/definition.rb | 29 +++++++++++++++-------- test/ruby_lsp_rails/definition_test.rb | 21 ++++++++++++++++ 3 files changed, 41 insertions(+), 18 deletions(-) diff --git a/lib/ruby_lsp/ruby_lsp_rails/code_lens.rb b/lib/ruby_lsp/ruby_lsp_rails/code_lens.rb index a3a85bbb..2012b21c 100644 --- a/lib/ruby_lsp/ruby_lsp_rails/code_lens.rb +++ b/lib/ruby_lsp/ruby_lsp_rails/code_lens.rb @@ -194,14 +194,7 @@ def add_jump_to_view(node) action_name = node.name controller_name = underscore(class_name.delete_suffix("Controller")) - controller_info = @client.controller(class_name) - return unless controller_info - - view_paths = controller_info[:view_paths].select do |path| - path.start_with?(@client.rails_root) - end - - view_uris = Dir.glob("{#{view_paths.join(",")}}/#{controller_name}/#{action_name}*").filter_map do |path| + view_uris = Dir.glob("#{@client.rails_root}/app/views/#{controller_name}/#{action_name}*").filter_map do |path| # it's possible we could have a directory with the same name as the action, so we need to skip those next if File.directory?(path) diff --git a/lib/ruby_lsp/ruby_lsp_rails/definition.rb b/lib/ruby_lsp/ruby_lsp_rails/definition.rb index 52d31d25..48165583 100644 --- a/lib/ruby_lsp/ruby_lsp_rails/definition.rb +++ b/lib/ruby_lsp/ruby_lsp_rails/definition.rb @@ -1,6 +1,8 @@ # typed: strict # frozen_string_literal: true +require "pathname" + module RubyLsp module Rails # ![Definition demo](../../definition.gif) @@ -280,20 +282,27 @@ def view_template_details(arguments) end end + # Determine controller name for given template path by matching segments + # of its directory path to controller paths and checking if the controllers' + # view paths complete the rest of the template directory path. + # #: (String template_path) -> String? def controller_for_template(template_path) - controller_info = @client.controller("ActionController::Base") - return unless controller_info - - view_paths = controller_info[:view_paths] - template_directory = File.dirname(template_path) - - view_path = view_paths.find { |path| template_directory.start_with?(path + "/") } - return unless view_path + template_directory = Pathname(template_path).dirname.relative_path_from(@client.rails_root) + directory_segments = template_directory.each_filename.to_a + possible_controller_paths = (1..directory_segments.count).map do |n| + directory_segments.last(n).join("/") + end - controller_path = template_directory.delete_prefix(view_path + "/") + controller_path = possible_controller_paths.find do |controller_path| + controller_name = camelize(controller_path) + "Controller" + view_paths = @client.controller(controller_name)&.dig(:view_paths) || [] + view_paths.any? do |view_path| + File.join(view_path, controller_path) == File.dirname(template_path) + end + end - camelize(controller_path) + "Controller" + camelize(controller_path) + "Controller" if controller_path end end end diff --git a/test/ruby_lsp_rails/definition_test.rb b/test/ruby_lsp_rails/definition_test.rb index d2f0d2d1..7b1fc35e 100644 --- a/test/ruby_lsp_rails/definition_test.rb +++ b/test/ruby_lsp_rails/definition_test.rb @@ -503,6 +503,27 @@ def name; end FileUtils.rm("#{dummy_root}/app/views/users/_partial.html.erb") end + test "handles custom view paths" do + FileUtils.mkdir_p("#{dummy_root}/app/custom/views/admin") + FileUtils.touch("#{dummy_root}/app/custom/views/admin/_partial.html.erb") + File.write("#{dummy_root}/app/controllers/admin_controller.rb", <<~RUBY) + class AdminController < ApplicationController + prepend_view_path "#{dummy_root}/app/custom/views" + end + RUBY + + uri = Kernel.URI("file://#{dummy_root}/app/custom/views/admin/render.html.erb") + source = <<~ERB + <%= render "partial" %> + ERB + + response = generate_definitions_for_source(source, { line: 0, character: 12 }, uri) + assert_equal("file://#{dummy_root}/app/custom/views/admin/_partial.html.erb", response.first.uri) + ensure + FileUtils.rm_r("#{dummy_root}/app/custom/views/admin") + FileUtils.rm("#{dummy_root}/app/controllers/admin_controller.rb") + end + test "handles template formats, variants and handlers" do FileUtils.touch("#{dummy_root}/app/views/users/_partial.html.erb") FileUtils.touch("#{dummy_root}/app/views/users/_partial.text.erb") From a5ca7859bb1982f427bce39fb29b454534aaf6a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janko=20Marohni=C4=87?= Date: Mon, 10 Nov 2025 14:24:45 +0100 Subject: [PATCH 4/4] Handle render calls from controllerless template directories In our app, we have an `app/views/shared/components` directory that contains shared "component" partials. It doesn't have any matching controller, and components can render other components, you just need to specify the full path to the render call (we have a helper method for that). We should make go to definition work for these cases as well. --- lib/ruby_lsp/ruby_lsp_rails/definition.rb | 6 +++++- lib/ruby_lsp/ruby_lsp_rails/server.rb | 2 +- test/ruby_lsp_rails/definition_test.rb | 15 +++++++++++++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/lib/ruby_lsp/ruby_lsp_rails/definition.rb b/lib/ruby_lsp/ruby_lsp_rails/definition.rb index 48165583..ac245a9d 100644 --- a/lib/ruby_lsp/ruby_lsp_rails/definition.rb +++ b/lib/ruby_lsp/ruby_lsp_rails/definition.rb @@ -302,7 +302,11 @@ def controller_for_template(template_path) end end - camelize(controller_path) + "Controller" if controller_path + if controller_path + camelize(controller_path) + "Controller" + else + "ActionController::Base" + 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 3428e8ee..1fe3689e 100644 --- a/lib/ruby_lsp/ruby_lsp_rails/server.rb +++ b/lib/ruby_lsp/ruby_lsp_rails/server.rb @@ -462,7 +462,7 @@ def lookup_view_template(params) details = params[:details].transform_values { |value| Array(value).map(&:to_sym) } lookup_context = const.new.lookup_context - prefixes = path.include?("/") || !partial ? [] : lookup_context.prefixes + prefixes = path.include?("/") || !partial || const.abstract? ? [] : lookup_context.prefixes # Disable the lookup cache to ensure template changes are reflected template = lookup_context.disable_cache do diff --git a/test/ruby_lsp_rails/definition_test.rb b/test/ruby_lsp_rails/definition_test.rb index 7b1fc35e..3acc4825 100644 --- a/test/ruby_lsp_rails/definition_test.rb +++ b/test/ruby_lsp_rails/definition_test.rb @@ -524,6 +524,21 @@ class AdminController < ApplicationController FileUtils.rm("#{dummy_root}/app/controllers/admin_controller.rb") end + test "handles template directories not matching any controller path" do + FileUtils.mkdir_p("#{dummy_root}/app/views/components") + FileUtils.touch("#{dummy_root}/app/views/components/_foo.html.erb") + + uri = Kernel.URI("file://#{dummy_root}/app/views/components/_bar.html.erb") + source = <<~ERB + <%= render "components/foo" %> + ERB + + response = generate_definitions_for_source(source, { line: 0, character: 12 }, uri) + assert_equal("file://#{dummy_root}/app/views/components/_foo.html.erb", response.first.uri) + ensure + FileUtils.rm_r("#{dummy_root}/app/views/components") + end + test "handles template formats, variants and handlers" do FileUtils.touch("#{dummy_root}/app/views/users/_partial.html.erb") FileUtils.touch("#{dummy_root}/app/views/users/_partial.text.erb")