internal/lsp: delete the source.Diagnostic.File field
Since diagnostics are published with the URI separately, there's no need
for us to keep the FileIdentity around in two places.
Change-Id: I5724b9582e5eee49f66fcf9f08625f14a69e3fc0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208263
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
diff --git a/internal/lsp/cmd/test/check.go b/internal/lsp/cmd/test/check.go
index 74b27cb..98d390c 100644
--- a/internal/lsp/cmd/test/check.go
+++ b/internal/lsp/cmd/test/check.go
@@ -51,12 +51,12 @@
got[l] = struct{}{}
}
for _, diag := range want {
- expect := fmt.Sprintf("%v:%v:%v: %v", diag.File.URI.Filename(), diag.Range.Start.Line+1, diag.Range.Start.Character+1, diag.Message)
+ expect := fmt.Sprintf("%v:%v:%v: %v", uri.Filename(), diag.Range.Start.Line+1, diag.Range.Start.Character+1, diag.Message)
if diag.Range.Start.Character == 0 {
- expect = fmt.Sprintf("%v:%v: %v", diag.File.URI.Filename(), diag.Range.Start.Line+1, diag.Message)
+ expect = fmt.Sprintf("%v:%v: %v", uri.Filename(), diag.Range.Start.Line+1, diag.Message)
}
// Skip the badimport test for now, until we do a better job with diagnostic ranges.
- if strings.Contains(diag.File.URI.Filename(), "badimport") {
+ if strings.Contains(uri.Filename(), "badimport") {
continue
}
_, found := got[expect]
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index e4cb28b..938da45 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -91,7 +91,7 @@
}
return
}
- if diff := tests.DiffDiagnostics(want, got); diff != "" {
+ if diff := tests.DiffDiagnostics(uri, want, got); diff != "" {
t.Error(diff)
}
}
diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go
index a3c8e8c..6019b63 100644
--- a/internal/lsp/source/diagnostics.go
+++ b/internal/lsp/source/diagnostics.go
@@ -14,10 +14,10 @@
"golang.org/x/tools/internal/span"
"golang.org/x/tools/internal/telemetry/log"
"golang.org/x/tools/internal/telemetry/trace"
+ errors "golang.org/x/xerrors"
)
type Diagnostic struct {
- File FileIdentity
Range protocol.Range
Message string
Source string
@@ -82,7 +82,7 @@
}
// Run diagnostics for the package that this URI belongs to.
- if !diagnostics(ctx, pkg, reports) {
+ if !diagnostics(ctx, snapshot, pkg, reports) {
// 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()))
@@ -98,7 +98,7 @@
for _, fh := range pkg.CompiledGoFiles() {
clearReports(snapshot, reports, fh.File().Identity())
}
- diagnostics(ctx, pkg, reports)
+ diagnostics(ctx, snapshot, pkg, reports)
}
return reports, warningMsg, nil
}
@@ -107,7 +107,7 @@
listErrors, parseErrors, typeErrors []*Diagnostic
}
-func diagnostics(ctx context.Context, pkg Package, reports map[FileIdentity][]Diagnostic) bool {
+func diagnostics(ctx context.Context, snapshot Snapshot, pkg Package, reports map[FileIdentity][]Diagnostic) bool {
ctx, done := trace.StartSpan(ctx, "source.diagnostics", telemetry.Package.Of(pkg.ID()))
_ = ctx // circumvent SA4006
defer done()
@@ -115,7 +115,6 @@
diagSets := make(map[FileIdentity]*diagnosticSet)
for _, e := range pkg.GetErrors() {
diag := &Diagnostic{
- File: e.File,
Message: e.Message,
Range: e.Range,
Severity: protocol.SeverityError,
@@ -149,11 +148,7 @@
if len(diags) > 0 {
nonEmptyDiagnostics = true
}
- for _, diag := range diags {
- if _, ok := reports[fileID]; ok {
- reports[fileID] = append(reports[fileID], *diag)
- }
- }
+ addReports(ctx, reports, snapshot, fileID, diags...)
}
return nonEmptyDiagnostics
}
@@ -181,8 +176,7 @@
if onlyDeletions(e.SuggestedFixes) {
tags = append(tags, protocol.Unnecessary)
}
- addReport(ctx, snapshot, reports, Diagnostic{
- File: e.File,
+ addReports(ctx, reports, snapshot, e.File, &Diagnostic{
Range: e.Range,
Message: e.Message,
Source: e.Category,
@@ -202,25 +196,32 @@
reports[fileID] = []Diagnostic{}
}
-func addReport(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][]Diagnostic, diagnostic Diagnostic) error {
- if snapshot.View().Ignore(diagnostic.File.URI) {
+func addReports(ctx context.Context, reports map[FileIdentity][]Diagnostic, snapshot Snapshot, fileID FileIdentity, diagnostics ...*Diagnostic) error {
+ if snapshot.View().Ignore(fileID.URI) {
return nil
}
- if _, ok := reports[diagnostic.File]; ok {
- reports[diagnostic.File] = append(reports[diagnostic.File], diagnostic)
+ if _, ok := reports[fileID]; !ok {
+ return errors.Errorf("diagnostics for unexpected file %s", fileID.URI)
+ }
+ for _, diag := range diagnostics {
+ if diag == nil {
+ continue
+ }
+ reports[fileID] = append(reports[fileID], *diag)
}
return nil
}
func singleDiagnostic(fileID FileIdentity, format string, a ...interface{}) map[FileIdentity][]Diagnostic {
return map[FileIdentity][]Diagnostic{
- fileID: []Diagnostic{{
- File: fileID,
- Source: "gopls",
- Range: protocol.Range{},
- Message: fmt.Sprintf(format, a...),
- Severity: protocol.SeverityError,
- }},
+ fileID: []Diagnostic{
+ {
+ Source: "gopls",
+ Range: protocol.Range{},
+ Message: fmt.Sprintf(format, a...),
+ Severity: protocol.SeverityError,
+ },
+ },
}
}
diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go
index b2c0f53..67b1aea 100644
--- a/internal/lsp/source/source_test.go
+++ b/internal/lsp/source/source_test.go
@@ -85,7 +85,7 @@
}
return
}
- if diff := tests.DiffDiagnostics(want, got); diff != "" {
+ if diff := tests.DiffDiagnostics(fileID.URI, want, got); diff != "" {
t.Error(diff)
}
}
diff --git a/internal/lsp/tests/diagnostics.go b/internal/lsp/tests/diagnostics.go
index 8ec1c6e..6b0c1cf 100644
--- a/internal/lsp/tests/diagnostics.go
+++ b/internal/lsp/tests/diagnostics.go
@@ -13,34 +13,34 @@
// DiffDiagnostics prints the diff between expected and actual diagnostics test
// results.
-func DiffDiagnostics(want, got []source.Diagnostic) string {
+func DiffDiagnostics(uri span.URI, want, got []source.Diagnostic) string {
sortDiagnostics(want)
sortDiagnostics(got)
if len(got) != len(want) {
- return summarizeDiagnostics(-1, want, got, "different lengths got %v want %v", len(got), len(want))
+ return summarizeDiagnostics(-1, uri, want, got, "different lengths got %v want %v", len(got), len(want))
}
for i, w := range want {
g := got[i]
if w.Message != g.Message {
- return summarizeDiagnostics(i, want, got, "incorrect Message got %v want %v", g.Message, w.Message)
+ return summarizeDiagnostics(i, uri, want, got, "incorrect Message got %v want %v", g.Message, w.Message)
}
if w.Severity != g.Severity {
- return summarizeDiagnostics(i, want, got, "incorrect Severity got %v want %v", g.Severity, w.Severity)
+ return summarizeDiagnostics(i, uri, want, got, "incorrect Severity got %v want %v", g.Severity, w.Severity)
}
if w.Source != g.Source {
- return summarizeDiagnostics(i, want, got, "incorrect Source got %v want %v", g.Source, w.Source)
+ return summarizeDiagnostics(i, uri, want, got, "incorrect Source got %v want %v", g.Source, w.Source)
}
// Don't check the range on the badimport test.
- if strings.Contains(string(g.File.URI), "badimport") {
+ if strings.Contains(uri.Filename(), "badimport") {
continue
}
if protocol.ComparePosition(w.Range.Start, g.Range.Start) != 0 {
- return summarizeDiagnostics(i, want, got, "incorrect Start got %v want %v", g.Range.Start, w.Range.Start)
+ return summarizeDiagnostics(i, uri, want, got, "incorrect Start got %v want %v", g.Range.Start, w.Range.Start)
}
if !protocol.IsPoint(g.Range) { // Accept any 'want' range if the diagnostic returns a zero-length range.
if protocol.ComparePosition(w.Range.End, g.Range.End) != 0 {
- return summarizeDiagnostics(i, want, got, "incorrect End got %v want %v", g.Range.End, w.Range.End)
+ return summarizeDiagnostics(i, uri, want, got, "incorrect End got %v want %v", g.Range.End, w.Range.End)
}
}
}
@@ -54,9 +54,6 @@
}
func compareDiagnostic(a, b source.Diagnostic) int {
- if r := span.CompareURI(a.File.URI, b.File.URI); r != 0 {
- return r
- }
if r := protocol.CompareRange(a.Range, b.Range); r != 0 {
return r
}
@@ -70,7 +67,7 @@
}
}
-func summarizeDiagnostics(i int, want []source.Diagnostic, got []source.Diagnostic, reason string, args ...interface{}) string {
+func summarizeDiagnostics(i int, uri span.URI, want []source.Diagnostic, got []source.Diagnostic, reason string, args ...interface{}) string {
msg := &bytes.Buffer{}
fmt.Fprint(msg, "diagnostics failed")
if i >= 0 {
@@ -80,11 +77,11 @@
fmt.Fprintf(msg, reason, args...)
fmt.Fprint(msg, ":\nexpected:\n")
for _, d := range want {
- fmt.Fprintf(msg, " %s:%v: %s\n", d.File.URI, d.Range, d.Message)
+ fmt.Fprintf(msg, " %s:%v: %s\n", uri, d.Range, d.Message)
}
fmt.Fprintf(msg, "got:\n")
for _, d := range got {
- fmt.Fprintf(msg, " %s:%v: %s\n", d.File.URI, d.Range, d.Message)
+ fmt.Fprintf(msg, " %s:%v: %s\n", uri, d.Range, d.Message)
}
return msg.String()
}
diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go
index 6aeebbd..7919440 100644
--- a/internal/lsp/tests/tests.go
+++ b/internal/lsp/tests/tests.go
@@ -704,9 +704,6 @@
// This is not the correct way to do this,
// but it seems excessive to do the full conversion here.
want := source.Diagnostic{
- File: source.FileIdentity{
- URI: spn.URI(),
- },
Range: protocol.Range{
Start: protocol.Position{
Line: float64(spn.Start().Line()) - 1,
