Skip to content

Add Definition support for routes #331

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

Merged
merged 2 commits into from
May 1, 2024
Merged
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
4 changes: 1 addition & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,8 @@ jobs:
gemfile:
- Gemfile
- gemfiles/Gemfile-rails-main
ruby: ["3.0", "3.1", "3.2", "3.3", "head"]
ruby: ["3.0", "3.1", "3.2", "3.3"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems that the behaviour added in https://github.com/rails/rails/pull/47877/files is not working properly with Ruby head. Since Rails itself doesn’t test against Ruby head, I think it’s reasonable if we don’t either, and wait until this is fixed upstream.

include:
- ruby: "head"
experimental: true
- gemfile: "gemfiles/Gemfile-rails-main"
experimental: true
exclude:
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Ruby LSP Rails is a [Ruby LSP](https://github.com/Shopify/ruby-lsp) addon for ex
* Run or debug a test by clicking on the code lens which appears above the test class, or an individual test.
* Navigate to associations, validations, callbacks and test cases using your editor's "Go to Symbol" feature, or outline view.
* Jump to the definition of callbacks using your editor's "Go to Definition" feature.
* Jump to the declaration of a route.

## Installation

Expand Down
4 changes: 2 additions & 2 deletions lib/ruby_lsp/ruby_lsp_rails/addon.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def activate(global_state, message_queue)
@global_state = T.let(global_state, T.nilable(RubyLsp::GlobalState))
$stderr.puts("Activating Ruby LSP Rails addon v#{VERSION}")
# Start booting the real client in a background thread. Until this completes, the client will be a NullClient
Thread.new { @client = RunnerClient.create_client }
Thread.new { @client = RunnerClient.create_client }.join
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(corrected in #356)

end

sig { override.void }
Expand Down Expand Up @@ -84,7 +84,7 @@ def create_document_symbol_listener(response_builder, dispatcher)
end
def create_definition_listener(response_builder, uri, nesting, dispatcher)
index = T.must(@global_state).index
Definition.new(response_builder, nesting, index, dispatcher)
Definition.new(@client, response_builder, nesting, index, dispatcher)
end

sig { params(changes: T::Array[{ uri: String, type: Integer }]).void }
Expand Down
41 changes: 38 additions & 3 deletions lib/ruby_lsp/ruby_lsp_rails/definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,25 +11,35 @@ module Rails
#
# Currently supported targets:
# - Callbacks
# - Named routes (e.g. `users_path`)
#
# # Example
#
# ```ruby
# before_action :foo # <- Go to definition on this symbol will jump to the method if it is defined in the same class
# ```
#
# Notes for named routes:
# - It is available only in Rails 7.1 or newer.
# - Route may be defined across multiple files, e.g. using `draw`, rather than in `routes.rb`.
# - Routes won't be found if not defined for the Rails development environment.
# - If using `constraints`, the route can only be found if the constraints are met.
# - Changes to routes won't be picked up until the server is restarted.
class Definition
extend T::Sig
include Requests::Support::Common

sig do
params(
client: RunnerClient,
response_builder: ResponseBuilders::CollectionResponseBuilder[Interface::Location],
nesting: T::Array[String],
index: RubyIndexer::Index,
dispatcher: Prism::Dispatcher,
).void
end
def initialize(response_builder, nesting, index, dispatcher)
def initialize(client, response_builder, nesting, index, dispatcher)
@client = client
@response_builder = response_builder
@nesting = nesting
@index = index
Expand All @@ -43,8 +53,19 @@ def on_call_node_enter(node)

message = node.message

return unless message && Support::Callbacks::ALL.include?(message)
return unless message

if Support::Callbacks::ALL.include?(message)
handle_callback(node)
elsif message.end_with?("_path") || message.end_with?("_url")
handle_route(node)
end
end

private

sig { params(node: Prism::CallNode).void }
def handle_callback(node)
arguments = node.arguments&.arguments
return unless arguments&.any?

Expand All @@ -62,7 +83,21 @@ def on_call_node_enter(node)
end
end

private
sig { params(node: Prism::CallNode).void }
def handle_route(node)
result = @client.route_location(T.must(node.message))
return unless result

file_path, line = result.fetch(:location).split(":")

@response_builder << Interface::Location.new(
uri: URI::Generic.from_path(path: file_path).to_s,
range: Interface::Range.new(
start: Interface::Position.new(line: Integer(line) - 1, character: 0),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

in @tenderlove's prototype he gets the character position by looking for the first non-whitespace character on the line, but I'm not sure it's really necessary.

end: Interface::Position.new(line: Integer(line) - 1, character: 0),
),
)
end

sig { params(name: String).void }
def collect_definitions(name)
Expand Down
8 changes: 8 additions & 0 deletions lib/ruby_lsp/ruby_lsp_rails/runner_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,14 @@ def model(name)
nil
end

sig { params(name: String).returns(T.nilable(T::Hash[Symbol, T.untyped])) }
def route_location(name)
make_request("route_location", name: name)
rescue IncompleteMessageError
$stderr.puts("Ruby LSP Rails failed to get route location: #{@stderr.read}")
nil
end

sig { void }
def trigger_reload
$stderr.puts("Reloading Rails application")
Expand Down
30 changes: 30 additions & 0 deletions lib/ruby_lsp/ruby_lsp_rails/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ def execute(request, params)
when "reload"
::Rails.application.reloader.reload!
VOID
when "route_location"
route_location(params.fetch(:name))
else
VOID
end
Expand All @@ -53,6 +55,34 @@ def execute(request, params)

private

# Older versions of Rails don't support `route_source_locations`.
# We also check that it's enabled.
if ActionDispatch::Routing::Mapper.respond_to?(:route_source_locations) &&
ActionDispatch::Routing::Mapper.route_source_locations
def route_location(name)
match_data = name.match(/^(.+)(_path|_url)$/)
return { result: nil } unless match_data

key = match_data[1]

# A token could match the _path or _url pattern, but not be an actual route.
route = ::Rails.application.routes.named_routes.get(key)
return { result: nil } unless route&.source_location

{
result: {
location: ::Rails.root.join(route.source_location).to_s,
},
}
rescue => e
{ error: e.full_message(highlight: false) }
end
else
def route_location(name)
{ result: nil }
end
end

def resolve_database_info_from_model(model_name)
const = ActiveSupport::Inflector.safe_constantize(model_name)
unless active_record_model?(const)
Expand Down
7 changes: 7 additions & 0 deletions test/dummy/app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
# frozen_string_literal: true

class ApplicationController < ActionController::Base
def create
user_path(1)
user_url(1)
users_path
archive_users_path
invalid_path
end
end
5 changes: 5 additions & 0 deletions test/dummy/config/initializers/action_dispatch.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# typed: true
# frozen_string_literal: true

# Route source locations are normally only available in development, so we need to enable this in test mode.
ActionDispatch::Routing::Mapper.route_source_locations = true
3 changes: 3 additions & 0 deletions test/dummy/config/routes.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
# frozen_string_literal: true

Rails.application.routes.draw do
resources :users do
get :archive, on: :collection
end
end
40 changes: 40 additions & 0 deletions test/ruby_lsp_rails/definition_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,46 @@ def baz; end
assert_equal(14, response[1].range.end.character)
end

test "provides the definition of a route" do
response = generate_definitions_for_source(<<~RUBY, { line: 0, character: 0 })
users_path
RUBY

assert_equal(1, response.size)
dummy_root = File.expand_path("../dummy", __dir__)
assert_equal("file://#{dummy_root}/config/routes.rb", response[0].uri)
assert_equal(3, response[0].range.start.line)
assert_equal(3, response[0].range.end.line)
end

test "handles incomplete routes" do
response = generate_definitions_for_source(<<~RUBY, { line: 0, character: 0 })
_path
RUBY

assert_empty(response)
end

test "provides the definition of a custom route" do
response = generate_definitions_for_source(<<~RUBY, { line: 0, character: 0 })
archive_users_path
RUBY

assert_equal(1, response.size)
dummy_root = File.expand_path("../dummy", __dir__)
assert_equal("file://#{dummy_root}/config/routes.rb", response[0].uri)
assert_equal(4, response[0].range.start.line)
assert_equal(4, response[0].range.end.line)
end

test "ignored non-existing routes" do
response = generate_definitions_for_source(<<~RUBY, { line: 0, character: 0 })
invalid_path
RUBY

assert_empty(response)
end

private

def generate_definitions_for_source(source, position)
Expand Down
11 changes: 11 additions & 0 deletions test/ruby_lsp_rails/server_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,15 @@ def <(other)
ensure
ActiveRecord::Tasks::DatabaseTasks.send(:alias_method, :schema_dump_path, :old_schema_dump_path)
end

test "route location returns the location for a valid route" do
response = @server.execute("route_location", { name: "user_path" })
location = response[:result][:location]
assert_match %r{test/dummy/config/routes.rb:4$}, location
end

test "route location returns nil for invalid routes" do
response = @server.execute("route_location", { name: "invalid_path" })
assert_nil response[:result]
end
end