internal/lsp: don't associate package with snapshot

This change effectively reverts CL 202039. This CL was a mistake, as it
creates a cycle. Snapshots hold CheckPackageHandles, which in turn hold
pkgs.

Change-Id: I944304cb365f0ef98b5e54ea38edea6cece40453
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202740
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>

internal/lsp/cache/check.go

@@ -270,7 +270,7 @@ func (imp *importer) typeCheck(ctx context.Context, cph *checkPackageHandle) (*p
 	}
 
 	pkg := &pkg{
-		snapshot:   imp.snapshot,
+		view:       imp.snapshot.view,
 		id:         cph.m.id,
 		mode:       cph.mode,
 		pkgPath:    cph.m.pkgPath,

internal/lsp/cache/errors.go

@@ -27,7 +27,7 @@ func sourceError(ctx context.Context, pkg *pkg, e interface{}) (*source.Error, e
 		fixes         []source.SuggestedFix
 		related       []source.RelatedInformation
 	)
-	fset := pkg.snapshot.view.session.cache.fset
+	fset := pkg.view.session.cache.fset
 	switch e := e.(type) {
 	case packages.Error:
 		if e.Pos == "" {
@@ -64,18 +64,18 @@ func sourceError(ctx context.Context, pkg *pkg, e interface{}) (*source.Error, e
 		spn, err = typeErrorRange(ctx, fset, pkg, e.Pos)
 
 	case *analysis.Diagnostic:
-		spn, err = span.NewRange(pkg.snapshot.view.session.cache.fset, e.Pos, e.End).Span()
+		spn, err = span.NewRange(fset, e.Pos, e.End).Span()
 		if err != nil {
 			return nil, err
 		}
 		msg = e.Message
 		kind = source.Analysis
 		category = e.Category
-		fixes, err = suggestedFixes(ctx, pkg, e)
+		fixes, err = suggestedFixes(ctx, fset, pkg, e)
 		if err != nil {
 			return nil, err
 		}
-		related, err = relatedInformation(ctx, pkg, e)
+		related, err = relatedInformation(ctx, fset, pkg, e)
 		if err != nil {
 			return nil, err
 		}
@@ -95,12 +95,12 @@ func sourceError(ctx context.Context, pkg *pkg, e interface{}) (*source.Error, e
 	}, nil
 }
 
-func suggestedFixes(ctx context.Context, pkg *pkg, diag *analysis.Diagnostic) ([]source.SuggestedFix, error) {
+func suggestedFixes(ctx context.Context, fset *token.FileSet, pkg *pkg, diag *analysis.Diagnostic) ([]source.SuggestedFix, error) {
 	var fixes []source.SuggestedFix
 	for _, fix := range diag.SuggestedFixes {
 		edits := make(map[span.URI][]protocol.TextEdit)
 		for _, e := range fix.TextEdits {
-			spn, err := span.NewRange(pkg.Snapshot().View().Session().Cache().FileSet(), e.Pos, e.End).Span()
+			spn, err := span.NewRange(fset, e.Pos, e.End).Span()
 			if err != nil {
 				return nil, err
 			}
@@ -121,10 +121,10 @@ func suggestedFixes(ctx context.Context, pkg *pkg, diag *analysis.Diagnostic) ([
 	return fixes, nil
 }
 
-func relatedInformation(ctx context.Context, pkg *pkg, diag *analysis.Diagnostic) ([]source.RelatedInformation, error) {
+func relatedInformation(ctx context.Context, fset *token.FileSet, pkg *pkg, diag *analysis.Diagnostic) ([]source.RelatedInformation, error) {
 	var out []source.RelatedInformation
 	for _, related := range diag.Related {
-		spn, err := span.NewRange(pkg.Snapshot().View().Session().Cache().FileSet(), related.Pos, related.End).Span()
+		spn, err := span.NewRange(fset, related.Pos, related.End).Span()
 		if err != nil {
 			return nil, err
 		}

internal/lsp/cache/pkg.go

@@ -19,7 +19,7 @@ import (
 
 // pkg contains the type information needed by the source package.
 type pkg struct {
-	snapshot *snapshot
+	view *view
 
 	// ID and package path have their own types to avoid being used interchangeably.
 	id      packageID
@@ -43,8 +43,8 @@ type pkg struct {
 type packageID string
 type packagePath string
 
-func (p *pkg) Snapshot() source.Snapshot {
-	return p.snapshot
+func (p *pkg) View() source.View {
+	return p.view
 }
 
 func (p *pkg) ID() string {
@@ -139,8 +139,8 @@ func (p *pkg) FindDiagnostic(pdiag protocol.Diagnostic) (*source.Diagnostic, err
 
 func (p *pkg) FindFile(ctx context.Context, uri span.URI) (source.ParseGoHandle, source.Package, error) {
 	// Special case for ignored files.
-	if p.snapshot.view.Ignore(uri) {
-		return p.snapshot.view.findIgnoredFile(ctx, uri)
+	if p.view.Ignore(uri) {
+		return p.view.findIgnoredFile(ctx, uri)
 	}
 
 	queue := []*pkg{p}

internal/lsp/source/completion_format.go

@@ -136,7 +136,7 @@ func (c *completer) item(cand candidate) (CompletionItem, error) {
 	if !(file.Pos() <= obj.Pos() && obj.Pos() <= file.End()) {
 		return CompletionItem{}, errors.Errorf("no file for %s", obj.Name())
 	}
-	ident, err := findIdentifier(c.ctx, pkg, file, obj.Pos())
+	ident, err := findIdentifier(c.ctx, c.snapshot, pkg, file, obj.Pos())
 	if err != nil {
 		return CompletionItem{}, err
 	}

internal/lsp/source/hover.go

@@ -47,7 +47,7 @@ func (i *IdentifierInfo) Hover(ctx context.Context) (*HoverInformation, error) {
 	switch x := h.source.(type) {
 	case ast.Node:
 		var b strings.Builder
-		if err := format.Node(&b, i.Snapshot().View().Session().Cache().FileSet(), x); err != nil {
+		if err := format.Node(&b, i.Snapshot.View().Session().Cache().FileSet(), x); err != nil {
 			return nil, err
 		}
 		h.Signature = b.String()

internal/lsp/source/identifier.go

@@ -20,8 +20,9 @@ import (
 
 // IdentifierInfo holds information about an identifier in Go source.
 type IdentifierInfo struct {
-	Name string
-	File ParseGoHandle
+	Name     string
+	File     ParseGoHandle
+	Snapshot Snapshot
 	mappedRange
 
 	Type struct {
@@ -37,10 +38,6 @@ type IdentifierInfo struct {
 	qf               types.Qualifier
 }
 
-func (i *IdentifierInfo) Snapshot() Snapshot {
-	return i.pkg.Snapshot()
-}
-
 type Declaration struct {
 	mappedRange
 	node        ast.Node
@@ -51,7 +48,7 @@ type Declaration struct {
 // Identifier returns identifier information for a position
 // in a file, accounting for a potentially incomplete selector.
 func Identifier(ctx context.Context, view View, f File, pos protocol.Position) (*IdentifierInfo, error) {
-	_, cphs, err := view.CheckPackageHandles(ctx, f)
+	snapshot, cphs, err := view.CheckPackageHandles(ctx, f)
 	if err != nil {
 		return nil, err
 	}
@@ -79,17 +76,17 @@ func Identifier(ctx context.Context, view View, f File, pos protocol.Position) (
 	if err != nil {
 		return nil, err
 	}
-	return findIdentifier(ctx, pkg, file, rng.Start)
+	return findIdentifier(ctx, snapshot, pkg, file, rng.Start)
 }
 
-func findIdentifier(ctx context.Context, pkg Package, file *ast.File, pos token.Pos) (*IdentifierInfo, error) {
-	if result, err := identifier(ctx, pkg, file, pos); err != nil || result != nil {
+func findIdentifier(ctx context.Context, snapshot Snapshot, pkg Package, file *ast.File, pos token.Pos) (*IdentifierInfo, error) {
+	if result, err := identifier(ctx, snapshot, pkg, file, pos); err != nil || result != nil {
 		return result, err
 	}
 	// If the position is not an identifier but immediately follows
 	// an identifier or selector period (as is common when
 	// requesting a completion), use the path to the preceding node.
-	ident, err := identifier(ctx, pkg, file, pos-1)
+	ident, err := identifier(ctx, snapshot, pkg, file, pos-1)
 	if ident == nil && err == nil {
 		err = errors.New("no identifier found")
 	}
@@ -97,21 +94,21 @@ func findIdentifier(ctx context.Context, pkg Package, file *ast.File, pos token.
 }
 
 // identifier checks a single position for a potential identifier.
-func identifier(ctx context.Context, pkg Package, file *ast.File, pos token.Pos) (*IdentifierInfo, error) {
+func identifier(ctx context.Context, snapshot Snapshot, pkg Package, file *ast.File, pos token.Pos) (*IdentifierInfo, error) {
 	ctx, done := trace.StartSpan(ctx, "source.identifier")
 	defer done()
 
 	var err error
 
 	// Handle import specs separately, as there is no formal position for a package declaration.
-	if result, err := importSpec(ctx, pkg, file, pos); result != nil || err != nil {
+	if result, err := importSpec(ctx, snapshot, pkg, file, pos); result != nil || err != nil {
 		return result, err
 	}
 	path, _ := astutil.PathEnclosingInterval(file, pos, pos)
 	if path == nil {
 		return nil, errors.Errorf("can't find node enclosing position")
 	}
-	view := pkg.Snapshot().View()
+	view := pkg.View()
 	uri := span.FileURI(view.Session().Cache().FileSet().Position(pos).Filename)
 	var ph ParseGoHandle
 	for _, h := range pkg.Files() {
@@ -120,10 +117,11 @@ func identifier(ctx context.Context, pkg Package, file *ast.File, pos token.Pos)
 		}
 	}
 	result := &IdentifierInfo{
-		File:  ph,
-		qf:    qualifier(file, pkg.GetTypes(), pkg.GetTypesInfo()),
-		pkg:   pkg,
-		ident: searchForIdent(path[0]),
+		File:     ph,
+		Snapshot: snapshot,
+		qf:       qualifier(file, pkg.GetTypes(), pkg.GetTypesInfo()),
+		pkg:      pkg,
+		ident:    searchForIdent(path[0]),
 	}
 	// No identifier at the given position.
 	if result.ident == nil {
@@ -244,7 +242,7 @@ func hasErrorType(obj types.Object) bool {
 }
 
 func objToNode(ctx context.Context, pkg Package, obj types.Object) (ast.Decl, error) {
-	view := pkg.Snapshot().View()
+	view := pkg.View()
 	uri := span.FileURI(view.Session().Cache().FileSet().Position(obj.Pos()).Filename)
 	ph, _, err := pkg.FindFile(ctx, uri)
 	if err != nil {
@@ -280,7 +278,7 @@ func objToNode(ctx context.Context, pkg Package, obj types.Object) (ast.Decl, er
 }
 
 // importSpec handles positions inside of an *ast.ImportSpec.
-func importSpec(ctx context.Context, pkg Package, file *ast.File, pos token.Pos) (*IdentifierInfo, error) {
+func importSpec(ctx context.Context, snapshot Snapshot, pkg Package, file *ast.File, pos token.Pos) (*IdentifierInfo, error) {
 	var imp *ast.ImportSpec
 	for _, spec := range file.Imports {
 		if spec.Path.Pos() <= pos && pos < spec.Path.End() {
@@ -294,7 +292,7 @@ func importSpec(ctx context.Context, pkg Package, file *ast.File, pos token.Pos)
 	if err != nil {
 		return nil, errors.Errorf("import path not quoted: %s (%v)", imp.Path.Value, err)
 	}
-	uri := span.FileURI(pkg.Snapshot().View().Session().Cache().FileSet().Position(pos).Filename)
+	uri := span.FileURI(pkg.View().Session().Cache().FileSet().Position(pos).Filename)
 	var ph ParseGoHandle
 	for _, h := range pkg.Files() {
 		if h.File().Identity().URI == uri {
@@ -302,9 +300,10 @@ func importSpec(ctx context.Context, pkg Package, file *ast.File, pos token.Pos)
 		}
 	}
 	result := &IdentifierInfo{
-		File: ph,
-		Name: importPath,
-		pkg:  pkg,
+		File:     ph,
+		Snapshot: snapshot,
+		Name:     importPath,
+		pkg:      pkg,
 	}
 	if result.mappedRange, err = posToMappedRange(ctx, pkg, imp.Path.Pos(), imp.Path.End()); err != nil {
 		return nil, err

internal/lsp/source/rename.go

@@ -151,7 +151,7 @@ func (i *IdentifierInfo) Rename(ctx context.Context, view View, newName string)
 		if err != nil {
 			return nil, err
 		}
-		fh := i.Snapshot().Handle(ctx, f)
+		fh := i.Snapshot.Handle(ctx, f)
 		data, _, err := fh.Read(ctx)
 		if err != nil {
 			return nil, err
@@ -225,6 +225,7 @@ func getPkgNameIdentifier(ctx context.Context, ident *IdentifierInfo, pkgName *t
 		return nil, err
 	}
 	return &IdentifierInfo{
+		Snapshot:         ident.Snapshot,
 		Name:             pkgName.Name(),
 		mappedRange:      decl.mappedRange,
 		File:             ident.File,

internal/lsp/source/util.go

@@ -159,7 +159,7 @@ func posToMappedRange(ctx context.Context, pkg Package, pos, end token.Pos) (map
 	if err != nil {
 		return mappedRange{}, err
 	}
-	return posToRange(ctx, pkg.Snapshot().View(), m, pos, end)
+	return posToRange(ctx, pkg.View(), m, pos, end)
 }
 
 func posToRange(ctx context.Context, view View, m *protocol.ColumnMapper, pos, end token.Pos) (mappedRange, error) {
@@ -176,7 +176,7 @@ func posToRange(ctx context.Context, view View, m *protocol.ColumnMapper, pos, e
 }
 
 func posToMapper(ctx context.Context, pkg Package, pos token.Pos) (*protocol.ColumnMapper, error) {
-	posn := pkg.Snapshot().View().Session().Cache().FileSet().Position(pos)
+	posn := pkg.View().Session().Cache().FileSet().Position(pos)
 	ph, _, err := pkg.FindFile(ctx, span.FileURI(posn.Filename))
 	if err != nil {
 		return nil, err

internal/lsp/source/view.go

@@ -274,7 +274,6 @@ type File interface {
 // Package represents a Go package that has been type-checked. It maintains
 // only the relevant fields of a *go/packages.Package.
 type Package interface {
-	Snapshot() Snapshot
 	ID() string
 	PkgPath() string
 	Files() []ParseGoHandle
@@ -295,6 +294,8 @@ type Package interface {
 	// FindFile returns the AST and type information for a file that may
 	// belong to or be part of a dependency of the given package.
 	FindFile(ctx context.Context, uri span.URI) (ParseGoHandle, Package, error)
+
+	View() View
 }
 
 type Error struct {