diff --git a/gopls/internal/regtest/diagnostics/builtin_test.go b/gopls/internal/regtest/diagnostics/builtin_test.go new file mode 100644 index 0000000000..775e7ec0b1 --- /dev/null +++ b/gopls/internal/regtest/diagnostics/builtin_test.go @@ -0,0 +1,38 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package diagnostics + +import ( + "strings" + "testing" + + . "golang.org/x/tools/internal/lsp/regtest" +) + +func TestIssue44866(t *testing.T) { + src := ` +-- go.mod -- +module mod.com + +go 1.12 +-- a.go -- +package a + +const ( + c = iota +) +` + Run(t, src, func(t *testing.T, env *Env) { + env.OpenFile("a.go") + name, _ := env.GoToDefinition("a.go", env.RegexpSearch("a.go", "iota")) + if !strings.HasSuffix(name, "builtin.go") { + t.Fatalf("jumped to %q, want builtin.go", name) + } + env.Await(OnceMet( + env.DoneWithOpen(), + NoDiagnostics("builtin.go"), + )) + }) +} diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index 3215b7ea29..7c8dfae447 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -51,7 +51,7 @@ type snapshot struct { generation *memoize.Generation // builtin pins the AST and package for builtin.go in memory. - builtin *builtinPackageHandle + builtin *builtinPackageData // The snapshot's initialization state is controlled by the fields below. // @@ -1361,10 +1361,6 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC newGen.Inherit(s.workspaceDirHandle) } - if s.builtin != nil { - newGen.Inherit(s.builtin.handle) - } - // Copy all of the FileHandles. for k, v := range s.files { result.files[k] = v @@ -1755,12 +1751,18 @@ func (s *snapshot) BuiltinPackage(ctx context.Context) (*source.BuiltinPackage, if s.builtin == nil { return nil, errors.Errorf("no builtin package for view %s", s.view.name) } - d, err := s.builtin.handle.Get(ctx, s.generation, s) + return s.builtin.parsed, s.builtin.err +} + +func (s *snapshot) IsBuiltin(ctx context.Context, uri span.URI) bool { + builtin, err := s.BuiltinPackage(ctx) if err != nil { - return nil, err + event.Error(ctx, "checking for builtin", err) + return false } - data := d.(*builtinPackageData) - return data.parsed, data.err + // We should always get the builtin URI in a canonical form, so use simple + // string comparison here. span.CompareURI is too expensive. + return builtin.ParsedFile.URI == uri } func (s *snapshot) buildBuiltinPackage(ctx context.Context, goFiles []string) error { @@ -1775,31 +1777,30 @@ func (s *snapshot) buildBuiltinPackage(ctx context.Context, goFiles []string) er if err != nil { return err } - h := s.generation.Bind(fh.FileIdentity(), func(ctx context.Context, arg memoize.Arg) interface{} { - snapshot := arg.(*snapshot) - - pgh := snapshot.parseGoHandle(ctx, fh, source.ParseFull) - pgf, _, err := snapshot.parseGo(ctx, pgh) - if err != nil { - return &builtinPackageData{err: err} - } - pkg, err := ast.NewPackage(snapshot.view.session.cache.fset, map[string]*ast.File{ - pgf.URI.Filename(): pgf.File, - }, nil, nil) - if err != nil { - return &builtinPackageData{err: err} - } - return &builtinPackageData{ - parsed: &source.BuiltinPackage{ - ParsedFile: pgf, - Package: pkg, - }, - } - }, nil) - s.builtin = &builtinPackageHandle{handle: h} + s.builtin = buildBuiltinPackage(ctx, fh, s) return nil } +func buildBuiltinPackage(ctx context.Context, fh *fileHandle, snapshot *snapshot) *builtinPackageData { + pgh := snapshot.parseGoHandle(ctx, fh, source.ParseFull) + pgf, _, err := snapshot.parseGo(ctx, pgh) + if err != nil { + return &builtinPackageData{err: err} + } + pkg, err := ast.NewPackage(snapshot.view.session.cache.fset, map[string]*ast.File{ + pgf.URI.Filename(): pgf.File, + }, nil, nil) + if err != nil { + return &builtinPackageData{err: err} + } + return &builtinPackageData{ + parsed: &source.BuiltinPackage{ + ParsedFile: pgf, + Package: pkg, + }, + } +} + // BuildGoplsMod generates a go.mod file for all modules in the workspace. It // bypasses any existing gopls.mod. func BuildGoplsMod(ctx context.Context, root span.URI, s source.Snapshot) (*modfile.File, error) { diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index f93e525c10..f892910c26 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -121,6 +121,7 @@ func (s *Server) diagnoseSnapshot(snapshot source.Snapshot, changedURIs []span.U func (s *Server) diagnoseChangedFiles(ctx context.Context, snapshot source.Snapshot, uris []span.URI, onDisk bool) { ctx, done := event.Start(ctx, "Server.diagnoseChangedFiles", tag.Snapshot.Of(snapshot.ID())) defer done() + packages := make(map[source.Package]struct{}) for _, uri := range uris { // If the change is only on-disk and the file is not open, don't @@ -133,6 +134,11 @@ func (s *Server) diagnoseChangedFiles(ctx context.Context, snapshot source.Snaps if snapshot.FindFile(uri) == nil { continue } + // Don't call PackagesForFile for builtin.go, as it results in a + // command-line-arguments load. + if snapshot.IsBuiltin(ctx, uri) { + continue + } pkgs, err := snapshot.PackagesForFile(ctx, uri, source.TypecheckFull) if err != nil { // TODO (findleyr): we should probably do something with the error here, @@ -393,6 +399,10 @@ func (s *Server) checkForOrphanedFile(ctx context.Context, snapshot source.Snaps if fh.Kind() != source.Go { return nil } + // builtin files won't have a package, but they are never orphaned. + if snapshot.IsBuiltin(ctx, fh.URI()) { + return nil + } pkgs, err := snapshot.PackagesForFile(ctx, fh.URI(), source.TypecheckWorkspace) if len(pkgs) > 0 || err == nil { return nil diff --git a/internal/lsp/fake/editor.go b/internal/lsp/fake/editor.go index f10ea44022..cceebe3472 100644 --- a/internal/lsp/fake/editor.go +++ b/internal/lsp/fake/editor.go @@ -693,7 +693,7 @@ func (e *Editor) setBufferContentLocked(ctx context.Context, path string, dirty } // GoToDefinition jumps to the definition of the symbol at the given position -// in an open buffer. +// in an open buffer. It returns the path and position of the resulting jump. func (e *Editor) GoToDefinition(ctx context.Context, path string, pos Pos) (string, Pos, error) { if err := e.checkBufferPosition(path, pos); err != nil { return "", Pos{}, err diff --git a/internal/lsp/regtest/wrappers.go b/internal/lsp/regtest/wrappers.go index 20388490eb..0137218366 100644 --- a/internal/lsp/regtest/wrappers.go +++ b/internal/lsp/regtest/wrappers.go @@ -156,7 +156,7 @@ func (e *Env) SaveBufferWithoutActions(name string) { } // GoToDefinition goes to definition in the editor, calling t.Fatal on any -// error. +// error. It returns the path and position of the resulting jump. func (e *Env) GoToDefinition(name string, pos fake.Pos) (string, fake.Pos) { e.T.Helper() n, p, err := e.Editor.GoToDefinition(e.Ctx, name, pos) diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 412866c0dc..2595823c0a 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -128,6 +128,9 @@ type Snapshot interface { // BuiltinPackage returns information about the special builtin package. BuiltinPackage(ctx context.Context) (*BuiltinPackage, error) + // IsBuiltin reports whether uri is part of the builtin package. + IsBuiltin(ctx context.Context, uri span.URI) bool + // PackagesForFile returns the packages that this file belongs to, checked // in mode. PackagesForFile(ctx context.Context, uri span.URI, mode TypecheckMode) ([]Package, error)