diff --git a/go/packages/packagestest/export.go b/go/packages/packagestest/export.go index 14424951..ea478a37 100644 --- a/go/packages/packagestest/export.go +++ b/go/packages/packagestest/export.go @@ -137,6 +137,7 @@ func Export(t testing.TB, exporter Exporter, modules []Module) *Exported { Env: append(os.Environ(), "GOPACKAGESDRIVER=off"), Overlay: make(map[string][]byte), Tests: true, + Mode: packages.LoadImports, }, Modules: modules, temp: temp, diff --git a/internal/lsp/cache/file.go b/internal/lsp/cache/file.go index 406fe3cb..cded8720 100644 --- a/internal/lsp/cache/file.go +++ b/internal/lsp/cache/file.go @@ -60,6 +60,9 @@ func (f *File) GetAST() (*ast.File, error) { if err := f.view.parse(f.URI); err != nil { return nil, err } + if f.ast == nil { + return nil, fmt.Errorf("failed to find or parse %v", f.URI) + } } return f.ast, nil } @@ -71,6 +74,9 @@ func (f *File) GetPackage() (*packages.Package, error) { if err := f.view.parse(f.URI); err != nil { return nil, err } + if f.pkg == nil { + return nil, fmt.Errorf("failed to find or parse %v", f.URI) + } } return f.pkg, nil } diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index a9f0ab1c..90393c80 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -7,7 +7,11 @@ package cache import ( "context" "fmt" + "go/ast" + "go/parser" + "go/scanner" "go/token" + "go/types" "log" "sync" @@ -105,35 +109,155 @@ func (v *View) parse(uri source.URI) error { } return err } - var foundPkg bool // true if we found a package for uri for _, pkg := range pkgs { - if len(pkg.Syntax) == 0 { - return fmt.Errorf("no syntax trees for %s", pkg.PkgPath) + imp := &importer{ + entries: make(map[string]*entry), + packages: make(map[string]*packages.Package), + v: v, + topLevelPkgPath: pkg.PkgPath, } - // Add every file in this package to our cache. - for _, fAST := range pkg.Syntax { - // TODO: If a file is in multiple packages, which package do we store? - if !fAST.Pos().IsValid() { - log.Printf("invalid position for AST %v", fAST.Name) - continue - } - fToken := v.Config.Fset.File(fAST.Pos()) - if fToken == nil { - log.Printf("no token.File for %v", fAST.Name) - continue - } - fURI := source.ToURI(fToken.Name()) - f := v.getFile(fURI) - f.token = fToken - f.ast = fAST - f.pkg = pkg - if fURI == uri { - foundPkg = true - } + if err := imp.addImports(pkg); err != nil { + return err } - } - if !foundPkg { - return fmt.Errorf("no package found for %v", uri) + imp.importPackage(pkg.PkgPath) } return nil } + +type importer struct { + mu sync.Mutex + entries map[string]*entry + packages map[string]*packages.Package + topLevelPkgPath string + + v *View +} + +type entry struct { + pkg *types.Package + err error + ready chan struct{} +} + +func (imp *importer) addImports(pkg *packages.Package) error { + imp.packages[pkg.PkgPath] = pkg + for _, i := range pkg.Imports { + if i.PkgPath == pkg.PkgPath { + return fmt.Errorf("import cycle: [%v]", pkg.PkgPath) + } + if err := imp.addImports(i); err != nil { + return err + } + } + return nil +} + +func (imp *importer) Import(path string) (*types.Package, error) { + if path == imp.topLevelPkgPath { + return nil, fmt.Errorf("import cycle: [%v]", path) + } + imp.mu.Lock() + e, ok := imp.entries[path] + if ok { + // cache hit + imp.mu.Unlock() + // wait for entry to become ready + <-e.ready + } else { + // cache miss + e = &entry{ready: make(chan struct{})} + imp.entries[path] = e + imp.mu.Unlock() + + // This goroutine becomes responsible for populating + // the entry and broadcasting its readiness. + e.pkg, e.err = imp.importPackage(path) + close(e.ready) + } + return e.pkg, e.err +} + +func (imp *importer) importPackage(pkgPath string) (*types.Package, error) { + imp.mu.Lock() + pkg, ok := imp.packages[pkgPath] + imp.mu.Unlock() + if !ok { + return nil, fmt.Errorf("no metadata for %v", pkgPath) + } + pkg.Fset = imp.v.Config.Fset + pkg.Syntax = make([]*ast.File, len(pkg.GoFiles)) + for i, filename := range pkg.GoFiles { + var src interface{} + overlay, ok := imp.v.Config.Overlay[filename] + if ok { + src = overlay + } + file, err := parser.ParseFile(imp.v.Config.Fset, filename, src, parser.AllErrors|parser.ParseComments) + if file == nil { + return nil, err + } + if err != nil { + switch err := err.(type) { + case *scanner.Error: + pkg.Errors = append(pkg.Errors, packages.Error{ + Pos: err.Pos.String(), + Msg: err.Msg, + Kind: packages.ParseError, + }) + case scanner.ErrorList: + // The first parser error is likely the root cause of the problem. + if err.Len() > 0 { + pkg.Errors = append(pkg.Errors, packages.Error{ + Pos: err[0].Pos.String(), + Msg: err[0].Msg, + Kind: packages.ParseError, + }) + } + } + } + pkg.Syntax[i] = file + } + cfg := &types.Config{ + Error: func(err error) { + if err, ok := err.(types.Error); ok { + pkg.Errors = append(pkg.Errors, packages.Error{ + Pos: imp.v.Config.Fset.Position(err.Pos).String(), + Msg: err.Msg, + Kind: packages.TypeError, + }) + } + }, + Importer: imp, + } + pkg.Types = types.NewPackage(pkg.PkgPath, pkg.Name) + pkg.TypesInfo = &types.Info{ + Types: make(map[ast.Expr]types.TypeAndValue), + Defs: make(map[*ast.Ident]types.Object), + Uses: make(map[*ast.Ident]types.Object), + Implicits: make(map[ast.Node]types.Object), + Selections: make(map[*ast.SelectorExpr]*types.Selection), + Scopes: make(map[ast.Node]*types.Scope), + } + check := types.NewChecker(cfg, imp.v.Config.Fset, pkg.Types, pkg.TypesInfo) + check.Files(pkg.Syntax) + + // Add every file in this package to our cache. + for _, file := range pkg.Syntax { + // TODO: If a file is in multiple packages, which package do we store? + if !file.Pos().IsValid() { + log.Printf("invalid position for file %v", file.Name) + continue + } + tok := imp.v.Config.Fset.File(file.Pos()) + if tok == nil { + log.Printf("no token.File for %v", file.Name) + continue + } + fURI := source.ToURI(tok.Name()) + f := imp.v.getFile(fURI) + f.token = tok + f.ast = file + f.pkg = pkg + } + return pkg.Types, nil +} diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index eb8b0f33..3425e383 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -14,7 +14,6 @@ import ( "strings" "testing" - "golang.org/x/tools/go/packages" "golang.org/x/tools/go/packages/packagestest" "golang.org/x/tools/internal/lsp/cache" "golang.org/x/tools/internal/lsp/diff" @@ -60,7 +59,6 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) { // Merge the exported.Config with the view.Config. cfg := *exported.Config cfg.Fset = token.NewFileSet() - cfg.Mode = packages.LoadSyntax s := &server{ view: cache.NewView(&cfg), diff --git a/internal/lsp/server.go b/internal/lsp/server.go index 2558da21..b70fec61 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -90,7 +90,7 @@ func (s *server) Initialize(ctx context.Context, params *protocol.InitializePara s.view = cache.NewView(&packages.Config{ Dir: rootPath, - Mode: packages.LoadSyntax, + Mode: packages.LoadImports, Fset: token.NewFileSet(), Tests: true, Overlay: make(map[string][]byte), @@ -170,9 +170,11 @@ func (s *server) DidChange(ctx context.Context, params *protocol.DidChangeTextDo return jsonrpc2.NewErrorf(jsonrpc2.CodeInternalError, "no content changes provided") } // We expect the full content of file, i.e. a single change with no range. - if change := params.ContentChanges[0]; change.RangeLength == 0 { - s.cacheAndDiagnose(ctx, params.TextDocument.URI, change.Text) + change := params.ContentChanges[0] + if change.RangeLength != 0 { + return jsonrpc2.NewErrorf(jsonrpc2.CodeInternalError, "unexpected change range provided") } + s.cacheAndDiagnose(ctx, params.TextDocument.URI, change.Text) return nil } diff --git a/internal/lsp/source/definition.go b/internal/lsp/source/definition.go index 27dc0d1a..bac247be 100644 --- a/internal/lsp/source/definition.go +++ b/internal/lsp/source/definition.go @@ -5,7 +5,6 @@ package source import ( - "bytes" "context" "fmt" "go/ast" @@ -142,33 +141,6 @@ func objToRange(ctx context.Context, v View, obj types.Object) (Range, error) { if !p.IsValid() { return Range{}, fmt.Errorf("invalid position for %v", obj.Name()) } - tok := v.FileSet().File(p) - pos := tok.Position(p) - if pos.Column == 1 { - // We do not have full position information because exportdata does not - // store the column. For now, we attempt to read the original source - // and find the identifier within the line. If we find it, we patch the - // column to match its offset. - // - // TODO: If we parse from source, we will never need this hack. - f, err := v.GetFile(ctx, ToURI(pos.Filename)) - if err != nil { - goto Return - } - src, err := f.Read() - if err != nil { - goto Return - } - tok, err := f.GetToken() - if err != nil { - goto Return - } - start := lineStart(tok, pos.Line) - offset := tok.Offset(start) - col := bytes.Index(src[offset:], []byte(obj.Name())) - p = tok.Pos(offset + col) - } -Return: return Range{ Start: p, End: p + token.Pos(identifierLen(obj.Name())),