Thank you for the report. We will investigate (for starters, we should bisect the regression).

CC @adonovan who is working on implementations currently. This may be related to recent changes in master.

As chance would have it, we just merged a new implementation of 'implementations' this morning, and I tried it on the example in your bug report.

jwt$ git remote get-url origin
https://github.com/golang-jwt/jwt
jwt$ cat foo/foo.go 
package foo

import "github.com/golang-jwt/jwt/v4"

type embedSomeStruct struct {
	jwt.RegisteredClaims

	Str string
	I   int
}

1. When I run 'implementations' on embedSomeStruct, the sole answer is type "Claims", which seems right.

2. When I run 'implementations' on the jwt.RegisteredClaims, I get an error ("RegisteredClaims is a var not a type"), which is a bug, because an embedded field is both a var and a type.

3. When I run implementations' on type Claims in claims.go, I get 7 types, including embedSomeStruct, so that's good.

I compared it to v0.11.0, and it gave essentially the same answers to all three queries, except that v0.11.0 returned an additional 2 elements in query number 3, for a total of 9. The additional ones were:

example_test.go
36: 	type MyCustomClaims struct {
78: 	type MyCustomClaims struct {

Both of these are function-local type declarations, which are only reported by the new algorithm if they appear in the same package as the query type. In this case, they do (specifically, a test variant), so that's another regression.

So your example has uncovered one bug in old and new algorithms, and one regression in the new algorithm, both of which I will fix. But I can't reproduce the specific problem you mention about embedSomeStruct not having any implementations. (I'm using Emacs+eglot as my LSP client, but I don't think that should matter much.)

Also, could you provide more information about compressWriter? I can't see the declaration of that type in your screenshot, nor does it appear to be part of jwt. Thanks.

Also, could you provide more information about compressWriter? I can't see the declaration of that type in your screenshot, nor does it appear to be part of jwt. Thanks.

It's under that wide popup with Writer in it:

type compressWriter struct {
	http.ResponseWriter
	io.Writer
}

// In case if it matters, that's the only method which redeclares Write:
func (cw *compressWriter) Write(b []byte) (int, error) {
	return cw.Writer.Write(b)
}

Change https://go.dev/cl/462658 mentions this issue: gopls/internal/lsp/source: make implementations2 work on embedded fields

It's under that wide popup with Writer in it:

Ah, so it is! I had assumed it was occluded by the popup, bit it was merely displaced downwards.

// In case if it matters, that's the only method which redeclares Write:

It does matter. Without this method, compressWriter would not satisfy the Writer interface because it would be ambiguous which of the two Write methods (one from each field, each at the same embedding depth) should "win". The explicit method essentially resolves in favor of "the second".

I tested the compressWriter example using three versions--the new implementation of 'implementations', the previous one from earlier this week, and the v0.11.0 release--and it correctly reports a result of two elements, ResponseWriter and Writer. So unfortunately I'm not able to reproduce the failure yet.

[adonovan] Both of these are function-local type declarations, which are only reported by the new algorithm if they appear in the same package as the query type. In this case, they do (specifically, a test variant), so that's another regression.

On closer inspection, the example_test.go file is in a different package--witness the package's _test suffix--so the fact that its function-local types are not among the query results is the intended behavior (even though it does differ in this special case from the previous implementation).

@findleyr and I just observed the correct behavior on the compressWriter example using VS Code, so that issue remains elusive. If you're able to provide any more details that might help us reproduce it, I'd be happy to fix it. Otherwise I plan to close this issue when https://go.dev/cl/462658 is merged. Thanks again.

This problem still persists, f.e. pb.GreeterServer is "classic" gRPC server with SayHello method, and it's embedded in outer GreeterLogging:

It actually doesn't matter which methods are exposed by embedded interface, more important is that it's already an interface.

Another example:

We will get to the bottom of this.

@inliquid In our extension we have an alternative implementation of finding implementations. You could try it by installing from here: https://marketplace.visualstudio.com/items?itemName=tooltitudeteam.tooltitude

Note: you need to use code lens to find usages, not shortcut for it to work. We work as an augmentation for gopls, so we need to work in this way.

@tooltitude-support while we welcome additions to the Go tools ecosystem, this issue is about a bug in gopls. Advocating for an alternative does not help us fix the bug. Please refrain from such comments in the future: they are not constructive, and in fact may be counter-productive if they distract from root-causing the bug in question. Furthermore, it makes me uncomfortable that users may be directed from this issue to (what appears to be) a closed-source extension. Do you publish your source code anywhere?

You are of course welcome to suggest solutions to the problem here, based on your experience writing your extension.

@findleyr Ok, I am sorry for the comment and that it distracts from the fixing of the issue. The extension is closed source and we don't have plan to open source it. However, I don't think it's completely counter productive. Having two implementations of the same functionality allows us to compare the results, and try to find the place where the issue reproduces.

Concerning the issue, I could add that I tried the results of our search compared to gopls search, and in all cases where I tried, they were consistent, i.e. all derived types were the same. There was a difference in library types, but I guess, that we collect results here differently. We are more aggressive in collecting files with non current build tags.

Also, feel free to do the same on some projects, in this way you might find inconsistencies, though, I tried hard to find at least one of them and failed at it. Maybe @inliquid could try doing the same.

@findleyr Concerning the projects, I tried, they were k8, and esbuild. k8 is a bit outlier since most of the dependencies are vendored. Esbuild, doesn't have that many interfaces.

@findleyr Actually, I looked into library files on k8.

Here's comparison of usages of io.WriteSeeker:

You miss closeOnce from exec.go which implements io.WriteSeeker via inclusion of os.File (we miss file from fd_unix due to attempt to load all tags).

@tooltitude-support thank you for investigating, and for the screenshots.

The extension is closed source and we don't have plan to open source it.

Without wishing to prolong this off-topic discussion, I think you @tooltitude-support would do well to recognize that it is not reasonable in 2023 to ask developers to install closed-source software from unknown organizations. Open source software such as Chrome, Emacs, Bash, GNU, or gopls allows a vigilant community of users to provide some collective (if imperfect) assurance that the software is not malicious. Closed-source software doesn't enable scrutiny of the code, but vendors (e.g. Apple, JetBrains, Adobe) may nonetheless demonstrate a good track record for security, and they risk losing customers and income when they screw up.

Tooltitude is closed source, not affiliated with a reputable organization, and, as far as I can tell, is the work of a single person. As a result it would be foolish of me to run your code on my machine. For all I know, the entire project could be a ruse to steal SSH keys from Go developers. I'm pretty sure that's not your intent, but I can't really be sure.

As @findleyr said, we welcome better tools for the Go ecosystem, but we also encourage secure development practices. Your users shouldn't have to choose between them.

For all I know, the entire project could be a ruse to steal SSH keys from Go developers. I'm pretty sure that's not your intent, but I can't really be sure.

We develop software in a secure way using best practices. We intend to be a trusted software vendor, and not a criminal enterprise. It's pretty disappointing to hear this gross description from you instead of just mentioning that you don't trust us enough to install the software.

P.S. If you don't trust software enough, you could always run it in a VM, either on your computer or in the cloud.

I don't believe providing closed source software is insecure development practice (look how many closed sourced programs you have on your personal computers, mobile devices, including from small businesses like ours, I bet there're plenty of them, but may be I am wrong).

Perhaps I'm unusual, but there are in fact very few closed-source programs on my (actually Google's) developer machine: only Spotify and macOS itself. Google's sysadmins may have installed more, but I trust them. (My personal phone is a different story, but mobiles have a different capability model that doesn't make it so trivial to exfiltrate any file).

