diff --git a/internal/lsp/cache/errors.go b/internal/lsp/cache/errors.go index 5c377aff43..6fdee56f26 100644 --- a/internal/lsp/cache/errors.go +++ b/internal/lsp/cache/errors.go @@ -87,8 +87,12 @@ func sourceError(ctx context.Context, fset *token.FileSet, pkg *pkg, e interface if err != nil { return nil, err } + ph, err := pkg.File(spn.URI()) + if err != nil { + return nil, err + } return &source.Error{ - URI: spn.URI(), + File: ph.File().Identity(), Range: rng, Message: msg, Kind: kind, diff --git a/internal/lsp/cmd/test/check.go b/internal/lsp/cmd/test/check.go index ad260d2e52..74b27cb5ee 100644 --- a/internal/lsp/cmd/test/check.go +++ b/internal/lsp/cmd/test/check.go @@ -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.URI.Filename(), diag.Range.Start.Line+1, diag.Range.Start.Character+1, diag.Message) + expect := fmt.Sprintf("%v:%v:%v: %v", diag.File.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.URI.Filename(), diag.Range.Start.Line+1, diag.Message) + expect = fmt.Sprintf("%v:%v: %v", diag.File.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.URI.Filename(), "badimport") { + if strings.Contains(diag.File.URI.Filename(), "badimport") { continue } _, found := got[expect] diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index a77b490719..8334d6d649 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -55,18 +55,18 @@ func (s *Server) diagnostics(snapshot source.Snapshot, uri span.URI) error { s.undeliveredMu.Lock() defer s.undeliveredMu.Unlock() - for uri, diagnostics := range reports { - if err := s.publishDiagnostics(ctx, uri, diagnostics); err != nil { + for fileID, diagnostics := range reports { + if err := s.publishDiagnostics(ctx, fileID, diagnostics); err != nil { if s.undelivered == nil { - s.undelivered = make(map[span.URI][]source.Diagnostic) + s.undelivered = make(map[source.FileIdentity][]source.Diagnostic) } - s.undelivered[uri] = diagnostics + s.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, uri) + delete(s.undelivered, fileID) } // Anytime we compute diagnostics, make sure to also send along any // undelivered ones (only for remaining URIs). @@ -81,10 +81,11 @@ func (s *Server) diagnostics(snapshot source.Snapshot, uri span.URI) error { return nil } -func (s *Server) publishDiagnostics(ctx context.Context, uri span.URI, diagnostics []source.Diagnostic) error { +func (s *Server) publishDiagnostics(ctx context.Context, fileID source.FileIdentity, diagnostics []source.Diagnostic) error { s.client.PublishDiagnostics(ctx, &protocol.PublishDiagnosticsParams{ Diagnostics: toProtocolDiagnostics(ctx, diagnostics), - URI: protocol.NewURI(uri), + URI: protocol.NewURI(fileID.URI), + Version: fileID.Version, }) return nil } diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 2895c9e41b..e4cb28b546 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -62,7 +62,7 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) { r := &runner{ server: &Server{ session: session, - undelivered: make(map[span.URI][]source.Diagnostic), + undelivered: make(map[source.FileIdentity][]source.Diagnostic), }, data: data, ctx: ctx, @@ -78,11 +78,12 @@ func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnosti if err != nil { 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) if err != nil { t.Fatal(err) } - got := results[uri] + got := results[identity] // A special case to test that there are no diagnostics for a file. if len(want) == 1 && want[0].Source == "no_diagnostics" { if len(got) != 0 { @@ -336,7 +337,9 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span) { if err != nil { t.Fatal(err) } - diagnostics, _, err := source.Diagnostics(r.ctx, view.Snapshot(), f, nil) + snapshot := view.Snapshot() + fileID := snapshot.Handle(r.ctx, f).Identity() + diagnostics, _, err := source.Diagnostics(r.ctx, snapshot, f, nil) if err != nil { t.Fatal(err) } @@ -346,7 +349,7 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span) { }, Context: protocol.CodeActionContext{ Only: []protocol.CodeActionKind{protocol.QuickFix}, - Diagnostics: toProtocolDiagnostics(r.ctx, diagnostics[uri]), + Diagnostics: toProtocolDiagnostics(r.ctx, diagnostics[fileID]), }, }) if err != nil { diff --git a/internal/lsp/server.go b/internal/lsp/server.go index c13047c5ba..dacfd48b50 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -14,7 +14,6 @@ import ( "golang.org/x/tools/internal/jsonrpc2" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" - "golang.org/x/tools/internal/span" ) // NewClientServer @@ -82,7 +81,7 @@ type Server struct { // undelivered is a cache of any diagnostics that the server // failed to deliver for some reason. undeliveredMu sync.Mutex - undelivered map[span.URI][]source.Diagnostic + undelivered map[source.FileIdentity][]source.Diagnostic // folders is only valid between initialize and initialized, and holds the // set of folders to build views for when we are ready diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index b112c081a3..a3c8e8cd33 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -17,7 +17,7 @@ import ( ) type Diagnostic struct { - URI span.URI + File FileIdentity Range protocol.Range Message string Source string @@ -39,10 +39,11 @@ type RelatedInformation struct { Message string } -func Diagnostics(ctx context.Context, snapshot Snapshot, f File, disabledAnalyses map[string]struct{}) (map[span.URI][]Diagnostic, string, error) { +func Diagnostics(ctx context.Context, snapshot Snapshot, f File, 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) if err != nil { return nil, "", err @@ -51,7 +52,6 @@ func Diagnostics(ctx context.Context, snapshot Snapshot, f File, disabledAnalyse if err != nil { return nil, "", err } - // If we are missing dependencies, it may because the user's workspace is // not correctly configured. Report errors, if possible. var warningMsg string @@ -64,21 +64,21 @@ func Diagnostics(ctx context.Context, snapshot Snapshot, f File, disabledAnalyse pkg, err := cph.Check(ctx) if err != nil { log.Error(ctx, "no package for file", err) - return singleDiagnostic(f.URI(), "%s is not part of a package", f.URI()), "", nil + return singleDiagnostic(fh.Identity(), "%s is not part of a package", f.URI()), "", nil } // Prepare the reports we will send for the files in this package. - reports := make(map[span.URI][]Diagnostic) + reports := make(map[FileIdentity][]Diagnostic) for _, fh := range pkg.CompiledGoFiles() { - clearReports(snapshot, reports, fh.File().Identity().URI) + clearReports(snapshot, reports, fh.File().Identity()) } // Prepare any additional reports for the errors in this package. - for _, err := range pkg.GetErrors() { - if err.Kind != ListError { + for _, e := range pkg.GetErrors() { + if e.Kind != ListError { continue } - clearReports(snapshot, reports, err.URI) + clearReports(snapshot, reports, e.File) } // Run diagnostics for the package that this URI belongs to. @@ -96,7 +96,7 @@ func Diagnostics(ctx context.Context, snapshot Snapshot, f File, disabledAnalyse return nil, warningMsg, err } for _, fh := range pkg.CompiledGoFiles() { - clearReports(snapshot, reports, fh.File().Identity().URI) + clearReports(snapshot, reports, fh.File().Identity()) } diagnostics(ctx, pkg, reports) } @@ -107,25 +107,25 @@ type diagnosticSet struct { listErrors, parseErrors, typeErrors []*Diagnostic } -func diagnostics(ctx context.Context, pkg Package, reports map[span.URI][]Diagnostic) bool { +func diagnostics(ctx context.Context, pkg Package, reports map[FileIdentity][]Diagnostic) bool { ctx, done := trace.StartSpan(ctx, "source.diagnostics", telemetry.Package.Of(pkg.ID())) _ = ctx // circumvent SA4006 defer done() - diagSets := make(map[span.URI]*diagnosticSet) - for _, err := range pkg.GetErrors() { + diagSets := make(map[FileIdentity]*diagnosticSet) + for _, e := range pkg.GetErrors() { diag := &Diagnostic{ - URI: err.URI, - Message: err.Message, - Range: err.Range, + File: e.File, + Message: e.Message, + Range: e.Range, Severity: protocol.SeverityError, } - set, ok := diagSets[diag.URI] + set, ok := diagSets[e.File] if !ok { set = &diagnosticSet{} - diagSets[diag.URI] = set + diagSets[e.File] = set } - switch err.Kind { + switch e.Kind { case ParseError: set.parseErrors = append(set.parseErrors, diag) diag.Source = "syntax" @@ -138,7 +138,7 @@ func diagnostics(ctx context.Context, pkg Package, reports map[span.URI][]Diagno } } var nonEmptyDiagnostics bool // track if we actually send non-empty diagnostics - for uri, set := range diagSets { + for fileID, set := range diagSets { // Don't report type errors if there are parse errors or list errors. diags := set.typeErrors if len(set.parseErrors) > 0 { @@ -150,15 +150,15 @@ func diagnostics(ctx context.Context, pkg Package, reports map[span.URI][]Diagno nonEmptyDiagnostics = true } for _, diag := range diags { - if _, ok := reports[uri]; ok { - reports[uri] = append(reports[uri], *diag) + if _, ok := reports[fileID]; ok { + reports[fileID] = append(reports[fileID], *diag) } } } return nonEmptyDiagnostics } -func analyses(ctx context.Context, snapshot Snapshot, cph CheckPackageHandle, disabledAnalyses map[string]struct{}, reports map[span.URI][]Diagnostic) error { +func analyses(ctx context.Context, snapshot Snapshot, cph CheckPackageHandle, disabledAnalyses map[string]struct{}, reports map[FileIdentity][]Diagnostic) error { var analyzers []*analysis.Analyzer for _, a := range snapshot.View().Options().Analyzers { if _, ok := disabledAnalyses[a.Name]; ok { @@ -181,8 +181,8 @@ func analyses(ctx context.Context, snapshot Snapshot, cph CheckPackageHandle, di if onlyDeletions(e.SuggestedFixes) { tags = append(tags, protocol.Unnecessary) } - addReport(snapshot, reports, Diagnostic{ - URI: e.URI, + addReport(ctx, snapshot, reports, Diagnostic{ + File: e.File, Range: e.Range, Message: e.Message, Source: e.Category, @@ -195,27 +195,28 @@ func analyses(ctx context.Context, snapshot Snapshot, cph CheckPackageHandle, di return nil } -func clearReports(snapshot Snapshot, reports map[span.URI][]Diagnostic, uri span.URI) { - if snapshot.View().Ignore(uri) { +func clearReports(snapshot Snapshot, reports map[FileIdentity][]Diagnostic, fileID FileIdentity) { + if snapshot.View().Ignore(fileID.URI) { return } - reports[uri] = []Diagnostic{} + reports[fileID] = []Diagnostic{} } -func addReport(snapshot Snapshot, reports map[span.URI][]Diagnostic, diagnostic Diagnostic) { - if snapshot.View().Ignore(diagnostic.URI) { - return +func addReport(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][]Diagnostic, diagnostic Diagnostic) error { + if snapshot.View().Ignore(diagnostic.File.URI) { + return nil } - if _, ok := reports[diagnostic.URI]; ok { - reports[diagnostic.URI] = append(reports[diagnostic.URI], diagnostic) + if _, ok := reports[diagnostic.File]; ok { + reports[diagnostic.File] = append(reports[diagnostic.File], diagnostic) } + return nil } -func singleDiagnostic(uri span.URI, format string, a ...interface{}) map[span.URI][]Diagnostic { - return map[span.URI][]Diagnostic{ - uri: []Diagnostic{{ - Source: "LSP", - URI: uri, +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, diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go index 021ec718c2..51059f368f 100644 --- a/internal/lsp/source/format.go +++ b/internal/lsp/source/format.go @@ -381,8 +381,8 @@ func PackageExports(ctx context.Context, view View, pkg, filename string) ([]imp // hasParseErrors returns true if the given file has parse errors. func hasParseErrors(pkg Package, uri span.URI) bool { - for _, err := range pkg.GetErrors() { - if err.URI == uri && err.Kind == ParseError { + for _, e := range pkg.GetErrors() { + if e.File.URI == uri && e.Kind == ParseError { return true } } diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index fb825a7e44..b2c0f539a2 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -71,11 +71,13 @@ func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnosti if err != nil { t.Fatal(err) } - results, _, err := source.Diagnostics(r.ctx, r.view.Snapshot(), f, nil) + snapshot := r.view.Snapshot() + fileID := snapshot.Handle(r.ctx, f).Identity() + results, _, err := source.Diagnostics(r.ctx, snapshot, f, nil) if err != nil { t.Fatal(err) } - got := results[uri] + got := results[fileID] // A special case to test that there are no diagnostics for a file. if len(want) == 1 && want[0].Source == "no_diagnostics" { if len(got) != 0 { diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index b41b8cf730..be5c39369f 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -287,8 +287,8 @@ type FileIdentity struct { Kind FileKind } -func (identity FileIdentity) String() string { - return fmt.Sprintf("%s%f%s%s", identity.URI, identity.Version, identity.Identifier, identity.Kind) +func (fileID FileIdentity) String() string { + return fmt.Sprintf("%s%f%s%s", fileID.URI, fileID.Version, fileID.Identifier, fileID.Kind) } // File represents a source file of any type. @@ -342,7 +342,7 @@ type Package interface { } type Error struct { - URI span.URI + File FileIdentity Range protocol.Range Kind ErrorKind Message string @@ -362,7 +362,7 @@ const ( ) func (e *Error) Error() string { - return fmt.Sprintf("%s:%s: %s", e.URI, e.Range, e.Message) + return fmt.Sprintf("%s:%s: %s", e.File, e.Range, e.Message) } type BuiltinPackage interface { diff --git a/internal/lsp/tests/diagnostics.go b/internal/lsp/tests/diagnostics.go index 92f948c1f7..8ec1c6ede3 100644 --- a/internal/lsp/tests/diagnostics.go +++ b/internal/lsp/tests/diagnostics.go @@ -32,7 +32,7 @@ func DiffDiagnostics(want, got []source.Diagnostic) string { return summarizeDiagnostics(i, 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.URI), "badimport") { + if strings.Contains(string(g.File.URI), "badimport") { continue } if protocol.ComparePosition(w.Range.Start, g.Range.Start) != 0 { @@ -54,7 +54,7 @@ func sortDiagnostics(d []source.Diagnostic) { } func compareDiagnostic(a, b source.Diagnostic) int { - if r := span.CompareURI(a.URI, b.URI); r != 0 { + if r := span.CompareURI(a.File.URI, b.File.URI); r != 0 { return r } if r := protocol.CompareRange(a.Range, b.Range); r != 0 { @@ -80,11 +80,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.URI, d.Range, d.Message) + fmt.Fprintf(msg, " %s:%v: %s\n", d.File.URI, d.Range, d.Message) } fmt.Fprintf(msg, "got:\n") for _, d := range got { - fmt.Fprintf(msg, " %s:%v: %s\n", d.URI, d.Range, d.Message) + fmt.Fprintf(msg, " %s:%v: %s\n", d.File.URI, d.Range, d.Message) } return msg.String() } diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go index d7c75eefc3..6aeebbdfc4 100644 --- a/internal/lsp/tests/tests.go +++ b/internal/lsp/tests/tests.go @@ -704,7 +704,9 @@ 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{ - URI: spn.URI(), + File: source.FileIdentity{ + URI: spn.URI(), + }, Range: protocol.Range{ Start: protocol.Position{ Line: float64(spn.Start().Line()) - 1, diff --git a/internal/lsp/watched_files.go b/internal/lsp/watched_files.go index a5efe0511a..87fa0f4b78 100644 --- a/internal/lsp/watched_files.go +++ b/internal/lsp/watched_files.go @@ -70,7 +70,9 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did // If this was the only file in the package, clear its diagnostics. if otherFile == nil { - if err := s.publishDiagnostics(ctx, uri, []source.Diagnostic{}); err != nil { + if err := s.publishDiagnostics(ctx, source.FileIdentity{ + URI: uri, + }, []source.Diagnostic{}); err != nil { log.Error(ctx, "failed to clear diagnostics", err, telemetry.URI.Of(uri)) } return nil