gopls/internal/lsp/mod: add related info

Related info in module vulncheck diagnostics carries the location
to the entry stack frame found from some of the call paths.

Change-Id: I99eb60ab1c2cb7cc356eae8d2ba35a2167cb179e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/450278
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
This commit is contained in:
Hana (Hyang-Ah) Kim 2022-11-11 22:34:29 -05:00 committed by Hyang-Ah Hana Kim
parent fc039936a9
commit 128f61d438
2 changed files with 112 additions and 23 deletions

View File

@ -18,6 +18,7 @@ import (
"golang.org/x/tools/gopls/internal/lsp/command"
"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/gopls/internal/lsp/source"
"golang.org/x/tools/gopls/internal/span"
"golang.org/x/tools/internal/event"
"golang.org/x/vuln/osv"
)
@ -213,6 +214,7 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot,
// Fixes will include only the upgrades for warning level diagnostics.
var fixes []source.SuggestedFix
var warning, info []string
var relatedInfo []source.RelatedInformation
for _, mv := range vulns {
mod, vuln := mv.mod, mv.vuln
// It is possible that the source code was changed since the last
@ -238,6 +240,7 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot,
info = append(info, vuln.OSV.ID)
} else {
warning = append(warning, vuln.OSV.ID)
relatedInfo = append(relatedInfo, listRelatedInfo(ctx, snapshot, vuln)...)
}
// Upgrade to the exact version we offer the user, not the most recent.
if fixedVersion := mod.FixedVersion; semver.IsValid(fixedVersion) && semver.Compare(req.Mod.Version, fixedVersion) < 0 {
@ -282,7 +285,6 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot,
default:
fmt.Fprintf(&b, "%v has vulnerabilities used in the code: %v.", req.Mod.Path, strings.Join(warning, ", "))
}
vulnDiagnostics = append(vulnDiagnostics, &source.Diagnostic{
URI: fh.URI(),
Range: rng,
@ -290,12 +292,54 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot,
Source: source.Vulncheck,
Message: b.String(),
SuggestedFixes: fixes,
Related: relatedInfo,
})
}
return vulnDiagnostics, nil
}
func listRelatedInfo(ctx context.Context, snapshot source.Snapshot, vuln *govulncheck.Vuln) []source.RelatedInformation {
var ri []source.RelatedInformation
for _, m := range vuln.Modules {
for _, p := range m.Packages {
for _, c := range p.CallStacks {
if len(c.Frames) == 0 {
continue
}
entry := c.Frames[0]
pos := entry.Position
if pos.Filename == "" {
continue // token.Position Filename is an optional field.
}
uri := span.URIFromPath(pos.Filename)
startPos := protocol.Position{
Line: uint32(pos.Line) - 1,
// We need to read the file contents to precisesly map
// token.Position (pos) to the UTF16-based column offset
// protocol.Position requires. That can be expensive.
// We need this related info to just help users to open
// the entry points of the callstack and once the file is
// open, we will compute the precise location based on the
// open file contents. So, use the beginning of the line
// as the position here instead of precise UTF16-based
// position computation.
Character: 0,
}
ri = append(ri, source.RelatedInformation{
URI: uri,
Range: protocol.Range{
Start: startPos,
End: startPos,
},
Message: fmt.Sprintf("[%v] %v -> %v.%v", vuln.OSV.ID, entry.Name(), p.Path, c.Symbol),
})
}
}
}
return ri
}
func formatMessage(v *govulncheck.Vuln) string {
details := []byte(v.OSV.Details)
// Remove any new lines that are not preceded or followed by a new line.

View File

@ -9,9 +9,12 @@ package misc
import (
"context"
"path/filepath"
"sort"
"strings"
"testing"
"github.com/google/go-cmp/cmp"
"golang.org/x/tools/gopls/internal/lsp/command"
"golang.org/x/tools/gopls/internal/lsp/protocol"
. "golang.org/x/tools/gopls/internal/lsp/regtest"
@ -329,6 +332,10 @@ func TestRunVulncheckWarning(t *testing.T) {
ReadDiagnostics("go.mod", gotDiagnostics),
),
)
env.OpenFile("x/x.go")
lineX := env.RegexpSearch("x/x.go", `c\.C1\(\)\.Vuln1\(\)`)
env.OpenFile("y/y.go")
lineY := env.RegexpSearch("y/y.go", `c\.C2\(\)\(\)`)
wantDiagnostics := map[string]vulnDiagExpectation{
"golang.org/amod": {
applyAction: "Upgrade to v1.0.4",
@ -340,6 +347,10 @@ func TestRunVulncheckWarning(t *testing.T) {
"Upgrade to latest",
"Upgrade to v1.0.4",
},
relatedInfo: []vulnRelatedInfo{
{"x.go", uint32(lineX.Line), "[GO-2022-01]"}, // avuln.VulnData.Vuln1
{"x.go", uint32(lineX.Line), "[GO-2022-01]"}, // avuln.VulnData.Vuln2
},
},
},
codeActions: []string{
@ -353,6 +364,9 @@ func TestRunVulncheckWarning(t *testing.T) {
{
msg: "golang.org/bmod has a vulnerability used in the code: GO-2022-02.",
severity: protocol.SeverityWarning,
relatedInfo: []vulnRelatedInfo{
{"y.go", uint32(lineY.Line), "[GO-2022-02]"}, // bvuln.Vuln
},
},
},
hover: []string{"GO-2022-02", "This is a long description of this vulnerability.", "No fix is available."},
@ -364,8 +378,8 @@ func TestRunVulncheckWarning(t *testing.T) {
// Check that the actions we get when including all diagnostics at a location return the same result
gotActions := env.CodeAction("go.mod", modPathDiagnostics)
if !sameCodeActions(gotActions, want.codeActions) {
t.Errorf("code actions for %q do not match, expected %v, got %v\n", mod, want.codeActions, gotActions)
if diff := diffCodeActions(gotActions, want.codeActions); diff != "" {
t.Errorf("code actions for %q do not match, expected %v, got %v\n%v\n", mod, want.codeActions, gotActions, diff)
continue
}
@ -399,20 +413,12 @@ require (
})
}
func sameCodeActions(gotActions []protocol.CodeAction, want []string) bool {
gotTitles := make([]string, len(gotActions))
for i, ca := range gotActions {
gotTitles[i] = ca.Title
func diffCodeActions(gotActions []protocol.CodeAction, want []string) string {
var gotTitles []string
for _, ca := range gotActions {
gotTitles = append(gotTitles, ca.Title)
}
if len(gotTitles) != len(want) {
return false
}
for i := range want {
if gotTitles[i] != want[i] {
return false
}
}
return true
return cmp.Diff(want, gotTitles)
}
const workspace2 = `
@ -483,8 +489,8 @@ func TestRunVulncheckInfo(t *testing.T) {
modPathDiagnostics := testVulnDiagnostics(t, env, mod, want, gotDiagnostics)
// Check that the actions we get when including all diagnostics at a location return the same result
gotActions := env.CodeAction("go.mod", modPathDiagnostics)
if !sameCodeActions(gotActions, want.codeActions) {
t.Errorf("code actions for %q do not match, expected %v, got %v\n", mod, want.codeActions, gotActions)
if diff := diffCodeActions(gotActions, want.codeActions); diff != "" {
t.Errorf("code actions for %q do not match, expected %v, got %v\n%v\n", mod, want.codeActions, gotActions, diff)
continue
}
}
@ -513,12 +519,16 @@ func testVulnDiagnostics(t *testing.T, env *Env, requireMod string, want vulnDia
continue
}
if diag.Severity != w.severity {
t.Errorf("incorrect severity for %q, expected %s got %s\n", w.msg, w.severity, diag.Severity)
t.Errorf("incorrect severity for %q, want %s got %s\n", w.msg, w.severity, diag.Severity)
}
sort.Slice(w.relatedInfo, func(i, j int) bool { return w.relatedInfo[i].less(w.relatedInfo[j]) })
if got, want := summarizeRelatedInfo(diag.RelatedInformation), w.relatedInfo; !cmp.Equal(got, want) {
t.Errorf("related info for %q do not match, want %v, got %v\n", w.msg, want, got)
}
// Check expected code actions appear.
gotActions := env.CodeAction("go.mod", []protocol.Diagnostic{*diag})
if !sameCodeActions(gotActions, w.codeActions) {
t.Errorf("code actions for %q do not match, expected %v, got %v\n", w.msg, w.codeActions, gotActions)
if diff := diffCodeActions(gotActions, w.codeActions); diff != "" {
t.Errorf("code actions for %q do not match, want %v, got %v\n%v\n", w.msg, w.codeActions, gotActions, diff)
continue
}
@ -527,7 +537,7 @@ func testVulnDiagnostics(t *testing.T, env *Env, requireMod string, want vulnDia
hover, _ := env.Hover("go.mod", pos)
for _, part := range want.hover {
if !strings.Contains(hover.Value, part) {
t.Errorf("hover contents for %q do not match, expected %v, got %v\n", w.msg, strings.Join(want.hover, ","), hover.Value)
t.Errorf("hover contents for %q do not match, want %v, got %v\n", w.msg, strings.Join(want.hover, ","), hover.Value)
break
}
}
@ -536,12 +546,47 @@ func testVulnDiagnostics(t *testing.T, env *Env, requireMod string, want vulnDia
return modPathDiagnostics
}
// summarizeRelatedInfo converts protocol.DiagnosticRelatedInformation to vulnRelatedInfo
// that captures only the part that we want to test.
func summarizeRelatedInfo(rinfo []protocol.DiagnosticRelatedInformation) []vulnRelatedInfo {
var res []vulnRelatedInfo
for _, r := range rinfo {
filename := filepath.Base(r.Location.URI.SpanURI().Filename())
message, _, _ := strings.Cut(r.Message, " ")
line := r.Location.Range.Start.Line
res = append(res, vulnRelatedInfo{filename, line, message})
}
sort.Slice(res, func(i, j int) bool {
return res[i].less(res[j])
})
return res
}
type vulnRelatedInfo struct {
Filename string
Line uint32
Message string
}
type vulnDiag struct {
msg string
severity protocol.DiagnosticSeverity
// codeActions is a list titles of code actions that we get with this
// diagnostics as the context.
codeActions []string
// relatedInfo is related info message prefixed by the file base.
// See summarizeRelatedInfo.
relatedInfo []vulnRelatedInfo
}
func (i vulnRelatedInfo) less(j vulnRelatedInfo) bool {
if i.Filename != j.Filename {
return i.Filename < j.Filename
}
if i.Line != j.Line {
return i.Line < j.Line
}
return i.Message < j.Message
}
// wantVulncheckModDiagnostics maps a module path in the require