gopls/internal/lsp/cache: remove package dependence on packages.Config
The cache.pkg type was a mix of metadata-related information and type
checking information, resulting in unnecessary relationships between
type-checking results (which are shared) and loading results (which are
not shared). As a result, the experimentalPackageCacheKey was more or
less a hope that these relationships were valid.
Avoid this relationship altogether by separating the shared
type-checking result from other derived calculations. This makes the
experimentalPackageCacheKey obsolete and lays the groundwork for
type-checking from export data.
Additionally:
- revisit the package cache key to ensure it covers all inputs into
type-checking, and make it more similar to the analysis key
- remove methods from the source.Package API that return source.Package:
we can't have edges between packages if they are going to be
standalone
- remove the experimentalPackageCacheKey setting
- add a test for go list errors
- use the proper types.Sizes when type-checking
- address a comment from an earlier CL in completion_test.go
Fixes golang/go#57853
Change-Id: I238913c7c8305cb534db77ebec5f062e96ed2503
Reviewed-on: https://go-review.googlesource.com/c/tools/+/461944
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/gopls/doc/settings.md b/gopls/doc/settings.md
index 1b8f6d7..52a7539 100644
--- a/gopls/doc/settings.md
+++ b/gopls/doc/settings.md
@@ -116,20 +116,6 @@
Default: `true`.
-#### **experimentalPackageCacheKey** *bool*
-
-**This setting is experimental and may be deleted.**
-
-experimentalPackageCacheKey controls whether to use a coarser cache key
-for package type information to increase cache hits. This setting removes
-the user's environment, build flags, and working directory from the cache
-key, which should be a safe change as all relevant inputs into the type
-checking pass are already hashed into the key. This is temporarily guarded
-by an experiment because caching behavior is subtle and difficult to
-comprehensively test.
-
-Default: `true`.
-
#### **allowModfileModifications** *bool*
**This setting is experimental and may be deleted.**
diff --git a/gopls/internal/lsp/cache/analysis.go b/gopls/internal/lsp/cache/analysis.go
index e9f1c7f..0005b0e 100644
--- a/gopls/internal/lsp/cache/analysis.go
+++ b/gopls/internal/lsp/cache/analysis.go
@@ -520,10 +520,8 @@
sz := m.TypesSizes.(*types.StdSizes)
fmt.Fprintf(hasher, "sizes: %d %d\n", sz.WordSize, sz.MaxAlign)
- // metadata errors
- for _, err := range m.Errors {
- fmt.Fprintf(hasher, "error: %q", err)
- }
+ // metadata errors: used for 'compiles' field
+ fmt.Fprintf(hasher, "errors: %d", len(m.Errors))
// module Go version
if m.Module != nil && m.Module.GoVersion != "" {
@@ -542,8 +540,8 @@
depIDs = append(depIDs, string(depID))
}
sort.Strings(depIDs)
- for _, id := range depIDs {
- vdep := vdeps[PackageID(id)]
+ for _, depID := range depIDs {
+ vdep := vdeps[PackageID(depID)]
fmt.Fprintf(hasher, "dep: %s\n", vdep.PkgPath)
fmt.Fprintf(hasher, "export: %s\n", vdep.DeepExportHash)
@@ -766,6 +764,7 @@
}
cfg := &types.Config{
+ Sizes: m.TypesSizes,
Error: func(e error) {
pkg.compiles = false // type error
pkg.typeErrors = append(pkg.typeErrors, e.(types.Error))
diff --git a/gopls/internal/lsp/cache/cache.go b/gopls/internal/lsp/cache/cache.go
index ef56b12..9da185c 100644
--- a/gopls/internal/lsp/cache/cache.go
+++ b/gopls/internal/lsp/cache/cache.go
@@ -209,7 +209,7 @@
typsCost := typesCost(v.pkg.types.Scope())
typInfoCost := typesInfoCost(v.pkg.typesInfo)
stat := packageStat{
- id: v.pkg.m.ID,
+ id: v.pkg.id,
mode: v.pkg.mode,
types: typsCost,
typesInfo: typInfoCost,
diff --git a/gopls/internal/lsp/cache/check.go b/gopls/internal/lsp/cache/check.go
index 722691d..03691e5 100644
--- a/gopls/internal/lsp/cache/check.go
+++ b/gopls/internal/lsp/cache/check.go
@@ -5,21 +5,21 @@
package cache
import (
- "bytes"
"context"
+ "crypto/sha256"
"errors"
"fmt"
"go/ast"
"go/types"
"path/filepath"
"regexp"
+ "sort"
"strings"
"sync"
"golang.org/x/mod/module"
"golang.org/x/sync/errgroup"
"golang.org/x/tools/go/ast/astutil"
- "golang.org/x/tools/go/packages"
"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/gopls/internal/lsp/source"
"golang.org/x/tools/gopls/internal/lsp/source/methodsets"
@@ -42,12 +42,14 @@
type packageHandleKey source.Hash
-// A packageHandle is a handle to the future result of type-checking a package.
-// The resulting package is obtained from the await() method.
+// A packageHandle holds package information, some of which may not be fully
+// evaluated.
+//
+// The only methods on packageHandle that are safe to call before calling await
+// are Metadata and await itself.
type packageHandle struct {
- promise *memoize.Promise // [typeCheckResult]
-
- // m is the metadata associated with the package.
+ // TODO(rfindley): remove metadata from packageHandle. It is only used for
+ // bug detection.
m *source.Metadata
// key is the hashed key for the package.
@@ -58,12 +60,29 @@
// enough. (The key for analysis actions could similarly
// hash only Facts of direct dependencies.)
key packageHandleKey
+
+ // The shared type-checking promise.
+ promise *memoize.Promise // [typeCheckResult]
+}
+
+// typeCheckInputs contains the inputs of a call to typeCheckImpl, which
+// type-checks a package.
+type typeCheckInputs struct {
+ id PackageID
+ pkgPath PackagePath
+ name PackageName
+ mode source.ParseMode
+ goFiles, compiledGoFiles []source.FileHandle
+ sizes types.Sizes
+ deps map[PackageID]*packageHandle
+ depsByImpPath map[ImportPath]PackageID
+ goVersion string // packages.Module.GoVersion, e.g. "1.18"
}
// typeCheckResult contains the result of a call to
// typeCheckImpl, which type-checks a package.
type typeCheckResult struct {
- pkg *pkg
+ pkg *syntaxPackage
err error
}
@@ -101,7 +120,6 @@
// for each package is computed by at most one thread, then do
// the recursive key building of dependencies in parallel.
deps := make(map[PackageID]*packageHandle)
- var depKey source.Hash // XOR of all unique deps
for _, depID := range m.DepsByPkgPath {
depHandle, err := s.buildPackageHandle(ctx, depID, s.workspaceParseMode(depID))
// Don't use invalid metadata for dependencies if the top-level
@@ -121,9 +139,6 @@
// checking the entire package. Leave depKeys[i] unset.
continue
}
-
- depKey.XORWith(source.Hash(depHandle.key))
-
deps[depID] = depHandle
}
@@ -139,12 +154,29 @@
return nil, err
}
+ goVersion := ""
+ if m.Module != nil && m.Module.GoVersion != "" {
+ goVersion = m.Module.GoVersion
+ }
+
+ inputs := typeCheckInputs{
+ id: m.ID,
+ pkgPath: m.PkgPath,
+ name: m.Name,
+ mode: mode,
+ goFiles: goFiles,
+ compiledGoFiles: compiledGoFiles,
+ sizes: m.TypesSizes,
+ deps: deps,
+ depsByImpPath: m.DepsByImpPath,
+ goVersion: goVersion,
+ }
+
// All the file reading has now been done.
// Create a handle for the result of type checking.
- experimentalKey := s.View().Options().ExperimentalPackageCacheKey
- phKey := computePackageKey(m.ID, compiledGoFiles, m, depKey, mode, experimentalKey)
+ phKey := computePackageKey(s, inputs)
promise, release := s.store.Promise(phKey, func(ctx context.Context, arg interface{}) interface{} {
- pkg, err := typeCheckImpl(ctx, arg.(*snapshot), goFiles, compiledGoFiles, m, mode, deps)
+ pkg, err := typeCheckImpl(ctx, arg.(*snapshot), inputs)
return typeCheckResult{pkg, err}
})
@@ -165,10 +197,11 @@
// handles for type-checking its immediate deps, at which
// point there will be no need to even access s.meta.)
if s.meta.metadata[ph.m.ID] != ph.m {
+ // TODO(rfindley): this should be bug.Errorf.
return nil, fmt.Errorf("stale metadata for %s", ph.m.ID)
}
- // Check cache again in case another thread got there first.
+ // Check cache again in case another goroutine got there first.
if prev, ok := s.packages.Get(packageKey); ok {
prevPH := prev.(*packageHandle)
release()
@@ -223,61 +256,64 @@
// computePackageKey returns a key representing the act of type checking
// a package named id containing the specified files, metadata, and
// combined dependency hash.
-func computePackageKey(id PackageID, files []source.FileHandle, m *source.Metadata, depsKey source.Hash, mode source.ParseMode, experimentalKey bool) packageHandleKey {
- // TODO(adonovan): opt: no need to materalize the bytes; hash them directly.
- // Also, use field separators to avoid spurious collisions.
- b := bytes.NewBuffer(nil)
- b.WriteString(string(id))
- if m.Module != nil {
- b.WriteString(m.Module.GoVersion) // go version affects type check errors.
- }
- if !experimentalKey {
- // cfg was used to produce the other hashed inputs (package ID, parsed Go
- // files, and deps). It should not otherwise affect the inputs to the type
- // checker, so this experiment omits it. This should increase cache hits on
- // the daemon as cfg contains the environment and working directory.
- hc := hashConfig(m.Config)
- b.Write(hc[:])
- }
- b.WriteByte(byte(mode))
- b.Write(depsKey[:])
- for _, file := range files {
- b.WriteString(file.FileIdentity().String())
- }
- // Metadata errors are interpreted and memoized on the computed package, so
- // we must hash them into the key here.
- //
- // TODO(rfindley): handle metadata diagnostics independently from
- // type-checking diagnostics.
- for _, err := range m.Errors {
- b.WriteString(err.Msg)
- b.WriteString(err.Pos)
- b.WriteRune(rune(err.Kind))
- }
- return packageHandleKey(source.HashOf(b.Bytes()))
-}
+func computePackageKey(s *snapshot, inputs typeCheckInputs) packageHandleKey {
+ hasher := sha256.New()
-// hashConfig returns the hash for the *packages.Config.
-func hashConfig(config *packages.Config) source.Hash {
- // TODO(adonovan): opt: don't materialize the bytes; hash them directly.
- // Also, use sound field separators to avoid collisions.
- var b bytes.Buffer
+ // In principle, a key must be the hash of an
+ // unambiguous encoding of all the relevant data.
+ // If it's ambiguous, we risk collisons.
- // Dir, Mode, Env, BuildFlags are the parts of the config that can change.
- b.WriteString(config.Dir)
- b.WriteRune(rune(config.Mode))
+ // package identifiers
+ fmt.Fprintf(hasher, "package: %s %s %s\n", inputs.id, inputs.name, inputs.pkgPath)
- for _, e := range config.Env {
- b.WriteString(e)
+ // module Go version
+ fmt.Fprintf(hasher, "go %s\n", inputs.goVersion)
+
+ // parse mode
+ fmt.Fprintf(hasher, "mode %d\n", inputs.mode)
+
+ // import map
+ importPaths := make([]string, 0, len(inputs.depsByImpPath))
+ for impPath := range inputs.depsByImpPath {
+ importPaths = append(importPaths, string(impPath))
}
- for _, f := range config.BuildFlags {
- b.WriteString(f)
+ sort.Strings(importPaths)
+ for _, impPath := range importPaths {
+ fmt.Fprintf(hasher, "import %s %s", impPath, string(inputs.depsByImpPath[ImportPath(impPath)]))
}
- return source.HashOf(b.Bytes())
+
+ // deps, in PackageID order
+ depIDs := make([]string, 0, len(inputs.deps))
+ for depID := range inputs.deps {
+ depIDs = append(depIDs, string(depID))
+ }
+ sort.Strings(depIDs)
+ for _, depID := range depIDs {
+ dep := inputs.deps[PackageID(depID)]
+ fmt.Fprintf(hasher, "dep: %s key:%s\n", dep.m.PkgPath, dep.key)
+ }
+
+ // file names and contents
+ fmt.Fprintf(hasher, "compiledGoFiles: %d\n", len(inputs.compiledGoFiles))
+ for _, fh := range inputs.compiledGoFiles {
+ fmt.Fprintln(hasher, fh.FileIdentity())
+ }
+ fmt.Fprintf(hasher, "goFiles: %d\n", len(inputs.goFiles))
+ for _, fh := range inputs.goFiles {
+ fmt.Fprintln(hasher, fh.FileIdentity())
+ }
+
+ // types sizes
+ sz := inputs.sizes.(*types.StdSizes)
+ fmt.Fprintf(hasher, "sizes: %d %d\n", sz.WordSize, sz.MaxAlign)
+
+ var hash [sha256.Size]byte
+ hasher.Sum(hash[:0])
+ return packageHandleKey(hash)
}
// await waits for typeCheckImpl to complete and returns its result.
-func (ph *packageHandle) await(ctx context.Context, s *snapshot) (*pkg, error) {
+func (ph *packageHandle) await(ctx context.Context, s *snapshot) (*syntaxPackage, error) {
v, err := s.awaitPromise(ctx, ph.promise)
if err != nil {
return nil, err
@@ -286,15 +322,7 @@
return data.pkg, data.err
}
-func (ph *packageHandle) CompiledGoFiles() []span.URI {
- return ph.m.CompiledGoFiles
-}
-
-func (ph *packageHandle) ID() string {
- return string(ph.m.ID)
-}
-
-func (ph *packageHandle) cached() (*pkg, error) {
+func (ph *packageHandle) cached() (*syntaxPackage, error) {
v := ph.promise.Cached()
if v == nil {
return nil, fmt.Errorf("no cached type information for %s", ph.m.PkgPath)
@@ -306,13 +334,13 @@
// typeCheckImpl type checks the parsed source files in compiledGoFiles.
// (The resulting pkg also holds the parsed but not type-checked goFiles.)
// deps holds the future results of type-checking the direct dependencies.
-func typeCheckImpl(ctx context.Context, snapshot *snapshot, goFiles, compiledGoFiles []source.FileHandle, m *source.Metadata, mode source.ParseMode, deps map[PackageID]*packageHandle) (*pkg, error) {
+func typeCheckImpl(ctx context.Context, snapshot *snapshot, inputs typeCheckInputs) (*syntaxPackage, error) {
// Start type checking of direct dependencies,
// in parallel and asynchronously.
// As the type checker imports each of these
// packages, it will wait for its completion.
var wg sync.WaitGroup
- for _, dep := range deps {
+ for _, dep := range inputs.deps {
wg.Add(1)
go func(dep *packageHandle) {
dep.await(ctx, snapshot) // ignore result
@@ -328,56 +356,39 @@
defer wg.Wait()
var filter *unexportedFilter
- if mode == source.ParseExported {
+ if inputs.mode == source.ParseExported {
filter = &unexportedFilter{uses: map[string]bool{}}
}
- pkg, err := doTypeCheck(ctx, snapshot, goFiles, compiledGoFiles, m, mode, deps, filter)
+ pkg, err := doTypeCheck(ctx, snapshot, inputs, filter)
if err != nil {
return nil, err
}
- if mode == source.ParseExported {
+ if inputs.mode == source.ParseExported {
// The AST filtering is a little buggy and may remove things it
// shouldn't. If we only got undeclared name errors, try one more
// time keeping those names.
missing, unexpected := filter.ProcessErrors(pkg.typeErrors)
if len(unexpected) == 0 && len(missing) != 0 {
- pkg, err = doTypeCheck(ctx, snapshot, goFiles, compiledGoFiles, m, mode, deps, filter)
+ pkg, err = doTypeCheck(ctx, snapshot, inputs, filter)
if err != nil {
return nil, err
}
missing, unexpected = filter.ProcessErrors(pkg.typeErrors)
}
if len(unexpected) != 0 || len(missing) != 0 {
- pkg, err = doTypeCheck(ctx, snapshot, goFiles, compiledGoFiles, m, mode, deps, nil)
+ pkg, err = doTypeCheck(ctx, snapshot, inputs, nil)
if err != nil {
return nil, err
}
}
}
- // If this is a replaced module in the workspace, the version is
- // meaningless, and we don't want clients to access it.
- if m.Module != nil {
- pkg.version = &module.Version{
- Path: m.Module.Path,
- Version: m.Module.Version,
- }
- }
// We don't care about a package's errors unless we have parsed it in full.
- if mode != source.ParseFull {
+ if inputs.mode != source.ParseFull {
return pkg, nil
}
- for _, e := range m.Errors {
- diags, err := goPackagesErrorDiagnostics(snapshot, pkg, e)
- if err != nil {
- event.Error(ctx, "unable to compute positions for list errors", err, tag.Package.Of(string(pkg.ID())))
- continue
- }
- pkg.diagnostics = append(pkg.diagnostics, diags...)
- }
-
// Our heuristic for whether to show type checking errors is:
// + If any file was 'fixed', don't show type checking errors as we
// can't guarantee that they reference accurate locations in the source.
@@ -394,7 +405,7 @@
for _, e := range pkg.parseErrors {
diags, err := parseErrorDiagnostics(snapshot, pkg, e)
if err != nil {
- event.Error(ctx, "unable to compute positions for parse errors", err, tag.Package.Of(string(pkg.ID())))
+ event.Error(ctx, "unable to compute positions for parse errors", err, tag.Package.Of(string(inputs.id)))
continue
}
for _, diag := range diags {
@@ -412,7 +423,7 @@
for _, e := range expandErrors(unexpanded, snapshot.View().Options().RelatedInformationSupported) {
diags, err := typeErrorDiagnostics(snapshot, pkg, e)
if err != nil {
- event.Error(ctx, "unable to compute positions for type errors", err, tag.Package.Of(string(pkg.ID())))
+ event.Error(ctx, "unable to compute positions for type errors", err, tag.Package.Of(string(inputs.id)))
continue
}
pkg.typeErrors = append(pkg.typeErrors, e.primary)
@@ -426,27 +437,20 @@
}
}
- depsErrors, err := snapshot.depsErrors(ctx, pkg)
- if err != nil {
- return nil, err
- }
- pkg.diagnostics = append(pkg.diagnostics, depsErrors...)
-
return pkg, nil
}
var goVersionRx = regexp.MustCompile(`^go([1-9][0-9]*)\.(0|[1-9][0-9]*)$`)
-func doTypeCheck(ctx context.Context, snapshot *snapshot, goFiles, compiledGoFiles []source.FileHandle, m *source.Metadata, mode source.ParseMode, deps map[PackageID]*packageHandle, astFilter *unexportedFilter) (*pkg, error) {
- ctx, done := event.Start(ctx, "cache.typeCheck", tag.Package.Of(string(m.ID)))
+func doTypeCheck(ctx context.Context, snapshot *snapshot, inputs typeCheckInputs, astFilter *unexportedFilter) (*syntaxPackage, error) {
+ ctx, done := event.Start(ctx, "cache.typeCheck", tag.Package.Of(string(inputs.id)))
defer done()
- pkg := &pkg{
- m: m,
- mode: mode,
+ pkg := &syntaxPackage{
+ id: inputs.id,
+ mode: inputs.mode,
fset: snapshot.FileSet(), // must match parse call below (snapshot.ParseGo for now)
- deps: make(map[PackageID]*pkg),
- types: types.NewPackage(string(m.PkgPath), string(m.Name)),
+ types: types.NewPackage(string(inputs.pkgPath), string(inputs.name)),
typesInfo: &types.Info{
Types: make(map[ast.Expr]types.TypeAndValue),
Defs: make(map[*ast.Ident]types.Object),
@@ -461,9 +465,9 @@
// Parse the non-compiled GoFiles. (These aren't presented to
// the type checker but are part of the returned pkg.)
// TODO(adonovan): opt: parallelize parsing.
- for _, fh := range goFiles {
- goMode := mode
- if mode == source.ParseExported {
+ for _, fh := range inputs.goFiles {
+ goMode := inputs.mode
+ if inputs.mode == source.ParseExported {
// This package is being loaded only for type information,
// to which non-compiled Go files are irrelevant,
// so parse only the header.
@@ -477,40 +481,32 @@
}
// Parse the CompiledGoFiles: those seen by the compiler/typechecker.
- if err := parseCompiledGoFiles(ctx, compiledGoFiles, snapshot, mode, pkg, astFilter); err != nil {
+ if err := parseCompiledGoFiles(ctx, inputs.compiledGoFiles, snapshot, inputs.mode, pkg, astFilter); err != nil {
return nil, err
}
// Use the default type information for the unsafe package.
- if m.PkgPath == "unsafe" {
+ if inputs.pkgPath == "unsafe" {
// Don't type check Unsafe: it's unnecessary, and doing so exposes a data
// race to Unsafe.completed.
// TODO(adonovan): factor (tail-merge) with the normal control path.
pkg.types = types.Unsafe
pkg.methodsets = methodsets.NewIndex(pkg.fset, pkg.types)
- pkg.xrefs = xrefs.Index(pkg)
+ pkg.xrefs = xrefs.Index(pkg.compiledGoFiles, pkg.types, pkg.typesInfo)
return pkg, nil
}
- if len(m.CompiledGoFiles) == 0 {
- // No files most likely means go/packages failed. Try to attach error
- // messages to the file as much as possible.
- var found bool
- for _, e := range m.Errors {
- srcDiags, err := goPackagesErrorDiagnostics(snapshot, pkg, e)
- if err != nil {
- continue
- }
- found = true
- pkg.diagnostics = append(pkg.diagnostics, srcDiags...)
- }
- if found {
- return pkg, nil
- }
- return nil, fmt.Errorf("no parsed files for package %s, expected: %v, errors: %v", pkg.m.PkgPath, pkg.compiledGoFiles, m.Errors)
+ if len(pkg.compiledGoFiles) == 0 {
+ // No files most likely means go/packages failed.
+ //
+ // TODO(rfindley): in the past, we would capture go list errors in this
+ // case, to present go list errors to the user. However we had no tests for
+ // this behavior. It is unclear if anything better can be done here.
+ return nil, fmt.Errorf("no parsed files for package %s", inputs.pkgPath)
}
cfg := &types.Config{
+ Sizes: inputs.sizes,
Error: func(e error) {
pkg.typeErrors = append(pkg.typeErrors, e.(types.Error))
},
@@ -519,30 +515,30 @@
// based on the metadata before we start type checking,
// reporting them via types.Importer places the errors
// at the correct source location.
- id, ok := pkg.m.DepsByImpPath[ImportPath(path)]
+ id, ok := inputs.depsByImpPath[ImportPath(path)]
if !ok {
// If the import declaration is broken,
// go list may fail to report metadata about it.
// See TestFixImportDecl for an example.
return nil, fmt.Errorf("missing metadata for import of %q", path)
}
- dep, ok := deps[id] // id may be ""
+ dep, ok := inputs.deps[id] // id may be ""
if !ok {
return nil, snapshot.missingPkgError(path)
}
- if !source.IsValidImport(m.PkgPath, dep.m.PkgPath) {
+ if !source.IsValidImport(inputs.pkgPath, dep.m.PkgPath) {
return nil, fmt.Errorf("invalid use of internal package %s", path)
}
depPkg, err := dep.await(ctx, snapshot)
if err != nil {
return nil, err
}
- pkg.deps[depPkg.m.ID] = depPkg
return depPkg.types, nil
}),
}
- if pkg.m.Module != nil && pkg.m.Module.GoVersion != "" {
- goVersion := "go" + pkg.m.Module.GoVersion
+
+ if inputs.goVersion != "" {
+ goVersion := "go" + inputs.goVersion
// types.NewChecker panics if GoVersion is invalid. An unparsable mod
// file should probably stop us before we get here, but double check
// just in case.
@@ -551,7 +547,7 @@
}
}
- if mode != source.ParseFull {
+ if inputs.mode != source.ParseFull {
cfg.DisableUnusedImportCheck = true
cfg.IgnoreFuncBodies = true
}
@@ -574,7 +570,7 @@
pkg.methodsets = methodsets.NewIndex(pkg.fset, pkg.types)
// Build global index of outbound cross-references.
- pkg.xrefs = xrefs.Index(pkg)
+ pkg.xrefs = xrefs.Index(pkg.compiledGoFiles, pkg.types, pkg.typesInfo)
// If the context was cancelled, we may have returned a ton of transient
// errors to the type checker. Swallow them.
@@ -584,7 +580,7 @@
return pkg, nil
}
-func parseCompiledGoFiles(ctx context.Context, compiledGoFiles []source.FileHandle, snapshot *snapshot, mode source.ParseMode, pkg *pkg, astFilter *unexportedFilter) error {
+func parseCompiledGoFiles(ctx context.Context, compiledGoFiles []source.FileHandle, snapshot *snapshot, mode source.ParseMode, pkg *syntaxPackage, astFilter *unexportedFilter) error {
// TODO(adonovan): opt: parallelize this loop, which takes 1-25ms.
for _, fh := range compiledGoFiles {
var pgf *source.ParsedGoFile
@@ -633,11 +629,16 @@
return nil
}
-func (s *snapshot) depsErrors(ctx context.Context, pkg *pkg) ([]*source.Diagnostic, error) {
+// depsErrors creates diagnostics for each metadata error (e.g. import cycle).
+// These may be attached to import declarations in the transitive source files
+// of pkg, or to 'requires' declarations in the package's go.mod file.
+//
+// TODO(rfindley): move this to errors.go
+func (s *snapshot) depsErrors(ctx context.Context, pkg *syntaxPackage, depsErrors []*packagesinternal.PackageError) ([]*source.Diagnostic, error) {
// Select packages that can't be found, and were imported in non-workspace packages.
// Workspace packages already show their own errors.
var relevantErrors []*packagesinternal.PackageError
- for _, depsError := range pkg.m.DepsErrors {
+ for _, depsError := range depsErrors {
// Up to Go 1.15, the missing package was included in the stack, which
// was presumably a bug. We want the next one up.
directImporterIdx := len(depsError.ImportStack) - 1
@@ -665,7 +666,7 @@
allImports := map[string][]fileImport{}
for _, cgf := range pkg.compiledGoFiles {
// TODO(adonovan): modify Imports() to accept a single token.File (cgf.Tok).
- for _, group := range astutil.Imports(s.FileSet(), cgf.File) {
+ for _, group := range astutil.Imports(pkg.fset, cgf.File) {
for _, imp := range group {
if imp.Path == nil {
continue
diff --git a/gopls/internal/lsp/cache/errors.go b/gopls/internal/lsp/cache/errors.go
index 8af1ac3..b0babc7 100644
--- a/gopls/internal/lsp/cache/errors.go
+++ b/gopls/internal/lsp/cache/errors.go
@@ -28,30 +28,20 @@
"golang.org/x/tools/internal/typesinternal"
)
-func goPackagesErrorDiagnostics(snapshot *snapshot, pkg *pkg, e packages.Error) ([]*source.Diagnostic, error) {
- if msg, spn, ok := parseGoListImportCycleError(snapshot, e, pkg); ok {
- rng, err := spanToRange(pkg, spn)
- if err != nil {
- return nil, err
- }
- return []*source.Diagnostic{{
- URI: spn.URI(),
- Range: rng,
- Severity: protocol.SeverityError,
- Source: source.ListError,
- Message: msg,
- }}, nil
+func goPackagesErrorDiagnostics(e packages.Error, pkg *syntaxPackage, fromDir string) (diags []*source.Diagnostic, rerr error) {
+ if diag, ok := parseGoListImportCycleError(e, pkg); ok {
+ return []*source.Diagnostic{diag}, nil
}
var spn span.Span
if e.Pos == "" {
- spn = parseGoListError(e.Msg, pkg.m.Config.Dir)
+ spn = parseGoListError(e.Msg, fromDir)
// We may not have been able to parse a valid span. Apply the errors to all files.
if _, err := spanToRange(pkg, spn); err != nil {
var diags []*source.Diagnostic
- for _, cgf := range pkg.compiledGoFiles {
+ for _, pgf := range pkg.compiledGoFiles {
diags = append(diags, &source.Diagnostic{
- URI: cgf.URI,
+ URI: pgf.URI,
Severity: protocol.SeverityError,
Source: source.ListError,
Message: e.Msg,
@@ -60,9 +50,20 @@
return diags, nil
}
} else {
- spn = span.ParseInDir(e.Pos, pkg.m.Config.Dir)
+ spn = span.ParseInDir(e.Pos, fromDir)
}
+ // TODO(rfindley): in some cases the go command outputs invalid spans, for
+ // example (from TestGoListErrors):
+ //
+ // package a
+ // import
+ //
+ // In this case, the go command will complain about a.go:2:8, which is after
+ // the trailing newline but still considered to be on the second line, most
+ // likely because *token.File lacks information about newline termination.
+ //
+ // We could do better here by handling that case.
rng, err := spanToRange(pkg, spn)
if err != nil {
return nil, err
@@ -76,7 +77,7 @@
}}, nil
}
-func parseErrorDiagnostics(snapshot *snapshot, pkg *pkg, errList scanner.ErrorList) ([]*source.Diagnostic, error) {
+func parseErrorDiagnostics(snapshot *snapshot, pkg *syntaxPackage, errList scanner.ErrorList) ([]*source.Diagnostic, error) {
// The first parser error is likely the root cause of the problem.
if errList.Len() <= 0 {
return nil, fmt.Errorf("no errors in %v", errList)
@@ -102,7 +103,7 @@
var importErrorRe = regexp.MustCompile(`could not import ([^\s]+)`)
var unsupportedFeatureRe = regexp.MustCompile(`.*require.* go(\d+\.\d+) or later`)
-func typeErrorDiagnostics(snapshot *snapshot, pkg *pkg, e extendedError) ([]*source.Diagnostic, error) {
+func typeErrorDiagnostics(snapshot *snapshot, pkg *syntaxPackage, e extendedError) ([]*source.Diagnostic, error) {
code, loc, err := typeErrorData(pkg, e.primary)
if err != nil {
return nil, err
@@ -286,7 +287,7 @@
return out
}
-func typeErrorData(pkg *pkg, terr types.Error) (typesinternal.ErrorCode, protocol.Location, error) {
+func typeErrorData(pkg *syntaxPackage, terr types.Error) (typesinternal.ErrorCode, protocol.Location, error) {
ecode, start, end, ok := typesinternal.ReadGo116ErrorData(terr)
if !ok {
start, end = terr.Pos, terr.Pos
@@ -299,7 +300,7 @@
}
// go/types errors retain their FileSet.
// Sanity-check that we're using the right one.
- fset := pkg.FileSet()
+ fset := pkg.fset
if fset != terr.Fset {
return 0, protocol.Location{}, bug.Errorf("wrong FileSet for type error")
}
@@ -320,7 +321,7 @@
// spanToRange converts a span.Span to a protocol.Range,
// assuming that the span belongs to the package whose diagnostics are being computed.
-func spanToRange(pkg *pkg, spn span.Span) (protocol.Range, error) {
+func spanToRange(pkg *syntaxPackage, spn span.Span) (protocol.Range, error) {
pgf, err := pkg.File(spn.URI())
if err != nil {
return protocol.Range{}, err
@@ -344,36 +345,39 @@
return span.ParseInDir(input[:msgIndex], wd)
}
-func parseGoListImportCycleError(snapshot *snapshot, e packages.Error, pkg *pkg) (string, span.Span, bool) {
+func parseGoListImportCycleError(e packages.Error, pkg *syntaxPackage) (*source.Diagnostic, bool) {
re := regexp.MustCompile(`(.*): import stack: \[(.+)\]`)
matches := re.FindStringSubmatch(strings.TrimSpace(e.Msg))
if len(matches) < 3 {
- return e.Msg, span.Span{}, false
+ return nil, false
}
msg := matches[1]
importList := strings.Split(matches[2], " ")
// Since the error is relative to the current package. The import that is causing
// the import cycle error is the second one in the list.
if len(importList) < 2 {
- return msg, span.Span{}, false
+ return nil, false
}
// Imports have quotation marks around them.
circImp := strconv.Quote(importList[1])
- for _, cgf := range pkg.compiledGoFiles {
+ for _, pgf := range pkg.compiledGoFiles {
// Search file imports for the import that is causing the import cycle.
- for _, imp := range cgf.File.Imports {
+ for _, imp := range pgf.File.Imports {
if imp.Path.Value == circImp {
- start, end, err := safetoken.Offsets(cgf.Tok, imp.Pos(), imp.End())
+ rng, err := pgf.PosMappedRange(imp.Pos(), imp.End())
if err != nil {
- return msg, span.Span{}, false
+ return nil, false
}
- spn, err := cgf.Mapper.OffsetSpan(start, end)
- if err != nil {
- return msg, span.Span{}, false
- }
- return msg, spn, true
+
+ return &source.Diagnostic{
+ URI: pgf.URI,
+ Range: rng.Range(),
+ Severity: protocol.SeverityError,
+ Source: source.ListError,
+ Message: msg,
+ }, true
}
}
}
- return msg, span.Span{}, false
+ return nil, false
}
diff --git a/gopls/internal/lsp/cache/error_test.go b/gopls/internal/lsp/cache/errors_test.go
similarity index 100%
rename from gopls/internal/lsp/cache/error_test.go
rename to gopls/internal/lsp/cache/errors_test.go
diff --git a/gopls/internal/lsp/cache/load.go b/gopls/internal/lsp/cache/load.go
index 1d69504..9b321ce 100644
--- a/gopls/internal/lsp/cache/load.go
+++ b/gopls/internal/lsp/cache/load.go
@@ -447,21 +447,13 @@
Name: PackageName(pkg.Name),
ForTest: PackagePath(packagesinternal.GetForTest(pkg)),
TypesSizes: pkg.TypesSizes,
- Config: cfg,
+ LoadDir: cfg.Dir,
Module: pkg.Module,
+ Errors: pkg.Errors,
DepsErrors: packagesinternal.GetDepsErrors(pkg),
}
- updates[id] = m
- for _, err := range pkg.Errors {
- // Filter out parse errors from go list. We'll get them when we
- // actually parse, and buggy overlay support may generate spurious
- // errors. (See TestNewModule_Issue38207.)
- if strings.Contains(err.Msg, "expected '") {
- continue
- }
- m.Errors = append(m.Errors, err)
- }
+ updates[id] = m
for _, filename := range pkg.CompiledGoFiles {
uri := span.URIFromPath(filename)
diff --git a/gopls/internal/lsp/cache/pkg.go b/gopls/internal/lsp/cache/pkg.go
index 37aae50..bb4823c 100644
--- a/gopls/internal/lsp/cache/pkg.go
+++ b/gopls/internal/lsp/cache/pkg.go
@@ -5,19 +5,22 @@
package cache
import (
+ "context"
"fmt"
"go/ast"
"go/scanner"
"go/token"
"go/types"
+ "strings"
- "golang.org/x/mod/module"
"golang.org/x/tools/go/types/objectpath"
"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/gopls/internal/lsp/source"
"golang.org/x/tools/gopls/internal/lsp/source/methodsets"
"golang.org/x/tools/gopls/internal/lsp/source/xrefs"
"golang.org/x/tools/gopls/internal/span"
+ "golang.org/x/tools/internal/event"
+ "golang.org/x/tools/internal/event/tag"
"golang.org/x/tools/internal/memoize"
)
@@ -29,30 +32,93 @@
ImportPath = source.ImportPath
)
-// pkg contains parse trees and type information for a package.
-type pkg struct {
+// A Package is the union of snapshot-local information (Metadata) and shared
+// type-checking information (a syntaxPackage).
+//
+// TODO(rfindley): for now, we do not persist the post-processing of
+// loadDiagnostics, because the value of the snapshot.packages map is just the
+// package handle. Fix this.
+type Package struct {
m *source.Metadata
- mode source.ParseMode
+ pkg *syntaxPackage
+ loadDiagnostics *memoize.Promise // post-processed errors from loading
+}
+
+func newPackage(m *source.Metadata, pkg *syntaxPackage) *Package {
+ p := &Package{
+ m: m,
+ pkg: pkg,
+ }
+ if len(m.Errors) > 0 || len(m.DepsErrors) > 0 {
+ p.loadDiagnostics = memoize.NewPromise(fmt.Sprintf("loadDiagnostics(%s)", m.ID), func(ctx context.Context, arg interface{}) interface{} {
+ s := arg.(*snapshot)
+ var diags []*source.Diagnostic
+ for _, packagesErr := range p.m.Errors {
+ // Filter out parse errors from go list. We'll get them when we
+ // actually parse, and buggy overlay support may generate spurious
+ // errors. (See TestNewModule_Issue38207.)
+ if strings.Contains(packagesErr.Msg, "expected '") {
+ continue
+ }
+ pkgDiags, err := goPackagesErrorDiagnostics(packagesErr, p.pkg, p.m.LoadDir)
+ if err != nil {
+ // There are certain cases where the go command returns invalid
+ // positions, so we cannot panic or even bug.Reportf here.
+ event.Error(ctx, "unable to compute positions for list errors", err, tag.Package.Of(string(p.m.ID)))
+ continue
+ }
+ diags = append(diags, pkgDiags...)
+ }
+
+ // TODO(rfindley): this is buggy: an insignificant change to a modfile
+ // (or an unsaved modfile) could affect the position of deps errors,
+ // without invalidating the package.
+ depsDiags, err := s.depsErrors(ctx, p.pkg, p.m.DepsErrors)
+ if err != nil {
+ if ctx.Err() == nil {
+ // TODO(rfindley): consider making this a bug.Reportf. depsErrors should
+ // not normally fail.
+ event.Error(ctx, "unable to compute deps errors", err, tag.Package.Of(string(p.m.ID)))
+ }
+ return nil
+ }
+ diags = append(diags, depsDiags...)
+ return diags
+ })
+ }
+ return p
+}
+
+// syntaxPackage contains parse trees and type information for a package.
+type syntaxPackage struct {
+ // -- identifiers --
+ id PackageID
+ mode source.ParseMode
+
+ // -- outputs --
fset *token.FileSet // for now, same as the snapshot's FileSet
goFiles []*source.ParsedGoFile
compiledGoFiles []*source.ParsedGoFile
diagnostics []*source.Diagnostic
- deps map[PackageID]*pkg // use m.DepsBy{Pkg,Imp}Path to look up ID
- version *module.Version // may be nil; may differ from m.Module.Version
parseErrors []scanner.ErrorList
typeErrors []types.Error
types *types.Package
typesInfo *types.Info
- hasFixedFiles bool // if true, AST was sufficiently mangled that we should hide type errors
- xrefs []byte // serializable index of outbound cross-references
-
- analyses memoize.Store // maps analyzer.Name to Promise[actionResult]
- methodsets *methodsets.Index // index of method sets of package-level types
+ hasFixedFiles bool // if true, AST was sufficiently mangled that we should hide type errors
+ xrefs []byte // serializable index of outbound cross-references
+ analyses memoize.Store // maps analyzer.Name to Promise[actionResult]
+ methodsets *methodsets.Index // index of method sets of package-level types
}
-func (p *pkg) String() string { return string(p.ID()) }
+func (p *Package) String() string { return string(p.m.ID) }
+
+func (p *Package) Metadata() *source.Metadata {
+ return p.m
+}
// A loadScope defines a package loading scope for use with go/packages.
+//
+// TODO(rfindley): move this to load.go.
type loadScope interface {
aScope()
}
@@ -70,127 +136,96 @@
func (moduleLoadScope) aScope() {}
func (viewLoadScope) aScope() {}
-func (p *pkg) ID() PackageID { return p.m.ID }
-func (p *pkg) Name() PackageName { return p.m.Name }
-func (p *pkg) PkgPath() PackagePath { return p.m.PkgPath }
-
-func (p *pkg) ParseMode() source.ParseMode {
- return p.mode
+func (p *Package) ParseMode() source.ParseMode {
+ return p.pkg.mode
}
-func (p *pkg) CompiledGoFiles() []*source.ParsedGoFile {
- return p.compiledGoFiles
+func (p *Package) CompiledGoFiles() []*source.ParsedGoFile {
+ return p.pkg.compiledGoFiles
}
-func (p *pkg) File(uri span.URI) (*source.ParsedGoFile, error) {
- for _, cgf := range p.compiledGoFiles {
+func (p *Package) File(uri span.URI) (*source.ParsedGoFile, error) {
+ return p.pkg.File(uri)
+}
+
+func (pkg *syntaxPackage) File(uri span.URI) (*source.ParsedGoFile, error) {
+ for _, cgf := range pkg.compiledGoFiles {
if cgf.URI == uri {
return cgf, nil
}
}
- for _, gf := range p.goFiles {
+ for _, gf := range pkg.goFiles {
if gf.URI == uri {
return gf, nil
}
}
- return nil, fmt.Errorf("no parsed file for %s in %v", uri, p.m.ID)
+ return nil, fmt.Errorf("no parsed file for %s in %v", uri, pkg.id)
}
-func (p *pkg) GetSyntax() []*ast.File {
+func (p *Package) GetSyntax() []*ast.File {
var syntax []*ast.File
- for _, pgf := range p.compiledGoFiles {
+ for _, pgf := range p.pkg.compiledGoFiles {
syntax = append(syntax, pgf.File)
}
return syntax
}
-func (p *pkg) FileSet() *token.FileSet {
- return p.fset
+func (p *Package) FileSet() *token.FileSet {
+ return p.pkg.fset
}
-func (p *pkg) GetTypes() *types.Package {
- return p.types
+func (p *Package) GetTypes() *types.Package {
+ return p.pkg.types
}
-func (p *pkg) GetTypesInfo() *types.Info {
- return p.typesInfo
+func (p *Package) GetTypesInfo() *types.Info {
+ return p.pkg.typesInfo
}
-func (p *pkg) GetTypesSizes() types.Sizes {
- return p.m.TypesSizes
+func (p *Package) HasParseErrors() bool {
+ return len(p.pkg.parseErrors) != 0
}
-func (p *pkg) ForTest() string {
- return string(p.m.ForTest)
+func (p *Package) HasTypeErrors() bool {
+ return len(p.pkg.typeErrors) != 0
}
-// DirectDep returns the directly imported dependency of this package,
-// given its PackagePath. (If you have an ImportPath, e.g. a string
-// from an import declaration, use ResolveImportPath instead.
-// They may differ in case of vendoring.)
-func (p *pkg) DirectDep(pkgPath PackagePath) (source.Package, error) {
- if id, ok := p.m.DepsByPkgPath[pkgPath]; ok {
- if imp := p.deps[id]; imp != nil {
- return imp, nil
- }
- }
- return nil, fmt.Errorf("package does not import package with path %s", pkgPath)
-}
-
-// ResolveImportPath returns the directly imported dependency of this package,
-// given its ImportPath. See also DirectDep.
-func (p *pkg) ResolveImportPath(importPath ImportPath) (source.Package, error) {
- if id, ok := p.m.DepsByImpPath[importPath]; ok && id != "" {
- if imp := p.deps[id]; imp != nil {
- return imp, nil
- }
- }
- return nil, fmt.Errorf("package does not import %s", importPath)
-}
-
-func (p *pkg) Imports() []source.Package {
- var result []source.Package // unordered
- for _, dep := range p.deps {
- result = append(result, dep)
- }
- return result
-}
-
-func (p *pkg) Version() *module.Version {
- return p.version
-}
-
-func (p *pkg) HasListOrParseErrors() bool {
- return len(p.m.Errors) != 0 || len(p.parseErrors) != 0
-}
-
-func (p *pkg) HasTypeErrors() bool {
- return len(p.typeErrors) != 0
-}
-
-func (p *pkg) DiagnosticsForFile(uri span.URI) []*source.Diagnostic {
- var res []*source.Diagnostic
- for _, diag := range p.diagnostics {
+func (p *Package) DiagnosticsForFile(ctx context.Context, s source.Snapshot, uri span.URI) ([]*source.Diagnostic, error) {
+ var diags []*source.Diagnostic
+ for _, diag := range p.pkg.diagnostics {
if diag.URI == uri {
- res = append(res, diag)
+ diags = append(diags, diag)
}
}
- return res
+
+ if p.loadDiagnostics != nil {
+ res, err := p.loadDiagnostics.Get(ctx, s)
+ if err != nil {
+ return nil, err
+ }
+ for _, diag := range res.([]*source.Diagnostic) {
+ if diag.URI == uri {
+ diags = append(diags, diag)
+ }
+ }
+ }
+
+ return diags, nil
}
// ReferencesTo returns the location of each reference within package p
// to one of the target objects denoted by the pair (package path, object path).
-func (p *pkg) ReferencesTo(targets map[PackagePath]map[objectpath.Path]struct{}) []protocol.Location {
+func (p *Package) ReferencesTo(targets map[PackagePath]map[objectpath.Path]struct{}) []protocol.Location {
// TODO(adonovan): In future, p.xrefs will be retrieved from a
// section of the cache file produced by type checking.
// (Other sections will include the package's export data,
// "implements" relations, exported symbols, etc.)
// For now we just hang it off the pkg.
- return xrefs.Lookup(p.m, p.xrefs, targets)
+ return xrefs.Lookup(p.m, p.pkg.xrefs, targets)
}
-func (p *pkg) MethodSetsIndex() *methodsets.Index {
+func (p *Package) MethodSetsIndex() *methodsets.Index {
// TODO(adonovan): In future, p.methodsets will be retrieved from a
// section of the cache file produced by type checking.
- return p.methodsets
+ return p.pkg.methodsets
}
diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go
index 51d7a79..995fba3 100644
--- a/gopls/internal/lsp/cache/snapshot.go
+++ b/gopls/internal/lsp/cache/snapshot.go
@@ -118,7 +118,7 @@
// analyses maps an analysisKey (which identifies a package
// and a set of analyzers) to the handle for the future result
// of loading the package and analyzing it.
- analyses *persistent.Map // from analysisKey to analysisHandle
+ analyses *persistent.Map // from analysisKey to analysisPromise
// workspacePackages contains the workspace's packages, which are loaded
// when the view is created.
@@ -626,10 +626,20 @@
}
// TypeCheck type-checks the specified packages in the given mode.
+//
+// The resulting packages slice always contains len(ids) entries, though some
+// of them may be nil if (and only if) the resulting error is non-nil.
+//
+// An error is returned if any of the packages fail to type-check. This is
+// different from having type-checking errors: a failure to type-check
+// indicates context cancellation or otherwise significant failure to perform
+// the type-checking operation.
func (s *snapshot) TypeCheck(ctx context.Context, mode source.TypecheckMode, ids ...PackageID) ([]source.Package, error) {
// Build all the handles...
- var phs []*packageHandle
- for _, id := range ids {
+ phs := make([]*packageHandle, len(ids))
+ pkgs := make([]source.Package, len(ids))
+ var firstErr error
+ for i, id := range ids {
parseMode := source.ParseFull
if mode == source.TypecheckWorkspace {
parseMode = s.workspaceParseMode(id)
@@ -637,21 +647,36 @@
ph, err := s.buildPackageHandle(ctx, id, parseMode)
if err != nil {
- return nil, err
+ if firstErr == nil {
+ firstErr = err
+ }
+ if ctx.Err() != nil {
+ return pkgs, firstErr
+ }
+ continue
}
- phs = append(phs, ph)
+ phs[i] = ph
}
// ...then await them all.
- var pkgs []source.Package
- for _, ph := range phs {
- pkg, err := ph.await(ctx, s)
- if err != nil {
- return nil, err
+ for i, ph := range phs {
+ if ph == nil {
+ continue
}
- pkgs = append(pkgs, pkg)
+ p, err := ph.await(ctx, s)
+ if err != nil {
+ if firstErr == nil {
+ firstErr = err
+ }
+ if ctx.Err() != nil {
+ return pkgs, firstErr
+ }
+ continue
+ }
+ pkgs[i] = newPackage(ph.m, p)
}
- return pkgs, nil
+
+ return pkgs, firstErr
}
func (s *snapshot) MetadataForFile(ctx context.Context, uri span.URI) ([]*source.Metadata, error) {
@@ -1058,7 +1083,7 @@
return meta, nil
}
-func (s *snapshot) CachedImportPaths(ctx context.Context) (map[PackagePath]source.Package, error) {
+func (s *snapshot) CachedImportPaths(ctx context.Context) (map[PackagePath]*types.Package, error) {
// Don't reload workspace package metadata.
// This function is meant to only return currently cached information.
s.AwaitInitialized(ctx)
@@ -1066,24 +1091,41 @@
s.mu.Lock()
defer s.mu.Unlock()
- results := map[PackagePath]source.Package{}
+ pkgs := make(map[PackagePath]*syntaxPackage)
+
+ // Find all cached packages that are imported a nonzero amount of time.
+ //
+ // TODO(rfindley): this is pre-existing behavior, and a test fails if we
+ // don't do the importCount filter, but why do we care if a package is
+ // imported a nonzero amount of times?
+ imported := make(map[PackagePath]bool)
s.packages.Range(func(_, v interface{}) {
- cachedPkg, err := v.(*packageHandle).cached()
+ ph := v.(*packageHandle)
+ for dep := range ph.m.DepsByPkgPath {
+ imported[dep] = true
+ }
+ if ph.m.Name == "main" {
+ return
+ }
+ pkg, err := ph.cached()
if err != nil {
return
}
- for _, newPkg := range cachedPkg.deps {
- pkgPath := newPkg.PkgPath()
- if oldPkg, ok := results[pkgPath]; ok {
- // Using the same trick as NarrowestPackage, prefer non-variants.
- if len(newPkg.compiledGoFiles) < len(oldPkg.(*pkg).compiledGoFiles) {
- results[pkgPath] = newPkg
- }
- } else {
- results[pkgPath] = newPkg
+ if old, ok := pkgs[ph.m.PkgPath]; ok {
+ if len(pkg.compiledGoFiles) < len(old.compiledGoFiles) {
+ pkgs[ph.m.PkgPath] = pkg
}
+ } else {
+ pkgs[ph.m.PkgPath] = pkg
}
})
+ results := make(map[PackagePath]*types.Package)
+ for pkgPath, pkg := range pkgs {
+ if imported[pkgPath] {
+ results[pkgPath] = pkg.types
+ }
+ }
+
return results, nil
}
diff --git a/gopls/internal/lsp/code_action.go b/gopls/internal/lsp/code_action.go
index ec918b7..c91def6 100644
--- a/gopls/internal/lsp/code_action.go
+++ b/gopls/internal/lsp/code_action.go
@@ -162,12 +162,16 @@
if err != nil {
return nil, err
}
- analysisDiags, err := source.Analyze(ctx, snapshot, pkg.ID(), true)
+ pkgDiags, err := pkg.DiagnosticsForFile(ctx, snapshot, uri)
+ if err != nil {
+ return nil, err
+ }
+ analysisDiags, err := source.Analyze(ctx, snapshot, pkg.Metadata().ID, true)
if err != nil {
return nil, err
}
var fileDiags []*source.Diagnostic
- source.CombineDiagnostics(pkg, fh.URI(), analysisDiags, &fileDiags, &fileDiags)
+ source.CombineDiagnostics(pkgDiags, analysisDiags[uri], &fileDiags, &fileDiags)
// Split diagnostics into fixes, which must match incoming diagnostics,
// and non-fixes, which must match the requested range. Build actions
diff --git a/gopls/internal/lsp/diagnostics.go b/gopls/internal/lsp/diagnostics.go
index ef30b6f..a58f404 100644
--- a/gopls/internal/lsp/diagnostics.go
+++ b/gopls/internal/lsp/diagnostics.go
@@ -401,8 +401,15 @@
if snapshot.IsBuiltin(ctx, cgf.URI) {
continue
}
+
+ pkgDiags, err := pkg.DiagnosticsForFile(ctx, snapshot, cgf.URI)
+ if err != nil {
+ event.Error(ctx, "warning: getting package diagnostics", err, append(source.SnapshotLabels(snapshot), tag.Package.Of(string(m.ID)))...)
+ return
+ }
+
var tdiags, adiags []*source.Diagnostic
- source.CombineDiagnostics(pkg, cgf.URI, analysisDiags, &tdiags, &adiags)
+ source.CombineDiagnostics(pkgDiags, analysisDiags[cgf.URI], &tdiags, &adiags)
s.storeDiagnostics(snapshot, cgf.URI, typeCheckSource, tdiags, true)
s.storeDiagnostics(snapshot, cgf.URI, analysisSource, adiags, true)
}
diff --git a/gopls/internal/lsp/mod/diagnostics.go b/gopls/internal/lsp/mod/diagnostics.go
index 3cbe352..25860ea 100644
--- a/gopls/internal/lsp/mod/diagnostics.go
+++ b/gopls/internal/lsp/mod/diagnostics.go
@@ -107,7 +107,11 @@
return nil, err
}
for _, pkg := range pkgs {
- diagnostics = append(diagnostics, pkg.DiagnosticsForFile(fh.URI())...)
+ pkgDiags, err := pkg.DiagnosticsForFile(ctx, snapshot, fh.URI())
+ if err != nil {
+ return nil, err
+ }
+ diagnostics = append(diagnostics, pkgDiags...)
}
}
diff --git a/gopls/internal/lsp/semantic.go b/gopls/internal/lsp/semantic.go
index 728f61d..9332877 100644
--- a/gopls/internal/lsp/semantic.go
+++ b/gopls/internal/lsp/semantic.go
@@ -76,10 +76,11 @@
// this is a little cumbersome to avoid both exporting 'encoded' and its methods
// and to avoid import cycles
e := &encoded{
- ctx: ctx,
- rng: rng,
- tokTypes: s.session.Options().SemanticTypes,
- tokMods: s.session.Options().SemanticMods,
+ ctx: ctx,
+ metadataSource: snapshot,
+ rng: rng,
+ tokTypes: s.session.Options().SemanticTypes,
+ tokMods: s.session.Options().SemanticMods,
}
add := func(line, start uint32, len uint32) {
e.add(line, start, len, tokMacro, nil)
@@ -103,16 +104,17 @@
return nil, err
}
e := &encoded{
- ctx: ctx,
- pgf: pgf,
- rng: rng,
- ti: pkg.GetTypesInfo(),
- pkg: pkg,
- fset: pkg.FileSet(),
- tokTypes: s.session.Options().SemanticTypes,
- tokMods: s.session.Options().SemanticMods,
- noStrings: vv.Options().NoSemanticString,
- noNumbers: vv.Options().NoSemanticNumber,
+ ctx: ctx,
+ metadataSource: snapshot,
+ pgf: pgf,
+ rng: rng,
+ ti: pkg.GetTypesInfo(),
+ pkg: pkg,
+ fset: pkg.FileSet(),
+ tokTypes: s.session.Options().SemanticTypes,
+ tokMods: s.session.Options().SemanticMods,
+ noStrings: vv.Options().NoSemanticString,
+ noNumbers: vv.Options().NoSemanticNumber,
}
if err := e.init(); err != nil {
// e.init should never return an error, unless there's some
@@ -216,7 +218,11 @@
noStrings bool
noNumbers bool
- ctx context.Context
+ ctx context.Context
+ // metadataSource is used to resolve imports
+ metadataSource interface {
+ Metadata(source.PackageID) *source.Metadata
+ }
tokTypes, tokMods []string
pgf *source.ParsedGoFile
rng *protocol.Range
@@ -908,20 +914,24 @@
return
}
// Import strings are implementation defined. Try to match with parse information.
- imported, err := e.pkg.ResolveImportPath(importPath)
- if err != nil {
+ depID := e.pkg.Metadata().DepsByImpPath[importPath]
+ if depID == "" {
+ return
+ }
+ depMD := e.metadataSource.Metadata(depID)
+ if depMD == nil {
// unexpected, but impact is that maybe some import is not colored
return
}
// Check whether the original literal contains the package's declared name.
- j := strings.LastIndex(d.Path.Value, string(imported.Name()))
+ j := strings.LastIndex(d.Path.Value, string(depMD.Name))
if j == -1 {
- // name doesn't show up, for whatever reason, so nothing to report
+ // Package name does not match import path, so there is nothing to report.
return
}
// Report virtual declaration at the position of the substring.
start := d.Path.Pos() + token.Pos(j)
- e.token(start, len(imported.Name()), tokNamespace, nil)
+ e.token(start, len(depMD.Name), tokNamespace, nil)
}
// log unexpected state
diff --git a/gopls/internal/lsp/source/api_json.go b/gopls/internal/lsp/source/api_json.go
index e97c147..3fa7ca2 100755
--- a/gopls/internal/lsp/source/api_json.go
+++ b/gopls/internal/lsp/source/api_json.go
@@ -57,14 +57,6 @@
Hierarchy: "build",
},
{
- Name: "experimentalPackageCacheKey",
- Type: "bool",
- Doc: "experimentalPackageCacheKey controls whether to use a coarser cache key\nfor package type information to increase cache hits. This setting removes\nthe user's environment, build flags, and working directory from the cache\nkey, which should be a safe change as all relevant inputs into the type\nchecking pass are already hashed into the key. This is temporarily guarded\nby an experiment because caching behavior is subtle and difficult to\ncomprehensively test.\n",
- Default: "true",
- Status: "experimental",
- Hierarchy: "build",
- },
- {
Name: "allowModfileModifications",
Type: "bool",
Doc: "allowModfileModifications disables -mod=readonly, allowing imports from\nout-of-scope modules. This option will eventually be removed.\n",
diff --git a/gopls/internal/lsp/source/completion/completion.go b/gopls/internal/lsp/source/completion/completion.go
index 056b289..3343a1a 100644
--- a/gopls/internal/lsp/source/completion/completion.go
+++ b/gopls/internal/lsp/source/completion/completion.go
@@ -1088,10 +1088,12 @@
// Is sel a qualified identifier?
if id, ok := sel.X.(*ast.Ident); ok {
if pkgName, ok := c.pkg.GetTypesInfo().Uses[id].(*types.PkgName); ok {
- pkg, _ := c.pkg.DirectDep(source.PackagePath(pkgName.Imported().Path()))
- // If the package is not imported, try searching for unimported
- // completions.
- if pkg == nil && c.opts.unimported {
+ // If this package path is not a known dep, it means that it resolves to
+ // an import path that couldn't be resolved by go/packages.
+ //
+ // Try to complete from the package cache.
+ pkgPath := source.PackagePath(pkgName.Imported().Path())
+ if _, ok := c.pkg.Metadata().DepsByPkgPath[pkgPath]; !ok && c.opts.unimported {
if err := c.unimportedMembers(ctx, id); err != nil {
return err
}
@@ -1133,7 +1135,7 @@
var paths []string
for path, pkg := range known {
- if pkg.GetTypes().Name() != id.Name {
+ if pkg.Name() != id.Name {
continue
}
paths = append(paths, string(path))
@@ -1155,16 +1157,16 @@
for _, path := range paths {
pkg := known[source.PackagePath(path)]
- if pkg.GetTypes().Name() != id.Name {
+ if pkg.Name() != id.Name {
continue
}
imp := &importInfo{
importPath: path,
}
- if imports.ImportPathToAssumedName(path) != pkg.GetTypes().Name() {
- imp.name = pkg.GetTypes().Name()
+ if imports.ImportPathToAssumedName(path) != pkg.Name() {
+ imp.name = pkg.Name()
}
- c.packageMembers(pkg.GetTypes(), unimportedScore(relevances[path]), imp, func(cand candidate) {
+ c.packageMembers(pkg, unimportedScore(relevances[path]), imp, func(cand candidate) {
c.deepState.enqueue(cand)
})
if len(c.items) >= unimportedMemberTarget {
@@ -1479,7 +1481,7 @@
}
var paths []string // actually PackagePaths
for path, pkg := range known {
- if !strings.HasPrefix(pkg.GetTypes().Name(), prefix) {
+ if !strings.HasPrefix(pkg.Name(), prefix) {
continue
}
paths = append(paths, string(path))
@@ -1508,21 +1510,21 @@
for _, path := range paths {
pkg := known[source.PackagePath(path)]
- if _, ok := seen[pkg.GetTypes().Name()]; ok {
+ if _, ok := seen[pkg.Name()]; ok {
continue
}
imp := &importInfo{
importPath: path,
}
- if imports.ImportPathToAssumedName(path) != pkg.GetTypes().Name() {
- imp.name = pkg.GetTypes().Name()
+ if imports.ImportPathToAssumedName(path) != pkg.Name() {
+ imp.name = pkg.Name()
}
if count >= maxUnimportedPackageNames {
return nil
}
c.deepState.enqueue(candidate{
// Pass an empty *types.Package to disable deep completions.
- obj: types.NewPkgName(0, nil, pkg.GetTypes().Name(), types.NewPackage(path, string(pkg.Name()))),
+ obj: types.NewPkgName(0, nil, pkg.Name(), types.NewPackage(path, string(pkg.Name()))),
score: unimportedScore(relevances[path]),
imp: imp,
})
diff --git a/gopls/internal/lsp/source/completion/format.go b/gopls/internal/lsp/source/completion/format.go
index bb3ccd7..16da642 100644
--- a/gopls/internal/lsp/source/completion/format.go
+++ b/gopls/internal/lsp/source/completion/format.go
@@ -81,7 +81,7 @@
if _, ok := obj.Type().(*types.Struct); ok {
detail = "struct{...}" // for anonymous structs
} else if obj.IsField() {
- detail = source.FormatVarType(c.pkg, obj, c.qf)
+ detail = source.FormatVarType(ctx, c.snapshot, c.pkg, obj, c.qf)
}
if obj.IsField() {
kind = protocol.FieldCompletion
@@ -238,7 +238,7 @@
uri := span.URIFromPath(pos.Filename)
// Find the source file of the candidate.
- pkg, err := source.FindPackageFromPos(c.pkg, obj.Pos())
+ pkg, err := source.FindPackageFromPos(ctx, c.snapshot, c.pkg, obj.Pos())
if err != nil {
return item, nil
}
diff --git a/gopls/internal/lsp/source/completion/literal.go b/gopls/internal/lsp/source/completion/literal.go
index 870b97a..9d4b0e6 100644
--- a/gopls/internal/lsp/source/completion/literal.go
+++ b/gopls/internal/lsp/source/completion/literal.go
@@ -162,7 +162,7 @@
if score := c.matcher.Score("func"); !cand.hasMod(reference) && score > 0 && (expType == nil || !types.IsInterface(expType)) {
switch t := literalType.Underlying().(type) {
case *types.Signature:
- c.functionLiteral(t, float64(score))
+ c.functionLiteral(ctx, t, float64(score))
}
}
}
@@ -175,7 +175,7 @@
// functionLiteral adds a function literal completion item for the
// given signature.
-func (c *completer) functionLiteral(sig *types.Signature, matchScore float64) {
+func (c *completer) functionLiteral(ctx context.Context, sig *types.Signature, matchScore float64) {
snip := &snippet.Builder{}
snip.WriteText("func(")
@@ -202,7 +202,7 @@
// If the param has no name in the signature, guess a name based
// on the type. Use an empty qualifier to ignore the package.
// For example, we want to name "http.Request" "r", not "hr".
- name = source.FormatVarType(c.pkg, p, func(p *types.Package) string {
+ name = source.FormatVarType(ctx, c.snapshot, c.pkg, p, func(p *types.Package) string {
return ""
})
name = abbreviateTypeName(name)
@@ -264,7 +264,7 @@
// of "i int, j int".
if i == sig.Params().Len()-1 || !types.Identical(p.Type(), sig.Params().At(i+1).Type()) {
snip.WriteText(" ")
- typeStr := source.FormatVarType(c.pkg, p, c.qf)
+ typeStr := source.FormatVarType(ctx, c.snapshot, c.pkg, p, c.qf)
if sig.Variadic() && i == sig.Params().Len()-1 {
typeStr = strings.Replace(typeStr, "[]", "...", 1)
}
@@ -314,7 +314,7 @@
snip.WriteText(name + " ")
}
- text := source.FormatVarType(c.pkg, r, c.qf)
+ text := source.FormatVarType(ctx, c.snapshot, c.pkg, r, c.qf)
if tp, _ := r.Type().(*typeparams.TypeParam); tp != nil && !c.typeParamInScope(tp) {
snip.WritePlaceholder(func(snip *snippet.Builder) {
snip.WriteText(text)
diff --git a/gopls/internal/lsp/source/completion/statements.go b/gopls/internal/lsp/source/completion/statements.go
index dcce76f..809cb80 100644
--- a/gopls/internal/lsp/source/completion/statements.go
+++ b/gopls/internal/lsp/source/completion/statements.go
@@ -330,24 +330,31 @@
return ""
}
+ var testingPkg *types.Package
+ for _, p := range pkg.GetTypes().Imports() {
+ if p.Path() == "testing" {
+ testingPkg = p
+ break
+ }
+ }
+ if testingPkg == nil {
+ return ""
+ }
+ tbObj := testingPkg.Scope().Lookup("TB")
+ if tbObj == nil {
+ return ""
+ }
+ iface, ok := tbObj.Type().Underlying().(*types.Interface)
+ if !ok {
+ return ""
+ }
+
sig := enclosingFunc.sig
for i := 0; i < sig.Params().Len(); i++ {
param := sig.Params().At(i)
if param.Name() == "_" {
continue
}
- testingPkg, err := pkg.DirectDep("testing")
- if err != nil {
- continue
- }
- tbObj := testingPkg.GetTypes().Scope().Lookup("TB")
- if tbObj == nil {
- continue
- }
- iface, ok := tbObj.Type().Underlying().(*types.Interface)
- if !ok {
- continue
- }
if !types.Implements(param.Type(), iface) {
continue
}
diff --git a/gopls/internal/lsp/source/diagnostics.go b/gopls/internal/lsp/source/diagnostics.go
index a7350b0..042db17 100644
--- a/gopls/internal/lsp/source/diagnostics.go
+++ b/gopls/internal/lsp/source/diagnostics.go
@@ -82,18 +82,22 @@
if err != nil {
return nil, nil, err
}
- adiags, err := Analyze(ctx, snapshot, pkg.ID(), false)
+ pkgDiags, err := pkg.DiagnosticsForFile(ctx, snapshot, uri)
+ if err != nil {
+ return nil, nil, err
+ }
+ adiags, err := Analyze(ctx, snapshot, pkg.Metadata().ID, false)
if err != nil {
return nil, nil, err
}
var fileDiags []*Diagnostic // combine load/parse/type + analysis diagnostics
- CombineDiagnostics(pkg, fh.URI(), adiags, &fileDiags, &fileDiags)
+ CombineDiagnostics(pkgDiags, adiags[uri], &fileDiags, &fileDiags)
return fh, fileDiags, nil
}
-// CombineDiagnostics combines and filters list/parse/type diagnostics
-// from pkg.DiagnosticsForFile(uri) with analysisDiagnostics[uri], and
-// appends the two lists to *outT and *outA, respectively.
+// CombineDiagnostics combines and filters list/parse/type diagnostics from
+// tdiags with adiags, and appends the two lists to *outT and *outA,
+// respectively.
//
// Type-error analyzers produce diagnostics that are redundant
// with type checker diagnostics, but more detailed (e.g. fixes).
@@ -111,7 +115,7 @@
// easily choose whether to keep the results separate or combined.
//
// The arguments are not modified.
-func CombineDiagnostics(pkg Package, uri span.URI, analysisDiagnostics map[span.URI][]*Diagnostic, outT, outA *[]*Diagnostic) {
+func CombineDiagnostics(tdiags []*Diagnostic, adiags []*Diagnostic, outT, outA *[]*Diagnostic) {
// Build index of (list+parse+)type errors.
type key struct {
@@ -119,14 +123,13 @@
message string
}
index := make(map[key]int) // maps (Range,Message) to index in tdiags slice
- tdiags := pkg.DiagnosticsForFile(uri)
for i, diag := range tdiags {
index[key{diag.Range, diag.Message}] = i
}
// Filter out analysis diagnostics that match type errors,
// retaining their suggested fix (etc) fields.
- for _, diag := range analysisDiagnostics[uri] {
+ for _, diag := range adiags {
if i, ok := index[key{diag.Range, diag.Message}]; ok {
copy := *tdiags[i]
copy.SuggestedFixes = diag.SuggestedFixes
diff --git a/gopls/internal/lsp/source/highlight.go b/gopls/internal/lsp/source/highlight.go
index 4fe3ae8..d0f77e6 100644
--- a/gopls/internal/lsp/source/highlight.go
+++ b/gopls/internal/lsp/source/highlight.go
@@ -54,7 +54,7 @@
}
var ranges []protocol.Range
for rng := range result {
- mRng, err := posToMappedRange(pkg, rng.start, rng.end)
+ mRng, err := posToMappedRange(ctx, snapshot, pkg, rng.start, rng.end)
if err != nil {
return nil, err
}
diff --git a/gopls/internal/lsp/source/hover.go b/gopls/internal/lsp/source/hover.go
index 9eee53a..c270656 100644
--- a/gopls/internal/lsp/source/hover.go
+++ b/gopls/internal/lsp/source/hover.go
@@ -327,7 +327,7 @@
// Check if the identifier is test-only (and is therefore not part of a
// package's API). This is true if the request originated in a test package,
// and if the declaration is also found in the same test package.
- if i.pkg != nil && obj.Pkg() != nil && i.pkg.ForTest() != "" {
+ if i.pkg != nil && obj.Pkg() != nil && i.pkg.Metadata().ForTest != "" {
if _, err := i.pkg.File(i.Declaration.MappedRange[0].URI()); err == nil {
return h, nil
}
@@ -443,18 +443,22 @@
if strings.ToLower(i.Snapshot.View().Options().LinkTarget) != "pkg.go.dev" {
return "", "", false
}
- impPkg, err := i.pkg.DirectDep(PackagePath(path))
- if err != nil {
+ impID, ok := i.pkg.Metadata().DepsByPkgPath[PackagePath(path)]
+ if !ok {
return "", "", false
}
- if impPkg.Version() == nil {
+ impMeta := i.Snapshot.Metadata(impID)
+ if impMeta == nil {
return "", "", false
}
- version, modpath := impPkg.Version().Version, impPkg.Version().Path
- if modpath == "" || version == "" {
+ module := impMeta.Module
+ if module == nil {
return "", "", false
}
- return modpath, version, true
+ if module.Path == "" || module.Version == "" {
+ return "", "", false
+ }
+ return module.Path, module.Version, true
}
// objectString is a wrapper around the types.ObjectString function.
@@ -534,7 +538,9 @@
if err != nil {
return nil, err
}
- imp, err := pkg.ResolveImportPath(ImportPath(importPath))
+ // TODO(rfindley): avoid type-checking here, by re-parsing the package with
+ // ParseHeader.
+ imp, err := ResolveImportPath(ctx, s, pkg.Metadata().ID, ImportPath(importPath))
if err != nil {
return nil, err
}
diff --git a/gopls/internal/lsp/source/identifier.go b/gopls/internal/lsp/source/identifier.go
index 39743b2..0be26b5 100644
--- a/gopls/internal/lsp/source/identifier.go
+++ b/gopls/internal/lsp/source/identifier.go
@@ -98,7 +98,7 @@
file := pgf.File
// Handle import specs separately, as there is no formal position for a
// package declaration.
- if result, err := importSpec(snapshot, pkg, pgf, pos); result != nil || err != nil {
+ if result, err := importSpec(ctx, snapshot, pkg, pgf, pos); result != nil || err != nil {
return result, err
}
path := pathEnclosingObjNode(file, pos)
@@ -155,7 +155,7 @@
result.Name = result.ident.Name
var err error
- if result.MappedRange, err = posToMappedRange(pkg, result.ident.Pos(), result.ident.End()); err != nil {
+ if result.MappedRange, err = posToMappedRange(ctx, snapshot, pkg, result.ident.Pos(), result.ident.End()); err != nil {
return nil, err
}
@@ -266,7 +266,7 @@
// findFileInDeps, which is also called below. Refactor
// objToMappedRange to separate the find-file from the
// lookup-position steps to avoid the redundancy.
- rng, err := objToMappedRange(pkg, result.Declaration.obj)
+ rng, err := objToMappedRange(ctx, snapshot, pkg, result.Declaration.obj)
if err != nil {
return nil, err
}
@@ -274,7 +274,7 @@
declPos := result.Declaration.obj.Pos()
objURI := span.URIFromPath(pkg.FileSet().File(declPos).Name())
- declFile, declPkg, err := findFileInDeps(pkg, objURI)
+ declFile, declPkg, err := findFileInDeps(ctx, snapshot, pkg, objURI)
if err != nil {
return nil, err
}
@@ -301,7 +301,7 @@
if hasErrorType(result.Type.Object) {
return result, nil
}
- if result.Type.MappedRange, err = objToMappedRange(pkg, result.Type.Object); err != nil {
+ if result.Type.MappedRange, err = objToMappedRange(ctx, snapshot, pkg, result.Type.Object); err != nil {
return nil, err
}
}
@@ -449,7 +449,7 @@
}
// importSpec handles positions inside of an *ast.ImportSpec.
-func importSpec(snapshot Snapshot, pkg Package, pgf *ParsedGoFile, pos token.Pos) (*IdentifierInfo, error) {
+func importSpec(ctx context.Context, snapshot Snapshot, pkg Package, pgf *ParsedGoFile, pos token.Pos) (*IdentifierInfo, error) {
var imp *ast.ImportSpec
for _, spec := range pgf.File.Imports {
if spec.Path.Pos() <= pos && pos < spec.Path.End() {
@@ -463,7 +463,7 @@
if err != nil {
return nil, fmt.Errorf("import path not quoted: %s (%v)", imp.Path.Value, err)
}
- imported, err := pkg.ResolveImportPath(ImportPath(importPath))
+ imported, err := ResolveImportPath(ctx, snapshot, pkg.Metadata().ID, ImportPath(importPath))
if err != nil {
return nil, err
}
@@ -472,13 +472,13 @@
Name: importPath, // should this perhaps be imported.PkgPath()?
pkg: pkg,
}
- if result.MappedRange, err = posToMappedRange(pkg, imp.Path.Pos(), imp.Path.End()); err != nil {
+ if result.MappedRange, err = posToMappedRange(ctx, snapshot, pkg, imp.Path.Pos(), imp.Path.End()); err != nil {
return nil, err
}
// Consider the "declaration" of an import spec to be the imported package.
// Return all of the files in the package as the definition of the import spec.
for _, dst := range imported.GetSyntax() {
- rng, err := posToMappedRange(pkg, dst.Pos(), dst.End())
+ rng, err := posToMappedRange(ctx, snapshot, pkg, dst.Pos(), dst.End())
if err != nil {
return nil, err
}
diff --git a/gopls/internal/lsp/source/implementation.go b/gopls/internal/lsp/source/implementation.go
index be29973..5052026 100644
--- a/gopls/internal/lsp/source/implementation.go
+++ b/gopls/internal/lsp/source/implementation.go
@@ -324,22 +324,28 @@
}
// Get all of the transitive dependencies of the search package.
- pkgs := make(map[*types.Package]Package)
- var addPkg func(pkg Package)
- addPkg = func(pkg Package) {
- pkgs[pkg.GetTypes()] = pkg
- for _, imp := range pkg.Imports() {
- if _, ok := pkgs[imp.GetTypes()]; !ok {
- addPkg(imp)
- }
+ pkgSet := map[*types.Package]Package{
+ searchpkg.GetTypes(): searchpkg,
+ }
+ deps := recursiveDeps(s, searchpkg.Metadata())[1:]
+ // Ignore the error from type checking, but check if the context was
+ // canceled (which would have caused TypeCheck to exit early).
+ depPkgs, _ := s.TypeCheck(ctx, TypecheckWorkspace, deps...)
+ if ctx.Err() != nil {
+ return nil, ctx.Err()
+ }
+ for _, dep := range depPkgs {
+ // Since we ignored the error from type checking, pkg may be nil.
+ if dep != nil {
+ pkgSet[dep.GetTypes()] = dep
}
}
- addPkg(searchpkg)
+
for _, obj := range objs {
if obj.Parent() == types.Universe {
return nil, fmt.Errorf("%q: %w", obj.Name(), errBuiltin)
}
- pkg, ok := pkgs[obj.Pkg()]
+ pkg, ok := pkgSet[obj.Pkg()]
if !ok {
event.Error(ctx, fmt.Sprintf("no package for obj %s: %v", obj, obj.Pkg()), err)
continue
diff --git a/gopls/internal/lsp/source/implementation2.go b/gopls/internal/lsp/source/implementation2.go
index 09659cc..a776c03 100644
--- a/gopls/internal/lsp/source/implementation2.go
+++ b/gopls/internal/lsp/source/implementation2.go
@@ -166,7 +166,7 @@
}
globalIDs := make([]PackageID, 0, len(globalMetas))
for _, m := range globalMetas {
- if m.PkgPath == declPkg.PkgPath() {
+ if m.PkgPath == declPkg.Metadata().PkgPath {
continue // declaring package is handled by local implementation
}
globalIDs = append(globalIDs, m.ID)
diff --git a/gopls/internal/lsp/source/options.go b/gopls/internal/lsp/source/options.go
index a32ed12..bb0b5ec 100644
--- a/gopls/internal/lsp/source/options.go
+++ b/gopls/internal/lsp/source/options.go
@@ -116,12 +116,11 @@
},
UserOptions: UserOptions{
BuildOptions: BuildOptions{
- ExpandWorkspaceToModule: true,
- ExperimentalPackageCacheKey: true,
- MemoryMode: ModeNormal,
- DirectoryFilters: []string{"-**/node_modules"},
- TemplateExtensions: []string{},
- StandaloneTags: []string{"ignore"},
+ ExpandWorkspaceToModule: true,
+ MemoryMode: ModeNormal,
+ DirectoryFilters: []string{"-**/node_modules"},
+ TemplateExtensions: []string{},
+ StandaloneTags: []string{"ignore"},
},
UIOptions: UIOptions{
DiagnosticOptions: DiagnosticOptions{
@@ -269,15 +268,6 @@
// a go.mod file, narrowing the scope to that directory if it exists.
ExpandWorkspaceToModule bool `status:"experimental"`
- // ExperimentalPackageCacheKey controls whether to use a coarser cache key
- // for package type information to increase cache hits. This setting removes
- // the user's environment, build flags, and working directory from the cache
- // key, which should be a safe change as all relevant inputs into the type
- // checking pass are already hashed into the key. This is temporarily guarded
- // by an experiment because caching behavior is subtle and difficult to
- // comprehensively test.
- ExperimentalPackageCacheKey bool `status:"experimental"`
-
// AllowModfileModifications disables -mod=readonly, allowing imports from
// out-of-scope modules. This option will eventually be removed.
AllowModfileModifications bool `status:"experimental"`
@@ -1113,7 +1103,7 @@
result.deprecated("")
case "experimentalPackageCacheKey":
- result.setBool(&o.ExperimentalPackageCacheKey)
+ result.deprecated("")
case "allowModfileModifications":
result.setBool(&o.AllowModfileModifications)
diff --git a/gopls/internal/lsp/source/references.go b/gopls/internal/lsp/source/references.go
index f14d1d6..1fc7fde 100644
--- a/gopls/internal/lsp/source/references.go
+++ b/gopls/internal/lsp/source/references.go
@@ -93,7 +93,7 @@
// transitively for (e.g.) capitalized local variables.
// We could do better by checking for an objectpath.
transitive := qo.obj.Pkg().Scope().Lookup(qo.obj.Name()) != qo.obj
- rdeps, err := snapshot.ReverseDependencies(ctx, qo.pkg.ID(), transitive)
+ rdeps, err := snapshot.ReverseDependencies(ctx, qo.pkg.Metadata().ID, transitive)
if err != nil {
return nil, err
}
@@ -134,14 +134,14 @@
}
key, found := packagePositionKey(pkg, ident.Pos())
if !found {
- bug.Reportf("ident %v (pos: %v) not found in package %v", ident.Name, ident.Pos(), pkg.Name())
+ bug.Reportf("ident %v (pos: %v) not found in package %v", ident.Name, ident.Pos(), pkg.Metadata().ID)
continue
}
if seen[key] {
continue
}
seen[key] = true
- rng, err := posToMappedRange(pkg, ident.Pos(), ident.End())
+ rng, err := posToMappedRange(ctx, snapshot, pkg, ident.Pos(), ident.End())
if err != nil {
return nil, err
}
diff --git a/gopls/internal/lsp/source/references2.go b/gopls/internal/lsp/source/references2.go
index 4b6ba4c..1a01eb9 100644
--- a/gopls/internal/lsp/source/references2.go
+++ b/gopls/internal/lsp/source/references2.go
@@ -316,7 +316,7 @@
ref := &ReferenceInfoV2{
IsDeclaration: isDecl,
Location: loc,
- PkgPath: pkg.PkgPath(),
+ PkgPath: pkg.Metadata().PkgPath,
Name: obj.Name(),
}
refsMu.Lock()
@@ -402,7 +402,7 @@
// Expand global search scope to include rdeps of this pkg.
if len(results) > 0 {
- rdeps, err := snapshot.ReverseDependencies(ctx, pkg.ID(), true)
+ rdeps, err := snapshot.ReverseDependencies(ctx, pkg.Metadata().ID, true)
if err != nil {
return err
}
diff --git a/gopls/internal/lsp/source/rename.go b/gopls/internal/lsp/source/rename.go
index 1d2a03b..385dff4 100644
--- a/gopls/internal/lsp/source/rename.go
+++ b/gopls/internal/lsp/source/rename.go
@@ -30,6 +30,7 @@
type renamer struct {
ctx context.Context
+ snapshot Snapshot
refs []*ReferenceInfo
objsToUpdate map[types.Object]bool
hadConflicts bool
@@ -125,15 +126,15 @@
if err := checkRenamable(obj); err != nil {
return nil, nil, err
}
- result, err := computePrepareRenameResp(snapshot, pkg, node, obj.Name())
+ result, err := computePrepareRenameResp(ctx, snapshot, pkg, node, obj.Name())
if err != nil {
return nil, nil, err
}
return result, nil, nil
}
-func computePrepareRenameResp(snapshot Snapshot, pkg Package, node ast.Node, text string) (*PrepareItem, error) {
- mr, err := posToMappedRange(pkg, node.Pos(), node.End())
+func computePrepareRenameResp(ctx context.Context, snapshot Snapshot, pkg Package, node ast.Node, text string) (*PrepareItem, error) {
+ mr, err := posToMappedRange(ctx, snapshot, pkg, node.Pos(), node.End())
if err != nil {
return nil, err
}
@@ -592,6 +593,7 @@
}
r := renamer{
ctx: ctx,
+ snapshot: s,
refs: refs,
objsToUpdate: make(map[types.Object]bool),
from: obj.Name(),
@@ -735,7 +737,7 @@
// docComment returns the doc for an identifier.
func (r *renamer) docComment(pkg Package, id *ast.Ident) *ast.CommentGroup {
- _, tokFile, nodes, _ := pathEnclosingInterval(pkg, id.Pos(), id.End())
+ _, tokFile, nodes, _ := pathEnclosingInterval(r.ctx, r.snapshot, pkg, id.Pos(), id.End())
for _, node := range nodes {
switch decl := node.(type) {
case *ast.FuncDecl:
@@ -788,7 +790,7 @@
func (r *renamer) updatePkgName(pkgName *types.PkgName) (*diff.Edit, error) {
// Modify ImportSpec syntax to add or remove the Name as needed.
pkg := r.packages[pkgName.Pkg()]
- _, tokFile, path, _ := pathEnclosingInterval(pkg, pkgName.Pos(), pkgName.Pos())
+ _, tokFile, path, _ := pathEnclosingInterval(r.ctx, r.snapshot, pkg, pkgName.Pos(), pkgName.Pos())
if len(path) < 2 {
return nil, fmt.Errorf("no path enclosing interval for %s", pkgName.Name())
}
diff --git a/gopls/internal/lsp/source/rename_check.go b/gopls/internal/lsp/source/rename_check.go
index 0285fc0..e442dae 100644
--- a/gopls/internal/lsp/source/rename_check.go
+++ b/gopls/internal/lsp/source/rename_check.go
@@ -7,6 +7,7 @@
package source
import (
+ "context"
"fmt"
"go/ast"
"go/token"
@@ -371,7 +372,7 @@
if !ok {
return
}
- pkg, _, path, _ := pathEnclosingInterval(fromPkg, from.Pos(), from.Pos())
+ pkg, _, path, _ := pathEnclosingInterval(r.ctx, r.snapshot, fromPkg, from.Pos(), from.Pos())
if pkg == nil || path == nil {
return
}
@@ -790,10 +791,10 @@
// type-checker.
//
// Only proceed if all packages have no errors.
- if pkg.HasListOrParseErrors() || pkg.HasTypeErrors() {
+ if pkg.HasParseErrors() || pkg.HasTypeErrors() {
r.errorf(token.NoPos, // we don't have a position for this error.
"renaming %q to %q not possible because %q has errors",
- r.from, r.to, pkg.PkgPath())
+ r.from, r.to, pkg.Metadata().PkgPath)
return nil
}
f.Find(pkg.GetTypesInfo(), pkg.GetSyntax())
@@ -826,7 +827,9 @@
// exact is defined as for astutil.PathEnclosingInterval.
//
// The zero value is returned if not found.
-func pathEnclosingInterval(pkg Package, start, end token.Pos) (resPkg Package, tokFile *token.File, path []ast.Node, exact bool) {
+//
+// TODO(rfindley): this has some redundancy with FindPackageFromPos, etc. Refactor.
+func pathEnclosingInterval(ctx context.Context, s Snapshot, pkg Package, start, end token.Pos) (resPkg Package, tokFile *token.File, path []ast.Node, exact bool) {
pkgs := []Package{pkg}
for _, f := range pkg.GetSyntax() {
for _, imp := range f.Imports {
@@ -837,7 +840,7 @@
if importPath == "" {
continue
}
- imported, err := pkg.ResolveImportPath(importPath)
+ imported, err := ResolveImportPath(ctx, s, pkg.Metadata().ID, importPath)
if err != nil {
return nil, nil, nil, false
}
diff --git a/gopls/internal/lsp/source/signature_help.go b/gopls/internal/lsp/source/signature_help.go
index 3f81c27..e655481 100644
--- a/gopls/internal/lsp/source/signature_help.go
+++ b/gopls/internal/lsp/source/signature_help.go
@@ -94,7 +94,7 @@
comment *ast.CommentGroup
)
if obj != nil {
- declPkg, err := FindPackageFromPos(pkg, obj.Pos())
+ declPkg, err := FindPackageFromPos(ctx, snapshot, pkg, obj.Pos())
if err != nil {
return nil, 0, err
}
diff --git a/gopls/internal/lsp/source/types_format.go b/gopls/internal/lsp/source/types_format.go
index e3a884d..7a2605c 100644
--- a/gopls/internal/lsp/source/types_format.go
+++ b/gopls/internal/lsp/source/types_format.go
@@ -206,7 +206,7 @@
params := make([]string, 0, sig.Params().Len())
for i := 0; i < sig.Params().Len(); i++ {
el := sig.Params().At(i)
- typ := FormatVarType(pkg, el, qf)
+ typ := FormatVarType(ctx, s, pkg, el, qf)
p := typ
if el.Name() != "" {
p = el.Name() + " " + typ
@@ -221,7 +221,7 @@
needResultParens = true
}
el := sig.Results().At(i)
- typ := FormatVarType(pkg, el, qf)
+ typ := FormatVarType(ctx, s, pkg, el, qf)
if el.Name() == "" {
results = append(results, typ)
} else {
@@ -254,8 +254,8 @@
// FormatVarType formats a *types.Var, accounting for type aliases.
// To do this, it looks in the AST of the file in which the object is declared.
// On any errors, it always falls back to types.TypeString.
-func FormatVarType(srcpkg Package, obj *types.Var, qf types.Qualifier) string {
- pkg, err := FindPackageFromPos(srcpkg, obj.Pos())
+func FormatVarType(ctx context.Context, snapshot Snapshot, srcpkg Package, obj *types.Var, qf types.Qualifier) string {
+ pkg, err := FindPackageFromPos(ctx, snapshot, srcpkg, obj.Pos())
if err != nil {
return types.TypeString(obj.Type(), qf)
}
diff --git a/gopls/internal/lsp/source/util.go b/gopls/internal/lsp/source/util.go
index ec4c6bd..eacd9ce 100644
--- a/gopls/internal/lsp/source/util.go
+++ b/gopls/internal/lsp/source/util.go
@@ -51,7 +51,7 @@
return false
}
-func objToMappedRange(pkg Package, obj types.Object) (protocol.MappedRange, error) {
+func objToMappedRange(ctx context.Context, snapshot Snapshot, pkg Package, obj types.Object) (protocol.MappedRange, error) {
nameLen := len(obj.Name())
if pkgName, ok := obj.(*types.PkgName); ok {
// An imported Go package has a package-local, unqualified name.
@@ -68,7 +68,7 @@
nameLen = len(pkgName.Imported().Path()) + len(`""`)
}
}
- return posToMappedRange(pkg, obj.Pos(), obj.Pos()+token.Pos(nameLen))
+ return posToMappedRange(ctx, snapshot, pkg, obj.Pos(), obj.Pos()+token.Pos(nameLen))
}
// posToMappedRange returns the MappedRange for the given [start, end) span,
@@ -77,7 +77,7 @@
// TODO(adonovan): many of the callers need only the ParsedGoFile so
// that they can call pgf.PosRange(pos, end) to get a Range; they
// don't actually need a MappedRange.
-func posToMappedRange(pkg Package, pos, end token.Pos) (protocol.MappedRange, error) {
+func posToMappedRange(ctx context.Context, snapshot Snapshot, pkg Package, pos, end token.Pos) (protocol.MappedRange, error) {
if !pos.IsValid() {
return protocol.MappedRange{}, fmt.Errorf("invalid start position")
}
@@ -86,7 +86,7 @@
}
logicalFilename := pkg.FileSet().File(pos).Name() // ignore line directives
- pgf, _, err := findFileInDeps(pkg, span.URIFromPath(logicalFilename))
+ pgf, _, err := findFileInDeps(ctx, snapshot, pkg, span.URIFromPath(logicalFilename))
if err != nil {
return protocol.MappedRange{}, err
}
@@ -99,13 +99,13 @@
// TODO(rfindley): is this the best factoring of this API? This function is
// really a trivial wrapper around findFileInDeps, which may be a more useful
// function to expose.
-func FindPackageFromPos(pkg Package, pos token.Pos) (Package, error) {
+func FindPackageFromPos(ctx context.Context, snapshot Snapshot, pkg Package, pos token.Pos) (Package, error) {
if !pos.IsValid() {
return nil, fmt.Errorf("invalid position")
}
fileName := pkg.FileSet().File(pos).Name()
uri := span.URIFromPath(fileName)
- _, pkg, err := findFileInDeps(pkg, uri)
+ _, pkg, err := findFileInDeps(ctx, snapshot, pkg, uri)
return pkg, err
}
@@ -223,25 +223,52 @@
}
// findFileInDeps finds uri in pkg or its dependencies.
-func findFileInDeps(pkg Package, uri span.URI) (*ParsedGoFile, Package, error) {
- queue := []Package{pkg}
- seen := make(map[PackageID]bool)
-
- for len(queue) > 0 {
- pkg := queue[0]
- queue = queue[1:]
- seen[pkg.ID()] = true
-
+func findFileInDeps(ctx context.Context, snapshot Snapshot, pkg Package, uri span.URI) (*ParsedGoFile, Package, error) {
+ pkgs := []Package{pkg}
+ deps := recursiveDeps(snapshot, pkg.Metadata())[1:]
+ // Ignore the error from type checking, but check if the context was
+ // canceled (which would have caused TypeCheck to exit early).
+ depPkgs, _ := snapshot.TypeCheck(ctx, TypecheckWorkspace, deps...)
+ if ctx.Err() != nil {
+ return nil, nil, ctx.Err()
+ }
+ for _, dep := range depPkgs {
+ // Since we ignored the error from type checking, pkg may be nil.
+ if dep != nil {
+ pkgs = append(pkgs, dep)
+ }
+ }
+ for _, pkg := range pkgs {
if pgf, err := pkg.File(uri); err == nil {
return pgf, pkg, nil
}
- for _, dep := range pkg.Imports() {
- if !seen[dep.ID()] {
- queue = append(queue, dep)
- }
+ }
+ return nil, nil, fmt.Errorf("no file for %s in deps of package %s", uri, pkg.Metadata().ID)
+}
+
+// recursiveDeps finds unique transitive dependencies of m, including m itself.
+//
+// Invariant: for the resulting slice res, res[0] == m.ID.
+//
+// TODO(rfindley): consider replacing this with a snapshot.ForwardDependencies
+// method, or exposing the metadata graph itself.
+func recursiveDeps(s interface{ Metadata(PackageID) *Metadata }, m *Metadata) []PackageID {
+ seen := make(map[PackageID]bool)
+ var ids []PackageID
+ var add func(*Metadata)
+ add = func(m *Metadata) {
+ if seen[m.ID] {
+ return
+ }
+ seen[m.ID] = true
+ ids = append(ids, m.ID)
+ for _, dep := range m.DepsByPkgPath {
+ m := s.Metadata(dep)
+ add(m)
}
}
- return nil, nil, fmt.Errorf("no file for %s in package %s", uri, pkg.ID())
+ add(m)
+ return ids
}
// UnquoteImportPath returns the unquoted import path of s,
@@ -463,3 +490,23 @@
}
return nil
}
+
+// ResolveImportPath returns the directly imported dependency of the package with id fromID,
+// given its ImportPath, type-checked in its workspace parse mode.
+//
+// TODO(rfindley): eliminate this function, in favor of inlining where it is used.
+func ResolveImportPath(ctx context.Context, snapshot Snapshot, fromID PackageID, importPath ImportPath) (Package, error) {
+ meta := snapshot.Metadata(fromID)
+ if meta == nil {
+ return nil, fmt.Errorf("unknown package %s", fromID)
+ }
+ depID, ok := meta.DepsByImpPath[importPath]
+ if !ok {
+ return nil, fmt.Errorf("package does not import %s", importPath)
+ }
+ pkgs, err := snapshot.TypeCheck(ctx, TypecheckWorkspace, depID)
+ if err != nil {
+ return nil, fmt.Errorf("type checking dep: %v", err)
+ }
+ return pkgs[0], nil
+}
diff --git a/gopls/internal/lsp/source/view.go b/gopls/internal/lsp/source/view.go
index 81a91cc..3744e7c 100644
--- a/gopls/internal/lsp/source/view.go
+++ b/gopls/internal/lsp/source/view.go
@@ -17,7 +17,6 @@
"io"
"golang.org/x/mod/modfile"
- "golang.org/x/mod/module"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/packages"
"golang.org/x/tools/go/types/objectpath"
@@ -171,7 +170,7 @@
//
// To reduce latency, it does not wait for type-checking to complete.
// It is intended for use only in completions.
- CachedImportPaths(ctx context.Context) (map[PackagePath]Package, error)
+ CachedImportPaths(ctx context.Context) (map[PackagePath]*types.Package, error)
// ActiveMetadata returns a new, unordered slice containing
// metadata for all packages considered 'active' in the workspace.
@@ -460,9 +459,7 @@
DepsByPkgPath map[PackagePath]PackageID // values are unique and non-empty
Module *packages.Module
DepsErrors []*packagesinternal.PackageError
-
- // Config is the *packages.Config associated with the loaded package.
- Config *packages.Config
+ LoadDir string // directory from which go/packages was run
}
// IsIntermediateTestVariant reports whether the given package is an
@@ -750,13 +747,7 @@
// Package represents a Go package that has been parsed and type-checked.
// It maintains only the relevant fields of a *go/packages.Package.
type Package interface {
- // Metadata:
- ID() PackageID
- Name() PackageName
- PkgPath() PackagePath
- GetTypesSizes() types.Sizes
- ForTest() string
- Version() *module.Version
+ Metadata() *Metadata
// Results of parsing:
FileSet() *token.FileSet
@@ -764,17 +755,14 @@
CompiledGoFiles() []*ParsedGoFile // (borrowed)
File(uri span.URI) (*ParsedGoFile, error)
GetSyntax() []*ast.File // (borrowed)
- HasListOrParseErrors() bool
+ HasParseErrors() bool
// Results of type checking:
GetTypes() *types.Package
GetTypesInfo() *types.Info
- DirectDep(path PackagePath) (Package, error)
- ResolveImportPath(path ImportPath) (Package, error)
- Imports() []Package // new slice of all direct dependencies, unordered
HasTypeErrors() bool
- DiagnosticsForFile(uri span.URI) []*Diagnostic // new array of list/parse/type errors
- ReferencesTo(map[PackagePath]map[objectpath.Path]unit) []protocol.Location // new sorted array of xrefs
+ DiagnosticsForFile(ctx context.Context, s Snapshot, uri span.URI) ([]*Diagnostic, error)
+ ReferencesTo(map[PackagePath]map[objectpath.Path]unit) []protocol.Location
MethodSetsIndex() *methodsets.Index
}
diff --git a/gopls/internal/lsp/source/xrefs/xrefs.go b/gopls/internal/lsp/source/xrefs/xrefs.go
index 5f781f7..6a8b391 100644
--- a/gopls/internal/lsp/source/xrefs/xrefs.go
+++ b/gopls/internal/lsp/source/xrefs/xrefs.go
@@ -23,7 +23,7 @@
// Index constructs a serializable index of outbound cross-references
// for the specified type-checked package.
-func Index(pkg source.Package) []byte {
+func Index(files []*source.ParsedGoFile, pkg *types.Package, info *types.Info) []byte {
// pkgObjects maps each referenced package Q to a mapping:
// from each referenced symbol in Q to the ordered list
// of references to that symbol from this package.
@@ -41,7 +41,7 @@
return objects
}
- for fileIndex, pgf := range pkg.CompiledGoFiles() {
+ for fileIndex, pgf := range files {
nodeRange := func(n ast.Node) protocol.Range {
rng, err := pgf.PosRange(n.Pos(), n.End())
@@ -58,9 +58,9 @@
// uses a symbol exported from another package.
// (The built-in error.Error method has no package.)
if n.IsExported() {
- if obj, ok := pkg.GetTypesInfo().Uses[n]; ok &&
+ if obj, ok := info.Uses[n]; ok &&
obj.Pkg() != nil &&
- obj.Pkg() != pkg.GetTypes() {
+ obj.Pkg() != pkg {
objects := getObjects(obj.Pkg())
gobObj, ok := objects[obj]
@@ -87,9 +87,9 @@
// string to the imported package.
var obj types.Object
if n.Name != nil {
- obj = pkg.GetTypesInfo().Defs[n.Name]
+ obj = info.Defs[n.Name]
} else {
- obj = pkg.GetTypesInfo().Implicits[n]
+ obj = info.Implicits[n]
}
if obj == nil {
return true // missing import
diff --git a/gopls/internal/regtest/bench/completion_test.go b/gopls/internal/regtest/bench/completion_test.go
index aafc970..1a464ca 100644
--- a/gopls/internal/regtest/bench/completion_test.go
+++ b/gopls/internal/regtest/bench/completion_test.go
@@ -7,7 +7,6 @@
import (
"context"
"fmt"
- "strings"
"testing"
"golang.org/x/tools/gopls/internal/lsp/protocol"
@@ -87,14 +86,12 @@
// the given file.
func endRangeInBuffer(env *Env, name string) protocol.Range {
buffer := env.BufferText(name)
- lines := strings.Split(buffer, "\n")
- numLines := len(lines)
-
- end := protocol.Position{
- Line: uint32(numLines - 1),
- Character: uint32(len([]rune(lines[numLines-1]))),
+ m := protocol.NewMapper("", []byte(buffer))
+ rng, err := m.OffsetRange(len(buffer), len(buffer))
+ if err != nil {
+ env.T.Fatal(err)
}
- return protocol.Range{Start: end, End: end}
+ return rng
}
// Benchmark struct completion in tools codebase.
diff --git a/gopls/internal/regtest/diagnostics/diagnostics_test.go b/gopls/internal/regtest/diagnostics/diagnostics_test.go
index 800274b..f96a0aa 100644
--- a/gopls/internal/regtest/diagnostics/diagnostics_test.go
+++ b/gopls/internal/regtest/diagnostics/diagnostics_test.go
@@ -862,6 +862,7 @@
`
Run(t, mod, func(t *testing.T, env *Env) {
env.OpenFile("foo/bar_test.go")
+ env.AfterChange()
env.RegexpReplace("foo/bar_test.go", "package foo_", "package foo_test")
env.AfterChange(
NoDiagnostics(ForFile("foo/bar_test.go")),
diff --git a/gopls/internal/regtest/diagnostics/golist_test.go b/gopls/internal/regtest/diagnostics/golist_test.go
new file mode 100644
index 0000000..ec54a92
--- /dev/null
+++ b/gopls/internal/regtest/diagnostics/golist_test.go
@@ -0,0 +1,68 @@
+// Copyright 2023 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 (
+ "testing"
+
+ . "golang.org/x/tools/gopls/internal/lsp/regtest"
+ "golang.org/x/tools/gopls/internal/lsp/source"
+)
+
+func TestGoListErrors(t *testing.T) {
+ const src = `
+-- go.mod --
+module a.com
+
+go 1.18
+-- a/a.go --
+package a
+
+import
+-- c/c.go --
+package c
+
+/*
+int fortythree() { return 42; }
+*/
+import "C"
+
+func Foo() {
+ print(C.fortytwo())
+}
+-- p/p.go --
+package p
+
+import "a.com/q"
+
+const P = q.Q + 1
+-- q/q.go --
+package q
+
+import "a.com/p"
+
+const Q = p.P + 1
+`
+
+ Run(t, src, func(t *testing.T, env *Env) {
+ env.OnceMet(
+ InitialWorkspaceLoad,
+ Diagnostics(
+ env.AtRegexp("a/a.go", "import\n()"),
+ FromSource(string(source.ParseError)),
+ ),
+ Diagnostics(
+ AtPosition("c/c.go", 0, 0),
+ FromSource(string(source.ListError)),
+ WithMessage("may indicate failure to perform cgo processing"),
+ ),
+ Diagnostics(
+ env.AtRegexp("p/p.go", `"a.com/q"`),
+ FromSource(string(source.ListError)),
+ WithMessage("import cycle not allowed"),
+ ),
+ )
+ })
+}
diff --git a/gopls/internal/regtest/misc/hover_test.go b/gopls/internal/regtest/misc/hover_test.go
index dafcc62..013c4d1 100644
--- a/gopls/internal/regtest/misc/hover_test.go
+++ b/gopls/internal/regtest/misc/hover_test.go
@@ -204,6 +204,10 @@
env.OpenFile("main.go")
for _, test := range tests {
got, _ := env.Hover("main.go", env.RegexpSearch("main.go", test.hoverPackage))
+ if got == nil {
+ t.Error("nil hover for", test.hoverPackage)
+ continue
+ }
if !strings.Contains(got.Value, test.want) {
t.Errorf("Hover: got:\n%q\nwant:\n%q", got.Value, test.want)
}
