diff --git a/gopls/internal/cache/parsego/file.go b/gopls/internal/cache/parsego/file.go index 3e13d5b2c43..b03929e6c86 100644 --- a/gopls/internal/cache/parsego/file.go +++ b/gopls/internal/cache/parsego/file.go @@ -41,10 +41,12 @@ type File struct { ParseErr scanner.ErrorList } +func (pgf File) String() string { return string(pgf.URI) } + // Fixed reports whether p was "Fixed", meaning that its source or positions // may not correlate with the original file. -func (p File) Fixed() bool { - return p.fixedSrc || p.fixedAST +func (pgf File) Fixed() bool { + return pgf.fixedSrc || pgf.fixedAST } // -- go/token domain convenience helpers -- diff --git a/gopls/internal/golang/call_hierarchy.go b/gopls/internal/golang/call_hierarchy.go index 5331e6eaabf..4971208e79d 100644 --- a/gopls/internal/golang/call_hierarchy.go +++ b/gopls/internal/golang/call_hierarchy.go @@ -201,13 +201,8 @@ func OutgoingCalls(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle return nil, nil } - // Skip builtins. - if obj.Pkg() == nil { - return nil, nil - } - - if !obj.Pos().IsValid() { - return nil, bug.Errorf("internal error: object %s.%s missing position", obj.Pkg().Path(), obj.Name()) + if isBuiltin(obj) { + return nil, nil // built-ins have no position } declFile := pkg.FileSet().File(obj.Pos()) @@ -271,10 +266,8 @@ func OutgoingCalls(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle if obj == nil { continue } - - // ignore calls to builtin functions - if obj.Pkg() == nil { - continue + if isBuiltin(obj) { + continue // built-ins have no position } outgoingCall, ok := outgoingCalls[obj.Pos()] diff --git a/gopls/internal/golang/definition.go b/gopls/internal/golang/definition.go index d2a61e17f13..6184e292928 100644 --- a/gopls/internal/golang/definition.go +++ b/gopls/internal/golang/definition.go @@ -87,8 +87,8 @@ func Definition(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p return nil, nil } - // Handle objects with no position: builtin, unsafe. - if !obj.Pos().IsValid() { + // Built-ins have no position. + if isBuiltin(obj) { return builtinDefinition(ctx, snapshot, obj) } diff --git a/gopls/internal/golang/hover.go b/gopls/internal/golang/hover.go index 5c9bae36fce..1eb32d9e4b7 100644 --- a/gopls/internal/golang/hover.go +++ b/gopls/internal/golang/hover.go @@ -169,12 +169,14 @@ func hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pp pro // Handle hovering over a doc link if obj, rng, _ := parseDocLink(pkg, pgf, pos); obj != nil { - hoverRange = &rng - // Handle builtins, which don't have a package or position. - if !obj.Pos().IsValid() { + // Built-ins have no position. + if isBuiltin(obj) { h, err := hoverBuiltin(ctx, snapshot, obj) - return *hoverRange, h, err + return rng, h, err } + + // Find position in declaring file. + hoverRange = &rng objURI := safetoken.StartPosition(pkg.FileSet(), obj.Pos()) pkg, pgf, err = NarrowestPackageForFile(ctx, snapshot, protocol.URIFromPath(objURI.Filename)) if err != nil { @@ -240,8 +242,8 @@ func hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pp pro }, nil } - // Handle builtins, which don't have a package or position. - if !obj.Pos().IsValid() { + if isBuiltin(obj) { + // Built-ins have no position. h, err := hoverBuiltin(ctx, snapshot, obj) return *hoverRange, h, err } diff --git a/gopls/internal/golang/references.go b/gopls/internal/golang/references.go index e5d9f2a4581..954748fbc36 100644 --- a/gopls/internal/golang/references.go +++ b/gopls/internal/golang/references.go @@ -252,15 +252,9 @@ func ordinaryReferences(ctx context.Context, snapshot *cache.Snapshot, uri proto } // nil, error, error.Error, iota, or other built-in? - if obj.Pkg() == nil { + if isBuiltin(obj) { return nil, fmt.Errorf("references to builtin %q are not supported", obj.Name()) } - if !obj.Pos().IsValid() { - if obj.Pkg().Path() != "unsafe" { - bug.Reportf("references: object %v has no position", obj) - } - return nil, fmt.Errorf("references to unsafe.%s are not supported", obj.Name()) - } // Find metadata of all packages containing the object's defining file. // This may include the query pkg, and possibly other variants. diff --git a/gopls/internal/golang/signature_help.go b/gopls/internal/golang/signature_help.go index 488387f0a18..a91be296cbd 100644 --- a/gopls/internal/golang/signature_help.go +++ b/gopls/internal/golang/signature_help.go @@ -99,9 +99,7 @@ FindCall: case *ast.SelectorExpr: obj = info.ObjectOf(t.Sel) } - - // Call to built-in? - if obj != nil && !obj.Pos().IsValid() { + if obj != nil && isBuiltin(obj) { // function? if obj, ok := obj.(*types.Builtin); ok { return builtinSignature(ctx, snapshot, callExpr, obj.Name(), pos) diff --git a/gopls/internal/golang/type_definition.go b/gopls/internal/golang/type_definition.go index 306852cdcaf..a396793e48a 100644 --- a/gopls/internal/golang/type_definition.go +++ b/gopls/internal/golang/type_definition.go @@ -12,7 +12,6 @@ import ( "golang.org/x/tools/gopls/internal/cache" "golang.org/x/tools/gopls/internal/file" "golang.org/x/tools/gopls/internal/protocol" - "golang.org/x/tools/gopls/internal/util/bug" "golang.org/x/tools/internal/event" ) @@ -42,13 +41,8 @@ func TypeDefinition(ctx context.Context, snapshot *cache.Snapshot, fh file.Handl if tname == nil { return nil, fmt.Errorf("no type definition for %s", obj.Name()) } - - if !tname.Pos().IsValid() { - // The only defined types with no position are error and comparable. - if tname.Name() != "error" && tname.Name() != "comparable" { - bug.Reportf("unexpected type name with no position: %s", tname) - } - return nil, nil + if isBuiltin(tname) { + return nil, nil // built-ins (error, comparable) have no position } loc, err := mapPosition(ctx, pkg.FileSet(), snapshot, tname.Pos(), tname.Pos()+token.Pos(len(tname.Name()))) diff --git a/gopls/internal/golang/types_format.go b/gopls/internal/golang/types_format.go index ebdf46f3e74..51584bcb013 100644 --- a/gopls/internal/golang/types_format.go +++ b/gopls/internal/golang/types_format.go @@ -283,7 +283,7 @@ func FormatVarType(ctx context.Context, snapshot *cache.Snapshot, srcpkg *cache. return types.TypeString(obj.Type(), qf), nil } - if obj.Pkg() == nil || !obj.Pos().IsValid() { + if isBuiltin(obj) { // This is defensive, though it is extremely unlikely we'll ever have a // builtin var. return types.TypeString(obj.Type(), qf), nil diff --git a/gopls/internal/golang/util.go b/gopls/internal/golang/util.go index 903fdd97936..3279dc8fc2e 100644 --- a/gopls/internal/golang/util.go +++ b/gopls/internal/golang/util.go @@ -350,3 +350,7 @@ func embeddedIdent(x ast.Expr) *ast.Ident { type ImporterFunc func(path string) (*types.Package, error) func (f ImporterFunc) Import(path string) (*types.Package, error) { return f(path) } + +// isBuiltin reports whether obj is a built-in symbol (e.g. append, iota, error.Error, unsafe.Slice). +// All other symbols have a valid position and a valid package. +func isBuiltin(obj types.Object) bool { return !obj.Pos().IsValid() } diff --git a/gopls/internal/test/marker/testdata/callhierarchy/issue66923.txt b/gopls/internal/test/marker/testdata/callhierarchy/issue66923.txt new file mode 100644 index 00000000000..4a5e59f9f9a --- /dev/null +++ b/gopls/internal/test/marker/testdata/callhierarchy/issue66923.txt @@ -0,0 +1,15 @@ +Regression test for a crash (#66923) in outgoing calls +to a built-in function (unsafe.Slice). + +-- go.mod -- +module example.com +go 1.17 + +-- a/a.go -- +package a + +import "unsafe" + +func A() []int { //@ loc(A, "A") + return unsafe.Slice(new(int), 1) //@ outgoingcalls(A) +}