golang/go

x/tools/gopls: improve the 'implementation' query on interfaces

koonix opened this issue · 3 comments

koonix commented

gopls version

Build info
----------
golang.org/x/tools/gopls v0.16.1
    golang.org/x/tools/gopls@v0.16.1 h1:1hO/dCeUvjEYx3V0rVvCtOkwnpEpqS29paE+Jw4dcAc=
    github.com/BurntSushi/toml@v1.2.1 h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak=
    github.com/google/go-cmp@v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
    golang.org/x/exp/typeparams@v0.0.0-20221212164502-fae10dda9338 h1:2O2DON6y3XMJiQRAS1UWU+54aec2uopH3x7MAiqGW6Y=
    golang.org/x/mod@v0.18.0 h1:5+9lSbEzPSdWkH32vYPBwEpX8KwDbM52Ud9xBUvNlb0=
    golang.org/x/sync@v0.7.0 h1:YsImfSBoP9QPYL0xyKJPq0gcaJdG3rInoqxTWbfQu9M=
    golang.org/x/telemetry@v0.0.0-20240607193123-221703e18637 h1:3Wt8mZlbFwG8llny+t18kh7AXxyWePFycXMuVdHxnyM=
    golang.org/x/text@v0.16.0 h1:a94ExnEXNtEwYLGJSIUxnWoxoRz/ZcCsV63ROupILh4=
    golang.org/x/tools@v0.22.1-0.20240628205440-9c895dd76b34 h1:Kd+Z5Pm6uwYx3T2KEkeHMHUMZxDPb/q6b1m+zEcy62c=
    golang.org/x/vuln@v1.0.4 h1:SP0mPeg2PmGCu03V+61EcQiOjmpri2XijexKdzv8Z1I=
    honnef.co/go/tools@v0.4.7 h1:9MDAWxMoSnB6QoSqiVr7P5mtkT9pOc1kSxchzPCnqJs=
    mvdan.cc/gofumpt@v0.6.0 h1:G3QvahNDmpD+Aek/bNOLrFR2XC6ZAdo62dZu65gmwGo=
    mvdan.cc/xurls/v2@v2.5.0 h1:lyBNOm8Wo71UknhUs4QTFUNNMyxy2JEIaKKo0RWOh+8=
go: go1.22.5

go env

GO111MODULE=''
GOARCH='amd64'
GOBIN='/home/koonix/.local/bin-go'
GOCACHE='/home/koonix/.cache/go-build'
GOENV='/home/koonix/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/koonix/.local/share/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/koonix/.local/share/go'
GOPRIVATE=''
GOPROXY='direct'
GOROOT='/usr/lib/golang'
GOSUMDB='off'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/usr/lib/golang/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22.5'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/koonix/Repositories/other/gotest/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3909477011=/tmp/go-build -gno-record-gcc-switches'

What did you do?

package main

type A interface {
	Read([]byte) (int, error)
}

type B interface {
	Read([]byte) (int, error)
	Extra()
}

type X int

func (x X) Read([]byte) (int, error) {
	return 0, nil
}

func fn(param A) {}

func main() {

	// this is fine
	var a A
	fn(a)

	// this is also fine
	var b B
	fn(b)

	// so is this
	var x X
	fn(x)
}

What did you see happen?

implementation behaves like so:

  • For interface A, it returns all types that implement it (including X but not B).
  • For type X, it returns all interfaces that it implements (including A).
  • For interface B, it returns no results.

What did you expect to see?

Much like the relationship between A and X, A is implemented by B, and B implements A. Therefore:

  • I expect to see B among A's implementations.
  • I expect to see A among B's implementations.

Editor and settings

No response

Logs

No response

adonovan commented

Thanks for the bug report; I can reproduce it with your test case. (The functions fn and main are superfluous, BTW.)

According to the documentation, implementation(A) should indeed include B, since B implements A, so this is definitely a bug. However, implementation(B) is correct not to include A, since A does not implement B.

type A interface { // impls={X} (should include B)
	Read([]byte) (int, error)
}

type B interface { // impls={} (correct)
	Read([]byte) (int, error)
	Extra()
}

type X int // impls={A} (correct)

func (x X) Read([]byte) (int, error) { return 0, nil }
koonix commented

Thanks for your reply.

implementation(B) is correct not to include A, since A does not implement B

I understand that this is true according to the documentation, but could we change this?

What I was trying to show in main() was that the relationship between B and A is practically similar to that of X and A, and if implementation(X) contains A, perhaps implementation(B) should too.

In other words, concrete types can only implement, but interfaces can both implement and be implemented.

I understand that the textDocument/implementation request might not be a good place for this (backwards compatibility? not matching Microsoft's LSP spec?), but it would be nice to somehow be able to see what other interfaces an interface implements.