internal/lsp: add support for RelatedInformation in diagnostics
The type checker sometimes emits secondary diagnostics. For instance,
if a function is defined twice, then when it sees the second definition
it emits a diagnostic at the second definition and a secondary diagnostic
pointing to the first diagnostic. Presently gopls treats these as two
separate diagnostics. The changed code still produces two diagnostics,
but now the secondary diagnostic is also converted into a
RelatedInformation so the user sees a xpointer to the earlier definition.
Updates https://github.com/golang/go/issues/39062.
Change-Id: Ic421ec91d2b46c28681ab3ec010d5b02c0442e68
Reviewed-on: https://go-review.googlesource.com/c/tools/+/251617
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/gopls/internal/regtest/diagnostics_test.go b/gopls/internal/regtest/diagnostics_test.go
index a7973fe..96d61b1 100644
--- a/gopls/internal/regtest/diagnostics_test.go
+++ b/gopls/internal/regtest/diagnostics_test.go
@@ -1026,21 +1026,21 @@
writeGoVim(env, "p/p.go", p)
writeGoVim(env, "main.go", main)
writeGoVim(env, "p/p_test.go", `package p
-
+
import "testing"
-
+
func TestDoIt(t *testing.T) {
DoIt(5)
}
`)
writeGoVim(env, "p/x_test.go", `package p_test
-
+
import (
"testing"
"mod.com/p"
)
-
+
func TestDoIt(t *testing.T) {
p.DoIt(5)
}
@@ -1274,3 +1274,42 @@
env.Await(env.DiagnosticAtRegexp("main.go", `t{"msg"}`))
})
}
+
+// Test some secondary diagnostics
+func TestSecondaryDiagnostics(t *testing.T) {
+ const dir = `
+-- mod --
+module mod.com
+-- main.go --
+package main
+func main() {
+ panic("not here")
+}
+-- other.go --
+package main
+func main() {}
+`
+ runner.Run(t, dir, func(t *testing.T, env *Env) {
+ log.SetFlags(log.Lshortfile)
+ env.OpenFile("main.go")
+ env.OpenFile("other.go")
+ env.Await(CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromInitialWorkspaceLoad), 1))
+ x := env.DiagnosticsFor("main.go")
+ if len(x.Diagnostics) != 1 {
+ t.Errorf("main.go, got %d diagnostics, expected 1", len(x.Diagnostics))
+ }
+ keep := x.Diagnostics[0]
+ y := env.DiagnosticsFor("other.go")
+ if len(y.Diagnostics) != 1 {
+ t.Errorf("other.go: got %d diagnostics, expected 1", len(y.Diagnostics))
+ }
+ if len(y.Diagnostics[0].RelatedInformation) != 1 {
+ t.Errorf("got %d RelatedInformations, expected 1", len(y.Diagnostics[0].RelatedInformation))
+ }
+ // check that the RelatedInformation matches the error from main.go
+ c := y.Diagnostics[0].RelatedInformation[0]
+ if c.Location.Range != keep.Range {
+ t.Errorf("locations don't match. Got %v expected %v", c.Location.Range, keep.Range)
+ }
+ })
+}
diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go
index 75642c2..8311416 100644
--- a/internal/lsp/cache/check.go
+++ b/internal/lsp/cache/check.go
@@ -405,6 +405,7 @@
// We don't care about a package's errors unless we have parsed it in full.
if mode == source.ParseFull {
+ expandErrors(rawErrors)
for _, e := range rawErrors {
srcErr, err := sourceError(ctx, snapshot, pkg, e)
if err != nil {
@@ -412,8 +413,8 @@
continue
}
pkg.errors = append(pkg.errors, srcErr)
- if err, ok := e.(types.Error); ok {
- pkg.typeErrors = append(pkg.typeErrors, err)
+ if err, ok := e.(extendedError); ok {
+ pkg.typeErrors = append(pkg.typeErrors, err.primary)
}
}
}
@@ -421,6 +422,48 @@
return pkg, nil
}
+type extendedError struct {
+ primary types.Error
+ secondaries []types.Error
+}
+
+func (e extendedError) Error() string {
+ return e.primary.Error()
+}
+
+// expandErrors duplicates "secondary" errors by mapping them to their main
+// error. Some errors returned by the type checker are followed by secondary
+// errors which give more information about the error. These are errors in
+// their own right, and they are marked by starting with \t. For instance, when
+// there is a multiply-defined function, the secondary error points back to the
+// definition first noticed.
+//
+// This code associates the secondary error with its primary error, which can
+// then be used as RelatedInformation when the error becomes a diagnostic.
+func expandErrors(errs []error) []error {
+ for i := 0; i < len(errs); {
+ e, ok := errs[i].(types.Error)
+ if !ok {
+ i++
+ continue
+ }
+ enew := extendedError{
+ primary: e,
+ }
+ j := i + 1
+ for ; j < len(errs); j++ {
+ spl, ok := errs[j].(types.Error)
+ if !ok || len(spl.Msg) == 0 || spl.Msg[0] != '\t' {
+ break
+ }
+ enew.secondaries = append(enew.secondaries, spl)
+ }
+ errs[i] = enew
+ i = j
+ }
+ return errs
+}
+
// resolveImportPath resolves an import path in pkg to a package from deps.
// It should produce the same results as resolveImportPath:
// https://cs.opensource.google/go/go/+/master:src/cmd/go/internal/load/pkg.go;drc=641918ee09cb44d282a30ee8b66f99a0b63eaef9;l=990.
diff --git a/internal/lsp/cache/errors.go b/internal/lsp/cache/errors.go
index cf02417..1f55a58 100644
--- a/internal/lsp/cache/errors.go
+++ b/internal/lsp/cache/errors.go
@@ -94,7 +94,32 @@
if err != nil {
return nil, err
}
-
+ case extendedError:
+ perr := e.primary
+ msg = perr.Msg
+ kind = source.TypeError
+ if !perr.Pos.IsValid() {
+ return nil, fmt.Errorf("invalid position for type error %v", e)
+ }
+ spn, err = typeErrorRange(snapshot, fset, pkg, perr.Pos)
+ if err != nil {
+ return nil, err
+ }
+ for _, s := range e.secondaries {
+ var x source.RelatedInformation
+ x.Message = s.Msg
+ xspn, err := typeErrorRange(snapshot, fset, pkg, s.Pos)
+ if err != nil {
+ return nil, fmt.Errorf("invalid position for type error %v", s)
+ }
+ x.URI = xspn.URI()
+ rng, err := spanToRange(snapshot, pkg, xspn)
+ if err != nil {
+ return nil, err
+ }
+ x.Range = rng
+ related = append(related, x)
+ }
case *analysis.Diagnostic:
spn, err = span.NewRange(fset, e.Pos, e.End).Span()
if err != nil {
@@ -111,6 +136,8 @@
if err != nil {
return nil, err
}
+ default:
+ panic(fmt.Sprintf("%T unexpected", e))
}
rng, err := spanToRange(snapshot, pkg, spn)
if err != nil {
diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go
index 00bbc3b..71662f3 100644
--- a/internal/lsp/diagnostics.go
+++ b/internal/lsp/diagnostics.go
@@ -156,7 +156,7 @@
return
}
- // Add all reports to the global map, checking for duplciates.
+ // Add all reports to the global map, checking for duplicates.
reportsMu.Lock()
for id, diags := range pkgReports {
key := idWithAnalysis{
@@ -307,7 +307,8 @@
})
}
reports = append(reports, protocol.Diagnostic{
- Message: strings.TrimSpace(diag.Message), // go list returns errors prefixed by newline
+ // diag.Message might start with \n or \t
+ Message: strings.TrimSpace(diag.Message),
Range: diag.Range,
Severity: diag.Severity,
Source: diag.Source,
diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go
index 0ca38de..583bb31 100644
--- a/internal/lsp/source/diagnostics.go
+++ b/internal/lsp/source/diagnostics.go
@@ -159,6 +159,7 @@
Message: e.Message,
Range: e.Range,
Severity: protocol.SeverityError,
+ Related: e.Related,
}
set, ok := diagSets[e.URI]
if !ok {
