1
0
mirror of https://github.com/golang/go synced 2024-09-30 10:38:33 -06:00

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>
This commit is contained in:
Peter Weinbergr 2020-08-21 08:50:11 -04:00 committed by Peter Weinberger
parent 93eecc3576
commit 39188db588
5 changed files with 120 additions and 9 deletions

View File

@ -1026,21 +1026,21 @@ func main() {
writeGoVim(env, "p/p.go", p) writeGoVim(env, "p/p.go", p)
writeGoVim(env, "main.go", main) writeGoVim(env, "main.go", main)
writeGoVim(env, "p/p_test.go", `package p writeGoVim(env, "p/p_test.go", `package p
import "testing" import "testing"
func TestDoIt(t *testing.T) { func TestDoIt(t *testing.T) {
DoIt(5) DoIt(5)
} }
`) `)
writeGoVim(env, "p/x_test.go", `package p_test writeGoVim(env, "p/x_test.go", `package p_test
import ( import (
"testing" "testing"
"mod.com/p" "mod.com/p"
) )
func TestDoIt(t *testing.T) { func TestDoIt(t *testing.T) {
p.DoIt(5) p.DoIt(5)
} }
@ -1274,3 +1274,42 @@ func main() {
env.Await(env.DiagnosticAtRegexp("main.go", `t{"msg"}`)) 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)
}
})
}

View File

@ -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. // We don't care about a package's errors unless we have parsed it in full.
if mode == source.ParseFull { if mode == source.ParseFull {
expandErrors(rawErrors)
for _, e := range rawErrors { for _, e := range rawErrors {
srcErr, err := sourceError(ctx, snapshot, pkg, e) srcErr, err := sourceError(ctx, snapshot, pkg, e)
if err != nil { if err != nil {
@ -412,8 +413,8 @@ func typeCheck(ctx context.Context, snapshot *snapshot, m *metadata, mode source
continue continue
} }
pkg.errors = append(pkg.errors, srcErr) pkg.errors = append(pkg.errors, srcErr)
if err, ok := e.(types.Error); ok { if err, ok := e.(extendedError); ok {
pkg.typeErrors = append(pkg.typeErrors, err) 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 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. // resolveImportPath resolves an import path in pkg to a package from deps.
// It should produce the same results as resolveImportPath: // 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. // https://cs.opensource.google/go/go/+/master:src/cmd/go/internal/load/pkg.go;drc=641918ee09cb44d282a30ee8b66f99a0b63eaef9;l=990.

View File

@ -94,7 +94,32 @@ func sourceError(ctx context.Context, snapshot *snapshot, pkg *pkg, e interface{
if err != nil { if err != nil {
return nil, err 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: case *analysis.Diagnostic:
spn, err = span.NewRange(fset, e.Pos, e.End).Span() spn, err = span.NewRange(fset, e.Pos, e.End).Span()
if err != nil { if err != nil {
@ -111,6 +136,8 @@ func sourceError(ctx context.Context, snapshot *snapshot, pkg *pkg, e interface{
if err != nil { if err != nil {
return nil, err return nil, err
} }
default:
panic(fmt.Sprintf("%T unexpected", e))
} }
rng, err := spanToRange(snapshot, pkg, spn) rng, err := spanToRange(snapshot, pkg, spn)
if err != nil { if err != nil {

View File

@ -156,7 +156,7 @@ If you believe this is a mistake, please file an issue: https://github.com/golan
return return
} }
// Add all reports to the global map, checking for duplciates. // Add all reports to the global map, checking for duplicates.
reportsMu.Lock() reportsMu.Lock()
for id, diags := range pkgReports { for id, diags := range pkgReports {
key := idWithAnalysis{ key := idWithAnalysis{
@ -307,7 +307,8 @@ func toProtocolDiagnostics(diagnostics []*source.Diagnostic) []protocol.Diagnost
}) })
} }
reports = append(reports, protocol.Diagnostic{ 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, Range: diag.Range,
Severity: diag.Severity, Severity: diag.Severity,
Source: diag.Source, Source: diag.Source,

View File

@ -159,6 +159,7 @@ func diagnostics(ctx context.Context, snapshot Snapshot, reports map[VersionedFi
Message: e.Message, Message: e.Message,
Range: e.Range, Range: e.Range,
Severity: protocol.SeverityError, Severity: protocol.SeverityError,
Related: e.Related,
} }
set, ok := diagSets[e.URI] set, ok := diagSets[e.URI]
if !ok { if !ok {