gopls: upgrade staticcheck to v0.3.0

Upgrade staticcheck to v0.3.0 to pick up support for generics. Since
this version only supports Go 1.17+, increase the version at which we
install staticcheck to 1.17 (from 1.15). This change is likely to cause
confusion for users on Go 1.16, so show a warning when setting
the "staticcheck" option to an unsupported value. Slightly refactor our
setting of options along the way.

Fixes golang/go#52159

Change-Id: Id9b4cee340e31988c64ca712d98573343aaf5848
Reviewed-on: https://go-review.googlesource.com/c/tools/+/396974
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Trust: Peter Weinberger <pjw@google.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>

gopls/go.mod

@@ -9,9 +9,9 @@ require (
 	github.com/sergi/go-diff v1.1.0
 	golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3
 	golang.org/x/sys v0.0.0-20220209214540-3681064d5158
-	golang.org/x/tools v0.1.9
+	golang.org/x/tools v0.1.11-0.20220330174940-8e193c2ba95e
 	golang.org/x/vuln v0.0.0-20220324005316-18fd808f5c7f
-	honnef.co/go/tools v0.2.2
+	honnef.co/go/tools v0.3.0
 	mvdan.cc/gofumpt v0.3.0
 	mvdan.cc/xurls/v2 v2.4.0
 )
