From a8b9ed3e93f79481c8d1076d3d2ef91551fc4554 Mon Sep 17 00:00:00 2001 From: "Hana (Hyang-Ah) Kim" Date: Sun, 11 Sep 2022 18:04:30 -0400 Subject: [PATCH] gopls/internal/lsp/source: remove Govulncheck from Hooks Now lsp packages are moved to the gopls module where the vulncheck implementation exists. We no longer need to inject govulncheck through the hook. Change-Id: Ia627f1abe4c626d254d3e72b778535d6cb1ab41e Reviewed-on: https://go-review.googlesource.com/c/tools/+/429938 gopls-CI: kokoro TryBot-Result: Gopher Robot Reviewed-by: Robert Findley Run-TryBot: Hyang-Ah Hana Kim --- gopls/internal/hooks/hooks.go | 3 --- gopls/internal/lsp/cmd/vulncheck.go | 12 +++++++----- gopls/internal/lsp/command.go | 10 +++++++--- gopls/internal/lsp/source/options.go | 5 ----- gopls/internal/vulncheck/vulncheck.go | 5 +---- 5 files changed, 15 insertions(+), 20 deletions(-) diff --git a/gopls/internal/hooks/hooks.go b/gopls/internal/hooks/hooks.go index 9d497e9ed1..7c4ac9df8a 100644 --- a/gopls/internal/hooks/hooks.go +++ b/gopls/internal/hooks/hooks.go @@ -11,7 +11,6 @@ import ( "context" "golang.org/x/tools/gopls/internal/lsp/source" - "golang.org/x/tools/gopls/internal/vulncheck" "golang.org/x/tools/internal/diff" "mvdan.cc/gofumpt/format" "mvdan.cc/xurls/v2" @@ -37,6 +36,4 @@ func Options(options *source.Options) { }) } updateAnalyzers(options) - - options.Govulncheck = vulncheck.Govulncheck } diff --git a/gopls/internal/lsp/cmd/vulncheck.go b/gopls/internal/lsp/cmd/vulncheck.go index 770455cfd9..3ae2175a96 100644 --- a/gopls/internal/lsp/cmd/vulncheck.go +++ b/gopls/internal/lsp/cmd/vulncheck.go @@ -12,7 +12,7 @@ import ( "os" "golang.org/x/tools/go/packages" - "golang.org/x/tools/gopls/internal/lsp/source" + vulnchecklib "golang.org/x/tools/gopls/internal/vulncheck" "golang.org/x/tools/internal/tool" ) @@ -54,6 +54,10 @@ func (v *vulncheck) DetailedHelp(f *flag.FlagSet) { } func (v *vulncheck) Run(ctx context.Context, args ...string) error { + if vulnchecklib.Govulncheck == nil { + return fmt.Errorf("vulncheck command is available only in gopls compiled with go1.18 or newer") + } + if len(args) > 1 { return tool.CommandLineErrorf("vulncheck accepts at most one package pattern") } @@ -68,9 +72,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 { + if vulnchecklib.Govulncheck == nil { return fmt.Errorf("vulncheck feature is not available") } @@ -81,7 +83,7 @@ func (v *vulncheck) Run(ctx context.Context, args ...string) error { // inherit the current process's cwd and env. } - res, err := opts.Hooks.Govulncheck(ctx, loadCfg, pattern) + res, err := vulnchecklib.Govulncheck(ctx, loadCfg, pattern) if err != nil { return fmt.Errorf("vulncheck failed: %v", err) } diff --git a/gopls/internal/lsp/command.go b/gopls/internal/lsp/command.go index 1868255742..f01bcb1a23 100644 --- a/gopls/internal/lsp/command.go +++ b/gopls/internal/lsp/command.go @@ -20,13 +20,14 @@ import ( "golang.org/x/mod/modfile" "golang.org/x/tools/go/ast/astutil" - "golang.org/x/tools/internal/event" - "golang.org/x/tools/internal/gocommand" "golang.org/x/tools/gopls/internal/lsp/command" "golang.org/x/tools/gopls/internal/lsp/debug" "golang.org/x/tools/gopls/internal/lsp/progress" "golang.org/x/tools/gopls/internal/lsp/protocol" "golang.org/x/tools/gopls/internal/lsp/source" + "golang.org/x/tools/gopls/internal/vulncheck" + "golang.org/x/tools/internal/event" + "golang.org/x/tools/internal/gocommand" "golang.org/x/tools/internal/span" "golang.org/x/tools/internal/xcontext" ) @@ -829,7 +830,10 @@ func (c *commandHandler) RunVulncheckExp(ctx context.Context, args command.Vulnc }, func(ctx context.Context, deps commandDeps) error { view := deps.snapshot.View() opts := view.Options() - if opts == nil || opts.Hooks.Govulncheck == nil { + // quickly test if gopls is compiled to support govulncheck + // by checking vulncheck.Govulncheck. Alternatively, we can continue and + // let the `gopls vulncheck` command fail. This is lighter-weight. + if vulncheck.Govulncheck == nil { return errors.New("vulncheck feature is not available") } diff --git a/gopls/internal/lsp/source/options.go b/gopls/internal/lsp/source/options.go index fdcb98c522..45aeca99dd 100644 --- a/gopls/internal/lsp/source/options.go +++ b/gopls/internal/lsp/source/options.go @@ -49,7 +49,6 @@ import ( "golang.org/x/tools/go/analysis/passes/unsafeptr" "golang.org/x/tools/go/analysis/passes/unusedresult" "golang.org/x/tools/go/analysis/passes/unusedwrite" - "golang.org/x/tools/go/packages" "golang.org/x/tools/gopls/internal/lsp/analysis/embeddirective" "golang.org/x/tools/gopls/internal/lsp/analysis/fillreturns" "golang.org/x/tools/gopls/internal/lsp/analysis/fillstruct" @@ -519,9 +518,6 @@ type Hooks struct { TypeErrorAnalyzers map[string]*Analyzer ConvenienceAnalyzers map[string]*Analyzer StaticcheckAnalyzers map[string]*Analyzer - - // Govulncheck is the implementation of the Govulncheck gopls command. - Govulncheck func(context.Context, *packages.Config, string) (command.VulncheckResult, error) } // InternalOptions contains settings that are not intended for use by the @@ -773,7 +769,6 @@ func (o *Options) Clone() *Options { ComputeEdits: o.ComputeEdits, GofumptFormat: o.GofumptFormat, URLRegexp: o.URLRegexp, - Govulncheck: o.Govulncheck, }, ServerOptions: o.ServerOptions, UserOptions: o.UserOptions, diff --git a/gopls/internal/vulncheck/vulncheck.go b/gopls/internal/vulncheck/vulncheck.go index 227b57d7f9..d452045a5e 100644 --- a/gopls/internal/vulncheck/vulncheck.go +++ b/gopls/internal/vulncheck/vulncheck.go @@ -10,7 +10,6 @@ package vulncheck import ( "context" - "errors" "golang.org/x/tools/go/packages" "golang.org/x/tools/gopls/internal/lsp/command" @@ -18,6 +17,4 @@ 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, patterns string) (res command.VulncheckResult, _ error) { - return res, errors.New("not implemented") -} +var Govulncheck func(ctx context.Context, cfg *packages.Config, patterns string) (res command.VulncheckResult, _ error) = nil