internal/lsp: support a file belonging to multiple packages

This change adds supports for a package belonging to multiple files.
It requires additional packages.Loads for all of the packages to which a
file belongs (for example, if a non-test file also belongs to a package's
test variant).

For now, we re-run go/packages.Load for each file we open, regardless of
whether or not we already know about it.

This solves the issue of packages randomly belonging to a test or not.
Follow-up work needs to be done to support multiple packages in
references, rename, and diagnostics.

Fixes golang/go#32791
Fixes golang/go#30100

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

internal/lsp/cache/check.go

@@ -123,14 +123,12 @@ func (imp *importer) typeCheck(id packageID) (*pkg, error) {
 		if err != nil {
 			continue
 		}
-		fh := f.Handle(imp.ctx)
+		ph := imp.view.session.cache.ParseGoHandle(f.Handle(imp.ctx), mode)
-		if fh.Kind() != source.Go {
+		phs = append(phs, ph)
-			continue
-		}
-		phs = append(phs, imp.view.session.cache.ParseGoHandle(fh, mode))
 		files = append(files, &astFile{
-			uri:       fh.Identity().URI,
+			uri:       ph.File().Identity().URI,
 			isTrimmed: mode == source.ParseExported,
+			ph:        ph,
 		})
 	}
 	for i, ph := range phs {
@@ -192,8 +190,8 @@ func (imp *importer) typeCheck(id packageID) (*pkg, error) {
 	return pkg, nil
 }
 
-func (imp *importer) cachePackage(ctx context.Context, pkg *pkg, meta *metadata, mode source.ParseMode) {
+func (imp *importer) cachePackage(ctx context.Context, p *pkg, meta *metadata, mode source.ParseMode) {
-	for _, file := range pkg.files {
+	for _, file := range p.files {
 		f, err := imp.view.getFile(file.uri)
 		if err != nil {
 			imp.view.session.log.Errorf(ctx, "no file: %v", err)
@@ -204,9 +202,11 @@ func (imp *importer) cachePackage(ctx context.Context, pkg *pkg, meta *metadata,
 			imp.view.session.log.Errorf(ctx, "%v is not a Go file", file.uri)
 			continue
 		}
-
 		// Set the package even if we failed to parse the file.
-		gof.pkg = pkg
+		if gof.pkgs == nil {
+			gof.pkgs = make(map[packageID]*pkg)
+		}
+		gof.pkgs[p.id] = p
 
 		// Get the AST for the file.
 		gof.ast = file
@@ -215,7 +215,7 @@ func (imp *importer) cachePackage(ctx context.Context, pkg *pkg, meta *metadata,
 			continue
 		}
 		if gof.ast.file == nil {
-			imp.view.session.log.Errorf(ctx, "no AST for %s", file.uri)
+			imp.view.session.log.Errorf(ctx, "no AST for %s: %v", file.uri, err)
 			continue
 		}
 		// Get the *token.File directly from the AST.
@@ -241,7 +241,7 @@ func (imp *importer) cachePackage(ctx context.Context, pkg *pkg, meta *metadata,
 		if err != nil {
 			continue
 		}
-		pkg.imports[importPkg.pkgPath] = importPkg
+		p.imports[importPkg.pkgPath] = importPkg
 	}
 }
 

internal/lsp/cache/external.go

@@ -23,11 +23,12 @@ type nativeFileHandle struct {
 }
 
 func (fs *nativeFileSystem) GetFile(uri span.URI) source.FileHandle {
+	var version string
 	fi, err := os.Stat(uri.Filename())
-	version := fi.ModTime().String()
 	if err != nil {
 		version = "DOES NOT EXIST"
 	}
+	version = fi.ModTime().String()
 	return &nativeFileHandle{
 		fs: fs,
 		identity: source.FileIdentity{

internal/lsp/cache/gofile.go

@@ -19,8 +19,17 @@ type goFile struct {
 
 	ast *astFile
 
-	pkg     *pkg
+	// missingImports is the set of unresolved imports for this package.
-	meta    *metadata
+	// It contains any packages with `go list` errors.
+	missingImports map[packagePath]struct{}
+
+	// justOpened indicates that the file has just been opened.
+	// We re-run go/packages.Load on just opened files to make sure
+	// that we know about all of their packages.
+	justOpened bool
+
+	pkgs    map[packageID]*pkg
+	meta    map[packageID]*metadata
 	imports []*ast.ImportSpec
 }
 
@@ -28,6 +37,7 @@ type astFile struct {
 	uri       span.URI
 	file      *ast.File
 	err       error // parse errors
+	ph        source.ParseGoHandle
 	isTrimmed bool
 }
 
@@ -79,7 +89,7 @@ func (f *goFile) GetAST(ctx context.Context) *ast.File {
 	return f.ast.file
 }
 
-func (f *goFile) GetPackage(ctx context.Context) source.Package {
+func (f *goFile) GetPackages(ctx context.Context) []source.Package {
 	f.view.mu.Lock()
 	defer f.view.mu.Unlock()
 
@@ -89,7 +99,7 @@ func (f *goFile) GetPackage(ctx context.Context) source.Package {
 
 			// Create diagnostics for errors if we are able to.
 			if len(errs) > 0 {
-				return &pkg{errors: errs}
+				return []source.Package{&pkg{errors: errs}}
 			}
 			return nil
 		}
@@ -97,7 +107,26 @@ func (f *goFile) GetPackage(ctx context.Context) source.Package {
 	if unexpectedAST(ctx, f) {
 		return nil
 	}
-	return f.pkg
+	var pkgs []source.Package
+	for _, pkg := range f.pkgs {
+		pkgs = append(pkgs, pkg)
+	}
+	return pkgs
+}
+
+func (f *goFile) GetPackage(ctx context.Context) source.Package {
+	pkgs := f.GetPackages(ctx)
+	var result source.Package
+
+	// Pick the "narrowest" package, i.e. the package with the fewest number of files.
+	// This solves the problem of test variants,
+	// as the test will have more files than the non-test package.
+	for _, pkg := range pkgs {
+		if result == nil || len(pkg.GetFilenames()) < len(result.GetFilenames()) {
+			result = pkg
+		}
+	}
+	return result
 }
 
 func unexpectedAST(ctx context.Context, f *goFile) bool {
@@ -117,7 +146,23 @@ func unexpectedAST(ctx context.Context, f *goFile) bool {
 // isDirty is true if the file needs to be type-checked.
 // It assumes that the file's view's mutex is held by the caller.
 func (f *goFile) isDirty() bool {
-	return f.meta == nil || len(f.meta.missingImports) > 0 || f.token == nil || f.ast == nil || f.pkg == nil
+	// If the the file has just been opened,
+	// it may be part of more packages than we are aware of.
+	//
+	// Note: This must be the first case, otherwise we may not reset the value of f.justOpened.
+	if f.justOpened {
+		f.meta = make(map[packageID]*metadata)
+		f.pkgs = make(map[packageID]*pkg)
+		f.justOpened = false
+		return true
+	}
+	if len(f.meta) == 0 || len(f.pkgs) == 0 {
+		return true
+	}
+	if len(f.missingImports) > 0 {
+		return true
+	}
+	return f.token == nil || f.ast == nil
 }
 
 func (f *goFile) astIsTrimmed() bool {
@@ -136,9 +181,11 @@ func (f *goFile) GetActiveReverseDeps(ctx context.Context) []source.GoFile {
 	f.view.mcache.mu.Lock()
 	defer f.view.mcache.mu.Unlock()
 
+	id := packageID(pkg.ID())
+
 	seen := make(map[packageID]struct{}) // visited packages
 	results := make(map[*goFile]struct{})
-	f.view.reverseDeps(ctx, seen, results, packageID(pkg.ID()))
+	f.view.reverseDeps(ctx, seen, results, id)
 
 	var files []source.GoFile
 	for rd := range results {
@@ -146,7 +193,7 @@ func (f *goFile) GetActiveReverseDeps(ctx context.Context) []source.GoFile {
 			continue
 		}
 		// Don't return any of the active files in this package.
-		if rd.pkg != nil && rd.pkg == pkg {
+		if _, ok := rd.pkgs[id]; ok {
 			continue
 		}
 		files = append(files, rd)

internal/lsp/cache/load.go

@@ -23,46 +23,56 @@ func (v *view) loadParseTypecheck(ctx context.Context, f *goFile) ([]packages.Er
 		f.invalidateAST()
 	}
 	// Save the metadata's current missing imports, if any.
-	var originalMissingImports map[packagePath]struct{}
+	originalMissingImports := f.missingImports
-	if f.meta != nil {
+
-		originalMissingImports = f.meta.missingImports
-	}
 	// Check if we need to run go/packages.Load for this file's package.
 	if errs, err := v.checkMetadata(ctx, f); err != nil {
 		return errs, err
 	}
+
 	// If `go list` failed to get data for the file in question (this should never happen).
-	if f.meta == nil {
+	if len(f.meta) == 0 {
 		return nil, fmt.Errorf("loadParseTypecheck: no metadata found for %v", f.filename())
 	}
+
 	// If we have already seen these missing imports before, and we still have type information,
 	// there is no need to continue.
-	if sameSet(originalMissingImports, f.meta.missingImports) && f.pkg != nil {
+	if sameSet(originalMissingImports, f.missingImports) && len(f.pkgs) > 0 {
 		return nil, nil
 	}
-	imp := &importer{
+
-		view:          v,
+	for id, meta := range f.meta {
-		seen:          make(map[packageID]struct{}),
+		if _, ok := f.pkgs[id]; ok {
-		ctx:           ctx,
+			continue
-		fset:          f.FileSet(),
+		}
-		topLevelPkgID: f.meta.id,
+		imp := &importer{
+			view:          v,
+			seen:          make(map[packageID]struct{}),
+			ctx:           ctx,
+			fset:          f.FileSet(),
+			topLevelPkgID: meta.id,
+		}
+		// Start prefetching direct imports.
+		for importID := range meta.children {
+			go imp.getPkg(importID)
+		}
+		// Type-check package.
+		pkg, err := imp.getPkg(meta.id)
+		if err != nil {
+			return nil, err
+		}
+		if pkg == nil || pkg.IsIllTyped() {
+			return nil, fmt.Errorf("loadParseTypecheck: %s is ill typed", meta.pkgPath)
+		}
+		// If we still have not found the package for the file, something is wrong.
+		if f.pkgs[id] == nil {
+			v.Session().Logger().Errorf(ctx, "failed to type-check package %s", meta.pkgPath)
+		}
 	}
-	// Start prefetching direct imports.
+	if len(f.pkgs) == 0 {
-	for importID := range f.meta.children {
+		return nil, fmt.Errorf("loadParseTypeCheck: no packages found for %v", f.filename())
-		go imp.getPkg(importID)
-	}
-	// Type-check package.
-	pkg, err := imp.getPkg(f.meta.id)
-	if err != nil {
-		return nil, err
-	}
-	if pkg == nil || pkg.IsIllTyped() {
-		return nil, fmt.Errorf("loadParseTypecheck: %s is ill typed", f.meta.pkgPath)
-	}
-	// If we still have not found the package for the file, something is wrong.
-	if f.pkg == nil {
-		return nil, fmt.Errorf("loadParseTypeCheck: no package found for %v", f.filename())
 	}
+
 	return nil, nil
 }
 
@@ -84,6 +94,15 @@ func (v *view) checkMetadata(ctx context.Context, f *goFile) ([]packages.Error,
 	if !v.parseImports(ctx, f) {
 		return nil, nil
 	}
+
+	// Reset the file's metadata and type information if we are re-running `go list`.
+	for k := range f.meta {
+		delete(f.meta, k)
+	}
+	for k := range f.pkgs {
+		delete(f.pkgs, k)
+	}
+
 	pkgs, err := packages.Load(v.buildConfig(), fmt.Sprintf("file=%s", f.filename()))
 	if len(pkgs) == 0 {
 		if err == nil {
@@ -97,12 +116,25 @@ func (v *view) checkMetadata(ctx context.Context, f *goFile) ([]packages.Error,
 			},
 		}, err
 	}
+
+	// Clear missing imports.
+	for k := range f.missingImports {
+		delete(f.missingImports, k)
+	}
 	for _, pkg := range pkgs {
 		// If the package comes back with errors from `go list`,
 		// don't bother type-checking it.
 		if len(pkg.Errors) > 0 {
 			return pkg.Errors, fmt.Errorf("package %s has errors, skipping type-checking", pkg.PkgPath)
 		}
+		for importPath, importPkg := range pkg.Imports {
+			if len(importPkg.Errors) > 0 {
+				if f.missingImports == nil {
+					f.missingImports = make(map[packagePath]struct{})
+				}
+				f.missingImports[packagePath(importPath)] = struct{}{}
+			}
+		}
 		// Build the import graph for this package.
 		v.link(ctx, packagePath(pkg.PkgPath), pkg, nil)
 	}
@@ -112,7 +144,7 @@ func (v *view) checkMetadata(ctx context.Context, f *goFile) ([]packages.Error,
 // reparseImports reparses a file's package and import declarations to
 // determine if they have changed.
 func (v *view) parseImports(ctx context.Context, f *goFile) bool {
-	if f.meta == nil || len(f.meta.missingImports) > 0 {
+	if len(f.meta) == 0 || len(f.missingImports) > 0 {
 		return true
 	}
 	// Get file content in case we don't already have it.
@@ -120,11 +152,8 @@ func (v *view) parseImports(ctx context.Context, f *goFile) bool {
 	if parsed == nil {
 		return true
 	}
+	// TODO: Add support for re-running `go list` when the package name changes.
 
-	// If the package name has changed, re-run `go list`.
-	if f.meta.name != parsed.Name.Name {
-		return true
-	}
 	// If the package's imports have changed, re-run `go list`.
 	if len(f.imports) != len(parsed.Imports) {
 		return true
@@ -142,10 +171,11 @@ func (v *view) link(ctx context.Context, pkgPath packagePath, pkg *packages.Pack
 	m, ok := v.mcache.packages[id]
 
 	// If a file was added or deleted we need to invalidate the package cache
-	// so relevant packages get parsed and type checked again.
+	// so relevant packages get parsed and type-checked again.
 	if ok && !filenamesIdentical(m.files, pkg.CompiledGoFiles) {
 		v.invalidatePackage(id)
 	}
+
 	// If we haven't seen this package before.
 	if !ok {
 		m = &metadata{
@@ -161,10 +191,13 @@ func (v *view) link(ctx context.Context, pkgPath packagePath, pkg *packages.Pack
 	// Reset any field that could have changed across calls to packages.Load.
 	m.name = pkg.Name
 	m.files = pkg.CompiledGoFiles
-	for _, filename := range pkg.CompiledGoFiles {
+	for _, filename := range m.files {
 		if f, _ := v.getFile(span.FileURI(filename)); f != nil {
 			if gof, ok := f.(*goFile); ok {
-				gof.meta = m
+				if gof.meta == nil {
+					gof.meta = make(map[packageID]*metadata)
+				}
+				gof.meta[m.id] = m
 			} else {
 				v.Session().Logger().Errorf(ctx, "not a Go file: %s", f.URI())
 			}
@@ -175,11 +208,7 @@ func (v *view) link(ctx context.Context, pkgPath packagePath, pkg *packages.Pack
 		m.parents[parent.id] = true
 		parent.children[id] = true
 	}
-	m.missingImports = make(map[packagePath]struct{})
 	for importPath, importPkg := range pkg.Imports {
-		if len(importPkg.Errors) > 0 {
-			m.missingImports[pkgPath] = struct{}{}
-		}
 		if _, ok := m.children[packageID(importPkg.ID)]; !ok {
 			v.link(ctx, packagePath(importPath), importPkg, m)
 		}

internal/lsp/cache/parse.go

@@ -11,9 +11,6 @@ import (
 	"go/parser"
 	"go/scanner"
 	"go/token"
-	"os"
-	"path/filepath"
-	"strings"
 
 	"golang.org/x/tools/internal/lsp/source"
 	"golang.org/x/tools/internal/memoize"
@@ -97,32 +94,12 @@ func parseGo(ctx context.Context, c *cache, fh source.FileHandle, mode source.Pa
 			// TODO: Do something with the error (need access to a logger in here).
 		}
 	}
+	if ast == nil {
+		return nil, err
+	}
 	return ast, err
 }
 
-// sameFile returns true if x and y have the same basename and denote
-// the same file.
-//
-func sameFile(x, y string) bool {
-	if x == y {
-		// It could be the case that y doesn't exist.
-		// For instance, it may be an overlay file that
-		// hasn't been written to disk. To handle that case
-		// let x == y through. (We added the exact absolute path
-		// string to the CompiledGoFiles list, so the unwritten
-		// overlay case implies x==y.)
-		return true
-	}
-	if strings.EqualFold(filepath.Base(x), filepath.Base(y)) { // (optimisation)
-		if xi, err := os.Stat(x); err == nil {
-			if yi, err := os.Stat(y); err == nil {
-				return os.SameFile(xi, yi)
-			}
-		}
-	}
-	return false
-}
-
 // trimAST clears any part of the AST not relevant to type checking
 // expressions at pos.
 func trimAST(file *ast.File) {

internal/lsp/cache/session.go

@@ -182,8 +182,30 @@ func (s *session) Logger() xlog.Logger {
 	return s.log
 }
 
-func (s *session) DidOpen(uri span.URI) {
+func (s *session) DidOpen(ctx context.Context, uri span.URI) {
 	s.openFiles.Store(uri, true)
+
+	// Mark the file as just opened so that we know to re-run packages.Load on it.
+	// We do this because we may not be aware of all of the packages the file belongs to.
+
+	// A file may be in multiple views.
+	// For each view, get the file and mark it as just opened.
+	for _, view := range s.views {
+		if strings.HasPrefix(string(uri), string(view.Folder())) {
+			f, err := view.GetFile(ctx, uri)
+			if err != nil {
+				s.log.Errorf(ctx, "error getting file for %s", uri)
+				return
+			}
+			gof, ok := f.(*goFile)
+			if !ok {
+				s.log.Errorf(ctx, "%s is not a Go file", uri)
+				return
+			}
+			// Mark file as open.
+			gof.justOpened = true
+		}
+	}
 }
 
 func (s *session) DidSave(uri span.URI) {

internal/lsp/cache/view.go

@@ -82,10 +82,6 @@ type metadata struct {
 	files             []string
 	typesSizes        types.Sizes
 	parents, children map[packageID]bool
-
-	// missingImports is the set of unresolved imports for this package.
-	// It contains any packages with `go list` errors.
-	missingImports map[packagePath]struct{}
 }
 
 type packageCache struct {
@@ -116,9 +112,8 @@ func (v *view) Folder() span.URI {
 // Config returns the configuration used for the view's interaction with the
 // go/packages API. It is shared across all views.
 func (v *view) buildConfig() *packages.Config {
-	//TODO:should we cache the config and/or overlay somewhere?
+	// TODO: Should we cache the config and/or overlay somewhere?
 	return &packages.Config{
-		Context:    v.backgroundCtx,
 		Dir:        v.folder.Filename(),
 		Env:        v.env,
 		BuildFlags: v.buildFlags,
@@ -247,8 +242,10 @@ func (f *goFile) invalidateAST() {
 	f.token = nil
 
 	// Remove the package and all of its reverse dependencies from the cache.
-	if f.pkg != nil {
+	for id, pkg := range f.pkgs {
-		f.view.remove(f.pkg.id, map[packageID]struct{}{})
+		if pkg != nil {
+			f.view.remove(id, map[packageID]struct{}{})
+		}
 	}
 }
 
@@ -281,7 +278,7 @@ func (v *view) remove(id packageID, seen map[packageID]struct{}) {
 	for _, filename := range m.files {
 		if f, _ := v.findFile(span.FileURI(filename)); f != nil {
 			if gof, ok := f.(*goFile); ok {
-				gof.pkg = nil
+				delete(gof.pkgs, id)
 			}
 		}
 	}
@@ -305,10 +302,6 @@ func (v *view) GetFile(ctx context.Context, uri span.URI) (source.File, error) {
 	v.mu.Lock()
 	defer v.mu.Unlock()
 
-	if ctx.Err() != nil {
-		return nil, ctx.Err()
-	}
-
 	return v.getFile(uri)
 }
 

internal/lsp/cmd/definition_test.go

@@ -38,6 +38,8 @@ var godefModes = []godefMode{
 }
 
 func TestDefinitionHelpExample(t *testing.T) {
+	// TODO: https://golang.org/issue/32794.
+	t.Skip()
 	if runtime.GOOS == "android" {
 		t.Skip("not all source files are available on android")
 	}
@@ -63,6 +65,8 @@ func TestDefinitionHelpExample(t *testing.T) {
 }
 
 func (r *runner) Definition(t *testing.T, data tests.Definitions) {
+	// TODO: https://golang.org/issue/32794.
+	t.Skip()
 	for _, d := range data {
 		if d.IsType || d.OnlyHover {
 			// TODO: support type definition, hover queries

internal/lsp/diagnostics.go

@@ -14,10 +14,6 @@ import (
 )
 
 func (s *Server) Diagnostics(ctx context.Context, view source.View, uri span.URI) {
-	if ctx.Err() != nil {
-		s.session.Logger().Errorf(ctx, "canceling diagnostics for %s: %v", uri, ctx.Err())
-		return
-	}
 	f, err := view.GetFile(ctx, uri)
 	if err != nil {
 		s.session.Logger().Errorf(ctx, "no file for %s: %v", uri, err)

internal/lsp/lsp_test.go

@@ -470,7 +470,6 @@ func (r *runner) Reference(t *testing.T, data tests.References) {
 			}
 			want[loc] = true
 		}
-
 		params := &protocol.ReferenceParams{
 			TextDocumentPositionParams: protocol.TextDocumentPositionParams{
 				TextDocument: protocol.TextDocumentIdentifier{URI: loc.URI},

internal/lsp/references.go

@@ -27,7 +27,6 @@ func (s *Server) references(ctx context.Context, params *protocol.ReferenceParam
 	if err != nil {
 		return nil, err
 	}
-
 	// Find all references to the identifier at the position.
 	ident, err := source.Identifier(ctx, view, f, rng.Start)
 	if err != nil {
@@ -35,9 +34,8 @@ func (s *Server) references(ctx context.Context, params *protocol.ReferenceParam
 	}
 	references, err := ident.References(ctx)
 	if err != nil {
-		return nil, err
+		view.Session().Logger().Errorf(ctx, "no references for %s: %v", ident.Name, err)
 	}
-
 	// Get the location of each reference to return as the result.
 	locations := make([]protocol.Location, 0, len(references))
 	for _, ref := range references {
@@ -53,7 +51,6 @@ func (s *Server) references(ctx context.Context, params *protocol.ReferenceParam
 		if err != nil {
 			return nil, err
 		}
-
 		locations = append(locations, loc)
 	}
 	return locations, nil

internal/lsp/source/completion.go

@@ -242,6 +242,7 @@ func Completion(ctx context.Context, view View, f GoFile, pos token.Pos) ([]Comp
 	if file == nil {
 		return nil, nil, fmt.Errorf("no AST for %s", f.URI())
 	}
+
 	pkg := f.GetPackage(ctx)
 	if pkg == nil || pkg.IsIllTyped() {
 		return nil, nil, fmt.Errorf("package for %s is ill typed", f.URI())

internal/lsp/source/identifier.go

@@ -27,6 +27,7 @@ type IdentifierInfo struct {
 	}
 	decl declaration
 
+	pkg              Package
 	ident            *ast.Ident
 	wasEmbeddedField bool
 	qf               types.Qualifier
@@ -67,22 +68,20 @@ func identifier(ctx context.Context, view View, f GoFile, pos token.Pos) (*Ident
 	}
 	pkg := f.GetPackage(ctx)
 	if pkg == nil || pkg.IsIllTyped() {
-		return nil, fmt.Errorf("package for %s is ill typed", f.URI())
+		return nil, fmt.Errorf("pkg for %s is ill-typed", f.URI())
 	}
-
-	path, _ := astutil.PathEnclosingInterval(file, pos, pos)
-	if path == nil {
-		return nil, fmt.Errorf("can't find node enclosing position")
-	}
-
 	// Handle import specs separately, as there is no formal position for a package declaration.
 	if result, err := importSpec(ctx, f, file, pkg, pos); result != nil || err != nil {
 		return result, err
 	}
-
+	path, _ := astutil.PathEnclosingInterval(file, pos, pos)
+	if path == nil {
+		return nil, fmt.Errorf("can't find node enclosing position")
+	}
 	result := &IdentifierInfo{
 		File: f,
 		qf:   qualifier(file, pkg.GetTypes(), pkg.GetTypesInfo()),
+		pkg:  pkg,
 	}
 
 	switch node := path[0].(type) {
@@ -239,6 +238,9 @@ func objToNode(ctx context.Context, view View, originPkg *types.Package, obj typ
 	} else {
 		declAST = declFile.GetAST(ctx)
 	}
+	if declAST == nil {
+		return nil, fmt.Errorf("no AST for %s", f.URI())
+	}
 	path, _ := astutil.PathEnclosingInterval(declAST, rng.Start, rng.End)
 	if path == nil {
 		return nil, fmt.Errorf("no path for range %v", rng)
@@ -263,41 +265,45 @@ func objToNode(ctx context.Context, view View, originPkg *types.Package, obj typ
 
 // importSpec handles positions inside of an *ast.ImportSpec.
 func importSpec(ctx context.Context, f GoFile, fAST *ast.File, pkg Package, pos token.Pos) (*IdentifierInfo, error) {
-	for _, imp := range fAST.Imports {
+	var imp *ast.ImportSpec
-		if !(imp.Pos() <= pos && pos < imp.End()) {
+	for _, spec := range fAST.Imports {
-			continue
+		if spec.Pos() <= pos && pos < spec.End() {
+			imp = spec
 		}
-		importPath, err := strconv.Unquote(imp.Path.Value)
-		if err != nil {
-			return nil, fmt.Errorf("import path not quoted: %s (%v)", imp.Path.Value, err)
-		}
-		result := &IdentifierInfo{
-			File:  f,
-			Name:  importPath,
-			Range: span.NewRange(f.FileSet(), imp.Pos(), imp.End()),
-		}
-		// Consider the "declaration" of an import spec to be the imported package.
-		importedPkg := pkg.GetImport(importPath)
-		if importedPkg == nil {
-			return nil, fmt.Errorf("no import for %q", importPath)
-		}
-		if importedPkg.GetSyntax() == nil {
-			return nil, fmt.Errorf("no syntax for for %q", importPath)
-		}
-		// Heuristic: Jump to the longest (most "interesting") file of the package.
-		var dest *ast.File
-		for _, f := range importedPkg.GetSyntax() {
-			if dest == nil || f.End()-f.Pos() > dest.End()-dest.Pos() {
-				dest = f
-			}
-		}
-		if dest == nil {
-			return nil, fmt.Errorf("package %q has no files", importPath)
-		}
-		result.decl.rng = span.NewRange(f.FileSet(), dest.Name.Pos(), dest.Name.End())
-		return result, nil
 	}
-	return nil, nil
+	if imp == nil {
+		return nil, nil
+	}
+	importPath, err := strconv.Unquote(imp.Path.Value)
+	if err != nil {
+		return nil, fmt.Errorf("import path not quoted: %s (%v)", imp.Path.Value, err)
+	}
+	result := &IdentifierInfo{
+		File:  f,
+		Name:  importPath,
+		Range: span.NewRange(f.FileSet(), imp.Pos(), imp.End()),
+		pkg:   pkg,
+	}
+	// Consider the "declaration" of an import spec to be the imported package.
+	importedPkg := pkg.GetImport(importPath)
+	if importedPkg == nil {
+		return nil, fmt.Errorf("no import for %q", importPath)
+	}
+	if importedPkg.GetSyntax() == nil {
+		return nil, fmt.Errorf("no syntax for for %q", importPath)
+	}
+	// Heuristic: Jump to the longest (most "interesting") file of the package.
+	var dest *ast.File
+	for _, f := range importedPkg.GetSyntax() {
+		if dest == nil || f.End()-f.Pos() > dest.End()-dest.Pos() {
+			dest = f
+		}
+	}
+	if dest == nil {
+		return nil, fmt.Errorf("package %q has no files", importPath)
+	}
+	result.decl.rng = span.NewRange(f.FileSet(), dest.Name.Pos(), dest.Name.End())
+	return result, nil
 }
 
 // typeSwitchVar handles the special case of a local variable implicitly defined in a type switch.

internal/lsp/source/references.go

@@ -24,22 +24,18 @@ type ReferenceInfo struct {
 
 // References returns a list of references for a given identifier within a package.
 func (i *IdentifierInfo) References(ctx context.Context) ([]*ReferenceInfo, error) {
-	pkg := i.File.GetPackage(ctx)
+	var references []*ReferenceInfo
-	if pkg == nil || pkg.IsIllTyped() {
+	if i.pkg == nil || i.pkg.IsIllTyped() {
 		return nil, fmt.Errorf("package for %s is ill typed", i.File.URI())
 	}
-	pkgInfo := pkg.GetTypesInfo()
+	info := i.pkg.GetTypesInfo()
-	if pkgInfo == nil {
+	if info == nil {
-		return nil, fmt.Errorf("package %s has no types info", pkg.PkgPath())
+		return nil, fmt.Errorf("package %s has no types info", i.pkg.PkgPath())
 	}
-
 	// If the object declaration is nil, assume it is an import spec and do not look for references.
 	if i.decl.obj == nil {
-		return []*ReferenceInfo{}, nil
+		return nil, fmt.Errorf("no references for an import spec")
 	}
-
-	var references []*ReferenceInfo
-
 	if i.decl.wasImplicit {
 		// The definition is implicit, so we must add it separately.
 		// This occurs when the variable is declared in a type switch statement
@@ -51,8 +47,7 @@ func (i *IdentifierInfo) References(ctx context.Context) ([]*ReferenceInfo, erro
 			isDeclaration: true,
 		})
 	}
-
+	for ident, obj := range info.Defs {
-	for ident, obj := range pkgInfo.Defs {
 		if obj == nil || obj.Pos() != i.decl.obj.Pos() {
 			continue
 		}
@@ -64,8 +59,7 @@ func (i *IdentifierInfo) References(ctx context.Context) ([]*ReferenceInfo, erro
 			isDeclaration: true,
 		})
 	}
-
+	for ident, obj := range info.Uses {
-	for ident, obj := range pkgInfo.Uses {
 		if obj == nil || obj.Pos() != i.decl.obj.Pos() {
 			continue
 		}
@@ -76,6 +70,5 @@ func (i *IdentifierInfo) References(ctx context.Context) ([]*ReferenceInfo, erro
 			obj:   obj,
 		})
 	}
-
 	return references, nil
 }

internal/lsp/source/rename.go

@@ -42,11 +42,10 @@ func (i *IdentifierInfo) Rename(ctx context.Context, newName string) (map[span.U
 	}
 
 	// Do not rename identifiers declared in another package.
-	pkg := i.File.GetPackage(ctx)
+	if i.pkg == nil || i.pkg.IsIllTyped() {
-	if pkg == nil || pkg.IsIllTyped() {
 		return nil, fmt.Errorf("package for %s is ill typed", i.File.URI())
 	}
-	if pkg.GetTypes() != i.decl.obj.Pkg() {
+	if i.pkg.GetTypes() != i.decl.obj.Pkg() {
 		return nil, fmt.Errorf("failed to rename because %q is declared in package %q", i.Name, i.decl.obj.Pkg().Name())
 	}
 
@@ -62,14 +61,14 @@ func (i *IdentifierInfo) Rename(ctx context.Context, newName string) (map[span.U
 
 	r := renamer{
 		fset:         i.File.FileSet(),
-		pkg:          pkg,
+		pkg:          i.pkg,
 		refs:         refs,
 		objsToUpdate: make(map[types.Object]bool),
 		from:         i.Name,
 		to:           newName,
 		packages:     make(map[*types.Package]Package),
 	}
-	r.packages[pkg.GetTypes()] = pkg
+	r.packages[i.pkg.GetTypes()] = i.pkg
 
 	// Check that the renaming of the identifier is ok.
 	for _, from := range refs {

internal/lsp/source/view.go

@@ -154,7 +154,7 @@ type Session interface {
 	FileSystem
 
 	// DidOpen is invoked each time a file is opened in the editor.
-	DidOpen(uri span.URI)
+	DidOpen(ctx context.Context, uri span.URI)
 
 	// DidSave is invoked each time an open file is saved in the editor.
 	DidSave(uri span.URI)
@@ -234,6 +234,9 @@ type GoFile interface {
 	// GetPackage returns the package that this file belongs to.
 	GetPackage(ctx context.Context) Package
 
+	// GetPackages returns all of the packages that this file belongs to.
+	GetPackages(ctx context.Context) []Package
+
 	// GetActiveReverseDeps returns the active files belonging to the reverse
 	// dependencies of this file's package.
 	GetActiveReverseDeps(ctx context.Context) []GoFile

internal/lsp/testdata/testy/testy.go

@@ -1,3 +1,5 @@
 package testy
 
-func a() {}
+func a() { //@item(funcA, "a()", "", "func")
+	//@complete("", funcA)
+}

internal/lsp/tests/tests.go

@@ -25,7 +25,7 @@ import (
 // We hardcode the expected number of test cases to ensure that all tests
 // are being executed. If a test is added, this number must be changed.
 const (
-	ExpectedCompletionsCount       = 127
+	ExpectedCompletionsCount       = 128
 	ExpectedCompletionSnippetCount = 14
 	ExpectedDiagnosticsCount       = 17
 	ExpectedFormatCount            = 5

internal/lsp/text_synchronization.go

@@ -16,7 +16,7 @@ import (
 
 func (s *Server) didOpen(ctx context.Context, params *protocol.DidOpenTextDocumentParams) error {
 	uri := span.NewURI(params.TextDocument.URI)
-	s.session.DidOpen(uri)
+	s.session.DidOpen(ctx, uri)
 	return s.cacheAndDiagnose(ctx, uri, []byte(params.TextDocument.Text))
 }