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

x/tools/gopls: 'implementations' doesn't work with generics #59224

Closed
adonovan opened this issue Mar 24, 2023 · 18 comments
Closed

x/tools/gopls: 'implementations' doesn't work with generics #59224

adonovan opened this issue Mar 24, 2023 · 18 comments
Assignees
Labels
gopls/generics Issues related to gopls' support for generics NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@adonovan
Copy link
Member

Even some of the most basic scenarios of 'implementations' are not working at all with generics, for example when the concrete and interface types are both generalized over [T any] and are defined within the same file:

xtools$ cat d/d.go 
package d

type Collection[T any] interface {
	Push(T) error
	Pop() (*T, bool)
}

type C[T any] struct{}

func (C[T]) Push(t T) error { return nil }

func (C[T]) Pop() (*T, bool) {
	var t T
	return &t, true
}

var _ Collection[int] = C[int]{}

xtools$ go run ./gopls implementation ./d/d.go:#120  # within "Push" concrete method identifier
xtools$

This is not a recent regression:

xtools$ ./gopls-v0.11.0 implementation ./d/d.go:#120  
xtools$

A similar lack of results is obtained by querying the Push abstract method, or the Collection interface, or the C concrete type.

(This test case was a further reduction of the one described in https://youtrack.jetbrains.com/issue/GO-12702/Go-to-Declaration-Implementation-not-available-for-generics-interfaces, referenced by golang/vscode-go#2711.)

@adonovan adonovan self-assigned this Mar 24, 2023
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Mar 24, 2023
@gopherbot gopherbot added this to the Unreleased milestone Mar 24, 2023
@adonovan adonovan added gopls/generics Issues related to gopls' support for generics NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. labels Mar 24, 2023
@linjunhao1997
Copy link

I think this is urgently needed

@fangjh13
Copy link

I would like to know how the progress is going, it's really needed.

@adonovan
Copy link
Member Author

I would like to know how the progress is going, it's really needed.

No progress, sorry. Haven't even designed the algorithm yet. But let's make it a priority for 0.13.

@ajsqr
Copy link

ajsqr commented Feb 9, 2024

Hello, is a fix for this planned in the upcoming release ?

@Solverj
Copy link

Solverj commented Feb 9, 2024

I'm also currently googling / gpting / searching, atm I have to make tests on a struct to verify that I've created structs according to generic interface in neovim.

@adonovan
Copy link
Member Author

adonovan commented Feb 9, 2024

Hello, is a fix for this planned in the upcoming release ?

Unfortunately no, but likely the following one.

@HarishHary
Copy link

Hello any progress on this one?

@adonovan
Copy link
Member Author

adonovan commented Aug 6, 2024

Still no progress yet, sorry.

The problem is a little trickier than it may appear because of the design of the index (database) used to radically optimize gopls v0.15. The indexed design means that the problem of testing implements(T, I), where I is an interface, cannot be done by making queries on the type-checker data structures for T and I since we no longer have them. Instead, the problem is mapped onto the domain of sets of strings, each representing a single method, so that the query becomes a string subset test. This works well for non-generic methods, which can each be mapped to a special string called a fingerprint; but we have not yet had time to develop a theory for fingerprinting generic methods, so they would need to fall back to using the type-checker, undoing the benefits of the gopls v0.15 redesign. Of course there are various short-cuts we could apply for common and simple cases, but our current focus is on better support for refactoring, so unfortunately this is not going to happen in the next several months.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/634197 mentions this issue: gopls/internal/cache/methodsets: support generics in Implementations

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/634597 mentions this issue: internal/typeparams: delete GenericAssignableTo

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/634596 mentions this issue: gopls/internal/golang: Implementations: support generics

gopherbot pushed a commit to golang/tools that referenced this issue Dec 9, 2024
It is unused and, in the one place we actually wanted to use it,
it was the wrong tool for the job; what we need is unification.

Updates golang/go#59224
Updates golang/go#63982

Change-Id: I05b6fe6f56e3f81f434e76398c20496950822bfb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/634597
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
gopherbot pushed a commit to golang/tools that referenced this issue Dec 9, 2024
This CL adds support to gopls' index of methodsets to enable
matching of generic types and their methods. Previously, the
index was based on the notion of "fingerprints", strings that
encode the type of a method such that fingerprints are equal
if and only if types are equal. Of course this does not work
for generic types, which can match multiple distinct types.

This change redesigns the fingerprint encoding so that it
preserves the tree structure of the type, allowing it to be
parsed again later, supporting tree-based matching such
as unification. (The encoding is about 30% bigger.)

The actual tree matching in this CL is
very simple: each type parameter is treated as a wildcard
that can match any subtree at all, without regard to
consistency of substitution (as in true unification),
nor to type parameter constraint satisfaction.
Full unification would reduce false positives, but is
not urgently needed.

This CL addresses only the global algorithm using the
methodsets index. See CL 634596 for the local (type-based)
algorithm.

+ Test, relnote, doc

Updates golang/go#59224

Change-Id: I670511a8e5ce02ab23ff7b7bd60e7bd7b43c1cca
Reviewed-on: https://go-review.googlesource.com/c/tools/+/634197
Auto-Submit: Alan Donovan <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
@DingYuan0118
Copy link

Hello, half year passed. Any progress on this one?

@adonovan
Copy link
Member Author

Yes! Run go install golang.org/x/tools/gopls@master and let us know how this feature works for you.

@DingYuan0118
Copy link

Thanks for the quick response.

And I have got a question. I found that the Goland IDE already have a similar func like

Image

Is there any difference between it and gopls?

@DingYuan0118
Copy link

reports

/Users/.../go/go1.23.4/bin/go install golang.org/x/tools/gopls@master
go: golang.org/x/tools/gopls@master (in golang.org/x/tools/[email protected]):
        The go.mod file for the module providing named packages contains one or
        more replace directives. It must not contain directives that would cause
        it to be interpreted differently than if it were the main module.

@adonovan
Copy link
Member Author

adonovan commented Jan 24, 2025

Is there any difference between it and gopls?

I don't use goland, so I can't answer that question. I am sure the feature is similar in the broad strokes (connecting interfaces with implementations), but it may differ in the details. For example, gopls only reports package-level types from other packages, so a type such as type T struct { io.Reader } appearing with in a function will not be found be a query for implementations of io.Reader. Also, gopls' matching algorithm for generic types and instantiations is not perfectly accurate.

/Users/.../go/go1.23.4/bin/go install golang.org/x/tools/gopls@master ... [errors]

Ah, sorry about that, our go.mod file uses a replace directive so you can't go install direct from master, only tagged releases.

You'll need to git clone https://go.googlesource.com/tools and then cd gopls and go install.

@DingYuan0118
Copy link

It works good for this demo.

But our project can not upgrade go version so quickly and still use go1.19. So it is a little painful that I can not use this feature...

@adonovan
Copy link
Member Author

adonovan commented Jan 24, 2025

But our project can not upgrade go version so quickly and still use go1.19. So it is a little painful that I can not use this feature...

go1.19? That's very old: gopls v0.15 dropped support for go1.19 last April. You should find a way to upgrade your project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls/generics Issues related to gopls' support for generics NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

9 participants