internal/lsp: create types for package paths and IDs

This change replaces the strings that were previously used for both the
ID and package path fields. This is a precursor to the change that will
replace the uses of package path with package ID.

Change-Id: I353e98aedede9b85c7a183fdd49048ff43b1e26d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/181757
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>

internal/lsp/cache/check.go

@@ -22,24 +22,24 @@ type importer struct {
 
 	// seen maintains the set of previously imported packages.
 	// If we have seen a package that is already in this map, we have a circular import.
-	seen map[string]struct{}
+	seen map[packagePath]struct{}
 
 	// topLevelPkgID is the ID of the package from which type-checking began.
-	topLevelPkgID string
+	topLevelPkgID packageID
 
 	ctx  context.Context
 	fset *token.FileSet
 }
 
 func (imp *importer) Import(pkgPath string) (*types.Package, error) {
-	pkg, err := imp.getPkg(pkgPath)
+	pkg, err := imp.getPkg(packagePath(pkgPath))
 	if err != nil {
 		return nil, err
 	}
 	return pkg.types, nil
 }
 
-func (imp *importer) getPkg(pkgPath string) (*pkg, error) {
+func (imp *importer) getPkg(pkgPath packagePath) (*pkg, error) {
 	if _, ok := imp.seen[pkgPath]; ok {
 		return nil, fmt.Errorf("circular import detected")
 	}
@@ -80,7 +80,7 @@ func (imp *importer) getPkg(pkgPath string) (*pkg, error) {
 	return e.pkg, nil
 }
 
-func (imp *importer) typeCheck(pkgPath string) (*pkg, error) {
+func (imp *importer) typeCheck(pkgPath packagePath) (*pkg, error) {
 	meta, ok := imp.view.mcache.packages[pkgPath]
 	if !ok {
 		return nil, fmt.Errorf("no metadata for %v", pkgPath)
@@ -89,7 +89,7 @@ func (imp *importer) typeCheck(pkgPath string) (*pkg, error) {
 		id:         meta.id,
 		pkgPath:    meta.pkgPath,
 		files:      meta.files,
-		imports:    make(map[string]*pkg),
+		imports:    make(map[packagePath]*pkg),
 		typesSizes: meta.typesSizes,
 		typesInfo: &types.Info{
 			Types:      make(map[ast.Expr]types.TypeAndValue),
@@ -117,13 +117,13 @@ func (imp *importer) typeCheck(pkgPath string) (*pkg, error) {
 	} else if len(files) == 0 { // not the unsafe package, no parsed files
 		return nil, fmt.Errorf("no parsed files for package %s", pkg.pkgPath)
 	} else {
-		pkg.types = types.NewPackage(meta.pkgPath, meta.name)
+		pkg.types = types.NewPackage(string(meta.pkgPath), meta.name)
 	}
 
 	pkg.syntax = files
 
 	// Handle circular imports by copying previously seen imports.
-	seen := make(map[string]struct{})
+	seen := make(map[packagePath]struct{})
 	for k, v := range imp.seen {
 		seen[k] = v
 	}

internal/lsp/cache/gofile.go

@@ -131,9 +131,9 @@ func (f *goFile) GetActiveReverseDeps(ctx context.Context) []source.GoFile {
 	f.view.mcache.mu.Lock()
 	defer f.view.mcache.mu.Unlock()
 
-	seen := make(map[string]struct{}) // visited packages
+	seen := make(map[packagePath]struct{}) // visited packages
 	results := make(map[*goFile]struct{})
-	f.view.reverseDeps(ctx, seen, results, pkg.PkgPath())
+	f.view.reverseDeps(ctx, seen, results, packagePath(pkg.PkgPath()))
 
 	var files []source.GoFile
 	for rd := range results {
@@ -149,7 +149,7 @@ func (f *goFile) GetActiveReverseDeps(ctx context.Context) []source.GoFile {
 	return files
 }
 
-func (v *view) reverseDeps(ctx context.Context, seen map[string]struct{}, results map[*goFile]struct{}, pkgPath string) {
+func (v *view) reverseDeps(ctx context.Context, seen map[packagePath]struct{}, results map[*goFile]struct{}, pkgPath packagePath) {
 	if _, ok := seen[pkgPath]; ok {
 		return
 	}

internal/lsp/cache/load.go

@@ -20,7 +20,7 @@ func (v *view) loadParseTypecheck(ctx context.Context, f *goFile) ([]packages.Er
 	}
 
 	// Save the metadata's current missing imports, if any.
-	var originalMissingImports map[string]struct{}
+	var originalMissingImports map[packagePath]struct{}
 	if f.meta != nil {
 		originalMissingImports = f.meta.missingImports
 	}
@@ -40,7 +40,7 @@ func (v *view) loadParseTypecheck(ctx context.Context, f *goFile) ([]packages.Er
 
 	imp := &importer{
 		view:          v,
-		seen:          make(map[string]struct{}),
+		seen:          make(map[packagePath]struct{}),
 		ctx:           ctx,
 		fset:          f.FileSet(),
 		topLevelPkgID: f.meta.id,
@@ -48,7 +48,7 @@ func (v *view) loadParseTypecheck(ctx context.Context, f *goFile) ([]packages.Er
 
 	// Start prefetching direct imports.
 	for importPath := range f.meta.children {
-		go imp.Import(importPath)
+		go imp.Import(string(importPath))
 	}
 	// Type-check package.
 	pkg, err := imp.getPkg(f.meta.pkgPath)
@@ -65,7 +65,7 @@ func (v *view) loadParseTypecheck(ctx context.Context, f *goFile) ([]packages.Er
 	return nil, nil
 }
 
-func sameSet(x, y map[string]struct{}) bool {
+func sameSet(x, y map[packagePath]struct{}) bool {
 	if len(x) != len(y) {
 		return false
 	}
@@ -103,7 +103,7 @@ func (v *view) checkMetadata(ctx context.Context, f *goFile) ([]packages.Error,
 			return pkg.Errors, fmt.Errorf("package %s has errors, skipping type-checking", pkg.PkgPath)
 		}
 		// Build the import graph for this package.
-		v.link(ctx, pkg.PkgPath, pkg, nil)
+		v.link(ctx, packagePath(pkg.PkgPath), pkg, nil)
 	}
 	return nil, nil
 }
@@ -136,16 +136,16 @@ func (v *view) parseImports(ctx context.Context, f *goFile) bool {
 	return false
 }
 
-func (v *view) link(ctx context.Context, pkgPath string, pkg *packages.Package, parent *metadata) *metadata {
+func (v *view) link(ctx context.Context, pkgPath packagePath, pkg *packages.Package, parent *metadata) *metadata {
 	m, ok := v.mcache.packages[pkgPath]
 	if !ok {
 		m = &metadata{
 			pkgPath:        pkgPath,
-			id:             pkg.ID,
+			id:             packageID(pkg.ID),
 			typesSizes:     pkg.TypesSizes,
-			parents:        make(map[string]bool),
+			parents:        make(map[packagePath]bool),
-			children:       make(map[string]bool),
+			children:       make(map[packagePath]bool),
-			missingImports: make(map[string]struct{}),
+			missingImports: make(map[packagePath]struct{}),
 		}
 		v.mcache.packages[pkgPath] = m
 	}
@@ -168,15 +168,16 @@ func (v *view) link(ctx context.Context, pkgPath string, pkg *packages.Package,
 	}
 	for importPath, importPkg := range pkg.Imports {
 		if len(importPkg.Errors) > 0 {
-			m.missingImports[pkg.PkgPath] = struct{}{}
+			m.missingImports[pkgPath] = struct{}{}
 		}
-		if _, ok := m.children[importPath]; !ok {
+		importPkgPath := packagePath(importPath)
-			v.link(ctx, importPath, importPkg, m)
+		if _, ok := m.children[importPkgPath]; !ok {
+			v.link(ctx, importPkgPath, importPkg, m)
 		}
 	}
 	// Clear out any imports that have been removed.
 	for importPath := range m.children {
-		if _, ok := pkg.Imports[importPath]; !ok {
+		if _, ok := pkg.Imports[string(importPath)]; !ok {
 			delete(m.children, importPath)
 			if child, ok := v.mcache.packages[importPath]; ok {
 				delete(child.parents, pkgPath)

internal/lsp/cache/pkg.go

@@ -18,11 +18,14 @@ import (
 
 // pkg contains the type information needed by the source package.
 type pkg struct {
-	id, pkgPath string
+	// ID and package path have their own types to avoid being used interchangeably.
+	id      packageID
+	pkgPath packagePath
+
 	files      []string
 	syntax     []*astFile
 	errors     []packages.Error
-	imports     map[string]*pkg
+	imports    map[packagePath]*pkg
 	types      *types.Package
 	typesInfo  *types.Info
 	typesSizes types.Sizes
@@ -35,6 +38,12 @@ type pkg struct {
 	analyses map[*analysis.Analyzer]*analysisEntry
 }
 
+// packageID is a type that abstracts a package ID.
+type packageID string
+
+// packagePath is a type that abstracts a package path.
+type packagePath string
+
 type analysisEntry struct {
 	done      chan struct{}
 	succeeded bool
@@ -108,11 +117,11 @@ func (pkg *pkg) GetActionGraph(ctx context.Context, a *analysis.Analyzer) (*sour
 		if len(a.FactTypes) > 0 {
 			importPaths := make([]string, 0, len(pkg.imports))
 			for importPath := range pkg.imports {
-				importPaths = append(importPaths, importPath)
+				importPaths = append(importPaths, string(importPath))
 			}
 			sort.Strings(importPaths) // for determinism
 			for _, importPath := range importPaths {
-				dep, ok := pkg.imports[importPath]
+				dep, ok := pkg.imports[packagePath(importPath)]
 				if !ok {
 					continue
 				}
@@ -129,7 +138,7 @@ func (pkg *pkg) GetActionGraph(ctx context.Context, a *analysis.Analyzer) (*sour
 }
 
 func (pkg *pkg) PkgPath() string {
-	return pkg.pkgPath
+	return string(pkg.pkgPath)
 }
 
 func (pkg *pkg) GetFilenames() []string {
@@ -165,7 +174,7 @@ func (pkg *pkg) IsIllTyped() bool {
 }
 
 func (pkg *pkg) GetImport(pkgPath string) source.Package {
-	if imp := pkg.imports[pkgPath]; imp != nil {
+	if imp := pkg.imports[packagePath(pkgPath)]; imp != nil {
 		return imp
 	}
 	// Don't return a nil pointer because that still satisfies the interface.

internal/lsp/cache/session.go

@@ -81,10 +81,10 @@ func (s *session) NewView(name string, folder span.URI) source.View {
 		filesByURI:    make(map[span.URI]viewFile),
 		filesByBase:   make(map[string][]viewFile),
 		mcache: &metadataCache{
-			packages: make(map[string]*metadata),
+			packages: make(map[packagePath]*metadata),
 		},
 		pcache: &packageCache{
-			packages: make(map[string]*entry),
+			packages: make(map[packagePath]*entry),
 		},
 		ignoredURIs: make(map[span.URI]struct{}),
 	}

internal/lsp/cache/view.go

@@ -71,23 +71,25 @@ type view struct {
 
 type metadataCache struct {
 	mu       sync.Mutex
-	packages map[string]*metadata
+	packages map[packagePath]*metadata
 }
 
 type metadata struct {
-	id, pkgPath, name string
+	id                packageID
+	pkgPath           packagePath
+	name              string
 	files             []string
 	typesSizes        types.Sizes
-	parents, children map[string]bool
+	parents, children map[packagePath]bool
 
 	// missingImports is the set of unresolved imports for this package.
 	// It contains any packages with `go list` errors.
-	missingImports map[string]struct{}
+	missingImports map[packagePath]struct{}
 }
 
 type packageCache struct {
 	mu       sync.Mutex
-	packages map[string]*entry
+	packages map[packagePath]*entry
 }
 
 type entry struct {
@@ -227,20 +229,10 @@ func (v *view) SetContent(ctx context.Context, uri span.URI, content []byte) err
 // invalidateContent invalidates the content of a Go file,
 // including any position and type information that depends on it.
 func (f *goFile) invalidateContent() {
-	f.view.pcache.mu.Lock()
 	f.handleMu.Lock()
-	defer func() {
+	defer f.handleMu.Unlock()
-		f.handleMu.Unlock()
-		f.view.pcache.mu.Unlock()
-	}()
 
-	f.ast = nil
+	f.invalidateAST()
-	f.token = nil
-
-	// Remove the package and all of its reverse dependencies from the cache.
-	if f.pkg != nil {
-		f.view.remove(f.pkg.pkgPath, map[string]struct{}{})
-	}
 	f.handle = nil
 }
 
@@ -255,14 +247,14 @@ func (f *goFile) invalidateAST() {
 
 	// Remove the package and all of its reverse dependencies from the cache.
 	if f.pkg != nil {
-		f.view.remove(f.pkg.pkgPath, map[string]struct{}{})
+		f.view.remove(f.pkg.pkgPath, map[packagePath]struct{}{})
 	}
 }
 
 // remove invalidates a package and its reverse dependencies in the view's
 // package cache. It is assumed that the caller has locked both the mutexes
 // of both the mcache and the pcache.
-func (v *view) remove(pkgPath string, seen map[string]struct{}) {
+func (v *view) remove(pkgPath packagePath, seen map[packagePath]struct{}) {
 	if _, ok := seen[pkgPath]; ok {
 		return
 	}