Yep, we are pretty new and small, but it doesn't mean that we won't be able to establish a track record with time. We are working on it. Also, if you are concerned about security, you could install the software in an isolated VM without any problem.

I could, but that's a pretty big disincentive to adoption. And I'd still be trusting your program with the privacy of my source code.

Of course it's very disappointing to hear from you about stealing keys. Our company intends to develop secure software using best practices and sell it and not to perpetrate criminal activity. You could have just said that you don't trust it to be secure instead of explaining possibility of it in such a gross way.

To be clear, I don't actually believe you have any malicious intent; I was just inviting you to put yourself in the shoes of your more skeptical and security conscious users at a time when vulnerabilities are always making headlines.

Also, one question. Is there something which would help you have more trust in the software? Does providing information about who we are, our experience, our investors, or some certificates help you? Would you be more eager to install it if it was provided with some form of shared source instead of full open source? We plan to provide this information, but we have limited time, and we would rather invest it in the project than in explanations.

P.S. We would love to open source the code if we could find a sustainable way to develop it while it's open source. May be in the future we will find such an option, i.e. some form of OSS/community edition, but not now. We need to at least understand what premium and non premium functionality will be before doing anything in this direction. It will take a while.

Sorry, I'm not much of a business person, and I don't have any good ideas for how to cross the chasm between enticing users with an early product and running a mature trustworthy business that sells closed developer tools (such as JetBrains, Sublime). Developer tools has always been a tough business because so much is free. Best of luck though.

Thanks for reporting the gopls bug. It is much appreciated.

@adonovan Thanks for your answers! Really appreciate your feedback!

Best of luck though.

Thanks a lot!

P.S. Will stop the discussion to stop off-topic.

@tooltitude-support, I can't reproduce the closeOnce problem using gopls@master, go@1.19.5:

xtools$ git log | head -n 1
commit 40fb89cd55bdb68faa88c52593caafb733cdc880

# implementations on io.WriteSeeker
xtools$ ( PATH=$HOME/sdk/go1.20/bin:$PATH  go run ./gopls implementation ~/sdk/go1.19.5/src/io/io.go:#6010 )
/Users/adonovan/sdk/go1.19.5/src/internal/poll/fd_unix.go:18:6-8
/Users/adonovan/sdk/go1.19.5/src/os/exec/exec.go:735:6-15
/Users/adonovan/sdk/go1.19.5/src/os/types.go:16:6-10
/Users/adonovan/w/xtools/internal/lockedfile/lockedfile.go:24:6-10
/Users/adonovan/w/xtools/internal/lockedfile/lockedfile.go:31:6-12
xtools$ nl -ba /Users/adonovan/sdk/go1.19.5/src/os/exec/exec.go | grep 735
   735	type closeOnce struct {

@inliquid: I can't reproduce the GreeterServer issue by using type S struct { io.Reader } and doing an implementation query on S. Was that a sufficient test case for you?

@inliquid: The built-in error interface is a special case that is indeed not supported. I'll file a separate issue for that one.

I'm going to close this issue. Please file a new issue if you still believe any of these problems (or others) exist at master, as this issue refers to a number of separate problems.