mirror
/
go
mirror of https://github.com/golang/go.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

internal/lsp: stop returning metadata from (*snapshot).load 

This metadata was hardly being used, and it's not really necessary now
that we are creating package handles in load. There are still a number
of cases that can simplified because of this fact, but those will be
done in follow-ups.

Also, fix a stray staticcheck warning.

Change-Id: I12d1b4d568a8fd145d08397a926e7ba6f3428604
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217138
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This commit is contained in:
Rebecca Stambler 2020-01-30 20:44:00 -05:00
parent 9f574694fd
commit 403f1254bd
4 changed files with 30 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
internal/lsp/cache/check.go vendored
View File

@ -63,7 +63,6 @@ type packageData struct {
// buildPackageHandle returns a source.PackageHandle for a given package and config.
func (s *snapshot) buildPackageHandle(ctx context.Context, id packageID) (*packageHandle, error) {
// Check if we already have this PackageHandle cached.
if ph := s.getPackage(id); ph != nil {
return ph, nil
}

42
internal/lsp/cache/load.go vendored
View File

@ -37,7 +37,7 @@ type metadata struct {
config *packages.Config
}
func (s *snapshot) load(ctx context.Context, scopes ...interface{}) ([]*metadata, error) {
func (s *snapshot) load(ctx context.Context, scopes ...interface{}) error {
var query []string
var containsDir bool // for logging
for _, scope := range scopes {
@ -87,14 +87,13 @@ func (s *snapshot) load(ctx context.Context, scopes ...interface{}) ([]*metadata
// type-checking an incomplete result. Check the context directly,
// because go/packages adds extra information to the error.
if ctx.Err() != nil {
return nil, ctx.Err()
return ctx.Err()
}
log.Print(ctx, "go/packages.Load", tag.Of("snapshot", s.ID()), tag.Of("query", query), tag.Of("packages", len(pkgs)))
if len(pkgs) == 0 {
return nil, err
return err
}
var results []*metadata
for _, pkg := range pkgs {
if !containsDir || s.view.Options().VerboseOutput {
log.Print(ctx, "go/packages.Load", tag.Of("snapshot", s.ID()), tag.Of("package", pkg.PkgPath), tag.Of("files", pkg.CompiledGoFiles))
@ -107,7 +106,7 @@ func (s *snapshot) load(ctx context.Context, scopes ...interface{}) ([]*metadata
// Special case for the builtin package, as it has no dependencies.
if pkg.PkgPath == "builtin" {
if err := s.view.buildBuiltinPackage(ctx, pkg.GoFiles); err != nil {
return nil, err
return err
}
continue
}
@ -118,21 +117,16 @@ func (s *snapshot) load(ctx context.Context, scopes ...interface{}) ([]*metadata
// Set the metadata for this package.
m, err := s.setMetadata(ctx, packagePath(pkg.PkgPath), pkg, cfg, map[packageID]struct{}{})
if err != nil {
return nil, err
return err
}
if _, err := s.packageHandle(ctx, m.id); err != nil {
return nil, err
if _, err := s.buildPackageHandle(ctx, m.id); err != nil {
return err
}
results = append(results, m)
}
// Rebuild the import graph when the metadata is updated.
s.clearAndRebuildImportGraph()
if len(results) == 0 {
return nil, errors.Errorf("no metadata for %s", scopes)
}
return results, nil
return nil
}
func (s *snapshot) setMetadata(ctx context.Context, pkgPath packagePath, pkg *packages.Package, cfg *packages.Config, seen map[packageID]struct{}) (*metadata, error) {
@ -205,17 +199,17 @@ func (s *snapshot) setMetadata(ctx context.Context, pkgPath packagePath, pkg *pa
// Set the workspace packages. If any of the package's files belong to the
// view, then the package is considered to be a workspace package.
for _, uri := range append(m.compiledGoFiles, m.goFiles...) {
if !s.view.contains(uri) {
continue
// If the package's files are in this view, mark it as a workspace package.
if s.view.contains(uri) {
// A test variant of a package can only be loaded directly by loading
// the non-test variant with -test. Track the import path of the non-test variant.
if m.forTest != "" {
s.workspacePackages[m.id] = m.forTest
} else {
s.workspacePackages[m.id] = pkgPath
}
break
}
// A test variant of a package can only be loaded directly by loading
// the non-test variant with -test. Track the import path of the non-test variant.
if m.forTest != "" {
s.workspacePackages[m.id] = m.forTest
} else {
s.workspacePackages[m.id] = pkgPath
}
break
}
return m, nil
}

20
internal/lsp/cache/snapshot.go vendored
View File

@ -82,7 +82,7 @@ func (s *snapshot) PackageHandles(ctx context.Context, fh source.FileHandle) ([]
ctx = telemetry.File.With(ctx, fh.Identity().URI)
meta := s.getMetadataForURI(fh.Identity().URI)
phs, err := s.packageHandles(ctx, fileURI(fh.Identity().URI), meta)
phs, err := s.packageHandles(ctx, fh.Identity().URI, meta)
if err != nil {
return nil, err
}
@ -128,14 +128,16 @@ func (s *snapshot) packageHandle(ctx context.Context, id packageID) (*packageHan
return result, nil
}
func (s *snapshot) packageHandles(ctx context.Context, scope interface{}, meta []*metadata) ([]*packageHandle, error) {
func (s *snapshot) packageHandles(ctx context.Context, uri span.URI, meta []*metadata) ([]*packageHandle, error) {
// First, determine if we need to reload or recheck the package.
phs, load, check := s.shouldCheck(meta)
if load {
newMeta, err := s.load(ctx, scope)
if err != nil {
if err := s.load(ctx, fileURI(uri)); err != nil {
return nil, err
}
// TODO(rstambler): Now that we are creating new package handles in
// every load, this isn't really necessary.
newMeta := s.getMetadataForURI(uri)
newMissing := missingImports(newMeta)
if len(newMissing) != 0 {
// Type checking a package with the same missing imports over and over
@ -158,7 +160,7 @@ func (s *snapshot) packageHandles(ctx context.Context, scope interface{}, meta [
results = phs
}
if len(results) == 0 {
return nil, errors.Errorf("packageHandles: no package handles for %v", scope)
return nil, errors.Errorf("packageHandles: no package handles for %v", uri)
}
return results, nil
}
@ -541,8 +543,7 @@ func (s *snapshot) reloadWorkspace(ctx context.Context) error {
// If the view's build configuration is invalid, we cannot reload by package path.
// Just reload the directory instead.
if !s.view.hasValidBuildConfiguration {
_, err := s.load(ctx, viewLoadScope("LOAD_INVALID_VIEW"))
return err
return s.load(ctx, viewLoadScope("LOAD_INVALID_VIEW"))
}
// See which of the workspace packages are missing metadata.
@ -558,8 +559,7 @@ func (s *snapshot) reloadWorkspace(ctx context.Context) error {
if len(pkgPaths) == 0 {
return nil
}
_, err := s.load(ctx, pkgPaths...)
return err
return s.load(ctx, pkgPaths...)
}
func (s *snapshot) reloadOrphanedFiles(ctx context.Context) error {
@ -572,7 +572,7 @@ func (s *snapshot) reloadOrphanedFiles(ctx context.Context) error {
return nil
}
_, err := s.load(ctx, scopes...)
err := s.load(ctx, scopes...)
// If we failed to load some files, i.e. they have no metadata,
// mark the failures so we don't bother retrying until the file's

3
internal/lsp/cache/view.go vendored
View File

@ -551,7 +551,8 @@ func (v *view) getSnapshot() *snapshot {
func (v *view) initialize(ctx context.Context, s *snapshot) {
v.initializeOnce.Do(func() {
defer close(v.initialized)
if _, err := s.load(ctx, viewLoadScope("LOAD_VIEW"), packagePath("builtin")); err != nil {
if err := s.load(ctx, viewLoadScope("LOAD_VIEW"), packagePath("builtin")); err != nil {
log.Error(ctx, "initial workspace load failed", err)
}
})