diff options
-rw-r--r-- | internal/lsp/cache/check.go | 24 | ||||
-rw-r--r-- | internal/lsp/cache/external.go | 3 | ||||
-rw-r--r-- | internal/lsp/cache/gofile.go | 63 | ||||
-rw-r--r-- | internal/lsp/cache/load.go | 107 | ||||
-rw-r--r-- | internal/lsp/cache/parse.go | 29 | ||||
-rw-r--r-- | internal/lsp/cache/session.go | 24 | ||||
-rw-r--r-- | internal/lsp/cache/view.go | 19 | ||||
-rw-r--r-- | internal/lsp/cmd/definition_test.go | 4 | ||||
-rw-r--r-- | internal/lsp/diagnostics.go | 4 | ||||
-rw-r--r-- | internal/lsp/lsp_test.go | 1 | ||||
-rw-r--r-- | internal/lsp/references.go | 5 | ||||
-rw-r--r-- | internal/lsp/source/completion.go | 1 | ||||
-rw-r--r-- | internal/lsp/source/identifier.go | 86 | ||||
-rw-r--r-- | internal/lsp/source/references.go | 23 | ||||
-rw-r--r-- | internal/lsp/source/rename.go | 9 | ||||
-rw-r--r-- | internal/lsp/source/view.go | 5 | ||||
-rw-r--r-- | internal/lsp/testdata/testy/testy.go | 4 | ||||
-rw-r--r-- | internal/lsp/tests/tests.go | 2 | ||||
-rw-r--r-- | internal/lsp/text_synchronization.go | 2 |
19 files changed, 242 insertions, 173 deletions
diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go index 408f12bb0..fab03645d 100644 --- a/internal/lsp/cache/check.go +++ b/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) - 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 @@ 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) { - 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 @@ 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 } } diff --git a/internal/lsp/cache/external.go b/internal/lsp/cache/external.go index fafcd9b13..043d20ac5 100644 --- a/internal/lsp/cache/external.go +++ b/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{ diff --git a/internal/lsp/cache/gofile.go b/internal/lsp/cache/gofile.go index ff2112a22..36ec12f9a 100644 --- a/internal/lsp/cache/gofile.go +++ b/internal/lsp/cache/gofile.go @@ -19,8 +19,17 @@ type goFile struct { 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 @@ 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) diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go index 6937917a0..1368d56aa 100644 --- a/internal/lsp/cache/load.go +++ b/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{} - 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, - } - // Start prefetching direct imports. - for importID := range f.meta.children { - 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) + + 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) + } } - // 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()) + if len(f.pkgs) == 0 { + return nil, fmt.Errorf("loadParseTypeCheck: no packages 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) } diff --git a/internal/lsp/cache/parse.go b/internal/lsp/cache/parse.go index 84235e92c..ed3a2d8b7 100644 --- a/internal/lsp/cache/parse.go +++ b/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,30 +94,10 @@ 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). } } - 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) - } - } + if ast == nil { + return nil, err } - return false + return ast, err } // trimAST clears any part of the AST not relevant to type checking diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index 0ca79a701..908805395 100644 --- a/internal/lsp/cache/session.go +++ b/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) { diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index 5592db462..76fade1aa 100644 --- a/internal/lsp/cache/view.go +++ b/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 { - 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 @@ 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) } diff --git a/internal/lsp/cmd/definition_test.go b/internal/lsp/cmd/definition_test.go index 7b4a9a848..489045272 100644 --- a/internal/lsp/cmd/definition_test.go +++ b/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 diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index 0f7c2893c..faccbc664 100644 --- a/internal/lsp/diagnostics.go +++ b/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) diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 084528ba0..263f17d31 100644 --- a/internal/lsp/lsp_test.go +++ b/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}, diff --git a/internal/lsp/references.go b/internal/lsp/references.go index 5f0d6bce2..6be976a77 100644 --- a/internal/lsp/references.go +++ b/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 diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index a478e188d..81fbd991a 100644 --- a/internal/lsp/source/completion.go +++ b/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()) diff --git a/internal/lsp/source/identifier.go b/internal/lsp/source/identifier.go index 8a9897a42..981019f63 100644 --- a/internal/lsp/source/identifier.go +++ b/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 { - if !(imp.Pos() <= pos && pos < imp.End()) { - continue - } - importPath, err := strconv.Unquote(imp.Path.Value) - if err != nil { - return nil, fmt.Errorf("import path not quoted: %s (%v)", imp.Path.Value, err) + var imp *ast.ImportSpec + for _, spec := range fAST.Imports { + if spec.Pos() <= pos && pos < spec.End() { + imp = spec } - 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) + } + 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 } - result.decl.rng = span.NewRange(f.FileSet(), dest.Name.Pos(), dest.Name.End()) - return result, nil } - return nil, nil + 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 a9f0e1b2b..b5178e94b 100644 --- a/internal/lsp/source/references.go +++ b/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) - 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 @@ func (i *IdentifierInfo) References(ctx context.Context) ([]*ReferenceInfo, erro 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 @@ func (i *IdentifierInfo) References(ctx context.Context) ([]*ReferenceInfo, erro 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 @@ func (i *IdentifierInfo) References(ctx context.Context) ([]*ReferenceInfo, erro obj: obj, }) } - return references, nil } diff --git a/internal/lsp/source/rename.go b/internal/lsp/source/rename.go index 5df3ba8ca..9b045064e 100644 --- a/internal/lsp/source/rename.go +++ b/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 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 @@ 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 { diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 60dd3f4f6..9b9a1966e 100644 --- a/internal/lsp/source/view.go +++ b/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 diff --git a/internal/lsp/testdata/testy/testy.go b/internal/lsp/testdata/testy/testy.go index a72cef898..f55380767 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 ba89f4f6d..5ecc71a24 100644 --- a/internal/lsp/tests/tests.go +++ b/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 diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go index b0ffc3fe2..b20b1a2ca 100644 --- a/internal/lsp/text_synchronization.go +++ b/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)) } |