From 921b34c7d07f74c8b33bbbfae8e373bb20929ea2 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Tue, 14 May 2019 16:23:46 -0400 Subject: [PATCH] internal/lsp: ignore files in the builtin package This change stops diagnostics from running in files making up the "fake" builtin package. Fixes golang/go#31962 Change-Id: Ic54e1587e3ad54f0c1f5e82f1a6f3522b4c6bee9 Reviewed-on: https://go-review.googlesource.com/c/tools/+/177218 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Ian Cottrell --- internal/lsp/cache/view.go | 26 ++++++++++++++++++++++---- internal/lsp/diagnostics.go | 12 ------------ internal/lsp/server.go | 2 +- internal/lsp/source/source_test.go | 9 --------- internal/lsp/text_synchronization.go | 18 +++++++++++++++++- 5 files changed, 40 insertions(+), 27 deletions(-) diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index 3b59ed5b3c9..6ef8ca82240 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -6,6 +6,7 @@ package cache import ( "context" + "fmt" "go/ast" "go/parser" "go/token" @@ -272,15 +273,18 @@ 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) { + filename, err := uri.Filename() + if err != nil { + return nil, err + } + if v.isIgnored(filename) { + return nil, fmt.Errorf("%s is ignored", filename) + } if f, err := v.findFile(uri); err != nil { return nil, err } else if f != nil { return f, nil } - filename, err := uri.Filename() - if err != nil { - return nil, err - } f := &File{ view: v, filename: filename, @@ -289,6 +293,20 @@ func (v *View) getFile(uri span.URI) (*File, error) { return f, nil } +// 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 { + bpkg := v.BuiltinPackage() + if bpkg != nil { + for builtinFilename := range bpkg.Files { + if filename == builtinFilename { + return true + } + } + } + return false +} + // findFile checks the cache for any file matching the given uri. // // An error is only returned for an irreparable failure, for example, if the diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index 040c3837933..2af02dab617 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -13,18 +13,6 @@ import ( "golang.org/x/tools/internal/span" ) -func (s *Server) cacheAndDiagnose(ctx context.Context, uri span.URI, content string) error { - view := s.findView(ctx, uri) - if err := view.SetContent(ctx, uri, []byte(content)); err != nil { - return err - } - go func() { - ctx := view.BackgroundContext() - s.Diagnostics(ctx, view, uri) - }() - return nil -} - func (s *Server) Diagnostics(ctx context.Context, view *cache.View, uri span.URI) { if ctx.Err() != nil { s.log.Errorf(ctx, "canceling diagnostics for %s: %v", uri, ctx.Err()) diff --git a/internal/lsp/server.go b/internal/lsp/server.go index 0a23f69037d..047be4e733e 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -135,7 +135,7 @@ func (s *Server) ExecuteCommand(context.Context, *protocol.ExecuteCommandParams) // Text Synchronization func (s *Server) DidOpen(ctx context.Context, params *protocol.DidOpenTextDocumentParams) error { - return s.cacheAndDiagnose(ctx, span.NewURI(params.TextDocument.URI), params.TextDocument.Text) + return s.didOpen(ctx, params) } func (s *Server) DidChange(ctx context.Context, params *protocol.DidChangeTextDocumentParams) error { diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index f840e41d267..7c3db45f6e7 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -156,15 +156,6 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests t.Errorf("%s: %s", src, diff) } } - // Make sure we don't crash completing the first position in file set. - uri := span.FileURI(r.view.FileSet().File(1).Name()) - f, err := r.view.GetFile(ctx, uri) - if err != nil { - t.Fatalf("failed for %v: %v", uri, err) - } - source.Completion(ctx, f, 1) - - r.checkCompletionSnippets(ctx, t, snippets, items) } func (r *runner) checkCompletionSnippets(ctx context.Context, t *testing.T, data tests.CompletionSnippets, items tests.CompletionItems) { diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go index a29edd5bdeb..16677730c02 100644 --- a/internal/lsp/text_synchronization.go +++ b/internal/lsp/text_synchronization.go @@ -13,6 +13,10 @@ import ( "golang.org/x/tools/internal/span" ) +func (s *Server) didOpen(ctx context.Context, params *protocol.DidOpenTextDocumentParams) error { + return s.cacheAndDiagnose(ctx, span.NewURI(params.TextDocument.URI), []byte(params.TextDocument.Text)) +} + func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDocumentParams) error { if len(params.ContentChanges) < 1 { return jsonrpc2.NewErrorf(jsonrpc2.CodeInternalError, "no content changes provided") @@ -34,7 +38,19 @@ func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDo } text = change.Text } - return s.cacheAndDiagnose(ctx, span.NewURI(params.TextDocument.URI), text) + return s.cacheAndDiagnose(ctx, span.NewURI(params.TextDocument.URI), []byte(text)) +} + +func (s *Server) cacheAndDiagnose(ctx context.Context, uri span.URI, content []byte) error { + view := s.findView(ctx, uri) + if err := view.SetContent(ctx, uri, content); err != nil { + return err + } + go func() { + ctx := view.BackgroundContext() + s.Diagnostics(ctx, view, uri) + }() + return nil } func (s *Server) applyChanges(ctx context.Context, params *protocol.DidChangeTextDocumentParams) (string, error) {