mirror
/
go
mirror of https://github.com/golang/go.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

internal/lsp: don't run analyses on the entire view 

Running staticcheck on the entire workspace causes a slowdown, and most
likely users don't want to see staticcheck reports for every
subdirectory of their workspace. Only run staticcheck on open files.

Also, fixed a staticcheck warning that showed up along the way. Filed
golang/go#35718 to remind ourselves to fix all of the staticcheck warnings
that showed up when we ran gopls with staticcheck on x/tools.

Finally, made sure that we don't send empty diagnostics when diagnosing
the snapshot on start-up, as that is not necessary.

Change-Id: Ic51d1abfc80b1b53397057f06a4cfd7e2dc930f9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208098
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This commit is contained in:
Rebecca Stambler 2019-11-20 14:26:02 -05:00
parent 9f1eb44090
commit 73cd2cc3b5
20 changed files with 99 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
internal/lsp/cache/snapshot.go vendored
View File

@ -61,10 +61,9 @@ func (s *snapshot) View() source.View {
return s.view
}
func (s *snapshot) PackageHandles(ctx context.Context, f source.File) ([]source.CheckPackageHandle, error) {
ctx = telemetry.File.With(ctx, f.URI())
func (s *snapshot) PackageHandles(ctx context.Context, fh source.FileHandle) ([]source.CheckPackageHandle, error) {
ctx = telemetry.File.With(ctx, fh.Identity().URI)
fh := s.Handle(ctx, f)
metadata := s.getMetadataForURI(fh.Identity().URI)
// Determine if we need to type-check the package.
@ -74,7 +73,7 @@ func (s *snapshot) PackageHandles(ctx context.Context, f source.File) ([]source.
// We only need to this if it has been invalidated, and is therefore unvailable.
if load {
var err error
m, err := s.load(ctx, source.FileURI(f.URI()))
m, err := s.load(ctx, source.FileURI(fh.Identity().URI))
if err != nil {
return nil, err
}
@ -95,7 +94,7 @@ func (s *snapshot) PackageHandles(ctx context.Context, f source.File) ([]source.
cphs = results
}
if len(cphs) == 0 {
return nil, errors.Errorf("no CheckPackageHandles for %s", f.URI())
return nil, errors.Errorf("no CheckPackageHandles for %s", fh.Identity().URI)
}
return cphs, nil
}
@ -509,14 +508,6 @@ func (v *view) invalidateContent(ctx context.Context, f source.File, kind source
v.snapshotMu.Lock()
defer v.snapshotMu.Unlock()
// TODO: Handle the possibility of opening a file outside of the current view.
// For now, return early if we open a file. Clone the snapshot so that the file's version is updated.
// We assume that we are already tracking any files within the given view.
if action == source.Open {
v.snapshot = v.snapshot.clone(ctx, f.URI(), nil, nil)
return true
}
// Collect all of the package IDs that correspond to the given file.
for _, id := range v.snapshot.getIDs(f.URI()) {
ids[id] = struct{}{}

2
internal/lsp/cmd/check.go
View File

@ -56,7 +56,7 @@ func (c *check) Run(ctx context.Context, args ...string) error {
checking[uri] = file
}
// now wait for results
//TODO: maybe conn.ExecuteCommand(ctx, &protocol.ExecuteCommandParams{Command: "gopls-wait-idle"})
// TODO: maybe conn.ExecuteCommand(ctx, &protocol.ExecuteCommandParams{Command: "gopls-wait-idle"})
for _, file := range checking {
select {
case <-file.hasDiagnostics:

23
internal/lsp/cmd/cmd.go
View File

@ -323,17 +323,22 @@ func (c *cmdClient) ApplyEdit(ctx context.Context, p *protocol.ApplyWorkspaceEdi
}
func (c *cmdClient) PublishDiagnostics(ctx context.Context, p *protocol.PublishDiagnosticsParams) error {
// Don't worry about diagnostics without versions.
if p.Version == 0 {
return nil
}
c.filesMu.Lock()
defer c.filesMu.Unlock()
uri := span.URI(p.URI)
file := c.getFile(ctx, uri)
file.diagnosticsMu.Lock()
defer file.diagnosticsMu.Unlock()
hadDiagnostics := file.diagnostics != nil
file.diagnostics = p.Diagnostics
if file.diagnostics == nil {
file.diagnostics = []protocol.Diagnostic{}
}
if !hadDiagnostics {
close(file.hasDiagnostics)
}
@ -384,10 +389,14 @@ func (c *connection) AddFile(ctx context.Context, uri span.URI) *cmdFile {
return file
}
file.added = true
p := &protocol.DidOpenTextDocumentParams{}
p.TextDocument.URI = string(uri)
p.TextDocument.Text = string(file.mapper.Content)
p.TextDocument.LanguageID = source.DetectLanguage("", file.uri.Filename()).String()
p := &protocol.DidOpenTextDocumentParams{
TextDocument: protocol.TextDocumentItem{
URI: protocol.NewURI(uri),
LanguageID: source.DetectLanguage("", file.uri.Filename()).String(),
Version: 1,
Text: string(file.mapper.Content),
},
}
if err := c.Server.DidOpen(ctx, p); err != nil {
file.err = errors.Errorf("%v: %v", uri, err)
}

3
internal/lsp/cmd/test/cmdtest.go
View File

@ -124,8 +124,9 @@ func (r *runner) RunGoplsCmd(t testing.TB, args ...string) (string, string) {
}()
os.Stdout = wStdout
os.Stderr = wStderr
app := cmd.New("gopls-test", r.data.Config.Dir, r.data.Exported.Config.Env, r.options)
err = tool.Run(tests.Context(t),
cmd.New("gopls-test", r.data.Config.Dir, r.data.Exported.Config.Env, r.options),
app,
append([]string{"-remote=internal"}, args...))
if err != nil {
fmt.Fprint(os.Stderr, err)

4
internal/lsp/code_action.go
View File

@ -205,7 +205,9 @@ func importDiagnostics(fix *imports.ImportFix, diagnostics []protocol.Diagnostic
func quickFixes(ctx context.Context, snapshot source.Snapshot, f source.File, diagnostics []protocol.Diagnostic) ([]protocol.CodeAction, error) {
var codeActions []protocol.CodeAction
cphs, err := snapshot.PackageHandles(ctx, f)
fh := snapshot.Handle(ctx, f)
cphs, err := snapshot.PackageHandles(ctx, fh)
if err != nil {
return nil, err
}

60
internal/lsp/diagnostics.go
View File

@ -16,25 +16,37 @@ import (
"golang.org/x/tools/internal/telemetry/trace"
)
func (s *Server) diagnoseSnapshot(ctx context.Context, snapshot source.Snapshot, cphs []source.CheckPackageHandle) {
func (s *Server) diagnoseSnapshot(snapshot source.Snapshot, cphs []source.CheckPackageHandle) {
ctx := snapshot.View().BackgroundContext()
ctx, done := trace.StartSpan(ctx, "lsp:background-worker")
defer done()
for _, cph := range cphs {
if len(cph.CompiledGoFiles()) == 0 {
continue
}
// Find a file on which to call diagnostics.
uri := cph.CompiledGoFiles()[0].File().Identity().URI
f, err := snapshot.View().GetFile(ctx, uri)
if err != nil {
log.Error(ctx, "no file", err, telemetry.URI.Of(uri))
continue
}
// Run diagnostics on the workspace package.
go func(snapshot source.Snapshot, uri span.URI) {
if err := s.diagnostics(ctx, snapshot, uri); err != nil {
log.Error(snapshot.View().BackgroundContext(), "error computing diagnostics", err, telemetry.URI.Of(uri))
go func(snapshot source.Snapshot, f source.File) {
reports, _, err := source.Diagnostics(ctx, snapshot, f, false, snapshot.View().Options().DisabledAnalyses)
if err != nil {
log.Error(ctx, "no diagnostics", err, telemetry.URI.Of(f.URI()))
return
}
}(snapshot, uri)
// Don't publish empty diagnostics.
s.publishReports(ctx, reports, false)
}(snapshot, f)
}
}
func (s *Server) diagnostics(ctx context.Context, snapshot source.Snapshot, uri span.URI) error {
func (s *Server) diagnoseFile(snapshot source.Snapshot, uri span.URI) error {
ctx := snapshot.View().BackgroundContext()
ctx, done := trace.StartSpan(ctx, "lsp:background-worker")
defer done()
@ -44,7 +56,7 @@ func (s *Server) diagnostics(ctx context.Context, snapshot source.Snapshot, uri
if err != nil {
return err
}
reports, warningMsg, err := source.Diagnostics(ctx, snapshot, f, snapshot.View().Options().DisabledAnalyses)
reports, warningMsg, err := source.Diagnostics(ctx, snapshot, f, true, snapshot.View().Options().DisabledAnalyses)
if err != nil {
return err
}
@ -54,43 +66,45 @@ func (s *Server) diagnostics(ctx context.Context, snapshot source.Snapshot, uri
Message: warningMsg,
})
}
// Publish empty diagnostics for files.
s.publishReports(ctx, reports, true)
return nil
}
s.undeliveredMu.Lock()
defer s.undeliveredMu.Unlock()
func (s *Server) publishReports(ctx context.Context, reports map[source.FileIdentity][]source.Diagnostic, publishEmpty bool) {
undelivered := make(map[source.FileIdentity][]source.Diagnostic)
for fileID, diagnostics := range reports {
// Don't publish empty diagnostics unless specified.
if len(diagnostics) == 0 && !publishEmpty {
continue
}
if err := s.publishDiagnostics(ctx, fileID, diagnostics); err != nil {
if s.undelivered == nil {
s.undelivered = make(map[source.FileIdentity][]source.Diagnostic)
}
s.undelivered[fileID] = diagnostics
undelivered[fileID] = diagnostics
log.Error(ctx, "failed to deliver diagnostic (will retry)", err, telemetry.File)
continue
}
// In case we had old, undelivered diagnostics.
delete(s.undelivered, fileID)
delete(undelivered, fileID)
}
// Anytime we compute diagnostics, make sure to also send along any
// Any time we compute diagnostics, make sure to also send along any
// undelivered ones (only for remaining URIs).
for uri, diagnostics := range s.undelivered {
for uri, diagnostics := range undelivered {
if err := s.publishDiagnostics(ctx, uri, diagnostics); err != nil {
log.Error(ctx, "failed to deliver diagnostic for (will not retry)", err, telemetry.File)
}
// If we fail to deliver the same diagnostics twice, just give up.
delete(s.undelivered, uri)
delete(undelivered, uri)
}
return nil
}
func (s *Server) publishDiagnostics(ctx context.Context, fileID source.FileIdentity, diagnostics []source.Diagnostic) error {
s.client.PublishDiagnostics(ctx, &protocol.PublishDiagnosticsParams{
return s.client.PublishDiagnostics(ctx, &protocol.PublishDiagnosticsParams{
Diagnostics: toProtocolDiagnostics(ctx, diagnostics),
URI: protocol.NewURI(fileID.URI),
Version: fileID.Version,
})
return nil
}
func toProtocolDiagnostics(ctx context.Context, diagnostics []source.Diagnostic) []protocol.Diagnostic {

2
internal/lsp/general.go
View File

@ -172,7 +172,7 @@ func (s *Server) addFolders(ctx context.Context, folders []protocol.WorkspaceFol
viewErrors[uri] = err
continue
}
go s.diagnoseSnapshot(ctx, view.Snapshot(), workspacePackages)
go s.diagnoseSnapshot(view.Snapshot(), workspacePackages)
}
if len(viewErrors) > 0 {
errMsg := fmt.Sprintf("Error loading workspace folders (expected %v, got %v)\n", len(folders), len(s.session.Views())-originalViews)

7
internal/lsp/lsp_test.go
View File

@ -61,8 +61,7 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) {
}
r := &runner{
server: &Server{
session: session,
undelivered: make(map[source.FileIdentity][]source.Diagnostic),
session: session,
},
data: data,
ctx: ctx,
@ -79,7 +78,7 @@ func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnosti
t.Fatalf("no file for %s: %v", f, err)
}
identity := v.Snapshot().Handle(r.ctx, f).Identity()
results, _, err := source.Diagnostics(r.ctx, v.Snapshot(), f, nil)
results, _, err := source.Diagnostics(r.ctx, v.Snapshot(), f, true, nil)
if err != nil {
t.Fatal(err)
}
@ -339,7 +338,7 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span) {
}
snapshot := view.Snapshot()
fileID := snapshot.Handle(r.ctx, f).Identity()
diagnostics, _, err := source.Diagnostics(r.ctx, snapshot, f, nil)
diagnostics, _, err := source.Diagnostics(r.ctx, snapshot, f, true, nil)
if err != nil {
t.Fatal(err)
}

5
internal/lsp/server.go
View File

@ -79,11 +79,6 @@ type Server struct {
session source.Session
// undelivered is a cache of any diagnostics that the server
// failed to deliver for some reason.
undeliveredMu sync.Mutex
undelivered map[source.FileIdentity][]source.Diagnostic
// changedFiles tracks files for which there has been a textDocument/didChange.
changedFiles map[span.URI]struct{}

3
internal/lsp/source/completion.go
View File

@ -394,7 +394,8 @@ func Completion(ctx context.Context, snapshot Snapshot, f File, pos protocol.Pos
startTime := time.Now()
cphs, err := snapshot.PackageHandles(ctx, f)
fh := snapshot.Handle(ctx, f)
cphs, err := snapshot.PackageHandles(ctx, fh)
if err != nil {
return nil, nil, err
}

6
internal/lsp/source/diagnostics.go
View File

@ -39,12 +39,12 @@ type RelatedInformation struct {
Message string
}
func Diagnostics(ctx context.Context, snapshot Snapshot, f File, disabledAnalyses map[string]struct{}) (map[FileIdentity][]Diagnostic, string, error) {
func Diagnostics(ctx context.Context, snapshot Snapshot, f File, withAnalysis bool, disabledAnalyses map[string]struct{}) (map[FileIdentity][]Diagnostic, string, error) {
ctx, done := trace.StartSpan(ctx, "source.Diagnostics", telemetry.File.Of(f.URI()))
defer done()
fh := snapshot.Handle(ctx, f)
cphs, err := snapshot.PackageHandles(ctx, f)
cphs, err := snapshot.PackageHandles(ctx, fh)
if err != nil {
return nil, "", err
}
@ -79,7 +79,7 @@ func Diagnostics(ctx context.Context, snapshot Snapshot, f File, disabledAnalyse
clearReports(snapshot, reports, e.File)
}
// Run diagnostics for the package that this URI belongs to.
if !diagnostics(ctx, snapshot, pkg, reports) {
if !diagnostics(ctx, snapshot, pkg, reports) && withAnalysis {
// If we don't have any list, parse, or type errors, run analyses.
if err := analyses(ctx, snapshot, cph, disabledAnalyses, reports); err != nil {
log.Error(ctx, "failed to run analyses", err, telemetry.File.Of(f.URI()))

6
internal/lsp/source/format.go
View File

@ -27,7 +27,8 @@ func Format(ctx context.Context, snapshot Snapshot, f File) ([]protocol.TextEdit
ctx, done := trace.StartSpan(ctx, "source.Format")
defer done()
cphs, err := snapshot.PackageHandles(ctx, f)
fh := snapshot.Handle(ctx, f)
cphs, err := snapshot.PackageHandles(ctx, fh)
if err != nil {
return nil, err
}
@ -101,7 +102,8 @@ func AllImportsFixes(ctx context.Context, snapshot Snapshot, f File) (allFixEdit
ctx, done := trace.StartSpan(ctx, "source.AllImportsFixes")
defer done()
cphs, err := snapshot.PackageHandles(ctx, f)
fh := snapshot.Handle(ctx, f)
cphs, err := snapshot.PackageHandles(ctx, fh)
if err != nil {
return nil, nil, err
}

3
internal/lsp/source/highlight.go
View File

@ -19,7 +19,8 @@ func Highlight(ctx context.Context, snapshot Snapshot, f File, pos protocol.Posi
ctx, done := trace.StartSpan(ctx, "source.Highlight")
defer done()
cphs, err := snapshot.PackageHandles(ctx, f)
fh := snapshot.Handle(ctx, f)
cphs, err := snapshot.PackageHandles(ctx, fh)
if err != nil {
return nil, err
}

3
internal/lsp/source/identifier.go
View File

@ -50,7 +50,8 @@ func Identifier(ctx context.Context, snapshot Snapshot, f File, pos protocol.Pos
ctx, done := trace.StartSpan(ctx, "source.Identifier")
defer done()
cphs, err := snapshot.PackageHandles(ctx, f)
fh := snapshot.Handle(ctx, f)
cphs, err := snapshot.PackageHandles(ctx, fh)
if err != nil {
return nil, err
}

3
internal/lsp/source/signature_help.go
View File

@ -31,7 +31,8 @@ func SignatureHelp(ctx context.Context, snapshot Snapshot, f File, pos protocol.
ctx, done := trace.StartSpan(ctx, "source.SignatureHelp")
defer done()
cphs, err := snapshot.PackageHandles(ctx, f)
fh := snapshot.Handle(ctx, f)
cphs, err := snapshot.PackageHandles(ctx, fh)
if err != nil {
return nil, err
}

2
internal/lsp/source/source_test.go
View File

@ -73,7 +73,7 @@ func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnosti
}
snapshot := r.view.Snapshot()
fileID := snapshot.Handle(r.ctx, f).Identity()
results, _, err := source.Diagnostics(r.ctx, snapshot, f, nil)
results, _, err := source.Diagnostics(r.ctx, snapshot, f, true, nil)
if err != nil {
t.Fatal(err)
}

3
internal/lsp/source/symbols.go
View File

@ -18,7 +18,8 @@ func DocumentSymbols(ctx context.Context, snapshot Snapshot, f File) ([]protocol
ctx, done := trace.StartSpan(ctx, "source.DocumentSymbols")
defer done()
cphs, err := snapshot.PackageHandles(ctx, f)
fh := snapshot.Handle(ctx, f)
cphs, err := snapshot.PackageHandles(ctx, fh)
if err != nil {
return nil, err
}

6
internal/lsp/source/view.go
View File

@ -38,7 +38,7 @@ type Snapshot interface {
// PackageHandles returns the CheckPackageHandles for the packages
// that this file belongs to.
PackageHandles(ctx context.Context, f File) ([]CheckPackageHandle, error)
PackageHandles(ctx context.Context, fh FileHandle) ([]CheckPackageHandle, error)
// GetActiveReverseDeps returns the active files belonging to the reverse
// dependencies of this file's package.
@ -291,7 +291,9 @@ type FileIdentity struct {
}
func (fileID FileIdentity) String() string {
return fmt.Sprintf("%s%f%s%s", fileID.URI, fileID.Version, fileID.Identifier, fileID.Kind)
// Version is not part of the FileIdentity string,
// as it can remain change even if the file does not.
return fmt.Sprintf("%s%s%s", fileID.URI, fileID.Identifier, fileID.Kind)
}
// File represents a source file of any type.

9
internal/lsp/text_synchronization.go
View File

@ -32,8 +32,10 @@ func (s *Server) didOpen(ctx context.Context, params *protocol.DidOpenTextDocume
if err != nil {
return err
}
// Run diagnostics on the newly-changed file.
go s.diagnostics(view.BackgroundContext(), view.Snapshot(), uri)
snapshot := view.Snapshot()
// Run diagnostics on the newly-opened file.
go s.diagnoseFile(snapshot, uri)
return nil
}
@ -78,9 +80,8 @@ func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDo
Type: protocol.Warning,
})
}
// Run diagnostics on the newly-changed file.
go s.diagnostics(view.BackgroundContext(), view.Snapshot(), uri)
go s.diagnoseFile(view.Snapshot(), uri)
return nil
}

8
internal/lsp/watched_files.go
View File

@ -34,7 +34,7 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did
if s.session.DidChangeOutOfBand(ctx, uri, action) {
// If we had been tracking the given file,
// recompute diagnostics to reflect updated file contents.
go s.diagnostics(view.BackgroundContext(), view.Snapshot(), uri)
go s.diagnoseFile(view.Snapshot(), uri)
}
case source.Delete:
f := view.FindFile(ctx, uri)
@ -42,7 +42,9 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did
if f == nil {
continue
}
cphs, err := view.Snapshot().PackageHandles(ctx, f)
snapshot := view.Snapshot()
fh := snapshot.Handle(ctx, f)
cphs, err := snapshot.PackageHandles(ctx, fh)
if err != nil {
log.Error(ctx, "didChangeWatchedFiles: CheckPackageHandles", err, telemetry.File)
continue
@ -79,7 +81,7 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did
}
// Refresh diagnostics for the package the file belonged to.
go s.diagnostics(view.BackgroundContext(), view.Snapshot(), otherFile.URI())
go s.diagnoseFile(view.Snapshot(), otherFile.URI())
}
}
}