Go to declaration places the caret at the beginning of the declaration instead of the beginning of its name identifier

[SCWells72]

In code like the following:

export default class Foo {
    foo: Foo;
}

Using the Navigate | Declaration or Usages action while here:

foo: Foo;
     ^

places the caret here:

export default class Foo {
^

instead of here:

export default class Foo {
                     ^

I've tracked that down to LSPDefinitionSupport#getDefinitionFor where it's doing the following:

return locations.getRight()
        .stream()
        .map(l -> new Location(l.getTargetUri(), l.getTargetRange()))
        .toList();

when it should be doing the following:

return locations.getRight()
        .stream()
        .map(l -> new Location(l.getTargetUri(), l.getTargetSelectionRange()))
        .toList();

Note the change from getTargetRange() to getTargetSelectionRange(). With that change, navigation to the declaration correctly lands on the declaration's name identifier instead of the beginning of the declaration.

It could be that the fix is as simple as that, but I'm concerned that perhaps the changed code above might be used for more than just Go To Declaration.

Thoughts?

[angelozerr]

You are right,we should fix that, but not only for declaration, for other LSP features which uses LocationLink like definition, typeDefinition, references etc

I suggest that you create a static method in LSP4IJ like

public static List<Location> toLocations(. Either<List<? extends Location>, List<? extends org. eclipse. lsp4j. LocationLink>> locations) {
  ...
}

and you use it for all LSP features like definition, typeDefition, etc.

As we cannot trust language server, I suggest that you try to get targetSelectionRange and if it is null you use targetRange.

Do you want to create a PR for that?

[SCWells72]

Sure, no problem. It won't likely be today as I have other obligations today, but I'll take a look and get a PR going for it.

[SCWells72]

@angelozerr please see https://github.com/redhat-developer/lsp4ij/pull/635. Let me know if that's not what you were wanting to see. Also note that I was able to verify go to definition and implementation but not declaration as neither of the language servers I'm using -- TypeScript and CSS -- seem to support that feature.

Are there plans to support the IDE's native Go To Implementation(s) and Go To Super actions as well as gutter markers for those relationships? The information is there, but at least for the latter (gutter markers), perhaps it's prohibitively expensive to query that information in real-time? It seems like the actions should be pretty easily supported when the LSP capabilities are also supported.

[angelozerr]

@angelozerr please see #635. Let me know if that's not what you were wanting to see.

It is exactly that I had in my mind. Good job!

Also note that I was able to verify go to definition and implementation but not declaration as neither of the language servers I'm using -- TypeScript and CSS -- seem to support that feature.

As code use now the same utilities class, it should work.

Are there plans to support the IDE's native Go To Implementation(s) and Go To Super actions as well as gutter markers for those relationships?

I wanted to use standard menu but those menus are linked to Psi element, I have not found a solution to use them. If you find a solution it would be very fantastic.

The information is there, but at least for the latter (gutter markers), perhaps it's prohibitively expensive to query that information in real-time? It seems like the actions should be pretty easily supported when the LSP capabilities are also supported.

Sorry I have not understood what you mean.

[SCWells72]

Are there plans to support the IDE's native Go To Implementation(s) and Go To Super actions as well as gutter markers for those relationships?

I wanted to use standard menu but those menus are linked to Psi element, I have not found a solution to use them. If you find a solution it would be very fantastic.

I'll take a look. Hopefully a FakePsiElement will do the trick.

The information is there, but at least for the latter (gutter markers), perhaps it's prohibitively expensive to query that information in real-time? It seems like the actions should be pretty easily supported when the LSP capabilities are also supported.

Sorry I have not understood what you mean.

I was referring to the gutter icons that show and allow navigation to inheritance relationships bidirectionally, e.g.:

Assuming/hoping I can figure out how to make the standard actions work, I'll see what it might mean to make those work. Because those are rendered on-the-fly, I'm not sure if you'd want that, though. Minimally it seems like it should have some level of client-side caching to avoid constantly running that LSP command for what is fundamentally optional information (as opposed to, say, static code analysis).