[gopls-release-branch.0.11] gopls/internal/lsp/mod: add Reset vulncheck result codelens

We hope this gives an option to more conveniently
hide unactionable govulncheck diagnostics while we are still
looking for a better option than placing "Run govulncheck to
verify" and "Reset govulncheck result" in Quick Fix.

The command for the code action is implemented by
extending the existing go mod diagnostics reset
command to accept an optional diagnostic source name.
If the name is given, only the go.mod diagnostics that
match the name will be reset.

This CL also renamed "Run govulncheck" to "Run govulncheck to
verify" codelens.

Change-Id: Id4e809083d572437f86380a22c70547ec12f9976
Reviewed-on: https://go-review.googlesource.com/c/tools/+/456256
Reviewed-by: Robert Findley <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
gopls-CI: kokoro <[email protected]>
Run-TryBot: Hyang-Ah Hana Kim <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/tools/+/456640

Author: hyangah

gopls/doc/commands.md

@@ -276,8 +276,12 @@ Args:
 
 ```
 {
-	// The file URI.
-	"URI": string,
+	"URIArg": {
+		"URI": string,
+	},
+	// Optional: source of the diagnostics to reset.
+	// If not set, all resettable go.mod diagnostics will be cleared.
+	"DiagnosticSource": string,
 }
 ```
 

gopls/internal/lsp/command.go

@@ -211,17 +211,22 @@ func (c *commandHandler) UpgradeDependency(ctx context.Context, args command.Dep
 	return c.GoGetModule(ctx, args)
 }
 
