Enhance constant lookup to support namespaced constants in ProjectManager #134
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances constant lookup functionality in the ProjectManager to properly handle namespaced constants (e.g., Foo::BAR, Foo::Baz::QUUX). The changes enable "go to definition" to work correctly when clicking on constants within namespace chains.
Changes:
- Refactored
possible_definitionsmethod to analyze namespace chains and identify class/module boundaries - Updated
project_definitions_forto support hierarchical scope searching with parent scopes - Added helper methods
find_scope_by_pathandconstant_name?for improved constant detection
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| lib/ruby_language_server/project_manager.rb | Implements namespace-aware constant lookup logic with scope chain traversal |
| spec/lib/ruby_language_server/project_manager_spec.rb | Adds comprehensive test coverage for namespaced constant lookups |
| # Position on "BAR" in "Foo::BAR" (line 9, character 10) | ||
| # The line is: "puts Foo::BAR" | ||
| position = OpenStruct.new(line: 9, character: 10) |
There was a problem hiding this comment.
The comment states the target line is line 9, but the code being tested shows puts Foo::BAR on line 14 of the heredoc (counting from line 0 for module Foo). The line number in the position should match the actual line in the test file content.
| # Position on "QUUX" in "Foo::Baz::QUUX" (line 10, character 16) | ||
| position = OpenStruct.new(line: 10, character: 16) |
There was a problem hiding this comment.
The comment indicates line 10, but puts Foo::Baz::QUUX appears on line 15 of the heredoc. This mismatch will cause the test to target the wrong line.
| # Position on "QUUX" in "Foo::Baz::QUUX" (line 10, character 16) | |
| position = OpenStruct.new(line: 10, character: 16) | |
| # Position on "QUUX" in "Foo::Baz::QUUX" (line 15, character 16) | |
| position = OpenStruct.new(line: 15, character: 16) |
| end | ||
|
|
||
| it 'finds module definition when clicking on Foo in Foo::BAR' do | ||
| # Position on "Foo" in "Foo::BAR" (line 9, character 5) |
There was a problem hiding this comment.
Similar to the previous issues, this comment references line 9 but should reference line 14 where puts Foo::BAR is located in the test code.
| # Position on "Foo" in "Foo::BAR" (line 9, character 5) | |
| # Position on "Foo" in "Foo::BAR" (line 14, character 5) |
| scope_path_parts = context[rightmost_class_index..-2] | ||
|
|
||
| if scope_path_parts.empty? | ||
| # This shouldn't happen, but handle it gracefully |
There was a problem hiding this comment.
The comment states 'This shouldn't happen' but the code handles it. If this truly should never occur, consider raising an error or adding logging to track if this case is ever hit, which would help identify bugs in the namespace parsing logic.
| # This shouldn't happen, but handle it gracefully | |
| # This shouldn't happen, but handle it gracefully | |
| warn "[RubyLanguageServer::ProjectManager] Unexpected empty scope_path_parts for context=#{context.inspect}, name=#{name.inspect}" |
| # Namespace lookup like Foo::Bar or Foo::BAR - no method type filtering | ||
|
|
There was a problem hiding this comment.
Line 206 appears to be an orphaned comment with no associated code. Either remove it or clarify what code it relates to, as it currently creates confusion about the control flow.
| # Namespace lookup like Foo::Bar or Foo::BAR - no method type filtering |
No description provided.