gopls/internal/hooks: respect the default checks of the staticcheck tool

Disable non-default checks by default. Also, update the regression
test because the previous version used a non-default check (ST1022).

Fixes golang/go#44712

Change-Id: I825cac8387b33307e529fc27ca2f54c2d0d50cc7
GitHub-Last-Rev: 3ad413a
GitHub-Pull-Request: #303
Reviewed-on: https://go-review.googlesource.com/c/tools/+/310449
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Peter Weinberger <pjw@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

gopls/internal/hooks/analysis.go

@@ -10,33 +10,31 @@ package hooks
 import (
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/internal/lsp/source"
+	"honnef.co/go/tools/analysis/lint"
 	"honnef.co/go/tools/simple"
 	"honnef.co/go/tools/staticcheck"
 	"honnef.co/go/tools/stylecheck"
 )
 
 func updateAnalyzers(options *source.Options) {
-	var analyzers []*analysis.Analyzer
-	for _, a := range simple.Analyzers {
-		analyzers = append(analyzers, a)
-	}
-	for _, a := range staticcheck.Analyzers {
-		switch a.Name {
-		case "SA5009":
-			// This check conflicts with the vet printf check (golang/go#34494).
-		case "SA5011":
-			// This check relies on facts from dependencies, which
-			// we don't currently compute.
-		default:
-			analyzers = append(analyzers, a)
+	add := func(analyzers map[string]*analysis.Analyzer, docs map[string]*lint.Documentation, skip map[string]struct{}) {
+		for check, a := range analyzers {
+			if _, ok := skip[check]; ok {
+				continue
+			}
+
+			enabled := !docs[check].NonDefault
+			options.AddStaticcheckAnalyzer(a, enabled)
 		}
 	}
-	for _, a := range stylecheck.Analyzers {
-		analyzers = append(analyzers, a)
-	}
-	// Always add hooks for all available analyzers, but disable them if the
-	// user does not have staticcheck enabled (they may enable it later on).
-	for _, a := range analyzers {
-		options.AddStaticcheckAnalyzer(a)
-	}
+
+	add(simple.Analyzers, simple.Docs, nil)
+	add(staticcheck.Analyzers, staticcheck.Docs, map[string]struct{}{
+		// This check conflicts with the vet printf check (golang/go#34494).
+		"SA5009": {},
+		// This check relies on facts from dependencies, which
+		// we don't currently compute.
+		"SA5011": {},
+	})
+	add(stylecheck.Analyzers, stylecheck.Docs, nil)
 }

gopls/internal/regtest/misc/configuration_test.go

@@ -27,8 +27,10 @@ go 1.12
 -- a/a.go --
 package a
 
-// NotThisVariable should really start with ThisVariable.
-const ThisVariable = 7
+import "errors"
+
+// FooErr should be called ErrFoo (ST1012)
+var FooErr = errors.New("foo")
 `
 	Run(t, files, func(t *testing.T, env *Env) {
 		env.OpenFile("a/a.go")
@@ -41,7 +43,7 @@ const ThisVariable = 7
 		cfg.EnableStaticcheck = true
 		env.ChangeConfiguration(t, cfg)
 		env.Await(
-			DiagnosticAt("a/a.go", 2, 0),
+			DiagnosticAt("a/a.go", 5, 4),
 		)
 	})
 }

internal/lsp/source/options.go

@@ -680,8 +680,8 @@ func (o *Options) Clone() *Options {
 	return result
 }
 
-func (o *Options) AddStaticcheckAnalyzer(a *analysis.Analyzer) {
-	o.StaticcheckAnalyzers[a.Name] = &Analyzer{Analyzer: a, Enabled: true}
+func (o *Options) AddStaticcheckAnalyzer(a *analysis.Analyzer, enabled bool) {
+	o.StaticcheckAnalyzers[a.Name] = &Analyzer{Analyzer: a, Enabled: enabled}
 }
 
 // enableAllExperiments turns on all of the experimental "off-by-default"