diff --git a/internal/lsp/cache/file.go b/internal/lsp/cache/file.go index b0dea34a..406fe3cb 100644 --- a/internal/lsp/cache/file.go +++ b/internal/lsp/cache/file.go @@ -25,35 +25,6 @@ type File struct { pkg *packages.Package } -// SetContent sets the overlay contents for a file. -// Setting it to nil will revert it to the on disk contents, and remove it -// from the active set. -func (f *File) SetContent(content []byte) { - f.view.mu.Lock() - defer f.view.mu.Unlock() - f.content = content - // the ast and token fields are invalid - f.ast = nil - f.token = nil - f.pkg = nil - // and we might need to update the overlay - switch { - case f.active && content == nil: - // we were active, and want to forget the content - f.active = false - if filename, err := f.URI.Filename(); err == nil { - delete(f.view.Config.Overlay, filename) - } - f.content = nil - case content != nil: - // an active overlay, update the map - f.active = true - if filename, err := f.URI.Filename(); err == nil { - f.view.Config.Overlay[filename] = f.content - } - } -} - // Read returns the contents of the file, reading it from file system if needed. func (f *File) Read() ([]byte, error) { f.view.mu.Lock() @@ -62,9 +33,6 @@ func (f *File) Read() ([]byte, error) { } func (f *File) GetFileSet() (*token.FileSet, error) { - if f.view.Config == nil { - return nil, fmt.Errorf("no config for file view") - } if f.view.Config.Fset == nil { return nil, fmt.Errorf("no fileset for file view config") } diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index f3286f5f..812fa1da 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -5,6 +5,7 @@ package cache import ( + "context" "fmt" "go/token" "sync" @@ -16,31 +17,66 @@ import ( type View struct { mu sync.Mutex // protects all mutable state of the view - Config *packages.Config + Config packages.Config files map[source.URI]*File } -func NewView(rootPath string) *View { +// NewView creates a new View, given a root path and go/packages configuration. +// If config is nil, one is created with the directory set to the rootPath. +func NewView(config *packages.Config) *View { return &View{ - Config: &packages.Config{ - Dir: rootPath, - Mode: packages.LoadSyntax, - Fset: token.NewFileSet(), - Tests: true, - Overlay: make(map[string][]byte), - }, - files: make(map[source.URI]*File), + Config: *config, + files: make(map[source.URI]*File), } } +func (v *View) FileSet() *token.FileSet { + return v.Config.Fset +} + +// SetContent sets the overlay contents for a file. A nil content value will +// remove the file from the active set and revert it to its on-disk contents. +func (v *View) SetContent(ctx context.Context, uri source.URI, content []byte) (source.View, error) { + v.mu.Lock() + defer v.mu.Unlock() + + f := v.getFile(uri) + f.content = content + + // Resetting the contents invalidates the ast, token, and pkg fields. + f.ast = nil + f.token = nil + f.pkg = nil + + // We might need to update the overlay. + switch { + case f.active && content == nil: + // The file was active, so we need to forget its content. + f.active = false + if filename, err := f.URI.Filename(); err == nil { + delete(f.view.Config.Overlay, filename) + } + f.content = nil + case content != nil: + // This is an active overlay, so we update the map. + f.active = true + if filename, err := f.URI.Filename(); err == nil { + f.view.Config.Overlay[filename] = f.content + } + } + + // TODO(rstambler): We should really return a new, updated view. + return v, nil +} + // 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(uri source.URI) source.File { +func (v *View) GetFile(ctx context.Context, uri source.URI) (source.File, error) { v.mu.Lock() f := v.getFile(uri) v.mu.Unlock() - return f + return f, nil } // getFile is the unlocked internal implementation of GetFile. @@ -51,7 +87,7 @@ func (v *View) getFile(uri source.URI) *File { URI: uri, view: v, } - v.files[f.URI] = f + v.files[uri] = f } return f } @@ -61,7 +97,7 @@ func (v *View) parse(uri source.URI) error { if err != nil { return err } - pkgs, err := packages.Load(v.Config, fmt.Sprintf("file=%s", path)) + pkgs, err := packages.Load(&v.Config, fmt.Sprintf("file=%s", path)) if len(pkgs) == 0 { if err == nil { err = fmt.Errorf("no packages found for %s", path) @@ -69,9 +105,9 @@ func (v *View) parse(uri source.URI) error { return err } for _, pkg := range pkgs { - // add everything we find to the files cache + // Add every file in this package to our cache. for _, fAST := range pkg.Syntax { - // if a file was in multiple packages, which token/ast/pkg do we store + // TODO: If a file is in multiple packages, which package do we store? fToken := v.Config.Fset.File(fAST.Pos()) fURI := source.ToURI(fToken.Name()) f := v.getFile(fURI) diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index dcc2cafb..02102867 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -8,18 +8,17 @@ import ( "context" "sort" - "golang.org/x/tools/internal/lsp/cache" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" ) -func (s *server) CacheAndDiagnose(ctx context.Context, uri protocol.DocumentURI, text string) { - f := s.view.GetFile(source.URI(uri)) - if f, ok := f.(*cache.File); ok { - f.SetContent([]byte(text)) +func (s *server) cacheAndDiagnose(ctx context.Context, uri protocol.DocumentURI, content string) { + sourceURI := fromProtocolURI(uri) + if err := s.setContent(ctx, sourceURI, []byte(content)); err != nil { + return // handle error? } go func() { - reports, err := source.Diagnostics(ctx, s.view, f) + reports, err := source.Diagnostics(ctx, s.view, sourceURI) if err != nil { return // handle error? } @@ -27,16 +26,32 @@ func (s *server) CacheAndDiagnose(ctx context.Context, uri protocol.DocumentURI, uri := source.ToURI(filename) s.client.PublishDiagnostics(ctx, &protocol.PublishDiagnosticsParams{ URI: protocol.DocumentURI(uri), - Diagnostics: toProtocolDiagnostics(s.view, uri, diagnostics), + Diagnostics: toProtocolDiagnostics(ctx, s.view, uri, diagnostics), }) } }() } -func toProtocolDiagnostics(v *cache.View, uri source.URI, diagnostics []source.Diagnostic) []protocol.Diagnostic { +func (s *server) setContent(ctx context.Context, uri source.URI, content []byte) error { + v, err := s.view.SetContent(ctx, uri, content) + if err != nil { + return err + } + + s.viewMu.Lock() + s.view = v + s.viewMu.Unlock() + + return nil +} + +func toProtocolDiagnostics(ctx context.Context, v source.View, uri source.URI, diagnostics []source.Diagnostic) []protocol.Diagnostic { reports := []protocol.Diagnostic{} for _, diag := range diagnostics { - f := v.GetFile(uri) + f, err := v.GetFile(ctx, uri) + if err != nil { + continue // handle error? + } tok, err := f.GetToken() if err != nil { continue // handle error? diff --git a/internal/lsp/format.go b/internal/lsp/format.go index ef7f1d50..8a238099 100644 --- a/internal/lsp/format.go +++ b/internal/lsp/format.go @@ -4,14 +4,16 @@ import ( "context" "go/token" - "golang.org/x/tools/internal/lsp/cache" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" ) // formatRange formats a document with a given range. -func formatRange(ctx context.Context, v *cache.View, uri protocol.DocumentURI, rng *protocol.Range) ([]protocol.TextEdit, error) { - f := v.GetFile(source.URI(uri)) +func formatRange(ctx context.Context, v source.View, uri protocol.DocumentURI, rng *protocol.Range) ([]protocol.TextEdit, error) { + f, err := v.GetFile(ctx, fromProtocolURI(uri)) + if err != nil { + return nil, err + } tok, err := f.GetToken() if err != nil { return nil, err diff --git a/internal/lsp/imports.go b/internal/lsp/imports.go index c3ecc132..e3b98187 100644 --- a/internal/lsp/imports.go +++ b/internal/lsp/imports.go @@ -7,13 +7,15 @@ 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" ) -func organizeImports(ctx context.Context, v *cache.View, uri protocol.DocumentURI) ([]protocol.TextEdit, error) { - f := v.GetFile(source.URI(uri)) +func organizeImports(ctx context.Context, v source.View, uri protocol.DocumentURI) ([]protocol.TextEdit, error) { + f, err := v.GetFile(ctx, fromProtocolURI(uri)) + if err != nil { + return nil, err + } tok, err := f.GetToken() if err != nil { return nil, err diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index ff0e69fb..73c1d4fb 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -56,15 +56,14 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) { exported := packagestest.Export(t, exporter, modules) defer exported.Cleanup() - s := &server{ - view: cache.NewView(exported.Config.Dir), - } // Merge the exported.Config with the view.Config. cfg := *exported.Config - cfg.Fset = s.view.Config.Fset + cfg.Fset = token.NewFileSet() cfg.Mode = packages.LoadSyntax - s.view.Config = &cfg + s := &server{ + view: cache.NewView(&cfg), + } // Do a first pass to collect special markers for completion. if err := exported.Expect(map[string]interface{}{ "item": func(name string, r packagestest.Range, _, _ string) { @@ -150,15 +149,15 @@ type completions map[token.Position][]token.Pos type formats map[string]string type definitions map[protocol.Location]protocol.Location -func (d diagnostics) test(t *testing.T, exported *packagestest.Exported, v *cache.View) int { +func (d diagnostics) test(t *testing.T, exported *packagestest.Exported, v source.View) int { count := 0 + ctx := context.Background() for filename, want := range d { - f := v.GetFile(source.ToURI(filename)) - sourceDiagnostics, err := source.Diagnostics(context.Background(), v, f) + sourceDiagnostics, err := source.Diagnostics(context.Background(), v, source.ToURI(filename)) if err != nil { t.Fatal(err) } - got := toProtocolDiagnostics(v, source.ToURI(filename), sourceDiagnostics[filename]) + got := toProtocolDiagnostics(ctx, v, source.ToURI(filename), sourceDiagnostics[filename]) sorted(got) if diff := diffDiagnostics(filename, want, got); diff != "" { t.Error(diff) diff --git a/internal/lsp/position.go b/internal/lsp/position.go index 4b3fc3f9..69266925 100644 --- a/internal/lsp/position.go +++ b/internal/lsp/position.go @@ -5,6 +5,7 @@ package lsp import ( + "context" "go/token" "golang.org/x/tools/internal/lsp/cache" @@ -12,11 +13,20 @@ import ( "golang.org/x/tools/internal/lsp/source" ) +// fromProtocolURI converts a protocol.DocumentURI to a source.URI. +// TODO(rstambler): Add logic here to support Windows. +func fromProtocolURI(uri protocol.DocumentURI) source.URI { + return source.URI(uri) +} + // fromProtocolLocation converts from a protocol location to a source range. // It will return an error if the file of the location was not valid. // It uses fromProtocolRange to convert the start and end positions. -func fromProtocolLocation(v *cache.View, loc protocol.Location) (source.Range, error) { - f := v.GetFile(source.URI(loc.URI)) +func fromProtocolLocation(ctx context.Context, v *cache.View, loc protocol.Location) (source.Range, error) { + f, err := v.GetFile(ctx, fromProtocolURI(loc.URI)) + if err != nil { + return source.Range{}, err + } tok, err := f.GetToken() if err != nil { return source.Range{}, err diff --git a/internal/lsp/server.go b/internal/lsp/server.go index 03169d75..cdd07f2c 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -6,9 +6,11 @@ package lsp import ( "context" + "go/token" "os" "sync" + "golang.org/x/tools/go/packages" "golang.org/x/tools/internal/jsonrpc2" "golang.org/x/tools/internal/lsp/cache" "golang.org/x/tools/internal/lsp/protocol" @@ -33,7 +35,8 @@ type server struct { signatureHelpEnabled bool snippetsSupported bool - view *cache.View + viewMu sync.Mutex + view source.View } func (s *server) Initialize(ctx context.Context, params *protocol.InitializeParams) (*protocol.InitializeResult, error) { @@ -48,11 +51,17 @@ func (s *server) Initialize(ctx context.Context, params *protocol.InitializePara s.snippetsSupported = params.Capabilities.TextDocument.Completion.CompletionItem.SnippetSupport s.signatureHelpEnabled = true - rootPath, err := source.URI(*params.RootURI).Filename() + rootPath, err := fromProtocolURI(*params.RootURI).Filename() if err != nil { return nil, err } - s.view = cache.NewView(rootPath) + s.view = cache.NewView(&packages.Config{ + Dir: rootPath, + Mode: packages.LoadSyntax, + Fset: token.NewFileSet(), + Tests: true, + Overlay: make(map[string][]byte), + }) return &protocol.InitializeResult{ Capabilities: protocol.ServerCapabilities{ @@ -119,7 +128,7 @@ func (s *server) ExecuteCommand(context.Context, *protocol.ExecuteCommandParams) } func (s *server) DidOpen(ctx context.Context, params *protocol.DidOpenTextDocumentParams) error { - s.CacheAndDiagnose(ctx, params.TextDocument.URI, params.TextDocument.Text) + s.cacheAndDiagnose(ctx, params.TextDocument.URI, params.TextDocument.Text) return nil } @@ -129,7 +138,7 @@ func (s *server) DidChange(ctx context.Context, params *protocol.DidChangeTextDo } // 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) + s.cacheAndDiagnose(ctx, params.TextDocument.URI, change.Text) } return nil } @@ -147,15 +156,15 @@ func (s *server) DidSave(context.Context, *protocol.DidSaveTextDocumentParams) e } func (s *server) DidClose(ctx context.Context, params *protocol.DidCloseTextDocumentParams) error { - f := s.view.GetFile(source.URI(params.TextDocument.URI)) - if f, ok := f.(*cache.File); ok { - f.SetContent(nil) - } + s.setContent(ctx, fromProtocolURI(params.TextDocument.URI), nil) return nil } func (s *server) Completion(ctx context.Context, params *protocol.CompletionParams) (*protocol.CompletionList, error) { - f := s.view.GetFile(source.URI(params.TextDocument.URI)) + f, err := s.view.GetFile(ctx, fromProtocolURI(params.TextDocument.URI)) + if err != nil { + return nil, err + } tok, err := f.GetToken() if err != nil { return nil, err @@ -176,7 +185,10 @@ func (s *server) CompletionResolve(context.Context, *protocol.CompletionItem) (* } func (s *server) Hover(ctx context.Context, params *protocol.TextDocumentPositionParams) (*protocol.Hover, error) { - f := s.view.GetFile(source.URI(params.TextDocument.URI)) + f, err := s.view.GetFile(ctx, fromProtocolURI(params.TextDocument.URI)) + if err != nil { + return nil, err + } tok, err := f.GetToken() if err != nil { return nil, err @@ -196,7 +208,10 @@ func (s *server) Hover(ctx context.Context, params *protocol.TextDocumentPositio } func (s *server) SignatureHelp(ctx context.Context, params *protocol.TextDocumentPositionParams) (*protocol.SignatureHelp, error) { - f := s.view.GetFile(source.URI(params.TextDocument.URI)) + f, err := s.view.GetFile(ctx, fromProtocolURI(params.TextDocument.URI)) + if err != nil { + return nil, err + } tok, err := f.GetToken() if err != nil { return nil, err @@ -210,7 +225,10 @@ func (s *server) SignatureHelp(ctx context.Context, params *protocol.TextDocumen } func (s *server) Definition(ctx context.Context, params *protocol.TextDocumentPositionParams) ([]protocol.Location, error) { - f := s.view.GetFile(source.URI(params.TextDocument.URI)) + f, err := s.view.GetFile(ctx, fromProtocolURI(params.TextDocument.URI)) + if err != nil { + return nil, err + } tok, err := f.GetToken() if err != nil { return nil, err @@ -220,11 +238,14 @@ func (s *server) Definition(ctx context.Context, params *protocol.TextDocumentPo if err != nil { return nil, err } - return []protocol.Location{toProtocolLocation(s.view.Config.Fset, r)}, nil + return []protocol.Location{toProtocolLocation(s.view.FileSet(), r)}, nil } func (s *server) TypeDefinition(ctx context.Context, params *protocol.TextDocumentPositionParams) ([]protocol.Location, error) { - f := s.view.GetFile(source.URI(params.TextDocument.URI)) + f, err := s.view.GetFile(ctx, fromProtocolURI(params.TextDocument.URI)) + if err != nil { + return nil, err + } tok, err := f.GetToken() if err != nil { return nil, err @@ -234,7 +255,7 @@ func (s *server) TypeDefinition(ctx context.Context, params *protocol.TextDocume if err != nil { return nil, err } - return []protocol.Location{toProtocolLocation(s.view.Config.Fset, r)}, nil + return []protocol.Location{toProtocolLocation(s.view.FileSet(), r)}, nil } func (s *server) Implementation(context.Context, *protocol.TextDocumentPositionParams) ([]protocol.Location, error) { diff --git a/internal/lsp/source/definition.go b/internal/lsp/source/definition.go index d97be3bf..79eb93ff 100644 --- a/internal/lsp/source/definition.go +++ b/internal/lsp/source/definition.go @@ -48,7 +48,7 @@ func Definition(ctx context.Context, v View, f File, pos token.Pos) (Range, erro if err != nil { return Range{}, err } - return objToRange(v, fset, obj), nil + return objToRange(ctx, v, fset, obj), nil } func TypeDefinition(ctx context.Context, v View, f File, pos token.Pos) (Range, error) { @@ -79,7 +79,7 @@ func TypeDefinition(ctx context.Context, v View, f File, pos token.Pos) (Range, if err != nil { return Range{}, err } - return objToRange(v, fset, obj), nil + return objToRange(ctx, v, fset, obj), nil } func typeToObject(typ types.Type) (obj types.Object) { @@ -137,7 +137,7 @@ func checkIdentifier(f *ast.File, pos token.Pos) (ident, error) { return result, nil } -func objToRange(v View, fset *token.FileSet, obj types.Object) Range { +func objToRange(ctx context.Context, v View, fset *token.FileSet, obj types.Object) Range { p := obj.Pos() f := fset.File(p) pos := f.Position(p) @@ -148,7 +148,10 @@ func objToRange(v View, fset *token.FileSet, obj types.Object) Range { // column to match its offset. // // TODO: If we parse from source, we will never need this hack. - f := v.GetFile(ToURI(pos.Filename)) + f, err := v.GetFile(ctx, ToURI(pos.Filename)) + if err != nil { + goto Return + } tok, err := f.GetToken() if err != nil { goto Return diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index 2c1e01ed..1b84d3c5 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -20,7 +20,11 @@ type Diagnostic struct { Message string } -func Diagnostics(ctx context.Context, v View, f File) (map[string][]Diagnostic, error) { +func Diagnostics(ctx context.Context, v View, uri URI) (map[string][]Diagnostic, error) { + f, err := v.GetFile(ctx, uri) + if err != nil { + return nil, err + } pkg, err := f.GetPackage() if err != nil { return nil, err @@ -49,7 +53,10 @@ func Diagnostics(ctx context.Context, v View, f File) (map[string][]Diagnostic, } for _, diag := range diags { pos := errorPos(diag) - diagFile := v.GetFile(ToURI(pos.Filename)) + diagFile, err := v.GetFile(ctx, ToURI(pos.Filename)) + if err != nil { + continue + } diagTok, err := diagFile.GetToken() if err != nil { continue diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index f0cd253e..395c06fd 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -5,6 +5,7 @@ package source import ( + "context" "go/ast" "go/token" @@ -15,8 +16,9 @@ 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 { - // Consider adding an error to this method, if users require it. - GetFile(uri URI) File + GetFile(ctx context.Context, uri URI) (File, error) + SetContent(ctx context.Context, uri URI, content []byte) (View, error) + FileSet() *token.FileSet } // File represents a Go source file that has been type-checked. It is the input