diff --git a/gopls/internal/regtest/diagnostics_test.go b/gopls/internal/regtest/diagnostics_test.go index a7973fe29b1..96d61b11a8a 100644 --- a/gopls/internal/regtest/diagnostics_test.go +++ b/gopls/internal/regtest/diagnostics_test.go @@ -1026,21 +1026,21 @@ func main() { 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 @@ func main() { 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 75642c20c67..8311416c82e 100644 --- a/internal/lsp/cache/check.go +++ b/internal/lsp/cache/check.go @@ -405,6 +405,7 @@ func typeCheck(ctx context.Context, snapshot *snapshot, m *metadata, mode source // 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 @@ func typeCheck(ctx context.Context, snapshot *snapshot, m *metadata, mode source 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 @@ func typeCheck(ctx context.Context, snapshot *snapshot, m *metadata, mode source 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 cf0241726a4..1f55a586b77 100644 --- a/internal/lsp/cache/errors.go +++ b/internal/lsp/cache/errors.go @@ -94,7 +94,32 @@ func sourceError(ctx context.Context, snapshot *snapshot, pkg *pkg, e interface{ 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 @@ func sourceError(ctx context.Context, snapshot *snapshot, pkg *pkg, e interface{ 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 00bbc3be10e..71662f32aca 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -156,7 +156,7 @@ If you believe this is a mistake, please file an issue: https://github.com/golan 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 @@ func toProtocolDiagnostics(diagnostics []*source.Diagnostic) []protocol.Diagnost }) } 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 0ca38de8c29..583bb314fa6 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -159,6 +159,7 @@ func diagnostics(ctx context.Context, snapshot Snapshot, reports map[VersionedFi Message: e.Message, Range: e.Range, Severity: protocol.SeverityError, + Related: e.Related, } set, ok := diagSets[e.URI] if !ok {