From c07b2d0cc8efd8c97712ab2f1349a71a3d5b7da8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Kondzior?= Date: Mon, 13 Oct 2025 15:28:23 +0200 Subject: [PATCH 1/5] Improve handling of class variables, aliases and constant aliases --- lib/ruby_indexer/lib/ruby_indexer/index.rb | 40 ++++++++++++++-- lib/ruby_lsp/requests/support/common.rb | 13 +++-- test/requests/workspace_symbol_test.rb | 55 ++++++++++++++++++++++ 3 files changed, 101 insertions(+), 7 deletions(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/index.rb b/lib/ruby_indexer/lib/ruby_indexer/index.rb index bc36927534..118f73660f 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/index.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/index.rb @@ -198,13 +198,14 @@ def prefix_search(query, nesting = nil) #: (String? query) -> Array[Entry] def fuzzy_search(query) unless query - entries = @entries.filter_map do |_name, entries| + entries = @entries.values.filter_map do |entries| next if entries.first.is_a?(Entry::SingletonClass) entries end - - return entries.flatten + return entries.flat_map do |entries| + skip_unresolved_entries(entries) + end end normalized_query = query.gsub("::", "").downcase @@ -216,7 +217,9 @@ def fuzzy_search(query) [entries, -similarity] if similarity > ENTRY_SIMILARITY_THRESHOLD end results.sort_by!(&:last) - results.flat_map(&:first) + results.flat_map do |entries, _similarity| + skip_unresolved_entries(entries) + end end #: (String? name, String receiver_name) -> Array[(Entry::Member | Entry::MethodAlias)] @@ -1065,5 +1068,34 @@ def resolve_method_alias(entry, receiver_name, seen_names) original_entries << resolved_alias resolved_alias end + + # Attempts to resolve an entry if it is an unresolved constant or method alias. + # Returns the resolved entry, or nil if it cannot be resolved. + #: (Entry) -> Entry? + def resolve_entry(entry) + case entry + when Entry::UnresolvedConstantAlias + resolved_entry = resolve_alias(entry, []) + resolved_entry unless resolved_entry.is_a?(Entry::UnresolvedConstantAlias) + when Entry::UnresolvedMethodAlias + resolved_entry = resolve_method_alias(entry, entry.owner&.name || "", []) + resolved_entry unless resolved_entry.is_a?(Entry::UnresolvedMethodAlias) + else + entry + end + end + + # Filters and resolves entries, skipping those that remain unresolved. + # Returns an array of successfully resolved entries. + #: (Array[Entry]) -> Array[Entry?] + def skip_unresolved_entries(entries) + entries.filter_map do |entry| + resolved_entry = resolve_entry(entry) + + next unless resolved_entry + + resolved_entry + end + end end end diff --git a/lib/ruby_lsp/requests/support/common.rb b/lib/ruby_lsp/requests/support/common.rb index 61f1824cd5..cc2ddc6b2c 100644 --- a/lib/ruby_lsp/requests/support/common.rb +++ b/lib/ruby_lsp/requests/support/common.rb @@ -140,21 +140,28 @@ def each_constant_path_part(node, &block) end end - #: (RubyIndexer::Entry entry) -> Integer? + #: (RubyIndexer::Entry entry) -> Integer def kind_for_entry(entry) case entry when RubyIndexer::Entry::Class Constant::SymbolKind::CLASS when RubyIndexer::Entry::Module Constant::SymbolKind::NAMESPACE - when RubyIndexer::Entry::Constant + when RubyIndexer::Entry::Constant, RubyIndexer::Entry::ConstantAlias Constant::SymbolKind::CONSTANT when RubyIndexer::Entry::Method entry.name == "initialize" ? Constant::SymbolKind::CONSTRUCTOR : Constant::SymbolKind::METHOD when RubyIndexer::Entry::Accessor Constant::SymbolKind::PROPERTY - when RubyIndexer::Entry::InstanceVariable + when RubyIndexer::Entry::InstanceVariable, RubyIndexer::Entry::ClassVariable Constant::SymbolKind::FIELD + when RubyIndexer::Entry::MethodAlias + Constant::SymbolKind::METHOD + else + # Kind shold be represented by one of + # [SymbolKind](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#symbolKind) variants + # $stderr.puts("Unknonw symbol kind #{entry.inspect}. Kind should be represented by one of SymbolKind enum variants. Please report this as a bug") + Constant::SymbolKind::NULL end end end diff --git a/test/requests/workspace_symbol_test.rb b/test/requests/workspace_symbol_test.rb index 0f683abb94..49f352c9c2 100644 --- a/test/requests/workspace_symbol_test.rb +++ b/test/requests/workspace_symbol_test.rb @@ -108,6 +108,61 @@ def bar; end assert_equal(RubyLsp::Constant::SymbolKind::PROPERTY, result&.kind) end + def test_returns_resolved_method_alias + @index.index_single(URI::Generic.from_path(path: "/fake.rb"), <<~RUBY) + class Foo + def test + end + alias whatever test + alias_method :bar, :to_a + alias_method "baz", "to_a" + end + RUBY + + result = RubyLsp::Requests::WorkspaceSymbol.new(@global_state, "whatever").perform.first + assert_equal("whatever", result&.name) + assert_equal(RubyLsp::Constant::SymbolKind::METHOD, result&.kind) + + result = RubyLsp::Requests::WorkspaceSymbol.new(@global_state, "bar").perform + assert_empty(result) + + result = RubyLsp::Requests::WorkspaceSymbol.new(@global_state, "bag").perform + assert_empty(result) + end + + def test_returns_resolved_constant_alias + @index.index_single(URI::Generic.from_path(path: "/fake.rb"), <<~RUBY) + OK = 'OK' + class Foo + BOK = OK + BAD = AD + end + RUBY + + result = RubyLsp::Requests::WorkspaceSymbol.new(@global_state, "OK").perform.first + assert_equal("OK", result&.name) + assert_equal(RubyLsp::Constant::SymbolKind::CONSTANT, result&.kind) + + result = RubyLsp::Requests::WorkspaceSymbol.new(@global_state, "Foo::OK").perform.first + assert_equal("Foo::BOK", result&.name) + assert_equal(RubyLsp::Constant::SymbolKind::CONSTANT, result&.kind) + + result = RubyLsp::Requests::WorkspaceSymbol.new(@global_state, "BAD").perform + assert_empty(result) + end + + def test_returns_class_variable + @index.index_single(URI::Generic.from_path(path: "/fake.rb"), <<~RUBY) + class Foo + @@test = '123' + end + RUBY + + result = RubyLsp::Requests::WorkspaceSymbol.new(@global_state, "test").perform.first + assert_equal("@@test", result&.name) + assert_equal(RubyLsp::Constant::SymbolKind::FIELD, result&.kind) + end + def test_returns_symbols_from_unsaved_files @index.index_single(URI("untitled:Untitled-1"), <<~RUBY) class Foo; end From 1f905773c4db7ab68e9707bd2a2a7c0cebb705ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Kondzior?= Date: Sun, 26 Oct 2025 10:27:33 +0100 Subject: [PATCH 2/5] Performance improvements --- lib/ruby_indexer/lib/ruby_indexer/entry.rb | 52 ++++++++++++++--- lib/ruby_indexer/lib/ruby_indexer/index.rb | 20 ++++++- lib/ruby_indexer/lib/ruby_indexer/uri.rb | 66 +++++++++++++++++----- lib/ruby_indexer/test/constant_test.rb | 51 ++++------------- lib/ruby_indexer/test/method_test.rb | 13 +++-- lib/ruby_indexer/test/uri_test.rb | 30 +++++----- lib/ruby_lsp/listeners/definition.rb | 6 +- lib/ruby_lsp/requests/workspace_symbol.rb | 8 +-- lib/ruby_lsp/server.rb | 3 + 9 files changed, 159 insertions(+), 90 deletions(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/entry.rb b/lib/ruby_indexer/lib/ruby_indexer/entry.rb index bc595a042a..24d318c617 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/entry.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/entry.rb @@ -41,9 +41,24 @@ def private? @visibility == :private end + #: -> bool + def resolved? + true + end + + #: -> bool + def in_dependencies? + @in_dependencies ||= file_path && ( + ::RubyLsp::BUNDLE_PATH && file_path.start_with?( + ::RubyLsp::BUNDLE_PATH, #: as !nil + ) || + file_path.start_with?(RbConfig::CONFIG["rubylibdir"]) + ) + end + #: -> String def file_name - if @uri.scheme == "untitled" + @file_name ||= if @uri.scheme == "untitled" @uri.opaque #: as !nil else File.basename( @@ -54,7 +69,7 @@ def file_name #: -> String? def file_path - @uri.full_path + @file_path ||= @uri.full_path end #: -> String @@ -373,6 +388,11 @@ def initialize(target, nesting, name, uri, location, comments) # rubocop:disable @target = target @nesting = nesting end + + #: -> Bool + def resolved? + false + end end # Alias represents a resolved alias, which points to an existing constant target @@ -386,11 +406,16 @@ def initialize(target, unresolved_alias) unresolved_alias.name, unresolved_alias.uri, unresolved_alias.location, - unresolved_alias.comments, + nil, ) @visibility = unresolved_alias.visibility @target = target + @unresolved_alias = unresolved_alias + end + + def comments + @comments ||= @unresolved_alias.comments end end @@ -439,6 +464,11 @@ def initialize(new_name, old_name, owner, uri, location, comments) # rubocop:dis @old_name = old_name @owner = owner end + + #: -> Bool + def resolved? + false + end end # A method alias is a resolved alias entry that points to the exact method target it refers to @@ -451,19 +481,25 @@ class MethodAlias < Entry #: ((Member | MethodAlias) target, UnresolvedMethodAlias unresolved_alias) -> void def initialize(target, unresolved_alias) - full_comments = +"Alias for #{target.name}\n" - full_comments << "#{unresolved_alias.comments}\n" - full_comments << target.comments - super( unresolved_alias.new_name, unresolved_alias.uri, unresolved_alias.location, - full_comments, + nil, ) @target = target @owner = unresolved_alias.owner #: Entry::Namespace? + @unresolved_alias = unresolved_alias + end + + #: -> String + def comments + @comments ||= <<~COMMENTS.chomp + Alias for #{@target.name} + #{@unresolved_alias.comments} + #{@target.comments} + COMMENTS end #: -> String diff --git a/lib/ruby_indexer/lib/ruby_indexer/index.rb b/lib/ruby_indexer/lib/ruby_indexer/index.rb index 118f73660f..fda3e1fc31 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/index.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/index.rb @@ -203,9 +203,7 @@ def fuzzy_search(query) entries end - return entries.flat_map do |entries| - skip_unresolved_entries(entries) - end + return entries.flatten end normalized_query = query.gsub("::", "").downcase @@ -389,6 +387,8 @@ def index_single(uri, source, collect_comments: true) require_path = uri.require_path @require_paths_tree.insert(require_path, uri) if require_path + process_unresolved_entries + indexing_errors = listener.indexing_errors.uniq indexing_errors.each { |error| $stderr.puts(error) } if indexing_errors.any? rescue SystemStackError => e @@ -734,6 +734,20 @@ def entries_for(uri, type = nil) entries&.grep(type) end + #: -> voide + def process_unresolved_entries + @entries.values.each do |entries| + entries.each do |entry| + case entry + when Entry::UnresolvedConstantAlias + resolve_alias(entry, []) + when Entry::UnresolvedMethodAlias + resolve_method_alias(entry, entry.owner&.name || "", []) + end + end + end + end + private # Always returns the linearized ancestors for the attached class, regardless of whether `name` refers to a singleton diff --git a/lib/ruby_indexer/lib/ruby_indexer/uri.rb b/lib/ruby_indexer/lib/ruby_indexer/uri.rb index 6eeff8b478..c05613547a 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/uri.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/uri.rb @@ -12,24 +12,37 @@ class Generic # NOTE: We also define this in the shim PARSER = const_defined?(:RFC2396_PARSER) ? RFC2396_PARSER : DEFAULT_PARSER + # This unsafe regex is the same one used in the URI::RFC2396_REGEXP class with the exception of the fact that we + # do not include colon as a safe character. VS Code URIs always escape colons and we need to ensure we do the + # same to avoid inconsistencies in our URIs, which are used to identify resources + UNSAFE_REGEX = %r{[^\-_.!~*'()a-zA-Z\d;/?@&=+$,\[\]]} + class << self #: (path: String, ?fragment: String?, ?scheme: String, ?load_path_entry: String?) -> URI::Generic - def from_path(path:, fragment: nil, scheme: "file", load_path_entry: nil) - # This unsafe regex is the same one used in the URI::RFC2396_REGEXP class with the exception of the fact that we - # do not include colon as a safe character. VS Code URIs always escape colons and we need to ensure we do the - # same to avoid inconsistencies in our URIs, which are used to identify resources - unsafe_regex = %r{[^\-_.!~*'()a-zA-Z\d;/?@&=+$,\[\]]} - + def from_win_path(path:, fragment: nil, scheme: "file", load_path_entry: nil) # On Windows, if the path begins with the disk name, we need to add a leading slash to make it a valid URI escaped_path = if /^[A-Z]:/i.match?(path) - PARSER.escape("/#{path}", unsafe_regex) + PARSER.escape("/#{path}", UNSAFE_REGEX) elsif path.start_with?("//?/") # Some paths on Windows start with "//?/". This is a special prefix that allows for long file paths - PARSER.escape(path.delete_prefix("//?"), unsafe_regex) + PARSER.escape(path.delete_prefix("//?"), UNSAFE_REGEX) else - PARSER.escape(path, unsafe_regex) + PARSER.escape(path, UNSAFE_REGEX) + end + + uri = build(scheme: scheme, path: escaped_path, fragment: fragment) + + if load_path_entry + uri.require_path = path.delete_prefix("#{load_path_entry}/").delete_suffix(".rb") end + uri + end + + #: (path: String, ?fragment: String?, ?scheme: String, ?load_path_entry: String?) -> URI::Generic + def from_unix_path(path:, fragment: nil, scheme: "file", load_path_entry: nil) + escaped_path = PARSER.escape(path, UNSAFE_REGEX) + uri = build(scheme: scheme, path: escaped_path, fragment: fragment) if load_path_entry @@ -38,6 +51,12 @@ def from_path(path:, fragment: nil, scheme: "file", load_path_entry: nil) uri end + + if Gem.win_platform? + alias_method :from_path, :from_win_path + else + alias_method :from_path, :from_unix_path + end end #: String? @@ -51,15 +70,18 @@ def add_require_path_from_load_entry(load_path_entry) self.require_path = path.delete_prefix("#{load_path_entry}/").delete_suffix(".rb") end - #: -> String? - def to_standardized_path + # On Windows, when we're getting the file system path back from the URI, we need to remove the leading forward + # slash + def to_standardized_win_path parsed_path = path + return unless parsed_path + # we can bail out parsing if there is nothing to unscape + return parsed_path unless parsed_path.match?(/%[0-9A-Fa-f]{2}/) + unescaped_path = PARSER.unescape(parsed_path) - # On Windows, when we're getting the file system path back from the URI, we need to remove the leading forward - # slash if %r{^/[A-Z]:}i.match?(unescaped_path) unescaped_path.delete_prefix("/") else @@ -67,6 +89,24 @@ def to_standardized_path end end + #: -> String? + def to_standardized_unix_path + unscapped_path = path + return unless unscapped_path + + # we can bail out parsing if there is nothing to unscape + return unscapped_path unless unscapped_path.match?(/%[0-9A-Fa-f]{2}/) + + PARSER.unescape(unscapped_path) + end + + + if Gem.win_platform? + alias_method :to_standardized_path, :to_standardized_win_path + else + alias_method :to_standardized_path, :to_standardized_unix_path + end + alias_method :full_path, :to_standardized_path end end diff --git a/lib/ruby_indexer/test/constant_test.rb b/lib/ruby_indexer/test/constant_test.rb index 7e70984b87..fbb9e3229b 100644 --- a/lib/ruby_indexer/test/constant_test.rb +++ b/lib/ruby_indexer/test/constant_test.rb @@ -207,10 +207,9 @@ module C SECOND = A::FIRST RUBY - unresolve_entry = @index["A::FIRST"]&.first #: as Entry::UnresolvedConstantAlias - assert_instance_of(Entry::UnresolvedConstantAlias, unresolve_entry) - assert_equal(["A"], unresolve_entry.nesting) - assert_equal("B::C", unresolve_entry.target) + resolved_entry = @index["A::FIRST"]&.first #: as Entry::ConstantAlias + assert_instance_of(Entry::ConstantAlias, resolved_entry) + assert_equal("A::B::C", resolved_entry.target) resolved_entry = @index.resolve("A::FIRST", [])&.first #: as Entry::ConstantAlias assert_instance_of(Entry::ConstantAlias, resolved_entry) @@ -233,12 +232,7 @@ module Other end RUBY - unresolve_entry = @index["A::ALIAS"]&.first #: as Entry::UnresolvedConstantAlias - assert_instance_of(Entry::UnresolvedConstantAlias, unresolve_entry) - assert_equal(["A"], unresolve_entry.nesting) - assert_equal("B", unresolve_entry.target) - - resolved_entry = @index.resolve("ALIAS", ["A"])&.first #: as Entry::ConstantAlias + resolved_entry = @index["A::ALIAS"]&.first #: as Entry::ConstantAlias assert_instance_of(Entry::ConstantAlias, resolved_entry) assert_equal("A::B", resolved_entry.target) @@ -246,10 +240,9 @@ module Other assert_instance_of(Entry::Module, resolved_entry) assert_equal("A::B::C", resolved_entry.name) - unresolve_entry = @index["Other::ONE_MORE"]&.first #: as Entry::UnresolvedConstantAlias - assert_instance_of(Entry::UnresolvedConstantAlias, unresolve_entry) - assert_equal(["Other"], unresolve_entry.nesting) - assert_equal("A::ALIAS", unresolve_entry.target) + resolved_entry = @index["Other::ONE_MORE"]&.first #: as Entry::ConstantAlias + assert_instance_of(Entry::ConstantAlias, resolved_entry) + assert_equal("A::ALIAS", resolved_entry.target) resolved_entry = @index.resolve("Other::ONE_MORE::C", [])&.first assert_instance_of(Entry::Module, resolved_entry) @@ -266,12 +259,7 @@ module A RUBY # B and C - unresolve_entry = @index["A::B"]&.first #: as Entry::UnresolvedConstantAlias - assert_instance_of(Entry::UnresolvedConstantAlias, unresolve_entry) - assert_equal(["A"], unresolve_entry.nesting) - assert_equal("C", unresolve_entry.target) - - resolved_entry = @index.resolve("A::B", [])&.first #: as Entry::ConstantAlias + resolved_entry = @index["A::B"]&.first #: as Entry::ConstantAlias assert_instance_of(Entry::ConstantAlias, resolved_entry) assert_equal("A::C", resolved_entry.target) @@ -279,32 +267,17 @@ module A assert_instance_of(Entry::Constant, constant) # D and E - unresolve_entry = @index["A::D"]&.first #: as Entry::UnresolvedConstantAlias - assert_instance_of(Entry::UnresolvedConstantAlias, unresolve_entry) - assert_equal(["A"], unresolve_entry.nesting) - assert_equal("E", unresolve_entry.target) - - resolved_entry = @index.resolve("A::D", [])&.first #: as Entry::ConstantAlias + resolved_entry = @index["A::D"]&.first #: as Entry::ConstantAlias assert_instance_of(Entry::ConstantAlias, resolved_entry) assert_equal("A::E", resolved_entry.target) # F and G::H - unresolve_entry = @index["A::F"]&.first #: as Entry::UnresolvedConstantAlias - assert_instance_of(Entry::UnresolvedConstantAlias, unresolve_entry) - assert_equal(["A"], unresolve_entry.nesting) - assert_equal("G::H", unresolve_entry.target) - - resolved_entry = @index.resolve("A::F", [])&.first #: as Entry::ConstantAlias + resolved_entry = @index["A::F"]&.first #: as Entry::ConstantAlias assert_instance_of(Entry::ConstantAlias, resolved_entry) assert_equal("A::G::H", resolved_entry.target) # I::J, K::L and M - unresolve_entry = @index["A::I::J"]&.first #: as Entry::UnresolvedConstantAlias - assert_instance_of(Entry::UnresolvedConstantAlias, unresolve_entry) - assert_equal(["A"], unresolve_entry.nesting) - assert_equal("K::L", unresolve_entry.target) - - resolved_entry = @index.resolve("A::I::J", [])&.first #: as Entry::ConstantAlias + resolved_entry = @index["A::I::J"]&.first #: as Entry::ConstantAlias assert_instance_of(Entry::ConstantAlias, resolved_entry) assert_equal("A::K::L", resolved_entry.target) @@ -356,7 +329,7 @@ module Real assert_entry("A::D::E", Entry::UnresolvedConstantAlias, "/fake/path/foo.rb:2-2:2-6") assert_entry("A::F::G", Entry::Constant, "/fake/path/foo.rb:2-8:2-12") assert_entry("A::H", Entry::Constant, "/fake/path/foo.rb:3-2:3-3") - assert_entry("A::I::J", Entry::UnresolvedConstantAlias, "/fake/path/foo.rb:3-5:3-9") + assert_entry("A::I::J", Entry::ConstantAlias, "/fake/path/foo.rb:3-5:3-9") assert_entry("A::K", Entry::Constant, "/fake/path/foo.rb:4-2:4-3") assert_entry("A::L", Entry::Constant, "/fake/path/foo.rb:4-5:4-6") end diff --git a/lib/ruby_indexer/test/method_test.rb b/lib/ruby_indexer/test/method_test.rb index c188d75950..1ca4c88df1 100644 --- a/lib/ruby_indexer/test/method_test.rb +++ b/lib/ruby_indexer/test/method_test.rb @@ -575,9 +575,13 @@ def third_method; end assert_equal("Foo", entry.owner&.name) end - def test_keeps_track_of_aliases + def test_keeps_track_of_aliases_and_comments index(<<~RUBY) class Foo + # A comment + def to_s + end + # Whatever Comment alias whatever to_s alias_method :foo, :to_a alias_method "bar", "to_a" @@ -588,9 +592,10 @@ class Foo end RUBY - assert_entry("whatever", Entry::UnresolvedMethodAlias, "/fake/path/foo.rb:1-8:1-16") - assert_entry("foo", Entry::UnresolvedMethodAlias, "/fake/path/foo.rb:2-15:2-19") - assert_entry("bar", Entry::UnresolvedMethodAlias, "/fake/path/foo.rb:3-15:3-20") + assert_entry("whatever", Entry::MethodAlias, "/fake/path/foo.rb:5-8:5-16") + assert_equal("Alias for to_s\nWhatever Comment\nA comment", @index["whatever"]&.first&.comments) + assert_entry("foo", Entry::UnresolvedMethodAlias, "/fake/path/foo.rb:6-15:6-19") + assert_entry("bar", Entry::UnresolvedMethodAlias, "/fake/path/foo.rb:7-15:7-20") # Foo plus 3 valid aliases assert_equal(4, @index.length - @default_indexed_entries.length) end diff --git a/lib/ruby_indexer/test/uri_test.rb b/lib/ruby_indexer/test/uri_test.rb index 8c064a1a77..39600066a9 100644 --- a/lib/ruby_indexer/test/uri_test.rb +++ b/lib/ruby_indexer/test/uri_test.rb @@ -6,38 +6,38 @@ module RubyIndexer class URITest < Minitest::Test def test_from_path_on_unix - uri = URI::Generic.from_path(path: "/some/unix/path/to/file.rb") + uri = URI::Generic.from_unix_path(path: "/some/unix/path/to/file.rb") assert_equal("/some/unix/path/to/file.rb", uri.path) end def test_from_path_on_windows - uri = URI::Generic.from_path(path: "C:/some/windows/path/to/file.rb") + uri = URI::Generic.from_win_path(path: "C:/some/windows/path/to/file.rb") assert_equal("/C%3A/some/windows/path/to/file.rb", uri.path) end def test_from_path_on_windows_with_lowercase_drive - uri = URI::Generic.from_path(path: "c:/some/windows/path/to/file.rb") + uri = URI::Generic.from_win_path(path: "c:/some/windows/path/to/file.rb") assert_equal("/c%3A/some/windows/path/to/file.rb", uri.path) end def test_to_standardized_path_on_unix - uri = URI::Generic.from_path(path: "/some/unix/path/to/file.rb") - assert_equal(uri.path, uri.to_standardized_path) + uri = URI::Generic.from_unix_path(path: "/some/unix/path/to/file.rb") + assert_equal(uri.path, uri.to_standardized_win_path) end def test_to_standardized_path_on_windows - uri = URI::Generic.from_path(path: "C:/some/windows/path/to/file.rb") - assert_equal("C:/some/windows/path/to/file.rb", uri.to_standardized_path) + uri = URI::Generic.from_win_path(path: "C:/some/windows/path/to/file.rb") + assert_equal("C:/some/windows/path/to/file.rb", uri.to_standardized_win_path) end def test_to_standardized_path_on_windows_with_lowercase_drive - uri = URI::Generic.from_path(path: "c:/some/windows/path/to/file.rb") - assert_equal("c:/some/windows/path/to/file.rb", uri.to_standardized_path) + uri = URI::Generic.from_win_path(path: "c:/some/windows/path/to/file.rb") + assert_equal("c:/some/windows/path/to/file.rb", uri.to_standardized_win_path) end def test_to_standardized_path_on_windows_with_received_uri uri = URI("file:///c%3A/some/windows/path/to/file.rb") - assert_equal("c:/some/windows/path/to/file.rb", uri.to_standardized_path) + assert_equal("c:/some/windows/path/to/file.rb", uri.to_standardized_win_path) end def test_plus_signs_are_properly_unescaped @@ -52,8 +52,8 @@ def test_from_path_with_fragment end def test_from_path_windows_long_file_paths - uri = URI::Generic.from_path(path: "//?/C:/hostedtoolcache/windows/Ruby/3.3.1/x64/lib/ruby/3.3.0/open-uri.rb") - assert_equal("C:/hostedtoolcache/windows/Ruby/3.3.1/x64/lib/ruby/3.3.0/open-uri.rb", uri.to_standardized_path) + uri = URI::Generic.from_win_path(path: "//?/C:/hostedtoolcache/windows/Ruby/3.3.1/x64/lib/ruby/3.3.0/open-uri.rb") + assert_equal("C:/hostedtoolcache/windows/Ruby/3.3.1/x64/lib/ruby/3.3.0/open-uri.rb", uri.to_standardized_win_path) end def test_from_path_computes_require_path_when_load_path_entry_is_given @@ -70,14 +70,14 @@ def test_allows_adding_require_path_with_load_path_entry end def test_from_path_escapes_colon_characters - uri = URI::Generic.from_path(path: "c:/some/windows/path with/spaces/file.rb") - assert_equal("c:/some/windows/path with/spaces/file.rb", uri.to_standardized_path) + uri = URI::Generic.from_win_path(path: "c:/some/windows/path with/spaces/file.rb") + assert_equal("c:/some/windows/path with/spaces/file.rb", uri.to_standardized_win_path) assert_equal("file:///c%3A/some/windows/path%20with/spaces/file.rb", uri.to_s) end def test_from_path_with_unicode_characters path = "/path/with/unicode/文件.rb" - uri = URI::Generic.from_path(path: path) + uri = URI::Generic.from_unix_path(path: path) assert_equal(path, uri.to_standardized_path) assert_equal("file:///path/with/unicode/%E6%96%87%E4%BB%B6.rb", uri.to_s) end diff --git a/lib/ruby_lsp/listeners/definition.rb b/lib/ruby_lsp/listeners/definition.rb index c0c3198d06..f2d56fe74a 100644 --- a/lib/ruby_lsp/listeners/definition.rb +++ b/lib/ruby_lsp/listeners/definition.rb @@ -389,15 +389,13 @@ def find_in_index(value) # If the project has Sorbet, then we only want to handle go to definition for constants defined in gems, as an # additional behavior on top of jumping to RBIs. The only sigil where Sorbet cannot handle constants is typed # ignore - uri = entry.uri - full_path = uri.full_path - if !@sorbet_level.ignore? && (!full_path || not_in_dependencies?(full_path)) + if !@sorbet_level.ignore? && !entry.in_dependencies? next end @response_builder << Interface::LocationLink.new( - target_uri: uri.to_s, + target_uri: entry.uri.to_s, target_range: range_from_location(entry.location), target_selection_range: range_from_location(entry.name_location), ) diff --git a/lib/ruby_lsp/requests/workspace_symbol.rb b/lib/ruby_lsp/requests/workspace_symbol.rb index 1abde2337c..6de7bfba3c 100644 --- a/lib/ruby_lsp/requests/workspace_symbol.rb +++ b/lib/ruby_lsp/requests/workspace_symbol.rb @@ -22,15 +22,15 @@ def initialize(global_state, query) def perform @index.fuzzy_search(@query).filter_map do |entry| uri = entry.uri - file_path = uri.full_path - # We only show symbols declared in the workspace - in_dependencies = file_path && !not_in_dependencies?(file_path) - next if in_dependencies + next if entry.in_dependencies? # We should never show private symbols when searching the entire workspace next if entry.private? + # We should present only resolved entries + next unless entry.resolved? + kind = kind_for_entry(entry) loc = entry.location diff --git a/lib/ruby_lsp/server.rb b/lib/ruby_lsp/server.rb index 394b85e451..5b76fee939 100644 --- a/lib/ruby_lsp/server.rb +++ b/lib/ruby_lsp/server.rb @@ -1251,6 +1251,9 @@ def perform_initial_indexing GC.compact unless @test_mode @global_state.synchronize do + # There are unresolved constant and method aliases we should take care of + @global_state.index.process_unresolved_entries + # If we linearize ancestors while the index is not fully populated, we may end up caching incorrect results # that were missing namespaces. After indexing is complete, we need to clear the ancestors cache and start # again From cd44b53209afc9782e8bfb6388840c33bdd28f6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Kondzior?= Date: Sun, 26 Oct 2025 14:44:21 +0100 Subject: [PATCH 3/5] Make srb happy --- lib/ruby_indexer/lib/ruby_indexer/entry.rb | 50 +++++++++++----------- lib/ruby_indexer/lib/ruby_indexer/index.rb | 35 +-------------- lib/ruby_indexer/lib/ruby_indexer/uri.rb | 29 +++++-------- lib/ruby_lsp/requests/support/common.rb | 4 +- sorbet/rbi/shims/uri.rbi | 18 ++++++++ test/integration_test.rb | 3 +- 6 files changed, 60 insertions(+), 79 deletions(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/entry.rb b/lib/ruby_indexer/lib/ruby_indexer/entry.rb index 24d318c617..9c9df20d52 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/entry.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/entry.rb @@ -17,6 +17,12 @@ class Entry #: Symbol attr_accessor :visibility + #: String? + attr_accessor :file_name + + #: String? + attr_accessor :file_path + #: (String name, URI::Generic uri, Location location, String? comments) -> void def initialize(name, uri, location, comments) @name = name @@ -24,6 +30,22 @@ def initialize(name, uri, location, comments) @comments = comments @visibility = :public #: Symbol @location = location + @file_path = @uri.full_path #: String? + @file_name = if @uri.scheme == "untitled" + @uri.opaque + else + File.basename( + @file_path, #: as !nil + ) + end #: String? + @in_dependencies = if @file_path + ::RubyLsp::BUNDLE_PATH && + @file_path.start_with?( + ::RubyLsp::BUNDLE_PATH, #: as !nil + ) || @file_path.start_with?(RbConfig::CONFIG["rubylibdir"]) + else + false + end #: bool end #: -> bool @@ -48,28 +70,7 @@ def resolved? #: -> bool def in_dependencies? - @in_dependencies ||= file_path && ( - ::RubyLsp::BUNDLE_PATH && file_path.start_with?( - ::RubyLsp::BUNDLE_PATH, #: as !nil - ) || - file_path.start_with?(RbConfig::CONFIG["rubylibdir"]) - ) - end - - #: -> String - def file_name - @file_name ||= if @uri.scheme == "untitled" - @uri.opaque #: as !nil - else - File.basename( - file_path, #: as !nil - ) - end - end - - #: -> String? - def file_path - @file_path ||= @uri.full_path + @in_dependencies end #: -> String @@ -389,7 +390,7 @@ def initialize(target, nesting, name, uri, location, comments) # rubocop:disable @nesting = nesting end - #: -> Bool + #: -> bool def resolved? false end @@ -414,6 +415,7 @@ def initialize(target, unresolved_alias) @unresolved_alias = unresolved_alias end + #: -> String? def comments @comments ||= @unresolved_alias.comments end @@ -465,7 +467,7 @@ def initialize(new_name, old_name, owner, uri, location, comments) # rubocop:dis @owner = owner end - #: -> Bool + #: -> bool def resolved? false end diff --git a/lib/ruby_indexer/lib/ruby_indexer/index.rb b/lib/ruby_indexer/lib/ruby_indexer/index.rb index fda3e1fc31..eee05353cd 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/index.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/index.rb @@ -215,9 +215,7 @@ def fuzzy_search(query) [entries, -similarity] if similarity > ENTRY_SIMILARITY_THRESHOLD end results.sort_by!(&:last) - results.flat_map do |entries, _similarity| - skip_unresolved_entries(entries) - end + results.flat_map(&:first) end #: (String? name, String receiver_name) -> Array[(Entry::Member | Entry::MethodAlias)] @@ -734,7 +732,7 @@ def entries_for(uri, type = nil) entries&.grep(type) end - #: -> voide + #: -> void def process_unresolved_entries @entries.values.each do |entries| entries.each do |entry| @@ -1082,34 +1080,5 @@ def resolve_method_alias(entry, receiver_name, seen_names) original_entries << resolved_alias resolved_alias end - - # Attempts to resolve an entry if it is an unresolved constant or method alias. - # Returns the resolved entry, or nil if it cannot be resolved. - #: (Entry) -> Entry? - def resolve_entry(entry) - case entry - when Entry::UnresolvedConstantAlias - resolved_entry = resolve_alias(entry, []) - resolved_entry unless resolved_entry.is_a?(Entry::UnresolvedConstantAlias) - when Entry::UnresolvedMethodAlias - resolved_entry = resolve_method_alias(entry, entry.owner&.name || "", []) - resolved_entry unless resolved_entry.is_a?(Entry::UnresolvedMethodAlias) - else - entry - end - end - - # Filters and resolves entries, skipping those that remain unresolved. - # Returns an array of successfully resolved entries. - #: (Array[Entry]) -> Array[Entry?] - def skip_unresolved_entries(entries) - entries.filter_map do |entry| - resolved_entry = resolve_entry(entry) - - next unless resolved_entry - - resolved_entry - end - end end end diff --git a/lib/ruby_indexer/lib/ruby_indexer/uri.rb b/lib/ruby_indexer/lib/ruby_indexer/uri.rb index c05613547a..61b1fed85c 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/uri.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/uri.rb @@ -52,11 +52,7 @@ def from_unix_path(path:, fragment: nil, scheme: "file", load_path_entry: nil) uri end - if Gem.win_platform? - alias_method :from_path, :from_win_path - else - alias_method :from_path, :from_unix_path - end + alias_method :from_path, Gem.win_platform? ? :from_win_path : :from_unix_path end #: String? @@ -70,6 +66,7 @@ def add_require_path_from_load_entry(load_path_entry) self.require_path = path.delete_prefix("#{load_path_entry}/").delete_suffix(".rb") end + #: -> String? # On Windows, when we're getting the file system path back from the URI, we need to remove the leading forward # slash def to_standardized_win_path @@ -77,7 +74,7 @@ def to_standardized_win_path return unless parsed_path - # we can bail out parsing if there is nothing to unscape + # we can bail out parsing if there is nothing to unescape return parsed_path unless parsed_path.match?(/%[0-9A-Fa-f]{2}/) unescaped_path = PARSER.unescape(parsed_path) @@ -91,22 +88,16 @@ def to_standardized_win_path #: -> String? def to_standardized_unix_path - unscapped_path = path - return unless unscapped_path - - # we can bail out parsing if there is nothing to unscape - return unscapped_path unless unscapped_path.match?(/%[0-9A-Fa-f]{2}/) - - PARSER.unescape(unscapped_path) - end + unescaped_path = path + return unless unescaped_path + # we can bail out parsing if there is nothing to be unescaped + return unescaped_path unless unescaped_path.match?(/%[0-9A-Fa-f]{2}/) - if Gem.win_platform? - alias_method :to_standardized_path, :to_standardized_win_path - else - alias_method :to_standardized_path, :to_standardized_unix_path + PARSER.unescape(unescaped_path) end - alias_method :full_path, :to_standardized_path + alias_method :to_standardized_path, Gem.win_platform? ? :to_standardized_win_path : :to_standardized_unix_path + alias_method :full_path, Gem.win_platform? ? :to_standardized_win_path : :to_standardized_unix_path end end diff --git a/lib/ruby_lsp/requests/support/common.rb b/lib/ruby_lsp/requests/support/common.rb index cc2ddc6b2c..32cbac7e3c 100644 --- a/lib/ruby_lsp/requests/support/common.rb +++ b/lib/ruby_lsp/requests/support/common.rb @@ -158,9 +158,9 @@ def kind_for_entry(entry) when RubyIndexer::Entry::MethodAlias Constant::SymbolKind::METHOD else - # Kind shold be represented by one of + # Kind should be represented by one of # [SymbolKind](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#symbolKind) variants - # $stderr.puts("Unknonw symbol kind #{entry.inspect}. Kind should be represented by one of SymbolKind enum variants. Please report this as a bug") + # $stderr.puts("Unknown symbol kind #{entry.inspect}. Kind should be represented by one of SymbolKind enum variants. Please report this as a bug") Constant::SymbolKind::NULL end end diff --git a/sorbet/rbi/shims/uri.rbi b/sorbet/rbi/shims/uri.rbi index 79ec8c59f8..3b539f8498 100644 --- a/sorbet/rbi/shims/uri.rbi +++ b/sorbet/rbi/shims/uri.rbi @@ -5,6 +5,24 @@ module URI class Generic PARSER = T.let(const_defined?(:RFC2396_PARSER) ? RFC2396_PARSER : DEFAULT_PARSER, RFC2396_Parser) + + class << self + sig do + params( + path: String, + fragment: T.nilable(String), + scheme: String, + load_path_entry: T.nilable(String) + ).returns(URI::Generic) + end + def from_path(path:, fragment: nil, scheme: "file", load_path_entry: nil); end + end + + sig { returns(T.nilable(String)) } + def to_standardized_path; end + + sig { returns(T.nilable(String)) } + def full_path; end end class File diff --git a/test/integration_test.rb b/test/integration_test.rb index 06136ba470..37b1bd1c3a 100644 --- a/test/integration_test.rb +++ b/test/integration_test.rb @@ -12,7 +12,8 @@ def test_ruby_lsp_doctor_works skip("CI only") unless ENV["CI"] in_isolation do - system("bundle exec ruby-lsp --doctor") + $stderr.puts "Running ruby-lsp --doctor in isolation" + system("bundle exec ruby-lsp --doctor", err: $stderr) assert_equal(0, $CHILD_STATUS) end end From 2cc7c9b50977366bd2e39fbb0776ba2c389a2470 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Kondzior?= Date: Mon, 27 Oct 2025 03:38:22 +0100 Subject: [PATCH 4/5] Fix indexing properly handle single and all files --- lib/ruby_indexer/lib/ruby_indexer/index.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/index.rb b/lib/ruby_indexer/lib/ruby_indexer/index.rb index eee05353cd..333e24590a 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/index.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/index.rb @@ -368,14 +368,14 @@ def index_all(uris: @configuration.indexable_uris, &block) break unless block.call(progress) end - index_file(uri, collect_comments: false) + index_file(uri, collect_comments: false, process_unresolved: false) end @initial_indexing_completed = true end - #: (URI::Generic uri, String source, ?collect_comments: bool) -> void - def index_single(uri, source, collect_comments: true) + #: (URI::Generic uri, String source, ?collect_comments: bool, ?process_unresolved: bool) -> void + def index_single(uri, source, collect_comments: true, process_unresolved: true) dispatcher = Prism::Dispatcher.new result = Prism.parse(source) @@ -385,7 +385,7 @@ def index_single(uri, source, collect_comments: true) require_path = uri.require_path @require_paths_tree.insert(require_path, uri) if require_path - process_unresolved_entries + process_unresolved_entries if process_unresolved indexing_errors = listener.indexing_errors.uniq indexing_errors.each { |error| $stderr.puts(error) } if indexing_errors.any? @@ -398,10 +398,10 @@ def index_single(uri, source, collect_comments: true) end # Indexes a File URI by reading the contents from disk - #: (URI::Generic uri, ?collect_comments: bool) -> void - def index_file(uri, collect_comments: true) + #: (URI::Generic uri, ?collect_comments: bool, ?process_unresolved: bool) -> void + def index_file(uri, collect_comments: true, process_unresolved: true) path = uri.full_path #: as !nil - index_single(uri, File.read(path), collect_comments: collect_comments) + index_single(uri, File.read(path), collect_comments: collect_comments, process_unresolved: process_unresolved) rescue Errno::EISDIR, Errno::ENOENT # If `path` is a directory, just ignore it and continue indexing. If the file doesn't exist, then we also ignore # it From 157c8b25dcfc30687f5d4c36563785329f43f199 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Kondzior?= Date: Mon, 27 Oct 2025 07:22:30 +0100 Subject: [PATCH 5/5] Fix stack level too deep on RbConfig constant alias resolution --- lib/ruby_indexer/lib/ruby_indexer/index.rb | 4 +++- lib/ruby_indexer/test/constant_test.rb | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/index.rb b/lib/ruby_indexer/lib/ruby_indexer/index.rb index 333e24590a..404ce57eb7 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/index.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/index.rb @@ -918,10 +918,12 @@ def resolve_alias(entry, seen_names) return entry if seen_names.include?(alias_name) seen_names << alias_name - target = resolve(entry.target, entry.nesting, seen_names) return entry unless target + # Self referential alias can be unresolved we should bail out from resolving + return entry if target.first == entry + target_name = target.first #: as !nil .name resolved_alias = Entry::ConstantAlias.new(target_name, entry) diff --git a/lib/ruby_indexer/test/constant_test.rb b/lib/ruby_indexer/test/constant_test.rb index fbb9e3229b..978d1f0cb3 100644 --- a/lib/ruby_indexer/test/constant_test.rb +++ b/lib/ruby_indexer/test/constant_test.rb @@ -291,6 +291,28 @@ module A assert_instance_of(Entry::Constant, constant) end + def test_self_referential_constant_alias + index(<<~RUBY) + module RealClass + CONSTANT = {} + end + + module Foo + SomeClass = ::SomeClass + RealClass = ::RealClass + + UNRESOLVED = SomeClass::CONSTANT + CONSTANT = RealClass::CONSTANT + end + RUBY + + assert_entry("RealClass", Entry::Module, "/fake/path/foo.rb:0-0:2-3") + assert_no_entry("SomeClass") + assert_entry("Foo", Entry::Module, "/fake/path/foo.rb:4-0:10-3") + assert_entry("RealClass::CONSTANT", Entry::Constant, "/fake/path/foo.rb:1-2:1-15") + assert_entry("Foo::UNRESOLVED", Entry::UnresolvedConstantAlias, "/fake/path/foo.rb:8-2:8-34") + end + def test_indexing_or_and_operator_nodes index(<<~RUBY) A ||= 1