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

go/doc: AllMethods mode erroneously includes methods shadowed by field declarations #62640

Open
bcmills opened this issue Sep 14, 2023 · 4 comments
Assignees
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@bcmills
Copy link
Member

bcmills commented Sep 14, 2023

What version of Go are you using (go version)?

$ go version
go version devel go1.22-ee788dba Sat Sep 9 01:48:44 2023 +0000 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/usr/local/google/home/bcmills/.cache/go-build'
GOENV='/usr/local/google/home/bcmills/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/tmp/tmp.VIszpiTnn8/.gopath/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/tmp/tmp.VIszpiTnn8/.gopath'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/google/home/bcmills/sdk/gotip'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/usr/local/google/home/bcmills/sdk/gotip/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='devel go1.22-ee788dba Sat Sep 9 01:48:44 2023 +0000'
GCCGO='/usr/bin/gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/tmp/tmp.VIszpiTnn8/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-build150804637=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Extract the following test to a local directory and run it:

-- example.go --
package example

type E struct{}

// F should be hidden within S because of the S.F field.
func (e E) F() string {
	return "blargh"
}

type S struct {
	E
	F int
}
-- example_test.go --
package example_test

import (
	"go/ast"
	"go/doc"
	"go/parser"
	"go/token"
	"testing"
)

func TestDoc(t *testing.T) {
	fset := token.NewFileSet()
	f, err := parser.ParseFile(fset, "example.go", nil, parser.ParseComments|parser.AllErrors)
	if err != nil {
		t.Fatal(err)
	}
	p, err := doc.NewFromFiles(fset, []*ast.File{f}, "example", doc.AllMethods)
	if err != nil {
		t.Fatal(err)
	}
	for _, typ := range p.Types {
		if typ.Name == "S" {
			for _, m := range typ.Methods {
				t.Fatalf("unexpected method on S: %v%s", p.Text(m.Doc))
			}
		}
	}
}

What did you expect to see?

The doc.Package for example.go should not include any methods on type S, since the method F is shadowed by the field S.F. (For confirmation that the field actually does shadow the method, see https://go.dev/play/p/OdO3BIxNHgd.)

What did you see instead?

$ go test .
--- FAIL: TestDoc (0.00s)
    example_test.go:24: unexpected method on S: F should be hidden within S because of the S.F field.
FAIL
FAIL    example 0.005s

(CC @griesemer @agnivade)

@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Sep 14, 2023
@bcmills bcmills added this to the Go1.22 milestone Sep 14, 2023
@bcmills bcmills self-assigned this Sep 14, 2023
@bcmills
Copy link
Member Author

bcmills commented Sep 14, 2023

(Found while addressing code review comments on https://go.dev/cl/510315, for #61394.)

@griesemer
Copy link
Contributor

go/doc doesn't use type information produced by the type checker and has various heuristics/approx. mechanisms to get things mostly right. Looks like this is another situation where these mechanisms need to be improved.

@gopherbot
Copy link

Change https://go.dev/cl/528402 mentions this issue: go/doc: add a golden test that reproduces #62640

@gopherbot
Copy link

Change https://go.dev/cl/510315 mentions this issue: go/doc: track struct fields during reading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants