internal/lsp/source: eliminate GetTypeCheckDiagnostics
Most callers of source.Package.GetDiagnostics do it via
GetTypeCheckDiagnostics. Push its logic up or down as appropriate and
delete it.
Rather than requiring fully populated maps of diagnostics, which was
rather subtle, call storeDiagnostics for every Go file in the package.
Change-Id: If43b0cc922af1013e80f969362246538df14985b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/297878
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
diff --git a/internal/lsp/cache/pkg.go b/internal/lsp/cache/pkg.go
index 5badb33..1ac6c83 100644
--- a/internal/lsp/cache/pkg.go
+++ b/internal/lsp/cache/pkg.go
@@ -86,8 +86,15 @@
return syntax
}
-func (p *pkg) GetDiagnostics() []*source.Diagnostic {
- return p.diagnostics
+func (p *pkg) GetDiagnostics() map[span.URI][]*source.Diagnostic {
+ diags := map[span.URI][]*source.Diagnostic{}
+ for _, diag := range p.diagnostics {
+ // As of writing, source.Analyze modifies the diagnostics it receives.
+ // Shallow copy to avoid races.
+ clone := *diag
+ diags[diag.URI] = append(diags[diag.URI], &clone)
+ }
+ return diags
}
func (p *pkg) GetTypes() *types.Package {
diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go
index 6c41dc3..952922b 100644
--- a/internal/lsp/code_action.go
+++ b/internal/lsp/code_action.go
@@ -126,7 +126,7 @@
return nil, err
}
if (wanted[protocol.QuickFix] || wanted[protocol.SourceFixAll]) && len(diagnostics) > 0 {
- pkgQuickFixes, err := quickFixesForDiagnostics(ctx, snapshot, diagnostics, pkg.GetDiagnostics())
+ pkgQuickFixes, err := quickFixesForDiagnostics(ctx, snapshot, diagnostics, pkg.GetDiagnostics()[fh.URI()])
if err != nil {
return nil, err
}
diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go
index 8b93058..f910398 100644
--- a/internal/lsp/diagnostics.go
+++ b/internal/lsp/diagnostics.go
@@ -245,14 +245,20 @@
func (s *Server) diagnosePkg(ctx context.Context, snapshot source.Snapshot, pkg source.Package, alwaysAnalyze bool) {
ctx, done := event.Start(ctx, "Server.diagnosePkg", tag.Snapshot.Of(snapshot.ID()), tag.Package.Of(pkg.ID()))
defer done()
+ enableDiagnostics := false
includeAnalysis := alwaysAnalyze // only run analyses for packages with open files
for _, pgf := range pkg.CompiledGoFiles() {
+ enableDiagnostics = enableDiagnostics || !snapshot.IgnoredFile(pgf.URI)
includeAnalysis = includeAnalysis || snapshot.IsOpen(pgf.URI)
}
+ // Don't show any diagnostics on ignored files.
+ if !enableDiagnostics {
+ return
+ }
- typeCheckDiagnostics := source.GetTypeCheckDiagnostics(ctx, snapshot, pkg)
- for uri, diags := range typeCheckDiagnostics {
- s.storeDiagnostics(snapshot, uri, typeCheckSource, diags)
+ typeCheckDiagnostics := pkg.GetDiagnostics()
+ for _, cgf := range pkg.CompiledGoFiles() {
+ s.storeDiagnostics(snapshot, cgf.URI, typeCheckSource, typeCheckDiagnostics[cgf.URI])
}
if includeAnalysis && !pkg.HasListOrParseErrors() {
reports, err := source.Analyze(ctx, snapshot, pkg, typeCheckDiagnostics)
@@ -260,8 +266,8 @@
event.Error(ctx, "warning: diagnose package", err, tag.Snapshot.Of(snapshot.ID()), tag.Package.Of(pkg.ID()))
return
}
- for uri, diags := range reports {
- s.storeDiagnostics(snapshot, uri, analysisSource, diags)
+ for _, cgf := range pkg.CompiledGoFiles() {
+ s.storeDiagnostics(snapshot, cgf.URI, analysisSource, reports[cgf.URI])
}
}
diff --git a/internal/lsp/mod/diagnostics.go b/internal/lsp/mod/diagnostics.go
index e515ada..ca636f4 100644
--- a/internal/lsp/mod/diagnostics.go
+++ b/internal/lsp/mod/diagnostics.go
@@ -92,11 +92,7 @@
}
if err == nil {
for _, pkg := range wspkgs {
- for _, diag := range pkg.GetDiagnostics() {
- if diag.URI == fh.URI() {
- diagnostics = append(diagnostics, diag)
- }
- }
+ diagnostics = append(diagnostics, pkg.GetDiagnostics()[fh.URI()]...)
}
}
diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go
index f9b82de..3e90d64 100644
--- a/internal/lsp/source/diagnostics.go
+++ b/internal/lsp/source/diagnostics.go
@@ -24,25 +24,6 @@
Message string
}
-func GetTypeCheckDiagnostics(ctx context.Context, snapshot Snapshot, pkg Package) map[span.URI][]*Diagnostic {
- onlyIgnoredFiles := true
- for _, pgf := range pkg.CompiledGoFiles() {
- onlyIgnoredFiles = onlyIgnoredFiles && snapshot.IgnoredFile(pgf.URI)
- }
- if onlyIgnoredFiles {
- return nil
- }
-
- diagSets := emptyDiagnostics(pkg)
- for _, diag := range pkg.GetDiagnostics() {
- diagSets[diag.URI] = append(diagSets[diag.URI], diag)
- }
- for uri, diags := range diagSets {
- diagSets[uri] = cloneDiagnostics(diags)
- }
- return diagSets
-}
-
func Analyze(ctx context.Context, snapshot Snapshot, pkg Package, typeCheckDiagnostics map[span.URI][]*Diagnostic) (map[span.URI][]*Diagnostic, error) {
// Exit early if the context has been canceled. This also protects us
// from a race on Options, see golang/go#36699.
@@ -57,7 +38,7 @@
}
analysisDiagnostics = cloneDiagnostics(analysisDiagnostics)
- reports := emptyDiagnostics(pkg)
+ reports := map[span.URI][]*Diagnostic{}
// Report diagnostics and errors from root analyzers.
for _, diag := range analysisDiagnostics {
// If the diagnostic comes from a "convenience" analyzer, it is not
@@ -134,26 +115,16 @@
if err != nil {
return VersionedFileIdentity{}, nil, err
}
- typeCheckDiagnostics := GetTypeCheckDiagnostics(ctx, snapshot, pkg)
- diagnostics := typeCheckDiagnostics[fh.URI()]
+ diagnostics := pkg.GetDiagnostics()
+ fileDiags := diagnostics[fh.URI()]
if !pkg.HasListOrParseErrors() {
- reports, err := Analyze(ctx, snapshot, pkg, typeCheckDiagnostics)
+ analysisDiags, err := Analyze(ctx, snapshot, pkg, diagnostics)
if err != nil {
return VersionedFileIdentity{}, nil, err
}
- diagnostics = append(diagnostics, reports[fh.URI()]...)
+ fileDiags = append(fileDiags, analysisDiags[fh.URI()]...)
}
- return fh.VersionedFileIdentity(), diagnostics, nil
-}
-
-func emptyDiagnostics(pkg Package) map[span.URI][]*Diagnostic {
- diags := map[span.URI][]*Diagnostic{}
- for _, pgf := range pkg.CompiledGoFiles() {
- if _, ok := diags[pgf.URI]; !ok {
- diags[pgf.URI] = nil
- }
- }
- return diags
+ return fh.VersionedFileIdentity(), fileDiags, nil
}
// onlyDeletions returns true if all of the suggested fixes are deletions.
diff --git a/internal/lsp/source/rename_check.go b/internal/lsp/source/rename_check.go
index 0f1bf92..a46254c 100644
--- a/internal/lsp/source/rename_check.go
+++ b/internal/lsp/source/rename_check.go
@@ -812,7 +812,7 @@
// type-checker.
//
// Only proceed if all packages have no errors.
- if diags := pkg.GetDiagnostics(); len(diags) > 0 {
+ if pkg.HasListOrParseErrors() || pkg.HasTypeErrors() {
r.errorf(token.NoPos, // we don't have a position for this error.
"renaming %q to %q not possible because %q has errors",
r.from, r.to, pkg.PkgPath())
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index 2e06670..a0e5252 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -548,7 +548,7 @@
CompiledGoFiles() []*ParsedGoFile
File(uri span.URI) (*ParsedGoFile, error)
GetSyntax() []*ast.File
- GetDiagnostics() []*Diagnostic
+ GetDiagnostics() map[span.URI][]*Diagnostic
GetTypes() *types.Package
GetTypesInfo() *types.Info
GetTypesSizes() types.Sizes
