gopls/internal/settings: remove experiments

Remove the concept of "experiments" from gopls settings and testing. In
the past, this was a way for us to selectively enable features in the VS
Code nightly build, but we have since switched to a model where we cut
earlier prereleases and endeavor to have more prerelease testing.

Fixes golang/go#65548

Change-Id: I64d739dd4b4899c19ce5cd0c3da45e498e9ae417
Reviewed-on: https://go-review.googlesource.com/c/tools/+/579395
Auto-Submit: Robert Findley <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: findleyr

gopls/internal/settings/settings.go

@@ -11,10 +11,8 @@ import (
 	"strings"
 	"time"
 
-	"golang.org/x/tools/gopls/internal/analysis/unusedvariable"
 	"golang.org/x/tools/gopls/internal/file"
 	"golang.org/x/tools/gopls/internal/protocol"
-	"golang.org/x/tools/gopls/internal/protocol/command"
 )
 
 type Annotation string
@@ -608,24 +606,10 @@ func SetOptions(options *Options, opts any) OptionResults {
 	switch opts := opts.(type) {
 	case nil:
 	case map[string]any:
-		// If the user's settings contains "allExperiments", set that first,
-		// and then let them override individual settings independently.
-		var enableExperiments bool
-		for name, value := range opts {
-			if b, ok := value.(bool); name == "allExperiments" && ok && b {
-				enableExperiments = true
-				options.EnableAllExperiments()
-			}
-		}
 		seen := map[string]struct{}{}
 		for name, value := range opts {
 			results = append(results, options.set(name, value, seen))
 		}
-		// Finally, enable any experimental features that are specified in
-		// maps, which allows users to individually toggle them on or off.
-		if enableExperiments {
-			options.enableAllExperimentMaps()
-		}
 	default:
 		results = append(results, OptionResult{
 			Value: opts,
@@ -694,8 +678,7 @@ func (o *Options) Clone() *Options {
 		ServerOptions:   o.ServerOptions,
 		UserOptions:     o.UserOptions,
 	}
-	// Fully clone any slice or map fields. Only Hooks, ExperimentalOptions,
-	// and UserOptions can be modified.
+	// Fully clone any slice or map fields. Only UserOptions can be modified.
 	copyStringMap := func(src map[string]bool) map[string]bool {
 		dst := make(map[string]bool)
 		for k, v := range src {
@@ -719,25 +702,6 @@ func (o *Options) Clone() *Options {
 	return result
 }
 
-// EnableAllExperiments turns on all of the experimental "off-by-default"
-// features offered by gopls. Any experimental features specified in maps
-// should be enabled in enableAllExperimentMaps.
-func (o *Options) EnableAllExperiments() {
-	o.SemanticTokens = true
-}
-
-func (o *Options) enableAllExperimentMaps() {
-	if _, ok := o.Codelenses[string(command.GCDetails)]; !ok {
-		o.Codelenses[string(command.GCDetails)] = true
-	}
-	if _, ok := o.Codelenses[string(command.RunGovulncheck)]; !ok {
-		o.Codelenses[string(command.RunGovulncheck)] = true
-	}
-	if _, ok := o.Analyses[unusedvariable.Analyzer.Name]; !ok {
-		o.Analyses[unusedvariable.Analyzer.Name] = true
-	}
-}
-
 // validateDirectoryFilter validates if the filter string
 // - is not empty
 // - start with either + or -
@@ -1034,8 +998,12 @@ func (o *Options) set(name string, value interface{}, seen map[string]struct{})
 		result.setStringSlice(&o.StandaloneTags)
 
 	case "allExperiments":
-		// This setting should be handled before all of the other options are
-		// processed, so do nothing here.
+		// golang/go#65548: this setting is a no-op, but we fail don't report it as
+		// deprecated, since the nightly VS Code injects it.
+		//
+		// If, in the future, VS Code stops injecting this, we could theoretically
+		// report an error here, but it also seems harmless to keep ignoring this
+		// setting forever.
 
 	case "newDiff":
 		result.deprecated("")

gopls/internal/test/integration/debug/debug_test.go

@@ -39,7 +39,7 @@ func TestBugNotification(t *testing.T) {
 // start the internal web server.
 func TestStartDebugging(t *testing.T) {
 	WithOptions(
-		Modes(Default|Experimental), // doesn't work in Forwarded mode
+		Modes(Default), // doesn't work in Forwarded mode
 	).Run(t, "", func(t *testing.T, env *Env) {
 		// Start a debugging server.
 		res, err := startDebugging(env.Ctx, env.Editor.Server, &command.DebuggingArgs{

gopls/internal/test/integration/diagnostics/diagnostics_test.go

@@ -15,7 +15,6 @@ import (
 	. "golang.org/x/tools/gopls/internal/test/integration"
 	"golang.org/x/tools/gopls/internal/test/integration/fake"
 	"golang.org/x/tools/gopls/internal/util/bug"
-	"golang.org/x/tools/gopls/internal/util/goversion"
 	"golang.org/x/tools/internal/testenv"
 )
 
@@ -1384,36 +1383,6 @@ func _() {
 	})
 }
 
-func TestEnableAllExperiments(t *testing.T) {
-	// Before the oldest supported Go version, gopls sends a warning to upgrade
-	// Go, which fails the expectation below.
-	testenv.NeedsGo1Point(t, goversion.OldestSupported())
-
-	const mod = `
--- go.mod --
-module mod.com
-
-go 1.12
--- main.go --
-package main
-
-import "bytes"
-
-func b(c bytes.Buffer) {
-	_ = 1
-}
-`
-	WithOptions(
-		Settings{"allExperiments": true},
-	).Run(t, mod, func(t *testing.T, env *Env) {
-		// Confirm that the setting doesn't cause any warnings.
-		env.OnceMet(
-			InitialWorkspaceLoad,
-			NoShownMessage(""), // empty substring to match any message
-		)
-	})
-}
-
 func TestSwig(t *testing.T) {
 	if _, err := exec.LookPath("swig"); err != nil {
 		t.Skip("skipping test: swig not available")

gopls/internal/test/integration/misc/semantictokens_test.go

@@ -30,7 +30,6 @@ func main() {}
 `
 	WithOptions(
 		Modes(Default),
-		Settings{"allExperiments": true},
 	).Run(t, src, func(t *testing.T, env *Env) {
 		params := &protocol.SemanticTokensParams{}
 		const badURI = "http://foo"

gopls/internal/test/integration/regtest.go

@@ -97,7 +97,10 @@ func (r RunMultiple) Run(t *testing.T, files string, f TestFunc) {
 func DefaultModes() Mode {
 	modes := Default
 	if !testing.Short() {
-		modes |= Experimental | Forwarded
+		// TODO(rfindley): we should just run a few select integration tests in
+		// "Forwarded" mode, and call it a day. No need to run every single test in
+		// two ways.
+		modes |= Forwarded
 	}
 	if *runSubprocessTests {
 		modes |= SeparateProcess

gopls/internal/test/integration/runner.go

@@ -24,7 +24,6 @@ import (
 	"golang.org/x/tools/gopls/internal/debug"
 	"golang.org/x/tools/gopls/internal/lsprpc"
 	"golang.org/x/tools/gopls/internal/protocol"
-	"golang.org/x/tools/gopls/internal/settings"
 	"golang.org/x/tools/gopls/internal/test/integration/fake"
 	"golang.org/x/tools/internal/jsonrpc2"
 	"golang.org/x/tools/internal/jsonrpc2/servertest"
@@ -45,17 +44,15 @@ import (
 // TODO(rfindley, cleanup): rather than using arbitrary names for these modes,
 // we can compose them explicitly out of the features described here, allowing
 // individual tests more freedom in constructing problematic execution modes.
-// For example, a test could assert on a certain behavior when running with
-// experimental options on a separate process. Moreover, we could unify 'Modes'
-// with 'Options', and use RunMultiple rather than a hard-coded loop through
-// modes.
+// For example, a test could assert on a certain behavior when running on a
+// separate process. Moreover, we could unify 'Modes' with 'Options', and use
+// RunMultiple rather than a hard-coded loop through modes.
 //
 // Mode            | Options      | Shared Cache? | Shared Server? | In-process?
 // ---------------------------------------------------------------------------
 // Default         | Default      | Y             | N              | Y
 // Forwarded       | Default      | Y             | Y              | Y
 // SeparateProcess | Default      | Y             | Y              | N
-// Experimental    | Experimental | N             | N              | Y
 type Mode int
 
 const (
@@ -76,13 +73,6 @@ const (
 	//
 	// Only supported on GOOS=linux.
 	SeparateProcess
-
-	// Experimental enables all of the experimental configurations that are
-	// being developed, and runs gopls in sidecar mode.
-	//
-	// It uses a separate cache for each test, to exercise races that may only
-	// appear with cache misses.
-	Experimental
 )
 
 func (m Mode) String() string {
@@ -93,8 +83,6 @@ func (m Mode) String() string {
 		return "forwarded"
 	case SeparateProcess:
 		return "separate process"
-	case Experimental:
-		return "experimental"
 	default:
 		return "unknown mode"
 	}
@@ -151,7 +139,6 @@ func (r *Runner) Run(t *testing.T, files string, test TestFunc, opts ...RunOptio
 		{"default", Default, r.defaultServer},
 		{"forwarded", Forwarded, r.forwardedServer},
 		{"separate_process", SeparateProcess, r.separateProcessServer},
-		{"experimental", Experimental, r.experimentalServer},
 	}
 
 	for _, tc := range tests {
@@ -349,14 +336,6 @@ func (r *Runner) defaultServer() jsonrpc2.StreamServer {
 	return lsprpc.NewStreamServer(cache.New(r.store), false, nil)
 }
 
-// experimentalServer handles the Experimental execution mode.
-func (r *Runner) experimentalServer() jsonrpc2.StreamServer {
-	options := func(o *settings.Options) {
-		o.EnableAllExperiments()
-	}
-	return lsprpc.NewStreamServer(cache.New(nil), false, options)
-}
-
 // forwardedServer handles the Forwarded execution mode.
 func (r *Runner) forwardedServer() jsonrpc2.StreamServer {
 	r.tsOnce.Do(func() {