From b5fd08821a5cee019211bfd5a3fe2340cd0462bb Mon Sep 17 00:00:00 2001 From: "Hana (Hyang-Ah) Kim" Date: Tue, 2 Aug 2022 22:39:32 -0400 Subject: [PATCH] internal/lsp/command: replace VulncheckArgs Dir with URI commandHandler.run expects a valid file for forURI and forURI is necessary to fully populate commandDeps. Our use of directory URI does not work. We plan to use this custom command triggered through codelenses on documents (e.g. go.mod), so we use the document's URI. Change-Id: I4de6d488dec5a4b71716499e7fc5e8328ecf3e49 Reviewed-on: https://go-review.googlesource.com/c/tools/+/420994 gopls-CI: kokoro TryBot-Result: Gopher Robot Reviewed-by: Robert Findley Reviewed-by: Suzy Mueller Run-TryBot: Hyang-Ah Hana Kim --- gopls/doc/commands.md | 4 ++-- gopls/internal/regtest/misc/vuln_test.go | 5 +++-- gopls/internal/vulncheck/command.go | 8 ++++---- gopls/internal/vulncheck/vulncheck.go | 2 +- internal/lsp/cmd/vulncheck.go | 8 ++------ internal/lsp/command.go | 10 ++++++---- internal/lsp/command/interface.go | 4 ++-- internal/lsp/source/api_json.go | 2 +- internal/lsp/source/options.go | 2 +- 9 files changed, 22 insertions(+), 23 deletions(-) diff --git a/gopls/doc/commands.md b/gopls/doc/commands.md index 37d8dcbdcc..c202b51a2c 100644 --- a/gopls/doc/commands.md +++ b/gopls/doc/commands.md @@ -274,8 +274,8 @@ Args: ``` { - // Dir is the directory from which vulncheck will run from. - "Dir": string, + // Any document in the directory from which govulncheck will run. + "URI": string, // Package pattern. E.g. "", ".", "./...". "Pattern": string, } diff --git a/gopls/internal/regtest/misc/vuln_test.go b/gopls/internal/regtest/misc/vuln_test.go index d39fd74ef5..9de68b61c7 100644 --- a/gopls/internal/regtest/misc/vuln_test.go +++ b/gopls/internal/regtest/misc/vuln_test.go @@ -26,7 +26,7 @@ package foo ` Run(t, files, func(t *testing.T, env *Env) { cmd, err := command.NewRunVulncheckExpCommand("Run Vulncheck Exp", command.VulncheckArgs{ - Dir: "/invalid/file/url", // invalid arg + URI: "/invalid/file/url", // invalid arg }) if err != nil { t.Fatal(err) @@ -81,7 +81,8 @@ func main() { }, ).Run(t, files, func(t *testing.T, env *Env) { cmd, err := command.NewRunVulncheckExpCommand("Run Vulncheck Exp", command.VulncheckArgs{ - Dir: env.Sandbox.Workdir.RootURI(), + URI: protocol.URIFromPath(env.Sandbox.Workdir.AbsPath("go.mod")), + Pattern: "./...", }) if err != nil { t.Fatal(err) diff --git a/gopls/internal/vulncheck/command.go b/gopls/internal/vulncheck/command.go index 8c88cf3d51..b84daa5d9f 100644 --- a/gopls/internal/vulncheck/command.go +++ b/gopls/internal/vulncheck/command.go @@ -26,9 +26,9 @@ func init() { Govulncheck = govulncheck } -func govulncheck(ctx context.Context, cfg *packages.Config, args command.VulncheckArgs) (res command.VulncheckResult, _ error) { - if args.Pattern == "" { - args.Pattern = "." +func govulncheck(ctx context.Context, cfg *packages.Config, patterns string) (res command.VulncheckResult, _ error) { + if patterns == "" { + patterns = "." } dbClient, err := client.NewClient(findGOVULNDB(cfg), client.Options{HTTPCache: gvc.DefaultCache()}) @@ -37,7 +37,7 @@ func govulncheck(ctx context.Context, cfg *packages.Config, args command.Vulnche } c := cmd{Client: dbClient} - vulns, err := c.Run(ctx, cfg, args.Pattern) + vulns, err := c.Run(ctx, cfg, patterns) if err != nil { return res, err } diff --git a/gopls/internal/vulncheck/vulncheck.go b/gopls/internal/vulncheck/vulncheck.go index 2c4d0d2978..7fc05ae423 100644 --- a/gopls/internal/vulncheck/vulncheck.go +++ b/gopls/internal/vulncheck/vulncheck.go @@ -18,6 +18,6 @@ import ( // Govulncheck runs the in-process govulncheck implementation. // With go1.18+, this is swapped with the real implementation. -var Govulncheck = func(ctx context.Context, cfg *packages.Config, args command.VulncheckArgs) (res command.VulncheckResult, _ error) { +var Govulncheck = func(ctx context.Context, cfg *packages.Config, patterns string) (res command.VulncheckResult, _ error) { return res, errors.New("not implemented") } diff --git a/internal/lsp/cmd/vulncheck.go b/internal/lsp/cmd/vulncheck.go index 4d245cecb6..19b3466a53 100644 --- a/internal/lsp/cmd/vulncheck.go +++ b/internal/lsp/cmd/vulncheck.go @@ -12,8 +12,6 @@ import ( "os" "golang.org/x/tools/go/packages" - "golang.org/x/tools/internal/lsp/command" - "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/tool" ) @@ -90,12 +88,10 @@ func (v *vulncheck) Run(ctx context.Context, args ...string) error { Tests: cfg.Tests, BuildFlags: cfg.BuildFlags, Env: cfg.Env, + Dir: cwd, } - res, err := opts.Hooks.Govulncheck(ctx, loadCfg, command.VulncheckArgs{ - Dir: protocol.URIFromPath(cwd), - Pattern: pattern, - }) + res, err := opts.Hooks.Govulncheck(ctx, loadCfg, pattern) if err != nil { return tool.CommandLineErrorf("govulncheck failed: %v", err) } diff --git a/internal/lsp/command.go b/internal/lsp/command.go index 23ade896a2..7348c17474 100644 --- a/internal/lsp/command.go +++ b/internal/lsp/command.go @@ -807,14 +807,14 @@ type pkgLoadConfig struct { } func (c *commandHandler) RunVulncheckExp(ctx context.Context, args command.VulncheckArgs) error { - if args.Dir == "" { - return errors.New("VulncheckArgs is missing Dir field") + if args.URI == "" { + return errors.New("VulncheckArgs is missing URI field") } err := c.run(ctx, commandConfig{ async: true, // need to be async to be cancellable progress: "Checking vulnerability", requireSave: true, - forURI: args.Dir, // Will dir work? + forURI: args.URI, }, func(ctx context.Context, deps commandDeps) error { view := deps.snapshot.View() opts := view.Options() @@ -823,7 +823,9 @@ func (c *commandHandler) RunVulncheckExp(ctx context.Context, args command.Vulnc } cmd := exec.Command(os.Args[0], "vulncheck", "-config", args.Pattern) - cmd.Dir = args.Dir.SpanURI().Filename() + // TODO(hyangah): if args.URI is not go.mod file, we need to + // adjust the directory accordingly. + cmd.Dir = filepath.Dir(args.URI.SpanURI().Filename()) var viewEnv []string if e := opts.EnvSlice(); e != nil { diff --git a/internal/lsp/command/interface.go b/internal/lsp/command/interface.go index 280fd616eb..4a4498a77e 100644 --- a/internal/lsp/command/interface.go +++ b/internal/lsp/command/interface.go @@ -314,8 +314,8 @@ type DebuggingResult struct { } type VulncheckArgs struct { - // Dir is the directory from which vulncheck will run from. - Dir protocol.DocumentURI + // Any document in the directory from which govulncheck will run. + URI protocol.DocumentURI // Package pattern. E.g. "", ".", "./...". Pattern string diff --git a/internal/lsp/source/api_json.go b/internal/lsp/source/api_json.go index 41f66d7c94..cf75792ff2 100755 --- a/internal/lsp/source/api_json.go +++ b/internal/lsp/source/api_json.go @@ -745,7 +745,7 @@ var GeneratedAPIJSON = &APIJSON{ Command: "gopls.run_vulncheck_exp", Title: "Run vulncheck (experimental)", Doc: "Run vulnerability check (`govulncheck`).", - ArgDoc: "{\n\t// Dir is the directory from which vulncheck will run from.\n\t\"Dir\": string,\n\t// Package pattern. E.g. \"\", \".\", \"./...\".\n\t\"Pattern\": string,\n}", + ArgDoc: "{\n\t// Any document in the directory from which govulncheck will run.\n\t\"URI\": string,\n\t// Package pattern. E.g. \"\", \".\", \"./...\".\n\t\"Pattern\": string,\n}", }, { Command: "gopls.start_debugging", diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go index 9073586939..126752bae7 100644 --- a/internal/lsp/source/options.go +++ b/internal/lsp/source/options.go @@ -514,7 +514,7 @@ type Hooks struct { StaticcheckAnalyzers map[string]*Analyzer // Govulncheck is the implementation of the Govulncheck gopls command. - Govulncheck func(context.Context, *packages.Config, command.VulncheckArgs) (command.VulncheckResult, error) + Govulncheck func(context.Context, *packages.Config, string) (command.VulncheckResult, error) } // InternalOptions contains settings that are not intended for use by the