internal/lsp: use options hooks to install diff driver
Change-Id: I2f94c2a68d0036a47ccac3fce07cf9f3b784d443
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200558
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/cmd/gopls/main.go b/cmd/gopls/main.go
index d8ab337..6b6e735 100644
--- a/cmd/gopls/main.go
+++ b/cmd/gopls/main.go
@@ -19,5 +19,5 @@
func main() {
debug.Version += "-cmd.gopls"
- tool.Main(context.Background(), cmd.New("gopls-legacy", "", nil), os.Args[1:])
+ tool.Main(context.Background(), cmd.New("gopls-legacy", "", nil, nil), os.Args[1:])
}
diff --git a/gopls/internal/hooks/analysis.go b/gopls/internal/hooks/analysis.go
index ef385a3..ca437ad 100644
--- a/gopls/internal/hooks/analysis.go
+++ b/gopls/internal/hooks/analysis.go
@@ -5,24 +5,22 @@
package hooks
import (
- "golang.org/x/tools/go/analysis"
"golang.org/x/tools/internal/lsp/source"
"honnef.co/go/tools/simple"
"honnef.co/go/tools/staticcheck"
"honnef.co/go/tools/stylecheck"
)
-func updateAnalyzers(v source.View, analyzers []*analysis.Analyzer) []*analysis.Analyzer {
- if v.Options().StaticCheck {
+func updateAnalyzers(options *source.Options) {
+ if options.StaticCheck {
for _, a := range simple.Analyzers {
- analyzers = append(analyzers, a)
+ options.Analyzers = append(options.Analyzers, a)
}
for _, a := range staticcheck.Analyzers {
- analyzers = append(analyzers, a)
+ options.Analyzers = append(options.Analyzers, a)
}
for _, a := range stylecheck.Analyzers {
- analyzers = append(analyzers, a)
+ options.Analyzers = append(options.Analyzers, a)
}
}
- return analyzers
}
diff --git a/gopls/internal/hooks/hooks.go b/gopls/internal/hooks/hooks.go
index f9127dc..9037c3b 100644
--- a/gopls/internal/hooks/hooks.go
+++ b/gopls/internal/hooks/hooks.go
@@ -8,12 +8,12 @@
package hooks // import "golang.org/x/tools/gopls/internal/hooks"
import (
- "context"
-
- "golang.org/x/tools/internal/lsp/cache"
+ "golang.org/x/tools/internal/lsp/source"
)
-func Install(ctx context.Context) context.Context {
- cache.UpdateAnalyzers = updateAnalyzers
- return ctx
+func Options(options *source.Options) {
+ if options.GoDiff {
+ options.ComputeEdits = ComputeEdits
+ }
+ updateAnalyzers(options)
}
diff --git a/gopls/main.go b/gopls/main.go
index 31d356e..0b4d889 100644
--- a/gopls/main.go
+++ b/gopls/main.go
@@ -19,6 +19,5 @@
func main() {
ctx := context.Background()
- ctx = hooks.Install(ctx)
- tool.Main(ctx, cmd.New("gopls", "", nil), os.Args[1:])
+ tool.Main(ctx, cmd.New("gopls", "", nil, hooks.Options), os.Args[1:])
}
diff --git a/gopls/test/gopls_test.go b/gopls/test/gopls_test.go
index 5245579..15ecfa4 100644
--- a/gopls/test/gopls_test.go
+++ b/gopls/test/gopls_test.go
@@ -5,7 +5,6 @@
package gopls_test
import (
- "context"
"os"
"testing"
@@ -30,9 +29,7 @@
if stat, err := os.Stat(testdata); err != nil || !stat.IsDir() {
t.Skip("testdata directory not present")
}
- ctx := context.Background()
- hooks.Install(ctx)
data := tests.Load(t, exporter, testdata)
defer data.Exported.Cleanup()
- tests.Run(t, cmdtest.NewRunner(exporter, data, tests.Context(t)), data)
+ tests.Run(t, cmdtest.NewRunner(exporter, data, tests.Context(t), hooks.Options), data)
}
diff --git a/internal/lsp/cache/analysis.go b/internal/lsp/cache/analysis.go
deleted file mode 100644
index 21a294f..0000000
--- a/internal/lsp/cache/analysis.go
+++ /dev/null
@@ -1,65 +0,0 @@
-// Copyright 2019 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package cache
-
-import (
- "golang.org/x/tools/go/analysis"
- "golang.org/x/tools/go/analysis/passes/asmdecl"
- "golang.org/x/tools/go/analysis/passes/assign"
- "golang.org/x/tools/go/analysis/passes/atomic"
- "golang.org/x/tools/go/analysis/passes/atomicalign"
- "golang.org/x/tools/go/analysis/passes/bools"
- "golang.org/x/tools/go/analysis/passes/buildtag"
- "golang.org/x/tools/go/analysis/passes/cgocall"
- "golang.org/x/tools/go/analysis/passes/composite"
- "golang.org/x/tools/go/analysis/passes/copylock"
- "golang.org/x/tools/go/analysis/passes/httpresponse"
- "golang.org/x/tools/go/analysis/passes/loopclosure"
- "golang.org/x/tools/go/analysis/passes/lostcancel"
- "golang.org/x/tools/go/analysis/passes/nilfunc"
- "golang.org/x/tools/go/analysis/passes/printf"
- "golang.org/x/tools/go/analysis/passes/shift"
- "golang.org/x/tools/go/analysis/passes/sortslice"
- "golang.org/x/tools/go/analysis/passes/stdmethods"
- "golang.org/x/tools/go/analysis/passes/structtag"
- "golang.org/x/tools/go/analysis/passes/tests"
- "golang.org/x/tools/go/analysis/passes/unmarshal"
- "golang.org/x/tools/go/analysis/passes/unreachable"
- "golang.org/x/tools/go/analysis/passes/unsafeptr"
- "golang.org/x/tools/go/analysis/passes/unusedresult"
- "golang.org/x/tools/internal/lsp/source"
-)
-
-var defaultAnalyzers = []*analysis.Analyzer{
- // The traditional vet suite:
- asmdecl.Analyzer,
- assign.Analyzer,
- atomic.Analyzer,
- atomicalign.Analyzer,
- bools.Analyzer,
- buildtag.Analyzer,
- cgocall.Analyzer,
- composite.Analyzer,
- copylock.Analyzer,
- httpresponse.Analyzer,
- loopclosure.Analyzer,
- lostcancel.Analyzer,
- nilfunc.Analyzer,
- printf.Analyzer,
- shift.Analyzer,
- stdmethods.Analyzer,
- structtag.Analyzer,
- tests.Analyzer,
- unmarshal.Analyzer,
- unreachable.Analyzer,
- unsafeptr.Analyzer,
- unusedresult.Analyzer,
- // Non-vet analyzers
- sortslice.Analyzer,
-}
-
-var UpdateAnalyzers = func(v source.View, analyzers []*analysis.Analyzer) []*analysis.Analyzer {
- return analyzers
-}
diff --git a/internal/lsp/cache/cache.go b/internal/lsp/cache/cache.go
index 764d077..e3a1396 100644
--- a/internal/lsp/cache/cache.go
+++ b/internal/lsp/cache/cache.go
@@ -18,21 +18,23 @@
"golang.org/x/tools/internal/span"
)
-func New() source.Cache {
+func New(options func(*source.Options)) source.Cache {
index := atomic.AddInt64(&cacheIndex, 1)
c := &cache{
- fs: &nativeFileSystem{},
- id: strconv.FormatInt(index, 10),
- fset: token.NewFileSet(),
+ fs: &nativeFileSystem{},
+ id: strconv.FormatInt(index, 10),
+ fset: token.NewFileSet(),
+ options: options,
}
debug.AddCache(debugCache{c})
return c
}
type cache struct {
- fs source.FileSystem
- id string
- fset *token.FileSet
+ fs source.FileSystem
+ id string
+ fset *token.FileSet
+ options func(*source.Options)
store memoize.Store
}
diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go
index 5a41423..805e8b5 100644
--- a/internal/lsp/cache/session.go
+++ b/internal/lsp/cache/session.go
@@ -110,7 +110,10 @@
}
v.snapshot.view = v
- v.analyzers = UpdateAnalyzers(v, defaultAnalyzers)
+ if v.session.cache.options != nil {
+ v.session.cache.options(&v.options)
+ }
+
// Preemptively build the builtin package,
// so we immediately add builtin.go to the list of ignored files.
v.buildBuiltinPackage(ctx)
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index c840a81..436ae3a 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -15,7 +15,6 @@
"strings"
"sync"
- "golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/packages"
"golang.org/x/tools/internal/imports"
"golang.org/x/tools/internal/lsp/debug"
@@ -80,8 +79,6 @@
// ignoredURIs is the set of URIs of files that we ignore.
ignoredURIsMu sync.Mutex
ignoredURIs map[span.URI]struct{}
-
- analyzers []*analysis.Analyzer
}
func (v *view) Session() source.Session {
@@ -396,10 +393,6 @@
return nil, nil
}
-func (v *view) Analyzers() []*analysis.Analyzer {
- return v.analyzers
-}
-
func (f *fileBase) addURI(uri span.URI) int {
f.uris = append(f.uris, uri)
return len(f.uris)
diff --git a/internal/lsp/cmd/cmd.go b/internal/lsp/cmd/cmd.go
index eff20ab..d74a5af 100644
--- a/internal/lsp/cmd/cmd.go
+++ b/internal/lsp/cmd/cmd.go
@@ -65,15 +65,19 @@
// Control ocagent export of telemetry
OCAgent string `flag:"ocagent" help:"The address of the ocagent, or off"`
+
+ // PrepareOptions is called to update the options when a new view is built.
+ // It is primarily to allow the behavior of gopls to be modified by hooks.
+ PrepareOptions func(*source.Options)
}
// Returns a new Application ready to run.
-func New(name, wd string, env []string) *Application {
+func New(name, wd string, env []string, options func(*source.Options)) *Application {
if wd == "" {
wd, _ = os.Getwd()
}
app := &Application{
- cache: cache.New(),
+ cache: cache.New(options),
name: name,
wd: wd,
env: env,
diff --git a/internal/lsp/cmd/cmd_test.go b/internal/lsp/cmd/cmd_test.go
index 1440459..af04e8d 100644
--- a/internal/lsp/cmd/cmd_test.go
+++ b/internal/lsp/cmd/cmd_test.go
@@ -32,7 +32,7 @@
func testCommandLine(t *testing.T, exporter packagestest.Exporter) {
data := tests.Load(t, exporter, "../testdata")
defer data.Exported.Cleanup()
- tests.Run(t, cmdtest.NewRunner(exporter, data, tests.Context(t)), data)
+ tests.Run(t, cmdtest.NewRunner(exporter, data, tests.Context(t), nil), data)
}
func TestDefinitionHelpExample(t *testing.T) {
@@ -54,7 +54,7 @@
fmt.Sprintf("%v:#%v", thisFile, cmd.ExampleOffset)} {
args := append(baseArgs, query)
got := cmdtest.CaptureStdOut(t, func() {
- _ = tool.Run(tests.Context(t), cmd.New("gopls-test", "", nil), args)
+ _ = tool.Run(tests.Context(t), cmd.New("gopls-test", "", nil, nil), args)
})
if !expect.MatchString(got) {
t.Errorf("test with %v\nexpected:\n%s\ngot:\n%s", args, expect, got)
diff --git a/internal/lsp/cmd/test/check.go b/internal/lsp/cmd/test/check.go
index 39dd3fb..c368998 100644
--- a/internal/lsp/cmd/test/check.go
+++ b/internal/lsp/cmd/test/check.go
@@ -22,7 +22,7 @@
}
fname := uri.Filename()
args := []string{"-remote=internal", "check", fname}
- app := cmd.New("gopls-test", r.data.Config.Dir, r.data.Exported.Config.Env)
+ app := cmd.New("gopls-test", r.data.Config.Dir, r.data.Exported.Config.Env, r.options)
out := CaptureStdOut(t, func() {
_ = tool.Run(r.ctx, app, args)
})
diff --git a/internal/lsp/cmd/test/cmdtest.go b/internal/lsp/cmd/test/cmdtest.go
index 13607f2..8aa8638 100644
--- a/internal/lsp/cmd/test/cmdtest.go
+++ b/internal/lsp/cmd/test/cmdtest.go
@@ -26,13 +26,15 @@
exporter packagestest.Exporter
data *tests.Data
ctx context.Context
+ options func(*source.Options)
}
-func NewRunner(exporter packagestest.Exporter, data *tests.Data, ctx context.Context) tests.Tests {
+func NewRunner(exporter packagestest.Exporter, data *tests.Data, ctx context.Context, options func(*source.Options)) tests.Tests {
return &runner{
exporter: exporter,
data: data,
ctx: ctx,
+ options: options,
}
}
diff --git a/internal/lsp/cmd/test/definition.go b/internal/lsp/cmd/test/definition.go
index 6bbcdd1..e91b511 100644
--- a/internal/lsp/cmd/test/definition.go
+++ b/internal/lsp/cmd/test/definition.go
@@ -54,7 +54,7 @@
uri := d.Src.URI()
args = append(args, fmt.Sprint(d.Src))
got := CaptureStdOut(t, func() {
- app := cmd.New("gopls-test", r.data.Config.Dir, r.data.Exported.Config.Env)
+ app := cmd.New("gopls-test", r.data.Config.Dir, r.data.Exported.Config.Env, r.options)
_ = tool.Run(r.ctx, app, args)
})
got = normalizePaths(r.data, got)
diff --git a/internal/lsp/cmd/test/format.go b/internal/lsp/cmd/test/format.go
index 0418b31..355f0d9 100644
--- a/internal/lsp/cmd/test/format.go
+++ b/internal/lsp/cmd/test/format.go
@@ -33,7 +33,7 @@
//TODO: our error handling differs, for now just skip unformattable files
t.Skip("Unformattable file")
}
- app := cmd.New("gopls-test", r.data.Config.Dir, r.data.Config.Env)
+ app := cmd.New("gopls-test", r.data.Config.Dir, r.data.Config.Env, r.options)
got := CaptureStdOut(t, func() {
_ = tool.Run(r.ctx, app, append([]string{"-remote=internal", "format"}, filename))
})
diff --git a/internal/lsp/cmd/test/rename.go b/internal/lsp/cmd/test/rename.go
index c5297b7..9b227b9 100644
--- a/internal/lsp/cmd/test/rename.go
+++ b/internal/lsp/cmd/test/rename.go
@@ -16,7 +16,7 @@
func (r *runner) Rename(t *testing.T, spn span.Span, newText string) {
filename := spn.URI().Filename()
goldenTag := newText + "-rename"
- app := cmd.New("gopls-test", r.data.Config.Dir, r.data.Config.Env)
+ app := cmd.New("gopls-test", r.data.Config.Dir, r.data.Config.Env, r.options)
loc := fmt.Sprintf("%v", spn)
var err error
got := CaptureStdOut(t, func() {
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index 4b2856c..283d458 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -48,7 +48,7 @@
data := tests.Load(t, exporter, "testdata")
defer data.Exported.Cleanup()
- cache := cache.New()
+ cache := cache.New(nil)
session := cache.NewSession(ctx)
options := tests.DefaultOptions()
session.SetOptions(options)
diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go
index 04c4737..a528bac 100644
--- a/internal/lsp/source/diagnostics.go
+++ b/internal/lsp/source/diagnostics.go
@@ -305,7 +305,7 @@
func runAnalyses(ctx context.Context, view View, cph CheckPackageHandle, disabledAnalyses map[string]struct{}, report func(a *analysis.Analyzer, diag analysis.Diagnostic) error) error {
var analyzers []*analysis.Analyzer
- for _, a := range view.Analyzers() {
+ for _, a := range view.Options().Analyzers {
if _, ok := disabledAnalyses[a.Name]; ok {
continue
}
diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go
index c89967d..e1bc782 100644
--- a/internal/lsp/source/options.go
+++ b/internal/lsp/source/options.go
@@ -9,6 +9,30 @@
"os"
"time"
+ "golang.org/x/tools/go/analysis"
+ "golang.org/x/tools/go/analysis/passes/asmdecl"
+ "golang.org/x/tools/go/analysis/passes/assign"
+ "golang.org/x/tools/go/analysis/passes/atomic"
+ "golang.org/x/tools/go/analysis/passes/atomicalign"
+ "golang.org/x/tools/go/analysis/passes/bools"
+ "golang.org/x/tools/go/analysis/passes/buildtag"
+ "golang.org/x/tools/go/analysis/passes/cgocall"
+ "golang.org/x/tools/go/analysis/passes/composite"
+ "golang.org/x/tools/go/analysis/passes/copylock"
+ "golang.org/x/tools/go/analysis/passes/httpresponse"
+ "golang.org/x/tools/go/analysis/passes/loopclosure"
+ "golang.org/x/tools/go/analysis/passes/lostcancel"
+ "golang.org/x/tools/go/analysis/passes/nilfunc"
+ "golang.org/x/tools/go/analysis/passes/printf"
+ "golang.org/x/tools/go/analysis/passes/shift"
+ "golang.org/x/tools/go/analysis/passes/sortslice"
+ "golang.org/x/tools/go/analysis/passes/stdmethods"
+ "golang.org/x/tools/go/analysis/passes/structtag"
+ "golang.org/x/tools/go/analysis/passes/tests"
+ "golang.org/x/tools/go/analysis/passes/unmarshal"
+ "golang.org/x/tools/go/analysis/passes/unreachable"
+ "golang.org/x/tools/go/analysis/passes/unsafeptr"
+ "golang.org/x/tools/go/analysis/passes/unusedresult"
"golang.org/x/tools/internal/lsp/diff"
"golang.org/x/tools/internal/lsp/diff/myers"
"golang.org/x/tools/internal/lsp/protocol"
@@ -43,6 +67,7 @@
Budget: 100 * time.Millisecond,
},
ComputeEdits: myers.ComputeEdits,
+ Analyzers: defaultAnalyzers,
}
)
@@ -57,6 +82,7 @@
DisabledAnalyses map[string]struct{}
StaticCheck bool
+ GoDiff bool
WatchFileChanges bool
InsertTextFormat protocol.InsertTextFormat
@@ -76,6 +102,8 @@
Completion CompletionOptions
ComputeEdits diff.ComputeEdits
+
+ Analyzers []*analysis.Analyzer
}
type CompletionOptions struct {
@@ -246,6 +274,9 @@
case "staticcheck":
result.setBool(&o.StaticCheck)
+ case "go-diff":
+ result.setBool(&o.GoDiff)
+
// Deprecated settings.
case "wantSuggestedFixes":
result.State = OptionDeprecated
@@ -290,3 +321,31 @@
*b = v
}
}
+
+var defaultAnalyzers = []*analysis.Analyzer{
+ // The traditional vet suite:
+ asmdecl.Analyzer,
+ assign.Analyzer,
+ atomic.Analyzer,
+ atomicalign.Analyzer,
+ bools.Analyzer,
+ buildtag.Analyzer,
+ cgocall.Analyzer,
+ composite.Analyzer,
+ copylock.Analyzer,
+ httpresponse.Analyzer,
+ loopclosure.Analyzer,
+ lostcancel.Analyzer,
+ nilfunc.Analyzer,
+ printf.Analyzer,
+ shift.Analyzer,
+ stdmethods.Analyzer,
+ structtag.Analyzer,
+ tests.Analyzer,
+ unmarshal.Analyzer,
+ unreachable.Analyzer,
+ unsafeptr.Analyzer,
+ unusedresult.Analyzer,
+ // Non-vet analyzers
+ sortslice.Analyzer,
+}
diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go
index 360c019..933dcb4 100644
--- a/internal/lsp/source/source_test.go
+++ b/internal/lsp/source/source_test.go
@@ -48,7 +48,7 @@
data := tests.Load(t, exporter, "../testdata")
defer data.Exported.Cleanup()
- cache := cache.New()
+ cache := cache.New(nil)
session := cache.NewSession(ctx)
options := tests.DefaultOptions()
options.Env = data.Config.Env
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index fa85bc0..d23d7cb 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -240,9 +240,6 @@
// This function does not correctly invalidate the view when needed.
SetOptions(Options)
- // Analyzers returns the set of Analyzers active for this view.
- Analyzers() []*analysis.Analyzer
-
// CheckPackageHandles returns the CheckPackageHandles for the packages
// that this file belongs to.
CheckPackageHandles(ctx context.Context, f File) (Snapshot, []CheckPackageHandle, error)
