Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement listing all returned results for LSP textDocument/implements #4755

Merged
merged 4 commits into from
May 1, 2024

Conversation

bgarber
Copy link
Contributor

@bgarber bgarber commented Apr 15, 2024

This PR has the intention of implementing the list feature for :ALEGoToDefinition and :ALEGoToImplementation. Specially :ALEGoToImplementation. There are times a LSP may return multiple results for the textDocument/implementation command. This is true when we try to find implementations of an abstract class or interface. The way ALE is implemented, only the first result is returned.

Take this example.

foobar.go:

type Foobarer interface {
    Do() bool
}

foobar_a.go

type FoobarA struct {
   foo int
}

func (f *FoobarA) Do() bool {
   fmt.Printf("This foobar is int (%d)\n", f.foo)
   return true
}

another_foobar.go

type AnotherFoobar struct {
   foo bool
}

func (f *AnotherFoobar) Do() bool {
   fmt.Printf("This foobar is bool (%v)\n", f.foo)
   return f.foo
}

When trying to find implementations of the Do function, it will only find the one from AnotherFoobar struct (because the LSP returns in alphabetic order). But I could want the other one. This change fixes this behavior.

Captura de tela 2024-04-14 221711

@bgarber
Copy link
Contributor Author

bgarber commented Apr 16, 2024

Please, let me know if this is good to go. Thank you!

Copy link
Member

@w0rp w0rp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Sorry for the delay.

Cheers! 🍻

@w0rp w0rp merged commit 70eeae5 into dense-analysis:master May 1, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants