Skip to content

Commit

Permalink
[gopls-release-branch.0.9] internal/lsp/cmd: fix vulncheck error hand…
Browse files Browse the repository at this point in the history
…ling

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 <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
  • Loading branch information
hyangah committed Sep 8, 2022
1 parent 58b9e2e commit ce39741
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 5 deletions.
2 changes: 2 additions & 0 deletions gopls/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,5 @@ require (
golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4
golang.org/x/text v0.3.7 // indirect
)

replace golang.org/x/tools => ../
5 changes: 3 additions & 2 deletions gopls/internal/vulncheck/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ package vulncheck

import (
"context"
"fmt"
"log"
"os"
"sort"
Expand Down Expand Up @@ -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))
Expand Down
6 changes: 3 additions & 3 deletions internal/lsp/cmd/vulncheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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
Expand Down

0 comments on commit ce39741

Please sign in to comment.