-func (c *commandHandler) ResetGoModDiagnostics(ctx context.Context, uri command.URIArg) error {
+func (c *commandHandler) ResetGoModDiagnostics(ctx context.Context, args command.ResetGoModDiagnosticsArgs) error {
 	return c.run(ctx, commandConfig{
-		forURI: uri.URI,
+		forURI: args.URI,
 	}, func(ctx context.Context, deps commandDeps) error {
-		deps.snapshot.View().ClearModuleUpgrades(uri.URI.SpanURI())
-		deps.snapshot.View().SetVulnerabilities(uri.URI.SpanURI(), nil)
 		// Clear all diagnostics coming from the upgrade check source and vulncheck.
 		// This will clear the diagnostics in all go.mod files, but they
 		// will be re-calculated when the snapshot is diagnosed again.
-		c.s.clearDiagnosticSource(modCheckUpgradesSource)
-		c.s.clearDiagnosticSource(modVulncheckSource)
+		if args.DiagnosticSource == "" || args.DiagnosticSource == string(source.UpgradeNotification) {
+			deps.snapshot.View().ClearModuleUpgrades(args.URI.SpanURI())
+			c.s.clearDiagnosticSource(modCheckUpgradesSource)
+		}
+
+		if args.DiagnosticSource == "" || args.DiagnosticSource == string(source.Vulncheck) {
+			deps.snapshot.View().SetVulnerabilities(args.URI.SpanURI(), nil)
+			c.s.clearDiagnosticSource(modVulncheckSource)
+		}
 
 		// Re-diagnose the snapshot to remove the diagnostics.
 		c.s.diagnoseSnapshot(deps.snapshot, nil, false)

gopls/internal/lsp/command/command_gen.go

@@ -102,7 +102,7 @@ type Interface interface {
 	// ResetGoModDiagnostics: Reset go.mod diagnostics
 	//
 	// Reset diagnostics in the go.mod file of a module.
-	ResetGoModDiagnostics(context.Context, URIArg) error
+	ResetGoModDiagnostics(context.Context, ResetGoModDiagnosticsArgs) error
 
 	// GoGetPackage: go get a package
 	//
@@ -309,6 +309,14 @@ type DebuggingResult struct {
 	URLs []string
 }
 
+type ResetGoModDiagnosticsArgs struct {
+	URIArg
+
+	// Optional: source of the diagnostics to reset.
+	// If not set, all resettable go.mod diagnostics will be cleared.
+	DiagnosticSource string
+}
+
 type VulncheckArgs struct {
 	// Any document in the directory from which govulncheck will run.
 	URI protocol.DocumentURI

gopls/internal/lsp/command/interface.go

@@ -32,7 +32,7 @@ func upgradeLenses(ctx context.Context, snapshot source.Snapshot, fh source.File
 		return nil, err
 	}
 	uri := protocol.URIFromSpanURI(fh.URI())
-	reset, err := command.NewResetGoModDiagnosticsCommand("Reset go.mod diagnostics", command.URIArg{URI: uri})
+	reset, err := command.NewResetGoModDiagnosticsCommand("Reset go.mod diagnostics", command.ResetGoModDiagnosticsArgs{URIArg: command.URIArg{URI: uri}})
 	if err != nil {
 		return nil, err
 	}
@@ -178,7 +178,7 @@ func vulncheckLenses(ctx context.Context, snapshot source.Snapshot, fh source.Fi
 		return nil, err
 	}
 
-	vulncheck, err := command.NewRunGovulncheckCommand("Run govulncheck", command.VulncheckArgs{
+	vulncheck, err := command.NewRunGovulncheckCommand("Run govulncheck to verify", command.VulncheckArgs{
 		URI:     uri,
 		Pattern: "./...",
 	})

gopls/internal/lsp/mod/code_lens.go

@@ -193,16 +193,11 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot,
 		return nil, nil
 	}
 
-	vulncheck, err := command.NewRunGovulncheckCommand("Run govulncheck", command.VulncheckArgs{
-		URI:     protocol.DocumentURI(fh.URI()),
-		Pattern: "./...",
-	})
+	suggestRunOrResetGovulncheck, err := suggestGovulncheckAction(fromGovulncheck, fh.URI())
 	if err != nil {
 		// must not happen
 		return nil, err // TODO: bug report
 	}
-	suggestVulncheck := source.SuggestedFixFromCommand(vulncheck, protocol.QuickFix)
-
 	type modVuln struct {
 		mod  *govulncheck.Module
 		vuln *govulncheck.Vuln
@@ -294,14 +289,12 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot,
 		if len(infoFixes) > 0 {
 			infoFixes = append(infoFixes, sf)
 		}
-		if !fromGovulncheck {
-			infoFixes = append(infoFixes, suggestVulncheck)
-		}
 
 		sort.Strings(warning)
 		sort.Strings(info)
 
 		if len(warning) > 0 {
+			warningFixes = append(warningFixes, suggestRunOrResetGovulncheck)
 			vulnDiagnostics = append(vulnDiagnostics, &source.Diagnostic{
 				URI:            fh.URI(),
 				Range:          rng,
@@ -313,6 +306,7 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot,
 			})
 		}
 		if len(info) > 0 {
+			infoFixes = append(infoFixes, suggestRunOrResetGovulncheck)
 			vulnDiagnostics = append(vulnDiagnostics, &source.Diagnostic{
 				URI:            fh.URI(),
 				Range:          rng,
@@ -355,20 +349,19 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot,
 			}
 		}
 		if len(warning) > 0 {
+			fixes := []source.SuggestedFix{suggestRunOrResetGovulncheck}
 			vulnDiagnostics = append(vulnDiagnostics, &source.Diagnostic{
-				URI:      fh.URI(),
-				Range:    rng,
-				Severity: protocol.SeverityWarning,
-				Source:   source.Vulncheck,
-				Message:  getVulnMessage(stdlib, warning, true, fromGovulncheck),
-				Related:  relatedInfo,
+				URI:            fh.URI(),
+				Range:          rng,
+				Severity:       protocol.SeverityWarning,
+				Source:         source.Vulncheck,
+				Message:        getVulnMessage(stdlib, warning, true, fromGovulncheck),
+				SuggestedFixes: fixes,
+				Related:        relatedInfo,
 			})
 		}
 		if len(info) > 0 {
-			var fixes []source.SuggestedFix
-			if !fromGovulncheck {
-				fixes = append(fixes, suggestVulncheck)
-			}
+			fixes := []source.SuggestedFix{suggestRunOrResetGovulncheck}
 			vulnDiagnostics = append(vulnDiagnostics, &source.Diagnostic{
 				URI:            fh.URI(),
 				Range:          rng,
@@ -384,6 +377,31 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot,
 	return vulnDiagnostics, nil
 }
 
+// suggestGovulncheckAction returns a code action that suggests either run govulncheck
+// for more accurate investigation (if the present vulncheck diagnostics are based on
+// analysis less accurate than govulncheck) or reset the existing govulncheck result
+// (if the present vulncheck diagnostics are already based on govulncheck run).
+func suggestGovulncheckAction(fromGovulncheck bool, uri span.URI) (source.SuggestedFix, error) {
+	if fromGovulncheck {
+		resetVulncheck, err := command.NewResetGoModDiagnosticsCommand("Reset govulncheck result", command.ResetGoModDiagnosticsArgs{
+			URIArg:           command.URIArg{URI: protocol.DocumentURI(uri)},
+			DiagnosticSource: string(source.Vulncheck),
+		})
+		if err != nil {
+			return source.SuggestedFix{}, err
+		}
+		return source.SuggestedFixFromCommand(resetVulncheck, protocol.QuickFix), nil
+	}
+	vulncheck, err := command.NewRunGovulncheckCommand("Run govulncheck to verify", command.VulncheckArgs{
+		URI:     protocol.DocumentURI(uri),
+		Pattern: "./...",
+	})
+	if err != nil {
+		return source.SuggestedFix{}, err
+	}
+	return source.SuggestedFixFromCommand(vulncheck, protocol.QuickFix), nil
+}
+
 func getVulnMessage(mod string, vulns []string, used, fromGovulncheck bool) string {
 	var b strings.Builder
 	if used {
@@ -493,18 +511,21 @@ func upgradeTitle(fixedVersion string) string {
 // SelectUpgradeCodeActions takes a list of code actions for a required module
 // and returns a more selective list of upgrade code actions,
 // where the code actions have been deduped. Code actions unrelated to upgrade
-// remain untouched.
+// are deduplicated by the name.
 func SelectUpgradeCodeActions(actions []protocol.CodeAction) []protocol.CodeAction {
 	if len(actions) <= 1 {
 		return actions // return early if no sorting necessary
 	}
 	var others []protocol.CodeAction
 
+	seen := make(map[string]bool)
+
 	set := make(map[string]protocol.CodeAction)
 	for _, action := range actions {
 		if strings.HasPrefix(action.Title, upgradeCodeActionPrefix) {
 			set[action.Command.Title] = action
-		} else {
+		} else if !seen[action.Command.Title] {
+			seen[action.Command.Title] = true
 			others = append(others, action)
 		}
 	}

gopls/internal/lsp/mod/diagnostics.go

@@ -507,14 +507,14 @@ func TestRunVulncheckPackageDiagnostics(t *testing.T) {
 						codeActions: []string{
 							"Upgrade to latest",
 							"Upgrade to v1.0.6",
-							"Run govulncheck",
+							"Run govulncheck to verify",
 						},
 					},
 				},
 				codeActions: []string{
 					"Upgrade to latest",
 					"Upgrade to v1.0.6",
-					"Run govulncheck",
+					"Run govulncheck to verify",
 				},
 				hover: []string{"GO-2022-01", "Fixed in v1.0.4.", "GO-2022-03"},
 			},
@@ -524,12 +524,12 @@ func TestRunVulncheckPackageDiagnostics(t *testing.T) {
 						msg:      "golang.org/bmod has a vulnerability GO-2022-02.",
 						severity: protocol.SeverityInformation,
 						codeActions: []string{
-							"Run govulncheck",
+							"Run govulncheck to verify",
 						},
 					},
 				},
 				codeActions: []string{
-					"Run govulncheck",
+					"Run govulncheck to verify",
 				},
 				hover: []string{"GO-2022-02", "This is a long description of this vulnerability.", "No fix is available."},
 			},
@@ -667,6 +667,7 @@ func TestRunVulncheckWarning(t *testing.T) {
 						codeActions: []string{
 							"Upgrade to latest",
 							"Upgrade to v1.0.4",
+							"Reset govulncheck result",
 						},
 						relatedInfo: []vulnRelatedInfo{
 							{"x.go", uint32(lineX.Line), "[GO-2022-01]"}, // avuln.VulnData.Vuln1
@@ -679,6 +680,7 @@ func TestRunVulncheckWarning(t *testing.T) {
 						codeActions: []string{
 							"Upgrade to latest",
 							"Upgrade to v1.0.6",
+							"Reset govulncheck result",
 						},
 						relatedInfo: []vulnRelatedInfo{
 							{"x.go", uint32(lineX.Line), "[GO-2022-01]"}, // avuln.VulnData.Vuln1
@@ -689,6 +691,7 @@ func TestRunVulncheckWarning(t *testing.T) {
 				codeActions: []string{
 					"Upgrade to latest",
 					"Upgrade to v1.0.6",
+					"Reset govulncheck result",
 				},
 				hover: []string{"GO-2022-01", "Fixed in v1.0.4.", "GO-2022-03"},
 			},
@@ -697,11 +700,17 @@ func TestRunVulncheckWarning(t *testing.T) {
 					{
 						msg:      "golang.org/bmod has a vulnerability used in the code: GO-2022-02.",
 						severity: protocol.SeverityWarning,
+						codeActions: []string{
+							"Reset govulncheck result", // no fix, but we should give an option to reset.
+						},
 						relatedInfo: []vulnRelatedInfo{
 							{"y.go", uint32(lineY.Line), "[GO-2022-02]"}, // bvuln.Vuln
 						},
 					},
 				},
+				codeActions: []string{
+					"Reset govulncheck result", // no fix, but we should give an option to reset.
+				},
 				hover: []string{"GO-2022-02", "This is a long description of this vulnerability.", "No fix is available."},
 			},
 		}
@@ -822,21 +831,44 @@ func TestGovulncheckInfo(t *testing.T) {
 					{
 						msg:      "golang.org/bmod has a vulnerability GO-2022-02 that is not used in the code.",
 						severity: protocol.SeverityInformation,
+						codeActions: []string{
+							"Reset govulncheck result",
+						},
 					},
 				},
+				codeActions: []string{
+					"Reset govulncheck result",
+				},
 				hover: []string{"GO-2022-02", "This is a long description of this vulnerability.", "No fix is available."},
 			},
 		}
 
+		var allActions []protocol.CodeAction
 		for mod, want := range wantDiagnostics {
 			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)
+			allActions = append(allActions, 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
 			}
 		}
+
+		// Clear Diagnostics by using one of the reset code actions.
+		var reset protocol.CodeAction
+		for _, a := range allActions {
+			if a.Title == "Reset govulncheck result" {
+				reset = a
+				break
+			}
+		}
+		if reset.Title != "Reset govulncheck result" {
+			t.Errorf("failed to find a 'Reset govulncheck result' code action, got %v", allActions)
+		}
+		env.ApplyCodeAction(reset)
+
+		env.Await(EmptyOrNoDiagnostics("go.mod"))
 	})
 }