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>
diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go
index 408f12b..fab0364 100644
--- a/internal/lsp/cache/check.go
+++ b/internal/lsp/cache/check.go
@@ -123,14 +123,12 @@
if err != nil {
continue
}
- fh := f.Handle(imp.ctx)
- if fh.Kind() != source.Go {
- continue
- }
- phs = append(phs, imp.view.session.cache.ParseGoHandle(fh, mode))
+ ph := imp.view.session.cache.ParseGoHandle(f.Handle(imp.ctx), mode)
+ phs = append(phs, ph)
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 @@
return pkg, nil
}
-func (imp *importer) cachePackage(ctx context.Context, pkg *pkg, meta *metadata, mode source.ParseMode) {
- for _, file := range pkg.files {
+func (imp *importer) cachePackage(ctx context.Context, p *pkg, meta *metadata, mode source.ParseMode) {
+ 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 @@
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 @@
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 @@
if err != nil {
continue
}
- pkg.imports[importPkg.pkgPath] = importPkg
+ p.imports[importPkg.pkgPath] = importPkg
}
}
diff --git a/internal/lsp/cache/external.go b/internal/lsp/cache/external.go
index fafcd9b..043d20a 100644
--- a/internal/lsp/cache/external.go
+++ b/internal/lsp/cache/external.go
@@ -23,11 +23,12 @@
}
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{
diff --git a/internal/lsp/cache/gofile.go b/internal/lsp/cache/gofile.go
index ff2112a..36ec12f 100644
--- a/internal/lsp/cache/gofile.go
+++ b/internal/lsp/cache/gofile.go
@@ -19,8 +19,17 @@
ast *astFile
- pkg *pkg
- meta *metadata
+ // missingImports is the set of unresolved imports for this package.
+ // 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 @@
uri span.URI
file *ast.File
err error // parse errors
+ ph source.ParseGoHandle
isTrimmed bool
}
@@ -79,7 +89,7 @@
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 @@
// 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 @@
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 @@
// 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 @@
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 @@
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)
diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go
index 6937917..1368d56 100644
--- a/internal/lsp/cache/load.go
+++ b/internal/lsp/cache/load.go
@@ -23,46 +23,56 @@
f.invalidateAST()
}
// Save the metadata's current missing imports, if any.
- var originalMissingImports map[packagePath]struct{}
- if f.meta != nil {
- originalMissingImports = f.meta.missingImports
- }
+ originalMissingImports := f.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,
- seen: make(map[packageID]struct{}),
- ctx: ctx,
- fset: f.FileSet(),
- topLevelPkgID: f.meta.id,
+
+ for id, meta := range f.meta {
+ if _, ok := f.pkgs[id]; ok {
+ continue
+ }
+ 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.
- for importID := range f.meta.children {
- go imp.getPkg(importID)
+ if len(f.pkgs) == 0 {
+ return nil, fmt.Errorf("loadParseTypeCheck: no packages found for %v", f.filename())
}
- // 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 @@
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 @@
},
}, 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 @@
// 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 @@
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 @@
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 @@
// 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 @@
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)
}
diff --git a/internal/lsp/cache/parse.go b/internal/lsp/cache/parse.go
index 84235e9..ed3a2d8 100644
--- a/internal/lsp/cache/parse.go
+++ b/internal/lsp/cache/parse.go
@@ -11,9 +11,6 @@
"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 @@
// 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) {
diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go
index 0ca79a7..9088053 100644
--- a/internal/lsp/cache/session.go
+++ b/internal/lsp/cache/session.go
@@ -182,8 +182,30 @@
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) {
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index 5592db4..76fade1 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -82,10 +82,6 @@
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 @@
// 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 @@
f.token = nil
// Remove the package and all of its reverse dependencies from the cache.
- if f.pkg != nil {
- f.view.remove(f.pkg.id, map[packageID]struct{}{})
+ for id, pkg := range f.pkgs {
+ if pkg != nil {
+ f.view.remove(id, map[packageID]struct{}{})
+ }
}
}
@@ -281,7 +278,7 @@
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 @@
v.mu.Lock()
defer v.mu.Unlock()
- if ctx.Err() != nil {
- return nil, ctx.Err()
- }
-
return v.getFile(uri)
}
diff --git a/internal/lsp/cmd/definition_test.go b/internal/lsp/cmd/definition_test.go
index 7b4a9a8..4890452 100644
--- a/internal/lsp/cmd/definition_test.go
+++ b/internal/lsp/cmd/definition_test.go
@@ -38,6 +38,8 @@
}
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 (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
diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go
index 0f7c289..faccbc6 100644
--- a/internal/lsp/diagnostics.go
+++ b/internal/lsp/diagnostics.go
@@ -14,10 +14,6 @@
)
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)
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index 084528b..263f17d 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -470,7 +470,6 @@
}
want[loc] = true
}
-
params := &protocol.ReferenceParams{
TextDocumentPositionParams: protocol.TextDocumentPositionParams{
TextDocument: protocol.TextDocumentIdentifier{URI: loc.URI},
diff --git a/internal/lsp/references.go b/internal/lsp/references.go
index 5f0d6bc..6be976a 100644
--- a/internal/lsp/references.go
+++ b/internal/lsp/references.go
@@ -27,7 +27,6 @@
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 @@
}
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 @@
if err != nil {
return nil, err
}
-
locations = append(locations, loc)
}
return locations, nil
diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go
index a478e18..81fbd99 100644
--- a/internal/lsp/source/completion.go
+++ b/internal/lsp/source/completion.go
@@ -242,6 +242,7 @@
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())
diff --git a/internal/lsp/source/identifier.go b/internal/lsp/source/identifier.go
index 8a9897a..981019f 100644
--- a/internal/lsp/source/identifier.go
+++ b/internal/lsp/source/identifier.go
@@ -27,6 +27,7 @@
}
decl declaration
+ pkg Package
ident *ast.Ident
wasEmbeddedField bool
qf types.Qualifier
@@ -67,22 +68,20 @@
}
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 @@
} 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 @@
// 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 {
- if !(imp.Pos() <= pos && pos < imp.End()) {
- continue
+ var imp *ast.ImportSpec
+ for _, spec := range fAST.Imports {
+ 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.
diff --git a/internal/lsp/source/references.go b/internal/lsp/source/references.go
index a9f0e1b..b5178e9 100644
--- a/internal/lsp/source/references.go
+++ b/internal/lsp/source/references.go
@@ -24,22 +24,18 @@
// 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)
- if pkg == nil || pkg.IsIllTyped() {
+ var references []*ReferenceInfo
+ if i.pkg == nil || i.pkg.IsIllTyped() {
return nil, fmt.Errorf("package for %s is ill typed", i.File.URI())
}
- pkgInfo := pkg.GetTypesInfo()
- if pkgInfo == nil {
- return nil, fmt.Errorf("package %s has no types info", pkg.PkgPath())
+ info := i.pkg.GetTypesInfo()
+ if info == nil {
+ 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 @@
isDeclaration: true,
})
}
-
- for ident, obj := range pkgInfo.Defs {
+ for ident, obj := range info.Defs {
if obj == nil || obj.Pos() != i.decl.obj.Pos() {
continue
}
@@ -64,8 +59,7 @@
isDeclaration: true,
})
}
-
- for ident, obj := range pkgInfo.Uses {
+ for ident, obj := range info.Uses {
if obj == nil || obj.Pos() != i.decl.obj.Pos() {
continue
}
@@ -76,6 +70,5 @@
obj: obj,
})
}
-
return references, nil
}
diff --git a/internal/lsp/source/rename.go b/internal/lsp/source/rename.go
index 5df3ba8..9b04506 100644
--- a/internal/lsp/source/rename.go
+++ b/internal/lsp/source/rename.go
@@ -42,11 +42,10 @@
}
// Do not rename identifiers declared in another package.
- pkg := i.File.GetPackage(ctx)
- 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())
}
- 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 @@
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 {
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index 60dd3f4..9b9a196 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -154,7 +154,7 @@
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 @@
// 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
diff --git a/internal/lsp/testdata/testy/testy.go b/internal/lsp/testdata/testy/testy.go
index a72cef8..f553807 100644
--- a/internal/lsp/testdata/testy/testy.go
+++ b/internal/lsp/testdata/testy/testy.go
@@ -1,3 +1,5 @@
package testy
-func a() {}
+func a() { //@item(funcA, "a()", "", "func")
+ //@complete("", funcA)
+}
diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go
index ba89f4f..5ecc71a 100644
--- a/internal/lsp/tests/tests.go
+++ b/internal/lsp/tests/tests.go
@@ -25,7 +25,7 @@
// 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
diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go
index b0ffc3f..b20b1a2 100644
--- a/internal/lsp/text_synchronization.go
+++ b/internal/lsp/text_synchronization.go
@@ -16,7 +16,7 @@
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))
}
