gopls/internal/lsp/source: eliminate Metadata interface

This change merges the source.Metadata interface with
its sole implementation, cache.Metadata.

One possible concern: the struct cannot have private
fields used only by the cache-package logic that
constructs these structs. We are ok with that.

Change-Id: I93c112f92dc812bd0da07d36e7244d5d77978312
Reviewed-on: https://go-review.googlesource.com/c/tools/+/452035
Run-TryBot: Alan Donovan <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Robert Findley <[email protected]>

Author: adonovan

gopls/internal/lsp/cache/check.go

@@ -46,7 +46,7 @@ type packageHandle struct {
 	promise *memoize.Promise // [typeCheckResult]
 
 	// m is the metadata associated with the package.
-	m *Metadata
+	m *source.Metadata
 
 	// key is the hashed key for the package.
 	//
@@ -184,7 +184,7 @@ func (s *snapshot) buildPackageHandle(ctx context.Context, id PackageID, mode so
 
 // readGoFiles reads the content of Metadata.GoFiles and
 // Metadata.CompiledGoFiles, in parallel.
-func readGoFiles(ctx context.Context, s *snapshot, m *Metadata) (goFiles, compiledGoFiles []source.FileHandle, err error) {
+func readGoFiles(ctx context.Context, s *snapshot, m *source.Metadata) (goFiles, compiledGoFiles []source.FileHandle, err error) {
 	var group errgroup.Group
 	getFileHandles := func(files []span.URI) []source.FileHandle {
 		fhs := make([]source.FileHandle, len(files))
@@ -221,7 +221,7 @@ func (s *snapshot) workspaceParseMode(id PackageID) source.ParseMode {
 // 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 *Metadata, depsKey source.Hash, mode source.ParseMode, experimentalKey bool) packageHandleKey {
+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)
@@ -304,7 +304,7 @@ func (ph *packageHandle) cached() (*pkg, error) {
 // 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 *Metadata, mode source.ParseMode, deps map[PackageID]*packageHandle) (*pkg, error) {
+func typeCheckImpl(ctx context.Context, snapshot *snapshot, goFiles, compiledGoFiles []source.FileHandle, m *source.Metadata, mode source.ParseMode, deps map[PackageID]*packageHandle) (*pkg, error) {
 	// Start type checking of direct dependencies,
 	// in parallel and asynchronously.
 	// As the type checker imports each of these
@@ -439,7 +439,7 @@ func typeCheckImpl(ctx context.Context, snapshot *snapshot, goFiles, compiledGoF
 
 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 *Metadata, mode source.ParseMode, deps map[PackageID]*packageHandle, astFilter *unexportedFilter) (*pkg, error) {
+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)))
 	defer done()
 
@@ -628,7 +628,7 @@ func (s *snapshot) depsErrors(ctx context.Context, pkg *pkg) ([]*source.Diagnost
 	// 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 pkg.m.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

gopls/internal/lsp/cache/graph.go

@@ -15,7 +15,7 @@ import (
 // graph of Go packages, as obtained from go/packages.
 type metadataGraph struct {
 	// metadata maps package IDs to their associated metadata.
-	metadata map[PackageID]*Metadata
+	metadata map[PackageID]*source.Metadata
 
 	// importedBy maps package IDs to the list of packages that import them.
 	importedBy map[PackageID][]PackageID
@@ -27,12 +27,12 @@ type metadataGraph struct {
 
 // Clone creates a new metadataGraph, applying the given updates to the
 // receiver.
-func (g *metadataGraph) Clone(updates map[PackageID]*Metadata) *metadataGraph {
+func (g *metadataGraph) Clone(updates map[PackageID]*source.Metadata) *metadataGraph {
 	if len(updates) == 0 {
 		// Optimization: since the graph is immutable, we can return the receiver.
 		return g
 	}
-	result := &metadataGraph{metadata: make(map[PackageID]*Metadata, len(g.metadata))}
+	result := &metadataGraph{metadata: make(map[PackageID]*source.Metadata, len(g.metadata))}
 	// Copy metadata.
 	for id, m := range g.metadata {
 		result.metadata[id] = m

gopls/internal/lsp/cache/load.go

@@ -157,7 +157,7 @@ func (s *snapshot) load(ctx context.Context, allowNetwork bool, scopes ...loadSc
 
 	moduleErrs := make(map[string][]packages.Error) // module path -> errors
 	filterer := buildFilterer(s.view.rootURI.Filename(), s.view.gomodcache, s.view.Options())
-	newMetadata := make(map[PackageID]*Metadata)
+	newMetadata := make(map[PackageID]*source.Metadata)
 	for _, pkg := range pkgs {
 		// The Go command returns synthetic list results for module queries that
 		// encountered module errors.
@@ -222,7 +222,7 @@ func (s *snapshot) load(ctx context.Context, allowNetwork bool, scopes ...loadSc
 	//
 	// TODO(rfindley): perform a sanity check that metadata matches here. If not,
 	// we have an invalidation bug elsewhere.
-	updates := make(map[PackageID]*Metadata)
+	updates := make(map[PackageID]*source.Metadata)
 	var updatedIDs []PackageID
 	for _, m := range newMetadata {
 		if existing := s.meta.metadata[m.ID]; existing == nil {
@@ -475,7 +475,7 @@ func makeWorkspaceDir(ctx context.Context, workspace *workspace, fs source.FileS
 // buildMetadata populates the updates map with metadata updates to
 // apply, based on the given pkg. It recurs through pkg.Imports to ensure that
 // metadata exists for all dependencies.
-func buildMetadata(ctx context.Context, pkg *packages.Package, cfg *packages.Config, query []string, updates map[PackageID]*Metadata, path []PackageID) error {
+func buildMetadata(ctx context.Context, pkg *packages.Package, cfg *packages.Config, query []string, updates map[PackageID]*source.Metadata, path []PackageID) error {
 	// Allow for multiple ad-hoc packages in the workspace (see #47584).
 	pkgPath := PackagePath(pkg.PkgPath)
 	id := PackageID(pkg.ID)
@@ -507,15 +507,15 @@ func buildMetadata(ctx context.Context, pkg *packages.Package, cfg *packages.Con
 	}
 
 	// Recreate the metadata rather than reusing it to avoid locking.
-	m := &Metadata{
+	m := &source.Metadata{
 		ID:         id,
 		PkgPath:    pkgPath,
 		Name:       PackageName(pkg.Name),
 		ForTest:    PackagePath(packagesinternal.GetForTest(pkg)),
 		TypesSizes: pkg.TypesSizes,
 		Config:     cfg,
 		Module:     pkg.Module,
-		depsErrors: packagesinternal.GetDepsErrors(pkg),
+		DepsErrors: packagesinternal.GetDepsErrors(pkg),
 	}
 	updates[id] = m
 
@@ -606,7 +606,7 @@ func buildMetadata(ctx context.Context, pkg *packages.Package, cfg *packages.Con
 // snapshot s.
 //
 // s.mu must be held while calling this function.
-func containsPackageLocked(s *snapshot, m *Metadata) bool {
+func containsPackageLocked(s *snapshot, m *source.Metadata) bool {
 	// In legacy workspace mode, or if a package does not have an associated
 	// module, a package is considered inside the workspace if any of its files
 	// are under the workspace root (and not excluded).
@@ -647,7 +647,7 @@ func containsPackageLocked(s *snapshot, m *Metadata) bool {
 // the snapshot s.
 //
 // s.mu must be held while calling this function.
-func containsOpenFileLocked(s *snapshot, m *Metadata) bool {
+func containsOpenFileLocked(s *snapshot, m *source.Metadata) bool {
 	uris := map[span.URI]struct{}{}
 	for _, uri := range m.CompiledGoFiles {
 		uris[uri] = struct{}{}
@@ -668,7 +668,7 @@ func containsOpenFileLocked(s *snapshot, m *Metadata) bool {
 // workspace of the snapshot s.
 //
 // s.mu must be held while calling this function.
-func containsFileInWorkspaceLocked(s *snapshot, m *Metadata) bool {
+func containsFileInWorkspaceLocked(s *snapshot, m *source.Metadata) bool {
 	uris := map[span.URI]struct{}{}
 	for _, uri := range m.CompiledGoFiles {
 		uris[uri] = struct{}{}
@@ -738,7 +738,7 @@ func computeWorkspacePackagesLocked(s *snapshot, meta *metadataGraph) map[Packag
 // function returns false.
 //
 // If m is not a command-line-arguments package, this is trivially true.
-func allFilesHaveRealPackages(g *metadataGraph, m *Metadata) bool {
+func allFilesHaveRealPackages(g *metadataGraph, m *source.Metadata) bool {
 	n := len(m.CompiledGoFiles)
 checkURIs:
 	for _, uri := range append(m.CompiledGoFiles[0:n:n], m.GoFiles...) {

gopls/internal/lsp/cache/metadata.go

@@ -18,9 +18,17 @@ import (
 	"golang.org/x/tools/internal/memoize"
 )
 
+// Convenient local aliases for typed strings.
+type (
+	PackageID   = source.PackageID
+	PackagePath = source.PackagePath
+	PackageName = source.PackageName
+	ImportPath  = source.ImportPath
+)
+
 // pkg contains the type information needed by the source package.
 type pkg struct {
-	m               *Metadata
+	m               *source.Metadata
 	mode            source.ParseMode
 	fset            *token.FileSet // for now, same as the snapshot's FileSet
 	goFiles         []*source.ParsedGoFile

gopls/internal/lsp/cache/pkg.go

@@ -1128,12 +1128,12 @@ func (s *snapshot) Symbols(ctx context.Context) map[span.URI][]source.Symbol {
 	return result
 }
 
-func (s *snapshot) MetadataForFile(ctx context.Context, uri span.URI) ([]source.Metadata, error) {
+func (s *snapshot) MetadataForFile(ctx context.Context, uri span.URI) ([]*source.Metadata, error) {
 	knownIDs, err := s.getOrLoadIDsForURI(ctx, uri)
 	if err != nil {
 		return nil, err
 	}
-	var mds []source.Metadata
+	var mds []*source.Metadata
 	for _, id := range knownIDs {
 		md := s.getMetadata(id)
 		// TODO(rfindley): knownIDs and metadata should be in sync, but existing
@@ -1168,15 +1168,15 @@ func (s *snapshot) KnownPackages(ctx context.Context) ([]source.Package, error)
 	return pkgs, nil
 }
 
-func (s *snapshot) AllValidMetadata(ctx context.Context) ([]source.Metadata, error) {
+func (s *snapshot) AllValidMetadata(ctx context.Context) ([]*source.Metadata, error) {
 	if err := s.awaitLoaded(ctx); err != nil {
 		return nil, err
 	}
 
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
-	var meta []source.Metadata
+	var meta []*source.Metadata
 	for _, m := range s.meta.metadata {
 		meta = append(meta, m)
 	}
@@ -1233,7 +1233,7 @@ func moduleForURI(modFiles map[span.URI]struct{}, uri span.URI) span.URI {
 	return match
 }
 
-func (s *snapshot) getMetadata(id PackageID) *Metadata {
+func (s *snapshot) getMetadata(id PackageID) *source.Metadata {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
@@ -1922,7 +1922,7 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC
 	// Compute which metadata updates are required. We only need to invalidate
 	// packages directly containing the affected file, and only if it changed in
 	// a relevant way.
-	metadataUpdates := make(map[PackageID]*Metadata)
+	metadataUpdates := make(map[PackageID]*source.Metadata)
 	for k, v := range s.meta.metadata {
 		invalidateMetadata := idsToInvalidate[k]
 

gopls/internal/lsp/cache/snapshot.go

@@ -74,7 +74,7 @@ func Format(ctx context.Context, snapshot Snapshot, fh FileHandle) ([]protocol.T
 		var langVersion, modulePath string
 		mds, err := snapshot.MetadataForFile(ctx, fh.URI())
 		if err == nil && len(mds) > 0 {
-			if mi := mds[0].ModuleInfo(); mi != nil {
+			if mi := mds[0].Module; mi != nil {
 				langVersion = mi.GoVersion
 				modulePath = mi.Path
 			}