qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-11-18 09:04:49 -07:00
Code Releases Activity

internal/lsp: use version numbers in diagnostic messages

Browse Source 
This change uses the FileIdentity when reporting an error message, so
that the version number can be propagated to through the
publishDiagnostics notification.

Change-Id: I6a2103e304717ca09895008ea40336e3ace3c66d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208260
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 00:08:58 -05:00
parent cb0506829d
commit eaeb383209
12 changed files with 83 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
internal/lsp/cache/errors.go vendored
View File

@ -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,

6
internal/lsp/cmd/test/check.go
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.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]

15
internal/lsp/diagnostics.go
View File

@ -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
}

11
internal/lsp/lsp_test.go
View File

@ -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 {

3
internal/lsp/server.go
View File

@ -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

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

@ -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,

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

@ -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
}
}

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

@ -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 {

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

@ -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 {

8
internal/lsp/tests/diagnostics.go
View File

@ -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()
}

4
internal/lsp/tests/tests.go
View File

@ -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,

4
internal/lsp/watched_files.go
View File

@ -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