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>

cmd/gopls/main.go

@@ -19,5 +19,5 @@ import (
 
 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:])
 }

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 {
+func updateAnalyzers(options *source.Options) {
-	if v.Options().StaticCheck {
+	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
 }

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/source"
-
-	"golang.org/x/tools/internal/lsp/cache"
 )
 
-func Install(ctx context.Context) context.Context {
+func Options(options *source.Options) {
-	cache.UpdateAnalyzers = updateAnalyzers
+	if options.GoDiff {
-	return ctx
+		options.ComputeEdits = ComputeEdits
+	}
+	updateAnalyzers(options)
 }

gopls/main.go

@@ -19,6 +19,5 @@ import (
 
 func main() {
 	ctx := context.Background()
-	ctx = hooks.Install(ctx)
+	tool.Main(ctx, cmd.New("gopls", "", nil, hooks.Options), os.Args[1:])
-	tool.Main(ctx, cmd.New("gopls", "", nil), os.Args[1:])
 }

gopls/test/gopls_test.go

@@ -5,7 +5,6 @@
 package gopls_test
 
 import (
-	"context"
 	"os"
 	"testing"
 
@@ -30,9 +29,7 @@ func testCommandLine(t *testing.T, exporter packagestest.Exporter) {
 	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)
 }

internal/lsp/cache/analysis.go

@@ -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
-}

internal/lsp/cache/cache.go

@@ -18,21 +18,23 @@ import (
 	"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{},
+		fs:      &nativeFileSystem{},
-		id:   strconv.FormatInt(index, 10),
+		id:      strconv.FormatInt(index, 10),
-		fset: token.NewFileSet(),
+		fset:    token.NewFileSet(),
+		options: options,
 	}
 	debug.AddCache(debugCache{c})
 	return c
 }
 
 type cache struct {
-	fs   source.FileSystem
+	fs      source.FileSystem
-	id   string
+	id      string
-	fset *token.FileSet
+	fset    *token.FileSet
+	options func(*source.Options)
 
 	store memoize.Store
 }

internal/lsp/cache/session.go

@@ -110,7 +110,10 @@ func (s *session) NewView(ctx context.Context, name string, folder span.URI, opt
 	}
 	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)

internal/lsp/cache/view.go

@@ -15,7 +15,6 @@ import (
 	"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 @@ type view struct {
 	// 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 @@ func (v *view) findFile(uri span.URI) (viewFile, error) {
 	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)

internal/lsp/cmd/cmd.go

@@ -65,15 +65,19 @@ type Application struct {
 
 	// 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,

internal/lsp/cmd/cmd_test.go

@@ -32,7 +32,7 @@ func TestCommandLine(t *testing.T) {
 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 @@ func TestDefinitionHelpExample(t *testing.T) {
 		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)

internal/lsp/cmd/test/check.go

@@ -22,7 +22,7 @@ func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnosti
 	}
 	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)
 	})

internal/lsp/cmd/test/cmdtest.go

@@ -26,13 +26,15 @@ type runner struct {
 	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,
 	}
 }
 

internal/lsp/cmd/test/definition.go

@@ -54,7 +54,7 @@ func (r *runner) Definition(t *testing.T, spn span.Span, d tests.Definition) {
 		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)

internal/lsp/cmd/test/format.go

@@ -33,7 +33,7 @@ func (r *runner) Format(t *testing.T, spn span.Span) {
 		//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))
 	})

internal/lsp/cmd/test/rename.go

@@ -16,7 +16,7 @@ import (
 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() {

internal/lsp/lsp_test.go

@@ -48,7 +48,7 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) {
 	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)

internal/lsp/source/diagnostics.go

@@ -305,7 +305,7 @@ func singleDiagnostic(uri span.URI, format string, a ...interface{}) map[span.UR
 
 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
 		}

internal/lsp/source/options.go

@@ -9,6 +9,30 @@ import (
 	"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 @@ var (
 			Budget:        100 * time.Millisecond,
 		},
 		ComputeEdits: myers.ComputeEdits,
+		Analyzers:    defaultAnalyzers,
 	}
 )
 
@@ -57,6 +82,7 @@ type Options struct {
 	DisabledAnalyses map[string]struct{}
 
 	StaticCheck bool
+	GoDiff      bool
 
 	WatchFileChanges              bool
 	InsertTextFormat              protocol.InsertTextFormat
@@ -76,6 +102,8 @@ type Options struct {
 	Completion CompletionOptions
 
 	ComputeEdits diff.ComputeEdits
+
+	Analyzers []*analysis.Analyzer
 }
 
 type CompletionOptions struct {
@@ -246,6 +274,9 @@ func (o *Options) set(name string, value interface{}) OptionResult {
 	case "staticcheck":
 		result.setBool(&o.StaticCheck)
 
+	case "go-diff":
+		result.setBool(&o.GoDiff)
+
 	// Deprecated settings.
 	case "wantSuggestedFixes":
 		result.State = OptionDeprecated
@@ -290,3 +321,31 @@ func (r *OptionResult) setBool(b *bool) {
 		*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,
+}

internal/lsp/source/source_test.go

@@ -48,7 +48,7 @@ func testSource(t *testing.T, exporter packagestest.Exporter) {
 	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

internal/lsp/source/view.go

@@ -240,9 +240,6 @@ type View interface {
 	// 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)