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>
This commit is contained in:
Rebecca Stambler 2019-11-21 15:14:48 -05:00
parent f51c1a7cd2
commit 004141db30
6 changed files with 40 additions and 45 deletions

View File

@ -51,12 +51,12 @@ func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnosti
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]

View File

@ -91,7 +91,7 @@ func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnosti
}
return
}
if diff := tests.DiffDiagnostics(want, got); diff != "" {
if diff := tests.DiffDiagnostics(uri, want, got); diff != "" {
t.Error(diff)
}
}

View File

@ -14,10 +14,10 @@ import (
"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 @@ func Diagnostics(ctx context.Context, snapshot Snapshot, f File, disabledAnalyse
}
// 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 @@ func Diagnostics(ctx context.Context, snapshot Snapshot, f File, disabledAnalyse
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 @@ type diagnosticSet struct {
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 @@ func diagnostics(ctx context.Context, pkg Package, reports map[FileIdentity][]Di
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 @@ func diagnostics(ctx context.Context, pkg Package, reports map[FileIdentity][]Di
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 @@ func analyses(ctx context.Context, snapshot Snapshot, cph CheckPackageHandle, di
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 @@ func clearReports(snapshot Snapshot, reports map[FileIdentity][]Diagnostic, file
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,
},
},
}
}

View File

@ -85,7 +85,7 @@ func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnosti
}
return
}
if diff := tests.DiffDiagnostics(want, got); diff != "" {
if diff := tests.DiffDiagnostics(fileID.URI, want, got); diff != "" {
t.Error(diff)
}
}

View File

@ -13,34 +13,34 @@ import (
// 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 sortDiagnostics(d []source.Diagnostic) {
}
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 compareDiagnostic(a, b source.Diagnostic) int {
}
}
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 @@ func summarizeDiagnostics(i int, want []source.Diagnostic, got []source.Diagnost
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()
}

View File

@ -704,9 +704,6 @@ func (data *Data) collectDiagnostics(spn span.Span, msgSource, msg string) {
// 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,