From 678efe0184ccc1ee853ed74216cfb7135ed5583d Mon Sep 17 00:00:00 2001 From: "Hana (Hyang-Ah) Kim" Date: Thu, 8 Sep 2022 09:58:25 -0400 Subject: [PATCH] gopls/internal/lsp/cmd: fix vulncheck error handling Fix 1 - print usage info only when appropriate: tool.CommandLineError is a special error that makes a command exits with full usage documentation. When Govulncheck hook runs and fails, it's unlikely the failure was from command line misuse and the extra usage doc will distract users from the root cause. Let's stop that by returning a plain error. Fix 2 - stop printing package loading errors twice: Package loading error was printed by the logger in gopls/internal/vulncheck.cmd and once more by the gopls command line handler. Print it once. For testing, added back the replace statement to go.mod. We will update go.mod back after this commit is merged. Change-Id: Ifaf569a31bbbc84d7b84e1b6d90a8fa0b27ac758 Reviewed-on: https://go-review.googlesource.com/c/tools/+/429515 Reviewed-by: Robert Findley gopls-CI: kokoro Run-TryBot: Hyang-Ah Hana Kim TryBot-Result: Gopher Robot Reviewed-on: https://go-review.googlesource.com/c/tools/+/429937 --- gopls/internal/lsp/cmd/vulncheck.go | 6 +++--- gopls/internal/vulncheck/command.go | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/gopls/internal/lsp/cmd/vulncheck.go b/gopls/internal/lsp/cmd/vulncheck.go index 5ee9b0e372..770455cfd9 100644 --- a/gopls/internal/lsp/cmd/vulncheck.go +++ b/gopls/internal/lsp/cmd/vulncheck.go @@ -71,7 +71,7 @@ func (v *vulncheck) Run(ctx context.Context, args ...string) error { opts := source.DefaultOptions().Clone() v.app.options(opts) // register hook if opts == nil || opts.Hooks.Govulncheck == nil { - return tool.CommandLineErrorf("vulncheck feature is not available") + return fmt.Errorf("vulncheck feature is not available") } loadCfg := &packages.Config{ @@ -83,11 +83,11 @@ func (v *vulncheck) Run(ctx context.Context, args ...string) error { res, err := opts.Hooks.Govulncheck(ctx, loadCfg, pattern) if err != nil { - return tool.CommandLineErrorf("govulncheck failed: %v", err) + return fmt.Errorf("vulncheck failed: %v", err) } data, err := json.MarshalIndent(res, " ", " ") if err != nil { - return tool.CommandLineErrorf("failed to decode results: %v", err) + return fmt.Errorf("vulncheck failed to encode result: %v", err) } fmt.Printf("%s", data) return nil diff --git a/gopls/internal/vulncheck/command.go b/gopls/internal/vulncheck/command.go index 9ed1e0ba2d..641c9ddeb6 100644 --- a/gopls/internal/vulncheck/command.go +++ b/gopls/internal/vulncheck/command.go @@ -9,6 +9,7 @@ package vulncheck import ( "context" + "fmt" "log" "os" "sort" @@ -78,8 +79,8 @@ func (c *cmd) Run(ctx context.Context, cfg *packages.Config, patterns ...string) logger.Println("loading packages...") loadedPkgs, err := gvc.LoadPackages(cfg, patterns...) if err != nil { - logger.Printf("package load failed: %v", err) - return nil, err + logger.Printf("%v", err) + return nil, fmt.Errorf("package load failed") } logger.Printf("analyzing %d packages...\n", len(loadedPkgs))