Go to definition fails on the last character of a reference

[vinistock]

Currently, we fail to locate the target for a go to definition request if the cursor is immediately after the reference the user is trying to jump to. For example:

Foo
#  ^ cursor position

Bar.something
#            ^ cursor position

Initially, I thought this implementation was correct, since the reference's location given to us by Prism really does not cover that one extra character. The cursor is in the next immediate character, not on top of the reference.

However, I checked what other LSPs do and apparently they do catch the reference even if the cursor is one character beyond.

I think we have two possible solutions for this:

1. The more general option is to consider an inclusive range here. In other words, just change that line to be

# Remove one of the dots to make the range inclusive
next unless (loc_start_offset..loc_end_offset).cover?(char_position)

If this works properly, without causing any weirdness in one of our features, then this is the appropriate solution.

2. If 1 is not viable, then we should perform some sort of off-by-one correction in the definition request when locating targets. This is not ideal, but would fix the scenario

[ZainIftikhar7vals]

Hi @vinistock , i want to work on this issue

Can you please explain how do I verify that solution 1 is not causing any weirdness in existing features.

Thanks

[vinistock]

Hello, @ZainIftikhar7vals

You'd need to test the features from the LSP that depend on locating nodes to see if they are triggering properly. Essentially, just performing a bunch of manual tests to see if the LSP is behaving as expected.

You'd need to primarily verify that definition, hover, completion and signature help are being triggered as expected. And we should verify edge cases since we'd now be including one extra character. Things like, trying to trigger completion on a variable with a 1 character name and so on.

[snutij]

Hello @ZainIftikhar7vals, are you still working on it? If you need help, you can push a draft; otherwise, I can push a fix for that. The first solution seems to do the job; we just need to consolidate it with some unit tests. 👍