gopls: add diagnostics for both vulns used and not used
Add separate diagnostics with different severity levels in order
to capture both reachable and unreachable vulnerabilites in the
diagnostics.
Change-Id: Ib7fa422d9ef7f84e85a5da30b6cf766618cb583c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/452837
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
diff --git a/gopls/internal/lsp/mod/diagnostics.go b/gopls/internal/lsp/mod/diagnostics.go
index 7202911..e338520 100644
--- a/gopls/internal/lsp/mod/diagnostics.go
+++ b/gopls/internal/lsp/mod/diagnostics.go
@@ -215,7 +215,7 @@
// Map affecting vulns to 'warning' level diagnostics,
// others to 'info' level diagnostics.
// Fixes will include only the upgrades for warning level diagnostics.
- var fixes []source.SuggestedFix
+ var warningFixes, infoFixes []source.SuggestedFix
var warning, info []string
var relatedInfo []source.RelatedInformation
for _, mv := range vulns {
@@ -251,52 +251,71 @@
if err != nil {
return nil, err // TODO: bug report
}
- fixes = append(fixes, source.SuggestedFixFromCommand(cmd, protocol.QuickFix))
+ sf := source.SuggestedFixFromCommand(cmd, protocol.QuickFix)
+ if !vuln.IsCalled() {
+ infoFixes = append(infoFixes, sf)
+ } else {
+ warningFixes = append(warningFixes, sf)
+ }
}
}
if len(warning) == 0 && len(info) == 0 {
continue
}
- severity := protocol.SeverityInformation
- if len(warning) > 0 {
- severity = protocol.SeverityWarning
+ // Add an upgrade for module@latest.
+ // TODO(suzmue): verify if latest is the same as fixedVersion.
+ latest, err := getUpgradeCodeAction(fh, req, "latest")
+ if err != nil {
+ return nil, err // TODO: bug report
}
- if len(fixes) > 0 {
- // Add an upgrade for module@latest.
- // TODO(suzmue): verify if latest is the same as fixedVersion.
- latest, err := getUpgradeCodeAction(fh, req, "latest")
- if err != nil {
- return nil, err // TODO: bug report
- }
- fixes = append([]source.SuggestedFix{source.SuggestedFixFromCommand(latest, protocol.QuickFix)}, fixes...)
+ sf := source.SuggestedFixFromCommand(latest, protocol.QuickFix)
+ if len(warningFixes) > 0 {
+ warningFixes = append(warningFixes, sf)
+ }
+ if len(infoFixes) > 0 {
+ infoFixes = append(infoFixes, sf)
}
sort.Strings(warning)
sort.Strings(info)
- var b strings.Builder
- switch len(warning) {
- case 0:
- if n := len(info); n == 1 {
+ if len(warning) > 0 {
+ var b strings.Builder
+ switch len(warning) {
+ case 1:
+ fmt.Fprintf(&b, "%v has a vulnerability used in the code: %v.", req.Mod.Path, warning[0])
+ 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,
+ Severity: protocol.SeverityWarning,
+ Source: source.Vulncheck,
+ Message: b.String(),
+ SuggestedFixes: warningFixes,
+ Related: relatedInfo,
+ })
+ }
+ if len(info) > 0 {
+ var b strings.Builder
+ switch len(info) {
+ case 1:
fmt.Fprintf(&b, "%v has a vulnerability %v that is not used in the code.", req.Mod.Path, info[0])
- } else if n > 1 {
+ default:
fmt.Fprintf(&b, "%v has known vulnerabilities %v that are not used in the code.", req.Mod.Path, strings.Join(info, ", "))
}
- case 1:
- fmt.Fprintf(&b, "%v has a vulnerability used in the code: %v.", req.Mod.Path, warning[0])
- 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,
+ Severity: protocol.SeverityInformation,
+ Source: source.Vulncheck,
+ Message: b.String(),
+ SuggestedFixes: infoFixes,
+ Related: relatedInfo,
+ })
}
- vulnDiagnostics = append(vulnDiagnostics, &source.Diagnostic{
- URI: fh.URI(),
- Range: rng,
- Severity: severity,
- Source: source.Vulncheck,
- Message: b.String(),
- SuggestedFixes: fixes,
- Related: relatedInfo,
- })
}
return vulnDiagnostics, nil
diff --git a/gopls/internal/regtest/misc/vuln_test.go b/gopls/internal/regtest/misc/vuln_test.go
index f2a5be5..e50c38f 100644
--- a/gopls/internal/regtest/misc/vuln_test.go
+++ b/gopls/internal/regtest/misc/vuln_test.go
@@ -404,6 +404,18 @@
severity: protocol.SeverityWarning,
codeActions: []string{
"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
+ },
+ },
+ {
+ msg: "golang.org/amod has a vulnerability GO-2022-03 that is not used in the code.",
+ severity: protocol.SeverityInformation,
+ codeActions: []string{
+ "Upgrade to latest",
"Upgrade to v1.0.6",
},
relatedInfo: []vulnRelatedInfo{
@@ -600,15 +612,14 @@
t.Errorf("code actions for %q do not match, want %v, got %v\n%v\n", w.msg, w.codeActions, gotActions, diff)
continue
}
-
- // Check that useful info is supplemented as hover.
- if len(want.hover) > 0 {
- 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, want %v, got %v\n", w.msg, strings.Join(want.hover, ","), hover.Value)
- break
- }
+ }
+ // Check that useful info is supplemented as hover.
+ if len(want.hover) > 0 {
+ 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, want %v, got %v\n", requireMod, strings.Join(want.hover, ","), hover.Value)
+ break
}
}
}
