From 9b5bafe36f85e86bd27ccf7989d811fa697f5872 Mon Sep 17 00:00:00 2001 From: Ian Cottrell Date: Fri, 2 Nov 2018 16:15:31 -0400 Subject: [PATCH] internal/lsp: extract view to its own package This allows us to write the lsp verbs in terms of a stable underlying source management layer. This should make it easier to refactor the underlying layer to add more powerful caching and incremental modes as we go. Change-Id: Iab97b061d80394a6fa6748a93a4c68f2deb46129 Reviewed-on: https://go-review.googlesource.com/c/147201 Reviewed-by: Rebecca Stambler --- internal/lsp/diagnostics.go | 5 +-- internal/lsp/diagnostics_test.go | 9 ++--- internal/lsp/format.go | 7 ++-- internal/lsp/server.go | 19 +++++------ internal/lsp/source/uri.go | 32 +++++++++++++++++ internal/lsp/{ => source}/view.go | 57 ++++++++++++++++--------------- 6 files changed, 82 insertions(+), 47 deletions(-) create mode 100644 internal/lsp/source/uri.go rename internal/lsp/{ => source}/view.go (51%) diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index c4dbb88e..94e8ff1d 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -11,10 +11,11 @@ import ( "golang.org/x/tools/go/packages" "golang.org/x/tools/internal/lsp/protocol" + "golang.org/x/tools/internal/lsp/source" ) -func (v *view) diagnostics(uri protocol.DocumentURI) (map[string][]protocol.Diagnostic, error) { - pkg, err := v.typeCheck(uri) +func diagnostics(v *source.View, uri protocol.DocumentURI) (map[string][]protocol.Diagnostic, error) { + pkg, err := v.TypeCheck(uri) if err != nil { return nil, err } diff --git a/internal/lsp/diagnostics_test.go b/internal/lsp/diagnostics_test.go index e547c6b7..093bfc39 100644 --- a/internal/lsp/diagnostics_test.go +++ b/internal/lsp/diagnostics_test.go @@ -15,6 +15,7 @@ import ( "golang.org/x/tools/go/packages" "golang.org/x/tools/go/packages/packagestest" "golang.org/x/tools/internal/lsp/protocol" + "golang.org/x/tools/internal/lsp/source" ) func TestDiagnostics(t *testing.T) { @@ -84,12 +85,12 @@ func testDiagnostics(t *testing.T, exporter packagestest.Exporter) { if err != nil { t.Fatal(err) } - v := newView() - v.config = exported.Config - v.config.Mode = packages.LoadSyntax + v := source.NewView() + v.Config = exported.Config + v.Config.Mode = packages.LoadSyntax for _, pkg := range pkgs { for _, filename := range pkg.GoFiles { - diagnostics, err := v.diagnostics(filenameToURI(filename)) + diagnostics, err := diagnostics(v, source.ToURI(filename)) if err != nil { t.Fatal(err) } diff --git a/internal/lsp/format.go b/internal/lsp/format.go index c78cf36a..9bfdb659 100644 --- a/internal/lsp/format.go +++ b/internal/lsp/format.go @@ -10,11 +10,12 @@ import ( "go/format" "golang.org/x/tools/internal/lsp/protocol" + "golang.org/x/tools/internal/lsp/source" ) -// format formats a document with a given range. -func (s *server) format(uri protocol.DocumentURI, rng *protocol.Range) ([]protocol.TextEdit, error) { - data, err := s.readActiveFile(uri) +// formatRange formats a document with a given range. +func formatRange(v *source.View, uri protocol.DocumentURI, rng *protocol.Range) ([]protocol.TextEdit, error) { + data, err := v.ReadActiveFile(uri) if err != nil { return nil, err } diff --git a/internal/lsp/server.go b/internal/lsp/server.go index 58cecde6..63c403e1 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -11,6 +11,7 @@ import ( "golang.org/x/tools/internal/jsonrpc2" "golang.org/x/tools/internal/lsp/protocol" + "golang.org/x/tools/internal/lsp/source" ) // RunServer starts an LSP server on the supplied stream, and waits until the @@ -28,7 +29,7 @@ type server struct { initializedMu sync.Mutex initialized bool // set once the server has received "initialize" request - *view + view *source.View } func (s *server) Initialize(ctx context.Context, params *protocol.InitializeParams) (*protocol.InitializeResult, error) { @@ -37,7 +38,7 @@ func (s *server) Initialize(ctx context.Context, params *protocol.InitializePara if s.initialized { return nil, jsonrpc2.NewErrorf(jsonrpc2.CodeInvalidRequest, "server already initialized") } - s.view = newView() + s.view = source.NewView() s.initialized = true return &protocol.InitializeResult{ Capabilities: protocol.ServerCapabilities{ @@ -110,15 +111,13 @@ func (s *server) DidChange(ctx context.Context, params *protocol.DidChangeTextDo } func (s *server) cacheAndDiagnoseFile(ctx context.Context, uri protocol.DocumentURI, text string) { - s.view.activeFilesMu.Lock() - s.view.activeFiles[uri] = []byte(text) - s.view.activeFilesMu.Unlock() + s.view.SetActiveFileContent(uri, []byte(text)) go func() { - reports, err := s.diagnostics(uri) + reports, err := diagnostics(s.view, uri) if err == nil { for filename, diagnostics := range reports { s.client.PublishDiagnostics(ctx, &protocol.PublishDiagnosticsParams{ - URI: filenameToURI(filename), + URI: source.ToURI(filename), Diagnostics: diagnostics, }) } @@ -140,7 +139,7 @@ func (s *server) DidSave(context.Context, *protocol.DidSaveTextDocumentParams) e } func (s *server) DidClose(ctx context.Context, params *protocol.DidCloseTextDocumentParams) error { - s.clearActiveFile(params.TextDocument.URI) + s.view.ClearActiveFile(params.TextDocument.URI) return nil } @@ -213,11 +212,11 @@ func (s *server) ColorPresentation(context.Context, *protocol.ColorPresentationP } func (s *server) Formatting(ctx context.Context, params *protocol.DocumentFormattingParams) ([]protocol.TextEdit, error) { - return s.format(params.TextDocument.URI, nil) + return formatRange(s.view, params.TextDocument.URI, nil) } func (s *server) RangeFormatting(ctx context.Context, params *protocol.DocumentRangeFormattingParams) ([]protocol.TextEdit, error) { - return s.format(params.TextDocument.URI, ¶ms.Range) + return formatRange(s.view, params.TextDocument.URI, ¶ms.Range) } func (s *server) OnTypeFormatting(context.Context, *protocol.DocumentOnTypeFormattingParams) ([]protocol.TextEdit, error) { diff --git a/internal/lsp/source/uri.go b/internal/lsp/source/uri.go new file mode 100644 index 00000000..c6b9d8f6 --- /dev/null +++ b/internal/lsp/source/uri.go @@ -0,0 +1,32 @@ +// Copyright 2018 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package source + +import ( + "fmt" + "path/filepath" + "strings" + + "golang.org/x/tools/internal/lsp/protocol" +) + +const fileSchemePrefix = "file://" + +// FromURI gets the file path for a given URI. +// It will return an error if the uri is not valid, or if the URI was not +// a file URI +func FromURI(uri protocol.DocumentURI) (string, error) { + s := string(uri) + if !strings.HasPrefix(s, fileSchemePrefix) { + return "", fmt.Errorf("only file URI's are supported, got %v", uri) + } + return filepath.FromSlash(s[len(fileSchemePrefix):]), nil +} + +// ToURI returns a protocol URI for the supplied path. +// It will always have the file scheme. +func ToURI(path string) protocol.DocumentURI { + return protocol.DocumentURI(fileSchemePrefix + filepath.ToSlash(path)) +} diff --git a/internal/lsp/view.go b/internal/lsp/source/view.go similarity index 51% rename from internal/lsp/view.go rename to internal/lsp/source/view.go index 77524d6a..27fb61b7 100644 --- a/internal/lsp/view.go +++ b/internal/lsp/source/view.go @@ -2,30 +2,30 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -package lsp +package source import ( "fmt" "go/token" - "strings" "sync" "golang.org/x/tools/go/packages" "golang.org/x/tools/internal/lsp/protocol" ) -type view struct { +type View struct { + Config *packages.Config + activeFilesMu sync.Mutex activeFiles map[protocol.DocumentURI][]byte - config *packages.Config fset *token.FileSet } -func newView() *view { +func NewView() *View { fset := token.NewFileSet() - return &view{ - config: &packages.Config{ + return &View{ + Config: &packages.Config{ Mode: packages.LoadSyntax, Fset: fset, Tests: true, @@ -35,53 +35,54 @@ func newView() *view { } } -func (v *view) overlay() map[string][]byte { +func (v *View) overlay() map[string][]byte { over := make(map[string][]byte) v.activeFilesMu.Lock() defer v.activeFilesMu.Unlock() for uri, content := range v.activeFiles { - over[uriToFilename(uri)] = content + filename, err := FromURI(uri) + if err == nil { + over[filename] = content + } } return over } -func (v *view) readActiveFile(uri protocol.DocumentURI) ([]byte, error) { +func (v *View) SetActiveFileContent(uri protocol.DocumentURI, content []byte) { v.activeFilesMu.Lock() - defer v.activeFilesMu.Unlock() + v.activeFiles[uri] = content + v.activeFilesMu.Unlock() +} +func (v *View) ReadActiveFile(uri protocol.DocumentURI) ([]byte, error) { + v.activeFilesMu.Lock() content, ok := v.activeFiles[uri] + v.activeFilesMu.Unlock() if !ok { - return nil, fmt.Errorf("file not found: %s", uri) + return nil, fmt.Errorf("uri not found: %s", uri) } return content, nil } -func (v *view) clearActiveFile(uri protocol.DocumentURI) { +func (v *View) ClearActiveFile(uri protocol.DocumentURI) { v.activeFilesMu.Lock() delete(v.activeFiles, uri) v.activeFilesMu.Unlock() } -// typeCheck type-checks the package for the given package path. -func (v *view) typeCheck(uri protocol.DocumentURI) (*packages.Package, error) { - v.config.Overlay = v.overlay() - pkgs, err := packages.Load(v.config, fmt.Sprintf("file=%s", uriToFilename(uri))) +// TypeCheck type-checks the package for the given package path. +func (v *View) TypeCheck(uri protocol.DocumentURI) (*packages.Package, error) { + v.Config.Overlay = v.overlay() + path, err := FromURI(uri) + if err != nil { + return nil, err + } + 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", uri) - } return nil, err } pkg := pkgs[0] return pkg, nil } - -func uriToFilename(uri protocol.DocumentURI) string { - return strings.TrimPrefix(string(uri), "file://") -} - -func filenameToURI(filename string) protocol.DocumentURI { - return protocol.DocumentURI("file://" + filename) -}