From ce09bef8aae21fe8b969f02e519f1e28826ead5d Mon Sep 17 00:00:00 2001 From: Ian Cottrell Date: Tue, 14 May 2019 23:04:23 -0400 Subject: [PATCH] internal/lsp: reduce the api surface of the cache package The cache now exposes only one symbol, NewView This is preparing the cache for a re-write Change-Id: I411c2cd7a7edc2e7c774218c6786f9fd4fcc53cb Reviewed-on: https://go-review.googlesource.com/c/tools/+/176924 Reviewed-by: Rebecca Stambler --- internal/lsp/cache/check.go | 30 +++++------ internal/lsp/cache/file.go | 38 +++++++------- internal/lsp/cache/parse.go | 16 +++--- internal/lsp/cache/pkg.go | 26 ++++----- internal/lsp/cache/view.go | 84 ++++++++++++++++++------------ internal/lsp/diagnostics.go | 5 +- internal/lsp/general.go | 8 +-- internal/lsp/lsp_test.go | 4 +- internal/lsp/server.go | 5 +- internal/lsp/source/source_test.go | 2 +- internal/lsp/source/view.go | 5 ++ internal/lsp/workspace.go | 17 +++--- 12 files changed, 132 insertions(+), 108 deletions(-) diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go index fca2804fc6f..4a9ec802a3b 100644 --- a/internal/lsp/cache/check.go +++ b/internal/lsp/cache/check.go @@ -17,7 +17,7 @@ import ( "golang.org/x/tools/internal/span" ) -func (v *View) parse(ctx context.Context, f *File) ([]packages.Error, error) { +func (v *view) parse(ctx context.Context, f *file) ([]packages.Error, error) { v.mcache.mu.Lock() defer v.mcache.mu.Unlock() @@ -61,9 +61,9 @@ func (v *View) parse(ctx context.Context, f *File) ([]packages.Error, error) { return nil, nil } -func (v *View) checkMetadata(ctx context.Context, f *File) ([]packages.Error, error) { +func (v *view) checkMetadata(ctx context.Context, f *file) ([]packages.Error, error) { if v.reparseImports(ctx, f, f.filename) { - cfg := v.Config + cfg := v.config cfg.Mode = packages.LoadImports | packages.NeedTypesSizes pkgs, err := packages.Load(&cfg, fmt.Sprintf("file=%s", f.filename)) if len(pkgs) == 0 { @@ -92,13 +92,13 @@ func (v *View) checkMetadata(ctx context.Context, f *File) ([]packages.Error, er // reparseImports reparses a file's import declarations to determine if they // have changed. -func (v *View) reparseImports(ctx context.Context, f *File, filename string) bool { +func (v *view) reparseImports(ctx context.Context, f *file, filename string) bool { if f.meta == nil { return true } // Get file content in case we don't already have it? f.read(ctx) - parsed, _ := parser.ParseFile(v.Config.Fset, filename, f.content, parser.ImportsOnly) + parsed, _ := parser.ParseFile(v.config.Fset, filename, f.content, parser.ImportsOnly) if parsed == nil { return true } @@ -113,7 +113,7 @@ func (v *View) reparseImports(ctx context.Context, f *File, filename string) boo return false } -func (v *View) link(pkgPath string, pkg *packages.Package, parent *metadata) *metadata { +func (v *view) link(pkgPath string, pkg *packages.Package, parent *metadata) *metadata { m, ok := v.mcache.packages[pkgPath] if !ok { m = &metadata{ @@ -156,7 +156,7 @@ func (v *View) link(pkgPath string, pkg *packages.Package, parent *metadata) *me } type importer struct { - view *View + view *view // seen maintains the set of previously imported packages. // If we have seen a package that is already in this map, we have a circular import. @@ -193,7 +193,7 @@ func (imp *importer) Import(pkgPath string) (*types.Package, error) { return e.pkg.types, nil } -func (imp *importer) typeCheck(pkgPath string) (*Package, error) { +func (imp *importer) typeCheck(pkgPath string) (*pkg, error) { meta, ok := imp.view.mcache.packages[pkgPath] if !ok { return nil, fmt.Errorf("no metadata for %v", pkgPath) @@ -205,11 +205,11 @@ func (imp *importer) typeCheck(pkgPath string) (*Package, error) { } else { typ = types.NewPackage(meta.pkgPath, meta.name) } - pkg := &Package{ + pkg := &pkg{ id: meta.id, pkgPath: meta.pkgPath, files: meta.files, - imports: make(map[string]*Package), + imports: make(map[string]*pkg), types: typ, typesSizes: meta.typesSizes, typesInfo: &types.Info{ @@ -246,7 +246,7 @@ func (imp *importer) typeCheck(pkgPath string) (*Package, error) { ctx: imp.ctx, }, } - check := types.NewChecker(cfg, imp.view.Config.Fset, pkg.types, pkg.typesInfo) + check := types.NewChecker(cfg, imp.view.config.Fset, pkg.types, pkg.typesInfo) check.Files(pkg.syntax) // Add every file in this package to our cache. @@ -255,14 +255,14 @@ func (imp *importer) typeCheck(pkgPath string) (*Package, error) { return pkg, nil } -func (v *View) cachePackage(ctx context.Context, pkg *Package, meta *metadata) { +func (v *view) cachePackage(ctx context.Context, pkg *pkg, meta *metadata) { for _, file := range pkg.GetSyntax() { // TODO: If a file is in multiple packages, which package do we store? if !file.Pos().IsValid() { v.Logger().Errorf(ctx, "invalid position for file %v", file.Name) continue } - tok := v.Config.Fset.File(file.Pos()) + tok := v.config.Fset.File(file.Pos()) if tok == nil { v.Logger().Errorf(ctx, "no token.File for %v", file.Name) continue @@ -302,7 +302,7 @@ func (v *View) cachePackage(ctx context.Context, pkg *Package, meta *metadata) { } } -func (v *View) appendPkgError(pkg *Package, err error) { +func (v *view) appendPkgError(pkg *pkg, err error) { if err == nil { return } @@ -325,7 +325,7 @@ func (v *View) appendPkgError(pkg *Package, err error) { } case types.Error: errs = append(errs, packages.Error{ - Pos: v.Config.Fset.Position(err.Pos).String(), + Pos: v.config.Fset.Position(err.Pos).String(), Msg: err.Msg, Kind: packages.TypeError, }) diff --git a/internal/lsp/cache/file.go b/internal/lsp/cache/file.go index 0a9b303df96..1e69dfe4fe2 100644 --- a/internal/lsp/cache/file.go +++ b/internal/lsp/cache/file.go @@ -16,18 +16,18 @@ import ( "golang.org/x/tools/internal/span" ) -// File holds all the information we know about a file. -type File struct { +// file holds all the information we know about a file. +type file struct { uris []span.URI filename string basename string - view *View + view *view active bool content []byte ast *ast.File token *token.File - pkg *Package + pkg *pkg meta *metadata imports []*ast.ImportSpec } @@ -36,17 +36,17 @@ func basename(filename string) string { return strings.ToLower(filepath.Base(filename)) } -func (f *File) URI() span.URI { +func (f *file) URI() span.URI { return f.uris[0] } // View returns the view associated with the file. -func (f *File) View() source.View { +func (f *file) View() source.View { return f.view } // GetContent returns the contents of the file, reading it from file system if needed. -func (f *File) GetContent(ctx context.Context) []byte { +func (f *file) GetContent(ctx context.Context) []byte { f.view.mu.Lock() defer f.view.mu.Unlock() @@ -57,11 +57,11 @@ func (f *File) GetContent(ctx context.Context) []byte { return f.content } -func (f *File) GetFileSet(ctx context.Context) *token.FileSet { - return f.view.Config.Fset +func (f *file) GetFileSet(ctx context.Context) *token.FileSet { + return f.view.config.Fset } -func (f *File) GetToken(ctx context.Context) *token.File { +func (f *file) GetToken(ctx context.Context) *token.File { f.view.mu.Lock() defer f.view.mu.Unlock() @@ -73,7 +73,7 @@ func (f *File) GetToken(ctx context.Context) *token.File { return f.token } -func (f *File) GetAST(ctx context.Context) *ast.File { +func (f *file) GetAST(ctx context.Context) *ast.File { f.view.mu.Lock() defer f.view.mu.Unlock() @@ -85,7 +85,7 @@ func (f *File) GetAST(ctx context.Context) *ast.File { return f.ast } -func (f *File) GetPackage(ctx context.Context) source.Package { +func (f *file) GetPackage(ctx context.Context) source.Package { f.view.mu.Lock() defer f.view.mu.Unlock() @@ -93,7 +93,7 @@ func (f *File) GetPackage(ctx context.Context) source.Package { if errs, err := f.view.parse(ctx, f); err != nil { // Create diagnostics for errors if we are able to. if len(errs) > 0 { - return &Package{errors: errs} + return &pkg{errors: errs} } return nil } @@ -103,7 +103,7 @@ func (f *File) GetPackage(ctx context.Context) source.Package { // read is the internal part of GetContent. It assumes that the caller is // holding the mutex of the file's view. -func (f *File) read(ctx context.Context) { +func (f *file) read(ctx context.Context) { if f.content != nil { if len(f.view.contentChanges) == 0 { return @@ -118,7 +118,7 @@ func (f *File) read(ctx context.Context) { } } // We might have the content saved in an overlay. - if content, ok := f.view.Config.Overlay[f.filename]; ok { + if content, ok := f.view.config.Overlay[f.filename]; ok { f.content = content return } @@ -132,11 +132,11 @@ func (f *File) read(ctx context.Context) { } // isPopulated returns true if all of the computed fields of the file are set. -func (f *File) isPopulated() bool { +func (f *file) isPopulated() bool { return f.ast != nil && f.token != nil && f.pkg != nil && f.meta != nil && f.imports != nil } -func (f *File) GetActiveReverseDeps(ctx context.Context) []source.File { +func (f *file) GetActiveReverseDeps(ctx context.Context) []source.File { pkg := f.GetPackage(ctx) if pkg == nil { return nil @@ -149,7 +149,7 @@ func (f *File) GetActiveReverseDeps(ctx context.Context) []source.File { defer f.view.mcache.mu.Unlock() seen := make(map[string]struct{}) // visited packages - results := make(map[*File]struct{}) + results := make(map[*file]struct{}) f.view.reverseDeps(ctx, seen, results, pkg.PkgPath()) files := make([]source.File, 0, len(results)) @@ -166,7 +166,7 @@ func (f *File) GetActiveReverseDeps(ctx context.Context) []source.File { return files } -func (v *View) reverseDeps(ctx context.Context, seen map[string]struct{}, results map[*File]struct{}, pkgPath string) { +func (v *view) reverseDeps(ctx context.Context, seen map[string]struct{}, results map[*file]struct{}, pkgPath string) { if _, ok := seen[pkgPath]; ok { return } diff --git a/internal/lsp/cache/parse.go b/internal/lsp/cache/parse.go index 29cd001eca6..0cb67e074e7 100644 --- a/internal/lsp/cache/parse.go +++ b/internal/lsp/cache/parse.go @@ -37,9 +37,9 @@ func (imp *importer) parseFiles(filenames []string) ([]*ast.File, []error) { parsed := make([]*ast.File, n) errors := make([]error, n) for i, filename := range filenames { - if imp.view.Config.Context.Err() != nil { + if imp.view.config.Context.Err() != nil { parsed[i] = nil - errors[i] = imp.view.Config.Context.Err() + errors[i] = imp.view.config.Context.Err() continue } @@ -63,7 +63,7 @@ func (imp *importer) parseFiles(filenames []string) ([]*ast.File, []error) { // We don't have a cached AST for this file. var src []byte // Check for an available overlay. - for f, contents := range imp.view.Config.Overlay { + for f, contents := range imp.view.config.Overlay { if sameFile(f, filename) { src = contents } @@ -77,11 +77,11 @@ func (imp *importer) parseFiles(filenames []string) ([]*ast.File, []error) { parsed[i], errors[i] = nil, err } else { // ParseFile may return both an AST and an error. - parsed[i], errors[i] = imp.view.Config.ParseFile(imp.view.Config.Fset, filename, src) + parsed[i], errors[i] = imp.view.config.ParseFile(imp.view.config.Fset, filename, src) // Fix any badly parsed parts of the AST. if file := parsed[i]; file != nil { - tok := imp.view.Config.Fset.File(file.Pos()) + tok := imp.view.config.Fset.File(file.Pos()) imp.view.fix(imp.ctx, parsed[i], tok, src) } } @@ -141,7 +141,7 @@ func sameFile(x, y string) bool { // fix inspects and potentially modifies any *ast.BadStmts or *ast.BadExprs in the AST. // We attempt to modify the AST such that we can type-check it more effectively. -func (v *View) fix(ctx context.Context, file *ast.File, tok *token.File, src []byte) { +func (v *view) fix(ctx context.Context, file *ast.File, tok *token.File, src []byte) { var parent ast.Node ast.Inspect(file, func(n ast.Node) bool { if n == nil { @@ -167,7 +167,7 @@ func (v *View) fix(ctx context.Context, file *ast.File, tok *token.File, src []b // this statement entirely, and we can't use the type information when completing. // Here, we try to generate a fake *ast.DeferStmt or *ast.GoStmt to put into the AST, // instead of the *ast.BadStmt. -func (v *View) parseDeferOrGoStmt(bad *ast.BadStmt, parent ast.Node, tok *token.File, src []byte) error { +func (v *view) parseDeferOrGoStmt(bad *ast.BadStmt, parent ast.Node, tok *token.File, src []byte) error { // Check if we have a bad statement containing either a "go" or "defer". s := &scanner.Scanner{} s.Init(tok, src, nil, 0) @@ -260,7 +260,7 @@ FindTo: // offsetPositions applies an offset to the positions in an ast.Node. // TODO(rstambler): Add more cases here as they become necessary. -func (v *View) offsetPositions(expr ast.Expr, offset token.Pos) { +func (v *view) offsetPositions(expr ast.Expr, offset token.Pos) { ast.Inspect(expr, func(n ast.Node) bool { switch n := n.(type) { case *ast.Ident: diff --git a/internal/lsp/cache/pkg.go b/internal/lsp/cache/pkg.go index 440b655e70e..6fc66228a84 100644 --- a/internal/lsp/cache/pkg.go +++ b/internal/lsp/cache/pkg.go @@ -16,13 +16,13 @@ import ( "golang.org/x/tools/internal/lsp/source" ) -// Package contains the type information needed by the source package. -type Package struct { +// pkg contains the type information needed by the source package. +type pkg struct { id, pkgPath string files []string syntax []*ast.File errors []packages.Error - imports map[string]*Package + imports map[string]*pkg types *types.Package typesInfo *types.Info typesSizes types.Sizes @@ -41,7 +41,7 @@ type analysisEntry struct { *source.Action } -func (pkg *Package) GetActionGraph(ctx context.Context, a *analysis.Analyzer) (*source.Action, error) { +func (pkg *pkg) GetActionGraph(ctx context.Context, a *analysis.Analyzer) (*source.Action, error) { if ctx.Err() != nil { return nil, ctx.Err() } @@ -128,39 +128,39 @@ func (pkg *Package) GetActionGraph(ctx context.Context, a *analysis.Analyzer) (* return e.Action, nil } -func (pkg *Package) PkgPath() string { +func (pkg *pkg) PkgPath() string { return pkg.pkgPath } -func (pkg *Package) GetFilenames() []string { +func (pkg *pkg) GetFilenames() []string { return pkg.files } -func (pkg *Package) GetSyntax() []*ast.File { +func (pkg *pkg) GetSyntax() []*ast.File { return pkg.syntax } -func (pkg *Package) GetErrors() []packages.Error { +func (pkg *pkg) GetErrors() []packages.Error { return pkg.errors } -func (pkg *Package) GetTypes() *types.Package { +func (pkg *pkg) GetTypes() *types.Package { return pkg.types } -func (pkg *Package) GetTypesInfo() *types.Info { +func (pkg *pkg) GetTypesInfo() *types.Info { return pkg.typesInfo } -func (pkg *Package) GetTypesSizes() types.Sizes { +func (pkg *pkg) GetTypesSizes() types.Sizes { return pkg.typesSizes } -func (pkg *Package) IsIllTyped() bool { +func (pkg *pkg) IsIllTyped() bool { return pkg.types == nil && pkg.typesInfo == nil } -func (pkg *Package) GetImport(pkgPath string) source.Package { +func (pkg *pkg) GetImport(pkgPath string) source.Package { imported := pkg.imports[pkgPath] // Be careful not to return a nil pointer because that still satisfies the // interface. diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index 6ef8ca82240..4329821ee92 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -20,7 +20,7 @@ import ( "golang.org/x/tools/internal/span" ) -type View struct { +type view struct { // mu protects all mutable state of the view. mu sync.Mutex @@ -40,19 +40,19 @@ type View struct { log xlog.Logger // Name is the user visible name of this view. - Name string + name string // Folder is the root of this view. - Folder span.URI + folder span.URI // Config is the configuration used for the view's interaction with the // go/packages API. It is shared across all views. - Config packages.Config + config packages.Config // keep track of files by uri and by basename, a single file may be mapped // to multiple uris, and the same basename may map to multiple files - filesByURI map[span.URI]*File - filesByBase map[string][]*File + filesByURI map[span.URI]*file + filesByBase map[string][]*file // contentChanges saves the content changes for a given state of the view. // When type information is requested by the view, all of the dirty changes @@ -89,24 +89,24 @@ type packageCache struct { } type entry struct { - pkg *Package + pkg *pkg err error ready chan struct{} // closed to broadcast ready condition } -func NewView(ctx context.Context, log xlog.Logger, name string, folder span.URI, config *packages.Config) *View { +func NewView(ctx context.Context, log xlog.Logger, name string, folder span.URI, config *packages.Config) source.View { backgroundCtx, cancel := context.WithCancel(ctx) - v := &View{ + v := &view{ baseCtx: ctx, backgroundCtx: backgroundCtx, builtinPkg: builtinPkg(*config), cancel: cancel, log: log, - Config: *config, - Name: name, - Folder: folder, - filesByURI: make(map[span.URI]*File), - filesByBase: make(map[string][]*File), + config: *config, + name: name, + folder: folder, + filesByURI: make(map[span.URI]*file), + filesByBase: make(map[string][]*file), contentChanges: make(map[span.URI]func()), mcache: &metadataCache{ packages: make(map[string]*metadata), @@ -118,14 +118,34 @@ func NewView(ctx context.Context, log xlog.Logger, name string, folder span.URI, return v } -func (v *View) BackgroundContext() context.Context { +// Name returns the user visible name of this view. +func (v *view) Name() string { + return v.name +} + +// Folder returns the root of this view. +func (v *view) Folder() span.URI { + return v.folder +} + +// Config returns the configuration used for the view's interaction with the +// go/packages API. It is shared across all views. +func (v *view) Config() packages.Config { + return v.config +} + +func (v *view) SetEnv(env []string) { + v.config.Env = env +} + +func (v *view) BackgroundContext() context.Context { v.mu.Lock() defer v.mu.Unlock() return v.backgroundCtx } -func (v *View) BuiltinPackage() *ast.Package { +func (v *view) BuiltinPackage() *ast.Package { return v.builtinPkg } @@ -151,12 +171,12 @@ func builtinPkg(cfg packages.Config) *ast.Package { return bpkg } -func (v *View) FileSet() *token.FileSet { - return v.Config.Fset +func (v *view) FileSet() *token.FileSet { + return v.config.Fset } // SetContent sets the overlay contents for a file. -func (v *View) SetContent(ctx context.Context, uri span.URI, content []byte) error { +func (v *view) SetContent(ctx context.Context, uri span.URI, content []byte) error { v.mu.Lock() defer v.mu.Unlock() @@ -175,7 +195,7 @@ func (v *View) SetContent(ctx context.Context, uri span.URI, content []byte) err // applyContentChanges applies all of the changed content stored in the view. // It is assumed that the caller has locked both the view's and the mcache's // mutexes. -func (v *View) applyContentChanges(ctx context.Context) error { +func (v *view) applyContentChanges(ctx context.Context) error { if ctx.Err() != nil { return ctx.Err() } @@ -193,7 +213,7 @@ func (v *View) applyContentChanges(ctx context.Context) error { // setContent applies a content update for a given file. It assumes that the // caller is holding the view's mutex. -func (v *View) applyContentChange(uri span.URI, content []byte) { +func (v *view) applyContentChange(uri span.URI, content []byte) { f, err := v.getFile(uri) if err != nil { return @@ -213,19 +233,19 @@ func (v *View) applyContentChange(uri span.URI, content []byte) { case f.active && content == nil: // The file was active, so we need to forget its content. f.active = false - delete(f.view.Config.Overlay, f.filename) + delete(f.view.config.Overlay, f.filename) f.content = nil case content != nil: // This is an active overlay, so we update the map. f.active = true - f.view.Config.Overlay[f.filename] = f.content + f.view.config.Overlay[f.filename] = f.content } } // remove invalidates a package and its reverse dependencies in the view's // package cache. It is assumed that the caller has locked both the mutexes // of both the mcache and the pcache. -func (v *View) remove(pkgPath string, seen map[string]struct{}) { +func (v *view) remove(pkgPath string, seen map[string]struct{}) { if _, ok := seen[pkgPath]; ok { return } @@ -248,7 +268,7 @@ func (v *View) remove(pkgPath string, seen map[string]struct{}) { } // FindFile returns the file if the given URI is already a part of the view. -func (v *View) FindFile(ctx context.Context, uri span.URI) *File { +func (v *view) FindFile(ctx context.Context, uri span.URI) *file { v.mu.Lock() defer v.mu.Unlock() f, err := v.findFile(uri) @@ -260,7 +280,7 @@ func (v *View) FindFile(ctx context.Context, uri span.URI) *File { // GetFile returns a File for the given URI. It will always succeed because it // adds the file to the managed set if needed. -func (v *View) GetFile(ctx context.Context, uri span.URI) (source.File, error) { +func (v *view) GetFile(ctx context.Context, uri span.URI) (source.File, error) { v.mu.Lock() defer v.mu.Unlock() @@ -272,7 +292,7 @@ func (v *View) GetFile(ctx context.Context, uri span.URI) (source.File, error) { } // getFile is the unlocked internal implementation of GetFile. -func (v *View) getFile(uri span.URI) (*File, error) { +func (v *view) getFile(uri span.URI) (*file, error) { filename, err := uri.Filename() if err != nil { return nil, err @@ -285,7 +305,7 @@ func (v *View) getFile(uri span.URI) (*File, error) { } else if f != nil { return f, nil } - f := &File{ + f := &file{ view: v, filename: filename, } @@ -295,7 +315,7 @@ func (v *View) getFile(uri span.URI) (*File, error) { // isIgnored checks if the given filename is a file we ignore. // As of right now, we only ignore files in the "builtin" package. -func (v *View) isIgnored(filename string) bool { +func (v *view) isIgnored(filename string) bool { bpkg := v.BuiltinPackage() if bpkg != nil { for builtinFilename := range bpkg.Files { @@ -311,7 +331,7 @@ func (v *View) isIgnored(filename string) bool { // // An error is only returned for an irreparable failure, for example, if the // filename in question does not exist. -func (v *View) findFile(uri span.URI) (*File, error) { +func (v *view) findFile(uri span.URI) (*file, error) { if f := v.filesByURI[uri]; f != nil { // a perfect match return f, nil @@ -344,7 +364,7 @@ func (v *View) findFile(uri span.URI) (*File, error) { return nil, nil } -func (v *View) mapFile(uri span.URI, f *File) { +func (v *view) mapFile(uri span.URI, f *file) { v.filesByURI[uri] = f f.uris = append(f.uris, uri) if f.basename == "" { @@ -353,6 +373,6 @@ func (v *View) mapFile(uri span.URI, f *File) { } } -func (v *View) Logger() xlog.Logger { +func (v *view) Logger() xlog.Logger { return v.log } diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index 2af02dab617..0312d4e09e4 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -7,13 +7,12 @@ package lsp import ( "context" - "golang.org/x/tools/internal/lsp/cache" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/span" ) -func (s *Server) Diagnostics(ctx context.Context, view *cache.View, uri span.URI) { +func (s *Server) Diagnostics(ctx context.Context, view source.View, uri span.URI) { if ctx.Err() != nil { s.log.Errorf(ctx, "canceling diagnostics for %s: %v", uri, ctx.Err()) return @@ -48,7 +47,7 @@ func (s *Server) Diagnostics(ctx context.Context, view *cache.View, uri span.URI } } -func (s *Server) publishDiagnostics(ctx context.Context, view *cache.View, uri span.URI, diagnostics []source.Diagnostic) error { +func (s *Server) publishDiagnostics(ctx context.Context, view source.View, uri span.URI, diagnostics []source.Diagnostic) error { protocolDiagnostics, err := toProtocolDiagnostics(ctx, view, diagnostics) if err != nil { return err diff --git a/internal/lsp/general.go b/internal/lsp/general.go index 09f0a4ec42d..dbc37031c2a 100644 --- a/internal/lsp/general.go +++ b/internal/lsp/general.go @@ -13,8 +13,8 @@ import ( "strings" "golang.org/x/tools/internal/jsonrpc2" - "golang.org/x/tools/internal/lsp/cache" "golang.org/x/tools/internal/lsp/protocol" + "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/span" ) @@ -128,7 +128,7 @@ func (s *Server) initialized(ctx context.Context, params *protocol.InitializedPa for _, view := range s.views { config, err := s.client.Configuration(ctx, &protocol.ConfigurationParams{ Items: []protocol.ConfigurationItem{{ - ScopeURI: protocol.NewURI(view.Folder), + ScopeURI: protocol.NewURI(view.Folder()), Section: "gopls", }}, }) @@ -146,7 +146,7 @@ func (s *Server) initialized(ctx context.Context, params *protocol.InitializedPa return nil } -func (s *Server) processConfig(view *cache.View, config interface{}) error { +func (s *Server) processConfig(view source.View, config interface{}) error { // TODO: We should probably store and process more of the config. if config == nil { return nil // ignore error if you don't have a config @@ -162,7 +162,7 @@ func (s *Server) processConfig(view *cache.View, config interface{}) error { return fmt.Errorf("invalid config gopls.env type %T", env) } for k, v := range menv { - view.Config.Env = applyEnv(view.Config.Env, k, v) + view.SetEnv(applyEnv(view.Config().Env, k, v)) } } // Check if placeholders are enabled. diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 7c4943b58e9..dd3c9c19bce 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -42,8 +42,8 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) { log := xlog.New(xlog.StdSink{}) r := &runner{ server: &Server{ - views: []*cache.View{cache.NewView(ctx, log, "lsp_test", span.FileURI(data.Config.Dir), &data.Config)}, - viewMap: make(map[span.URI]*cache.View), + views: []source.View{cache.NewView(ctx, log, "lsp_test", span.FileURI(data.Config.Dir), &data.Config)}, + viewMap: make(map[span.URI]source.View), undelivered: make(map[span.URI][]source.Diagnostic), log: log, }, diff --git a/internal/lsp/server.go b/internal/lsp/server.go index 047be4e733e..3ccd7f550a3 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -11,7 +11,6 @@ import ( "sync" "golang.org/x/tools/internal/jsonrpc2" - "golang.org/x/tools/internal/lsp/cache" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/lsp/xlog" @@ -83,8 +82,8 @@ type Server struct { textDocumentSyncKind protocol.TextDocumentSyncKind viewMu sync.Mutex - views []*cache.View - viewMap map[span.URI]*cache.View + views []source.View + viewMap map[span.URI]source.View // undelivered is a cache of any diagnostics that the server // failed to deliver for some reason. diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 708743090ed..577d3254f58 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -27,7 +27,7 @@ func TestSource(t *testing.T) { } type runner struct { - view *cache.View + view source.View data *tests.Data } diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 13e8d741529..44796a7dd11 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -22,11 +22,16 @@ import ( // package. The view provides access to files and their contents, so the source // package does not directly access the file system. type View interface { + Name() string + Folder() span.URI Logger() xlog.Logger FileSet() *token.FileSet BuiltinPackage() *ast.Package GetFile(ctx context.Context, uri span.URI) (File, error) SetContent(ctx context.Context, uri span.URI, content []byte) error + BackgroundContext() context.Context + Config() packages.Config + SetEnv([]string) } // File represents a Go source file that has been type-checked. It is the input diff --git a/internal/lsp/workspace.go b/internal/lsp/workspace.go index 1b90473c6db..44674b3fe45 100644 --- a/internal/lsp/workspace.go +++ b/internal/lsp/workspace.go @@ -16,6 +16,7 @@ import ( "golang.org/x/tools/go/packages" "golang.org/x/tools/internal/lsp/cache" "golang.org/x/tools/internal/lsp/protocol" + "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/span" ) @@ -59,7 +60,7 @@ func (s *Server) addView(ctx context.Context, name string, uri span.URI) error { Tests: true, })) // we always need to drop the view map - s.viewMap = make(map[span.URI]*cache.View) + s.viewMap = make(map[span.URI]source.View) return nil } @@ -67,10 +68,10 @@ func (s *Server) removeView(ctx context.Context, name string, uri span.URI) erro s.viewMu.Lock() defer s.viewMu.Unlock() // we always need to drop the view map - s.viewMap = make(map[span.URI]*cache.View) + s.viewMap = make(map[span.URI]source.View) s.log.Infof(ctx, "drop view %v as %v", name, uri) for i, view := range s.views { - if view.Name == name { + if view.Name() == name { // delete this view... we don't care about order but we do want to make // sure we can garbage collect the view s.views[i] = s.views[len(s.views)-1] @@ -85,7 +86,7 @@ func (s *Server) removeView(ctx context.Context, name string, uri span.URI) erro // findView returns the view corresponding to the given URI. // If the file is not already associated with a view, pick one using some heuristics. -func (s *Server) findView(ctx context.Context, uri span.URI) *cache.View { +func (s *Server) findView(ctx context.Context, uri span.URI) source.View { s.viewMu.Lock() defer s.viewMu.Unlock() @@ -102,14 +103,14 @@ func (s *Server) findView(ctx context.Context, uri span.URI) *cache.View { // bestView finds the best view to associate a given URI with. // viewMu must be held when calling this method. -func (s *Server) bestView(ctx context.Context, uri span.URI) *cache.View { +func (s *Server) bestView(ctx context.Context, uri span.URI) source.View { // we need to find the best view for this file - var longest *cache.View + var longest source.View for _, view := range s.views { - if longest != nil && len(longest.Folder) > len(view.Folder) { + if longest != nil && len(longest.Folder()) > len(view.Folder()) { continue } - if strings.HasPrefix(string(uri), string(view.Folder)) { + if strings.HasPrefix(string(uri), string(view.Folder())) { longest = view } }