Skip to content

Commit 50daa53

Browse files
authored
Check target precision for call node in hover and definition (#2870)
### Motivation Closes #2846 We had a regression in our call node target precisions. Essentially, the call node covers both the receiver and the message, but we only want to make the call node the target is the position covers the message location exactly. ### Implementation Added another branch to the checks for whether the position is covered or not. ### Automated Tests Added tests to ensure we don't regress.
1 parent 9926887 commit 50daa53

File tree

4 files changed

+72
-0
lines changed

4 files changed

+72
-0
lines changed

lib/ruby_lsp/requests/definition.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,8 @@ def position_outside_target?(position, target)
118118
Prism::InstanceVariableWriteNode
119119

120120
!covers_position?(target.name_loc, position)
121+
when Prism::CallNode
122+
!covers_position?(target.message_loc, position)
121123
else
122124
false
123125
end

lib/ruby_lsp/requests/hover.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ def position_outside_target?(position, target)
103103
Prism::GlobalVariableOrWriteNode,
104104
Prism::GlobalVariableWriteNode
105105
!covers_position?(target.name_loc, position)
106+
when Prism::CallNode
107+
!covers_position?(target.message_loc, position)
106108
else
107109
false
108110
end

test/requests/definition_expectations_test.rb

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1097,6 +1097,40 @@ def baz
10971097
end
10981098
end
10991099

1100+
def test_definition_call_node_precision
1101+
source = <<~RUBY
1102+
class Foo
1103+
def message
1104+
"hello!"
1105+
end
1106+
end
1107+
1108+
class Bar
1109+
def with_foo(foo)
1110+
@foo_message = foo.message
1111+
end
1112+
end
1113+
RUBY
1114+
1115+
with_server(source) do |server, uri|
1116+
# On the `foo` receiver, we should not show any results
1117+
server.process_message(
1118+
id: 1,
1119+
method: "textDocument/definition",
1120+
params: { textDocument: { uri: uri }, position: { character: 19, line: 8 } },
1121+
)
1122+
assert_empty(server.pop_response.response)
1123+
1124+
# On `message`, we should
1125+
server.process_message(
1126+
id: 2,
1127+
method: "textDocument/definition",
1128+
params: { textDocument: { uri: uri }, position: { character: 23, line: 8 } },
1129+
)
1130+
refute_empty(server.pop_response.response)
1131+
end
1132+
end
1133+
11001134
private
11011135

11021136
def create_definition_addon

test/requests/hover_expectations_test.rb

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -820,6 +820,40 @@ def foo
820820
end
821821
end
822822

823+
def test_hover_call_node_precision
824+
source = <<~RUBY
825+
class Foo
826+
def message
827+
"hello!"
828+
end
829+
end
830+
831+
class Bar
832+
def with_foo(foo)
833+
@foo_message = foo.message
834+
end
835+
end
836+
RUBY
837+
838+
with_server(source) do |server, uri|
839+
# On the `foo` receiver, we should not show any results
840+
server.process_message(
841+
id: 1,
842+
method: "textDocument/hover",
843+
params: { textDocument: { uri: uri }, position: { character: 19, line: 8 } },
844+
)
845+
assert_nil(server.pop_response.response)
846+
847+
# On `message`, we should
848+
server.process_message(
849+
id: 2,
850+
method: "textDocument/hover",
851+
params: { textDocument: { uri: uri }, position: { character: 23, line: 8 } },
852+
)
853+
refute_nil(server.pop_response.response)
854+
end
855+
end
856+
823857
private
824858

825859
def create_hover_addon

0 commit comments

Comments
 (0)