gopls: eliminate the hooks package

Simplify some of the legacy indirection that used to be required when
the bulk of gopls' implementation lived in the x/tools module. We no
longer need to set hooks on the settings.Options; hooked-in
implementations can simply be statically linked.

Remove the settings.Hooks type, and pull the thread as far as it will
go, cleaning up a bunch of unnecessary indirection. As a result, we no
longer need the hooks package.

Change-Id: Ifd71da13174af4b7bc733f97774830ec1d98a2bc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/578039
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>

Author: findleyr

gopls/doc/generate.go

@@ -111,7 +111,7 @@ func loadAPI() (*settings.APIJSON, error) {
 	defaults := settings.DefaultOptions()
 	api := &settings.APIJSON{
 		Options:   map[string][]*settings.OptionJSON{},
-		Analyzers: loadAnalyzers(defaults.DefaultAnalyzers), // no staticcheck analyzers
+		Analyzers: loadAnalyzers(settings.DefaultAnalyzers), // no staticcheck analyzers
 	}
 
 	api.Commands, err = loadCommands()
@@ -508,17 +508,17 @@ func loadLenses(commands []*settings.CommandJSON) []*settings.LensJSON {
 func loadAnalyzers(m map[string]*settings.Analyzer) []*settings.AnalyzerJSON {
 	var sorted []string
 	for _, a := range m {
-		sorted = append(sorted, a.Analyzer.Name)
+		sorted = append(sorted, a.Analyzer().Name)
 	}
 	sort.Strings(sorted)
 	var json []*settings.AnalyzerJSON
 	for _, name := range sorted {
 		a := m[name]
 		json = append(json, &settings.AnalyzerJSON{
-			Name:    a.Analyzer.Name,
-			Doc:     a.Analyzer.Doc,
-			URL:     a.Analyzer.URL,
-			Default: a.Enabled,
+			Name:    a.Analyzer().Name,
+			Doc:     a.Analyzer().Doc,
+			URL:     a.Analyzer().URL,
+			Default: a.EnabledByDefault(),
 		})
 	}
 	return json

gopls/internal/cache/analysis.go

@@ -192,30 +192,30 @@ func (s *Snapshot) Analyze(ctx context.Context, pkgs map[PackageID]*metadata.Pac
 	// Filter and sort enabled root analyzers.
 	// A disabled analyzer may still be run if required by another.
 	toSrc := make(map[*analysis.Analyzer]*settings.Analyzer)
-	var enabled []*analysis.Analyzer // enabled subset + transitive requirements
+	var enabledAnalyzers []*analysis.Analyzer // enabled subset + transitive requirements
 	for _, a := range analyzers {
-		if a.IsEnabled(s.Options()) {
-			toSrc[a.Analyzer] = a
-			enabled = append(enabled, a.Analyzer)
+		if enabled, ok := s.Options().Analyses[a.Analyzer().Name]; enabled || !ok && a.EnabledByDefault() {
+			toSrc[a.Analyzer()] = a
+			enabledAnalyzers = append(enabledAnalyzers, a.Analyzer())
 		}
 	}
-	sort.Slice(enabled, func(i, j int) bool {
-		return enabled[i].Name < enabled[j].Name
+	sort.Slice(enabledAnalyzers, func(i, j int) bool {
+		return enabledAnalyzers[i].Name < enabledAnalyzers[j].Name
 	})
 	analyzers = nil // prevent accidental use
 
-	enabled = requiredAnalyzers(enabled)
+	enabledAnalyzers = requiredAnalyzers(enabledAnalyzers)
 
 	// Perform basic sanity checks.
 	// (Ideally we would do this only once.)
-	if err := analysis.Validate(enabled); err != nil {
+	if err := analysis.Validate(enabledAnalyzers); err != nil {
 		return nil, fmt.Errorf("invalid analyzer configuration: %v", err)
 	}
 
 	stableNames := make(map[*analysis.Analyzer]string)
 
 	var facty []*analysis.Analyzer // facty subset of enabled + transitive requirements
-	for _, a := range enabled {
+	for _, a := range enabledAnalyzers {
 		// TODO(adonovan): reject duplicate stable names (very unlikely).
 		stableNames[a] = stableName(a)
 
@@ -317,7 +317,7 @@ func (s *Snapshot) Analyze(ctx context.Context, pkgs map[PackageID]*metadata.Pac
 		if err != nil {
 			return nil, err
 		}
-		root.analyzers = enabled
+		root.analyzers = enabledAnalyzers
 		roots = append(roots, root)
 	}
 
@@ -451,7 +451,7 @@ func (s *Snapshot) Analyze(ctx context.Context, pkgs map[PackageID]*metadata.Pac
 	// results, we should propagate the per-action errors.
 	var results []*Diagnostic
 	for _, root := range roots {
-		for _, a := range enabled {
+		for _, a := range enabledAnalyzers {
 			// Skip analyzers that were added only to
 			// fulfil requirements of the original set.
 			srcAnalyzer, ok := toSrc[a]

gopls/internal/cache/errors.go

@@ -279,7 +279,7 @@ func toSourceDiagnostic(srcAnalyzer *settings.Analyzer, gobDiag *gobDiagnostic)
 		related = append(related, protocol.DiagnosticRelatedInformation(gobRelated))
 	}
 
-	severity := srcAnalyzer.Severity
+	severity := srcAnalyzer.Severity()
 	if severity == 0 {
 		severity = protocol.SeverityWarning
 	}
@@ -293,15 +293,15 @@ func toSourceDiagnostic(srcAnalyzer *settings.Analyzer, gobDiag *gobDiagnostic)
 		Source:   DiagnosticSource(gobDiag.Source),
 		Message:  gobDiag.Message,
 		Related:  related,
-		Tags:     srcAnalyzer.Tag,
+		Tags:     srcAnalyzer.Tags(),
 	}
 
 	// We cross the set of fixes (whether edit- or command-based)
 	// with the set of kinds, as a single fix may represent more
 	// than one kind of action (e.g. refactor, quickfix, fixall),
 	// each corresponding to a distinct client UI element
 	// or operation.
-	kinds := srcAnalyzer.ActionKinds
+	kinds := srcAnalyzer.ActionKinds()
 	if len(kinds) == 0 {
 		kinds = []protocol.CodeActionKind{protocol.QuickFix}
 	}

gopls/internal/cmd/capabilities_test.go

@@ -40,7 +40,7 @@ func TestCapabilities(t *testing.T) {
 	}
 	defer os.RemoveAll(tmpDir)
 
-	app := New(nil)
+	app := New()
 
 	params := &protocol.ParamInitialize{}
 	params.RootURI = protocol.URIFromPath(tmpDir)

gopls/internal/cmd/check.go

@@ -45,7 +45,9 @@ func (c *check) Run(ctx context.Context, args ...string) error {
 	// false. Investigate.
 	origOptions := c.app.options
 	c.app.options = func(opts *settings.Options) {
-		origOptions(opts)
+		if origOptions != nil {
+			origOptions(opts)
+		}
 		opts.RelatedInformationSupported = true
 	}
 

gopls/internal/cmd/cmd.go

@@ -95,9 +95,8 @@ func (app *Application) verbose() bool {
 }
 
 // New returns a new Application ready to run.
-func New(options func(*settings.Options)) *Application {
+func New() *Application {
 	app := &Application{
-		options: options,
 		OCAgent: "off", //TODO: Remove this line to default the exporter to on
 
 		Serve: Serve{

gopls/internal/cmd/codelens.go

@@ -74,7 +74,9 @@ func (r *codelens) Run(ctx context.Context, args ...string) error {
 	// See golang.LensFuncs().
 	origOptions := r.app.options
 	r.app.options = func(opts *settings.Options) {
-		origOptions(opts)
+		if origOptions != nil {
+			origOptions(opts)
+		}
 		if opts.Codelenses == nil {
 			opts.Codelenses = make(map[string]bool)
 		}

gopls/internal/cmd/help_test.go

@@ -32,7 +32,7 @@ const appName = "gopls"
 
 func TestHelpFiles(t *testing.T) {
 	testenv.NeedsGoBuild(t) // This is a lie. We actually need the source code.
-	app := cmd.New(nil)
+	app := cmd.New()
 	ctx := context.Background()
 	for _, page := range append(app.Commands(), app) {
 		t.Run(page.Name(), func(t *testing.T) {
@@ -65,7 +65,7 @@ func TestHelpFiles(t *testing.T) {
 
 func TestVerboseHelp(t *testing.T) {
 	testenv.NeedsGoBuild(t) // This is a lie. We actually need the source code.
-	app := cmd.New(nil)
+	app := cmd.New()
 	ctx := context.Background()
 	var buf bytes.Buffer
 	s := flag.NewFlagSet(appName, flag.ContinueOnError)

gopls/internal/cmd/info.go

@@ -19,6 +19,7 @@ import (
 
 	"golang.org/x/tools/gopls/internal/debug"
 	"golang.org/x/tools/gopls/internal/filecache"
+	licensespkg "golang.org/x/tools/gopls/internal/licenses"
 	"golang.org/x/tools/gopls/internal/settings"
 	"golang.org/x/tools/gopls/internal/util/browser"
 	goplsbug "golang.org/x/tools/gopls/internal/util/bug"
@@ -301,12 +302,11 @@ gopls also includes software made available under these licenses:
 `
 
 func (l *licenses) Run(ctx context.Context, args ...string) error {
-	opts := settings.DefaultOptions(l.app.options)
 	txt := licensePreamble
-	if opts.LicensesText == "" {
+	if licensespkg.Text == "" {
 		txt += "(development gopls, license information not available)"
 	} else {
-		txt += opts.LicensesText
+		txt += licensespkg.Text
 	}
 	fmt.Fprint(os.Stdout, txt)
 	return nil

gopls/internal/cmd/integration_test.go

@@ -40,7 +40,6 @@ import (
 
 	"golang.org/x/tools/gopls/internal/cmd"
 	"golang.org/x/tools/gopls/internal/debug"
-	"golang.org/x/tools/gopls/internal/hooks"
 	"golang.org/x/tools/gopls/internal/protocol"
 	"golang.org/x/tools/gopls/internal/util/bug"
 	"golang.org/x/tools/gopls/internal/version"
@@ -1060,7 +1059,7 @@ func goplsMain() {
 		version.VersionOverride = v
 	}
 
-	tool.Main(context.Background(), cmd.New(hooks.Options), os.Args[1:])
+	tool.Main(context.Background(), cmd.New(), os.Args[1:])
 }
 
 // writeTree extracts a txtar archive into a new directory and returns its path.

gopls/internal/cmd/semantictokens.go

@@ -66,7 +66,9 @@ func (c *semtok) Run(ctx context.Context, args ...string) error {
 	// perhaps simpler if app had just had a FlagSet member
 	origOptions := c.app.options
 	c.app.options = func(opts *settings.Options) {
-		origOptions(opts)
+		if origOptions != nil {
+			origOptions(opts)
+		}
 		opts.SemanticTokens = true
 	}
 	conn, err := c.app.connect(ctx, nil)

gopls/internal/golang/diagnostics.go

@@ -19,24 +19,18 @@ import (
 //
 // If the provided tracker is non-nil, it may be used to provide notifications
 // of the ongoing analysis pass.
+//
+// TODO(rfindley): merge this with snapshot.Analyze.
 func Analyze(ctx context.Context, snapshot *cache.Snapshot, pkgIDs map[PackageID]*metadata.Package, tracker *progress.Tracker) (map[protocol.DocumentURI][]*cache.Diagnostic, error) {
 	// Exit early if the context has been canceled. This also protects us
 	// from a race on Options, see golang/go#36699.
 	if ctx.Err() != nil {
 		return nil, ctx.Err()
 	}
 
-	options := snapshot.Options()
-	categories := []map[string]*settings.Analyzer{
-		options.DefaultAnalyzers,
-		options.StaticcheckAnalyzers,
-	}
-
-	var analyzers []*settings.Analyzer
-	for _, cat := range categories {
-		for _, a := range cat {
-			analyzers = append(analyzers, a)
-		}
+	analyzers := maps.Values(settings.DefaultAnalyzers)
+	if snapshot.Options().Staticcheck {
+		analyzers = append(analyzers, maps.Values(settings.StaticcheckAnalyzers)...)
 	}
 
 	analysisDiagnostics, err := snapshot.Analyze(ctx, pkgIDs, analyzers, tracker)

gopls/internal/golang/format.go

@@ -21,6 +21,7 @@ import (
 	"golang.org/x/tools/gopls/internal/cache/parsego"
 	"golang.org/x/tools/gopls/internal/file"
 	"golang.org/x/tools/gopls/internal/protocol"
+	"golang.org/x/tools/gopls/internal/settings"
 	"golang.org/x/tools/gopls/internal/util/safetoken"
 	"golang.org/x/tools/internal/diff"
 	"golang.org/x/tools/internal/event"
@@ -66,7 +67,7 @@ func Format(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle) ([]pr
 
 	// Apply additional formatting, if any is supported. Currently, the only
 	// supported additional formatter is gofumpt.
-	if format := snapshot.Options().GofumptFormat; snapshot.Options().Gofumpt && format != nil {
+	if format := settings.GofumptFormat; snapshot.Options().Gofumpt && format != nil {
 		// gofumpt can customize formatting based on language version and module
 		// path, if available.
 		//

gopls/internal/hooks/hooks.go

@@ -16,9 +16,9 @@ cat > $tempfile <<END
 // license that can be found in the LICENSE file.
 
 //go:generate ./gen-licenses.sh licenses.go
-package hooks
+package licenses
 
-const licensesText = \`
+const Text = \`
 END
 
 # List all the modules gopls depends on, except other golang.org modules, which
@@ -35,4 +35,4 @@ for mod in $mods; do
 done
 
 echo "\`" >> $tempfile
-mv $tempfile $output
+mv $tempfile $output

gopls/internal/hooks/gen-licenses.sh renamed to gopls/internal/licenses/gen-licenses.sh

@@ -3,9 +3,9 @@
 // license that can be found in the LICENSE file.
 
 //go:generate ./gen-licenses.sh licenses.go
-package hooks
+package licenses
 
-const licensesText = `
+const Text = `
 -- github.com/BurntSushi/toml COPYING --
 
 The MIT License (MIT)

gopls/internal/hooks/licenses.go renamed to gopls/internal/licenses/licenses.go

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-package hooks
+package licenses_test
 
 import (
 	"bytes"
@@ -42,6 +42,6 @@ func TestLicenses(t *testing.T) {
 		t.Fatal(err)
 	}
 	if !bytes.Equal(got, want) {
-		t.Error("combined license text needs updating. Run: `go generate ./internal/hooks` from the gopls module.")
+		t.Error("combined license text needs updating. Run: `go generate ./internal/licenses` from the gopls module.")
 	}
 }

gopls/internal/hooks/licenses_test.go renamed to gopls/internal/licenses/licenses_test.go

@@ -25,6 +25,7 @@ import (
 	"golang.org/x/tools/gopls/internal/util/safetoken"
 	"golang.org/x/tools/internal/event"
 	"golang.org/x/tools/internal/event/tag"
+	"mvdan.cc/xurls/v2"
 )
 
 func (s *server) DocumentLink(ctx context.Context, params *protocol.DocumentLinkParams) (links []protocol.DocumentLink, err error) {
@@ -87,7 +88,7 @@ func modLinks(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle) ([]
 	}
 
 	// Get all the links that are contained in the comments of the file.
-	urlRegexp := snapshot.Options().URLRegexp
+	urlRegexp := xurls.Relaxed()
 	for _, expr := range pm.File.Syntax.Stmt {
 		comments := expr.Comment()
 		if comments == nil {
@@ -159,7 +160,7 @@ func goLinks(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle) ([]p
 		}
 	}
 
-	urlRegexp := snapshot.Options().URLRegexp
+	urlRegexp := xurls.Relaxed()
 
 	// Gather links found in string literals.
 	var str []*ast.BasicLit

gopls/internal/server/link.go

@@ -93,13 +93,7 @@ func (s *server) DidChangeConfiguration(ctx context.Context, _ *protocol.DidChan
 			return err
 		}
 
-		// Ignore hooks for the purposes of equality.
-		sameOptions := reflect.DeepEqual(folder.Options.ClientOptions, opts.ClientOptions) &&
-			reflect.DeepEqual(folder.Options.ServerOptions, opts.ServerOptions) &&
-			reflect.DeepEqual(folder.Options.UserOptions, opts.UserOptions) &&
-			reflect.DeepEqual(folder.Options.InternalOptions, opts.InternalOptions)
-
-		if !sameOptions {
+		if !reflect.DeepEqual(folder.Options, opts) {
 			changed = true
 		}
 		folderOpts[folder.Dir] = opts