@@ -19,6 +19,7 @@ require (
 require (
 	github.com/BurntSushi/toml v1.0.0 // indirect
 	github.com/google/safehtml v0.0.2 // indirect
+	golang.org/x/exp/typeparams v0.0.0-20220218215828-6cf2b201936e // indirect
 	golang.org/x/sync v0.0.0-20210220032951-036812b2e83c // indirect
 	golang.org/x/text v0.3.7 // indirect
 	golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect

gopls/go.sum

@@ -1,4 +1,5 @@
 github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
+github.com/BurntSushi/toml v0.4.1/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ=
 github.com/BurntSushi/toml v1.0.0 h1:dtDWrepsVPfW9H/4y7dDgFc2MBUSeJhlaDtK13CxFlU=
 github.com/BurntSushi/toml v1.0.0/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ=
 github.com/client9/misspell v0.3.4 h1:ta993UF76GwbvJcIo3Y68y/M3WxlpEHPWIGDkJYwzJI=
@@ -40,6 +41,8 @@ github.com/yuin/goldmark v1.4.1/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
+golang.org/x/exp/typeparams v0.0.0-20220218215828-6cf2b201936e h1:qyrTQ++p1afMkO4DPEeLGq/3oTsdlvdH4vqZUBWzUKM=
+golang.org/x/exp/typeparams v0.0.0-20220218215828-6cf2b201936e/go.mod h1:AbB0pIl9nAr9wVwH+Z2ZpaocVmF5I4GyWCDIsVjR0bk=
 golang.org/x/mod v0.5.1/go.mod h1:5OXOZSfqPIIbmVBIIKWRFfZjPR0E5r58TLhUjH0a2Ro=
 golang.org/x/mod v0.6.0-dev.0.20211013180041-c96bc1413d57/go.mod h1:3p9vT2HGsQu2K1YbXdKPJLVgG5VJdoTa1poYQBtP1AY=
 golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3 h1:kQgndtyPBW/JIYERgdxfwMYh3AVStj88WQTlNDi2a+o=
@@ -77,8 +80,9 @@ gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI=
 gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.2.4 h1:/eiJrUcujPVeJ3xlSWaiNi3uSVmDGBK1pDHUHAnao1I=
 gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
-honnef.co/go/tools v0.2.2 h1:MNh1AVMyVX23VUHE2O27jm6lNj3vjO5DexS4A1xvnzk=
 honnef.co/go/tools v0.2.2/go.mod h1:lPVVZ2BS5TfnjLyizF7o7hv7j9/L+8cZY2hLyjP9cGY=
+honnef.co/go/tools v0.3.0 h1:2LdYUZ7CIxnYgskbUZfY7FPggmqnh6shBqfWa8Tn3XU=
+honnef.co/go/tools v0.3.0/go.mod h1:vlRD9XErLMGT+mDuofSr0mMMquscM/1nQqtRSsh6m70=
 mvdan.cc/gofumpt v0.3.0 h1:kTojdZo9AcEYbQYhGuLf/zszYthRdhDNDUi2JKTxas4=
 mvdan.cc/gofumpt v0.3.0/go.mod h1:0+VyGZWleeIj5oostkOex+nDBA0eyavuDnDusAJ8ylo=
 mvdan.cc/unparam v0.0.0-20211214103731-d0ef000c54e5 h1:Jh3LAeMt1eGpxomyu3jVkmVZWW2MxZ1qIIV2TZ/nRio=

gopls/internal/hooks/analysis.go

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-//go:build go1.15
-// +build go1.15
+//go:build go1.17
+// +build go1.17
 
 package hooks
 
@@ -18,6 +18,8 @@ import (
 )
 
 func updateAnalyzers(options *source.Options) {
+	options.StaticcheckSupported = true
+
 	mapSeverity := func(severity lint.Severity) protocol.DiagnosticSeverity {
 		switch severity {
 		case lint.SeverityError:

gopls/internal/hooks/analysis_117.go

@@ -2,11 +2,13 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-//go:build !go1.15
-// +build !go1.15
+//go:build !go1.17
+// +build !go1.17
 
 package hooks
 
 import "golang.org/x/tools/internal/lsp/source"
 
-func updateAnalyzers(_ *source.Options) {}
+func updateAnalyzers(options *source.Options) {
+	options.StaticcheckSupported = false
+}

gopls/internal/hooks/licenses_test.go

@@ -17,7 +17,7 @@ import (
 func TestLicenses(t *testing.T) {
 	// License text differs for older Go versions because staticcheck isn't
 	// supported for those versions.
-	testenv.NeedsGo1Point(t, 15)
+	testenv.NeedsGo1Point(t, 17)
 
 	if runtime.GOOS != "linux" && runtime.GOOS != "darwin" {
 		t.Skip("generating licenses only works on Unixes")

gopls/internal/regtest/diagnostics/diagnostics_test.go

@@ -1265,7 +1265,11 @@ func main() {
 `
 
 	WithOptions(
-		EditorConfig{EnableStaticcheck: true},
+		EditorConfig{
+			Settings: map[string]interface{}{
+				"staticcheck": true,
+			},
+		},
 	).Run(t, files, func(t *testing.T, env *Env) {
 		env.OpenFile("main.go")
 		var d protocol.PublishDiagnosticsParams

gopls/internal/regtest/misc/configuration_test.go

@@ -16,8 +16,10 @@ import (
 // Test that enabling and disabling produces the expected results of showing
 // and hiding staticcheck analysis results.
 func TestChangeConfiguration(t *testing.T) {
-	// Staticcheck only supports Go versions > 1.14.
-	testenv.NeedsGo1Point(t, 15)
+	// Staticcheck only supports Go versions >= 1.17.
+	// Note: keep this in sync with TestStaticcheckWarning. Below this version we
+	// should get an error when setting staticcheck configuration.
+	testenv.NeedsGo1Point(t, 17)
 
 	const files = `
 -- go.mod --
@@ -40,10 +42,39 @@ var FooErr = errors.New("foo")
 		)
 		cfg := &fake.EditorConfig{}
 		*cfg = env.Editor.Config
-		cfg.EnableStaticcheck = true
+		cfg.Settings = map[string]interface{}{
+			"staticcheck": true,
+		}
 		env.ChangeConfiguration(t, cfg)
 		env.Await(
 			DiagnosticAt("a/a.go", 5, 4),
 		)
 	})
 }
+
+func TestStaticcheckWarning(t *testing.T) {
+	// Note: keep this in sync with TestChangeConfiguration.
+	testenv.SkipAfterGo1Point(t, 16)
+
+	const files = `
+-- go.mod --
+module mod.com
+
+go 1.12
+-- a/a.go --
+package a
+
+import "errors"
+
+// FooErr should be called ErrFoo (ST1012)
+var FooErr = errors.New("foo")
+`
+
+	WithOptions(EditorConfig{
+		Settings: map[string]interface{}{
+			"staticcheck": true,
+		},
+	}).Run(t, files, func(t *testing.T, env *Env) {
+		env.Await(ShownMessage("staticcheck is not supported"))
+	})
+}

internal/lsp/fake/editor.go

@@ -101,9 +101,6 @@ type EditorConfig struct {
 	// To explicitly send no workspace folders, use an empty (non-nil) slice.
 	WorkspaceFolders []string
 
-	// EnableStaticcheck enables staticcheck analyzers.
-	EnableStaticcheck bool
-
 	// AllExperiments sets the "allExperiments" configuration, which enables
 	// all of gopls's opt-in settings.
 	AllExperiments bool
@@ -258,9 +255,6 @@ func (e *Editor) configuration() map[string]interface{} {
 	if e.Config.SymbolStyle != nil {
 		config["symbolStyle"] = *e.Config.SymbolStyle
 	}
-	if e.Config.EnableStaticcheck {
-		config["staticcheck"] = true
-	}
 	if e.Config.AllExperiments {
 		config["allExperiments"] = true
 	}

internal/lsp/general.go

@@ -412,35 +412,25 @@ func (s *Server) eventuallyShowMessage(ctx context.Context, msg *protocol.ShowMe
 
 func (s *Server) handleOptionResults(ctx context.Context, results source.OptionResults) error {
 	for _, result := range results {
-		if result.Error != nil {
-			msg := &protocol.ShowMessageParams{
+		var msg *protocol.ShowMessageParams
+		switch result.Error.(type) {
+		case nil:
+			// nothing to do
+		case *source.SoftError:
+			msg = &protocol.ShowMessageParams{
+				Type:    protocol.Warning,
+				Message: result.Error.Error(),
+			}
+		default:
+			msg = &protocol.ShowMessageParams{
 				Type:    protocol.Error,
 				Message: result.Error.Error(),
 			}
-			if err := s.eventuallyShowMessage(ctx, msg); err != nil {
-				return err
-			}
 		}
-		switch result.State {
-		case source.OptionUnexpected:
-			msg := &protocol.ShowMessageParams{
-				Type:    protocol.Error,
-				Message: fmt.Sprintf("unexpected gopls setting %q", result.Name),
-			}
+		if msg != nil {
 			if err := s.eventuallyShowMessage(ctx, msg); err != nil {
 				return err
 			}
-		case source.OptionDeprecated:
-			msg := fmt.Sprintf("gopls setting %q is deprecated", result.Name)
-			if result.Replacement != "" {
-				msg = fmt.Sprintf("%s, use %q instead", msg, result.Replacement)
-			}
-			if err := s.eventuallyShowMessage(ctx, &protocol.ShowMessageParams{
-				Type:    protocol.Warning,
-				Message: msg,
-			}); err != nil {
-				return err
-			}
 		}
 	}
 	return nil

internal/lsp/regtest/expectation.go

@@ -152,12 +152,12 @@ func NoShowMessage() SimpleExpectation {
 	}
 }
 
-// ShownMessage asserts that the editor has received a ShownMessage with the
-// given title.
-func ShownMessage(title string) SimpleExpectation {
+// ShownMessage asserts that the editor has received a ShowMessageRequest
+// containing the given substring.
+func ShownMessage(containing string) SimpleExpectation {
 	check := func(s State) Verdict {
 		for _, m := range s.showMessage {
-			if strings.Contains(m.Message, title) {
+			if strings.Contains(m.Message, containing) {
 				return Met
 			}
 		}

internal/lsp/source/options.go

@@ -10,6 +10,7 @@ import (
 	"io"
 	"path/filepath"
 	"regexp"
+	"runtime"
 	"strings"
 	"sync"
 	"time"
@@ -463,10 +464,20 @@ func (u *UserOptions) SetEnvSlice(env []string) {
 // Hooks contains configuration that is provided to the Gopls command by the
 // main package.
 type Hooks struct {
+	// LicensesText holds third party licenses for software used by gopls.
 	LicensesText string
-	GoDiff       bool
+
+	// TODO(rfindley): is this even necessary?
+	GoDiff bool
+
+	// Whether staticcheck is supported.
+	StaticcheckSupported bool
+
+	// ComputeEdits is used to compute edits between file versions.
 	ComputeEdits diff.ComputeEdits
-	URLRegexp    *regexp.Regexp
+
+	// URLRegexp is used to find urls in comments and strings.
+	URLRegexp *regexp.Regexp
 
 	// GofumptFormat allows the gopls module to wire-in a call to
 	// gofumpt/format.Source. langVersion and modulePath are used for some
@@ -615,9 +626,6 @@ type OptionResult struct {
 	Name  string
 	Value interface{}
 	Error error
-
-	State       OptionState
-	Replacement string
 }
 
 type OptionState int
@@ -703,11 +711,12 @@ func (o *Options) Clone() *Options {
 		ClientOptions:   o.ClientOptions,
 		InternalOptions: o.InternalOptions,
 		Hooks: Hooks{
-			GoDiff:        o.GoDiff,
-			ComputeEdits:  o.ComputeEdits,
-			GofumptFormat: o.GofumptFormat,
-			URLRegexp:     o.URLRegexp,
-			Govulncheck:   o.Govulncheck,
+			GoDiff:               o.GoDiff,
+			StaticcheckSupported: o.StaticcheckSupported,
+			ComputeEdits:         o.ComputeEdits,
+			GofumptFormat:        o.GofumptFormat,
+			URLRegexp:            o.URLRegexp,
+			Govulncheck:          o.Govulncheck,
 		},
 		ServerOptions: o.ServerOptions,
 		UserOptions:   o.UserOptions,
@@ -915,12 +924,19 @@ func (o *Options) set(name string, value interface{}, seen map[string]struct{})
 		// codelens is deprecated, but still works for now.
 		// TODO(rstambler): Remove this for the gopls/v0.7.0 release.
 		if name == "codelens" {
-			result.State = OptionDeprecated
-			result.Replacement = "codelenses"
+			result.deprecated("codelenses")
 		}
 
 	case "staticcheck":
-		result.setBool(&o.Staticcheck)
+		if v, ok := result.asBool(); ok {
+			o.Staticcheck = v
+			if v && !o.StaticcheckSupported {
+				// Warn if the user is trying to enable staticcheck, but staticcheck is
+				// unsupported.
+				result.Error = fmt.Errorf("applying setting %q: staticcheck is not supported at %s\n"+
+					"\trebuild gopls with a more recent version of Go", result.Name, runtime.Version())
+			}
+		}
 
 	case "local":
 		result.setString(&o.Local)
@@ -946,11 +962,11 @@ func (o *Options) set(name string, value interface{}, seen map[string]struct{})
 	case "experimentalPostfixCompletions":
 		result.setBool(&o.ExperimentalPostfixCompletions)
 
-	case "experimentalWorkspaceModule":
+	case "experimentalWorkspaceModule": // TODO(rfindley): suggest go.work on go1.18+
 		result.setBool(&o.ExperimentalWorkspaceModule)
 
-	case "experimentalTemplateSupport": // remove after June 2022
-		result.State = OptionDeprecated
+	case "experimentalTemplateSupport": // TODO(pjw): remove after June 2022
+		result.deprecated("")
 
 	case "templateExtensions":
 		if iexts, ok := value.([]interface{}); ok {
@@ -968,8 +984,7 @@ func (o *Options) set(name string, value interface{}, seen map[string]struct{})
 		result.errorf(fmt.Sprintf("unexpected type %T not []string", value))
 	case "experimentalDiagnosticsDelay", "diagnosticsDelay":
 		if name == "experimentalDiagnosticsDelay" {
-			result.State = OptionDeprecated
-			result.Replacement = "diagnosticsDelay"
+			result.deprecated("diagnosticsDelay")
 		}
 		result.setDuration(&o.DiagnosticsDelay)
 
@@ -994,48 +1009,41 @@ func (o *Options) set(name string, value interface{}, seen map[string]struct{})
 
 	// Replaced settings.
 	case "experimentalDisabledAnalyses":
-		result.State = OptionDeprecated
-		result.Replacement = "analyses"
+		result.deprecated("analyses")
 
 	case "disableDeepCompletion":
-		result.State = OptionDeprecated
-		result.Replacement = "deepCompletion"
+		result.deprecated("deepCompletion")
 
 	case "disableFuzzyMatching":
-		result.State = OptionDeprecated
-		result.Replacement = "fuzzyMatching"
+		result.deprecated("fuzzyMatching")
 
 	case "wantCompletionDocumentation":
-		result.State = OptionDeprecated
-		result.Replacement = "completionDocumentation"
+		result.deprecated("completionDocumentation")
 
 	case "wantUnimportedCompletions":
-		result.State = OptionDeprecated
-		result.Replacement = "completeUnimported"
+		result.deprecated("completeUnimported")
 
 	case "fuzzyMatching":
-		result.State = OptionDeprecated
-		result.Replacement = "matcher"
+		result.deprecated("matcher")
 
 	case "caseSensitiveCompletion":
-		result.State = OptionDeprecated
-		result.Replacement = "matcher"
+		result.deprecated("matcher")
 
 	// Deprecated settings.
 	case "wantSuggestedFixes":
-		result.State = OptionDeprecated
+		result.deprecated("")
 
 	case "noIncrementalSync":
-		result.State = OptionDeprecated
+		result.deprecated("")
 
 	case "watchFileChanges":
-		result.State = OptionDeprecated
+		result.deprecated("")
 
 	case "go-diff":
-		result.State = OptionDeprecated
+		result.deprecated("")
 
 	default:
-		result.State = OptionUnexpected
+		result.unexpected()
 	}
 	return result
 }
@@ -1045,6 +1053,27 @@ func (r *OptionResult) errorf(msg string, values ...interface{}) {
 	r.Error = errors.Errorf(prefix+msg, values...)
 }
 
+// A SoftError is an error that does not affect the functionality of gopls.
+type SoftError struct {
+	msg string
+}
+
+func (e *SoftError) Error() string {
+	return e.msg
+}
+
+func (r *OptionResult) deprecated(replacement string) {
+	msg := fmt.Sprintf("gopls setting %q is deprecated", r.Name)
+	if replacement != "" {
+		msg = fmt.Sprintf("%s, use %q instead", msg, replacement)
+	}
+	r.Error = &SoftError{msg}
+}
+
+func (r *OptionResult) unexpected() {
+	r.Error = fmt.Errorf("unexpected gopls setting %q", r.Name)
+}
+
 func (r *OptionResult) asBool() (bool, bool) {
 	b, ok := r.Value.(bool)
 	if !ok {

internal/lsp/source/options_test.go

@@ -44,9 +44,10 @@ func TestSetOption(t *testing.T) {
 			check: func(o Options) bool { return o.CompletionBudget == 2*time.Second },
 		},
 		{
-			name:  "staticcheck",
-			value: true,
-			check: func(o Options) bool { return o.Staticcheck == true },
+			name:      "staticcheck",
+			value:     true,
+			check:     func(o Options) bool { return o.Staticcheck == true },
+			wantError: true, // o.StaticcheckSupported is unset
 		},
 		{
 			name:  "codelenses",