internal/lsp: temporarily disable type error analyzers by default
If we release gopls/v0.4.0 soon, we should keep these new analyzers off
by default. They were just merged, so they haven't been used enough to
be enabled, I think. We'll turn them on by default for gopls/v0.5.0.
Also, ended up creating a helper function to check if analysis has been
abled (which fixed a small bug in FindAnalysisError), and another helper
function to enable all analyses for testing purposes.
Updates golang/go#38212
Change-Id: I5ee94b3582dfc0863978650fc6ce51bfa0606c13
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226962
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md
index 90cf758..f3e5eee 100644
--- a/gopls/doc/analyzers.md
+++ b/gopls/doc/analyzers.md
@@ -341,6 +341,8 @@
This functionality is similar to [goreturns](https://github.com/sqs/goreturns).
+Default value: `false`.
+
### **nonewvars**
suggested fixes for "no new vars on left side of :="
@@ -357,7 +359,7 @@
z = 2
```
-Default value: `true`.
+Default value: `false`.
### **noresultvalues**
@@ -471,7 +473,7 @@
type `undeclared name: <>`. It will insert a new statement:
`<> := `.
-Default value: `true`.
+Default value: `false`.
### **unusedparams**
diff --git a/gopls/internal/hooks/analysis.go b/gopls/internal/hooks/analysis.go
index 338827a..3a4c00f 100644
--- a/gopls/internal/hooks/analysis.go
+++ b/gopls/internal/hooks/analysis.go
@@ -14,17 +14,17 @@
func updateAnalyzers(options *source.Options) {
if options.StaticCheck {
for _, a := range simple.Analyzers {
- options.DefaultAnalyzers[a.Name] = source.Analyzer{Analyzer: a, Enabled: true}
+ options.AddDefaultAnalyzer(a)
}
for _, a := range staticcheck.Analyzers {
// This check conflicts with the vet printf check (golang/go#34494).
if a.Name == "SA5009" {
continue
}
- options.DefaultAnalyzers[a.Name] = source.Analyzer{Analyzer: a, Enabled: true}
+ options.AddDefaultAnalyzer(a)
}
for _, a := range stylecheck.Analyzers {
- options.DefaultAnalyzers[a.Name] = source.Analyzer{Analyzer: a, Enabled: true}
+ options.AddDefaultAnalyzer(a)
}
}
}
diff --git a/internal/lsp/cache/pkg.go b/internal/lsp/cache/pkg.go
index 4e51038..9fe7a02 100644
--- a/internal/lsp/cache/pkg.go
+++ b/internal/lsp/cache/pkg.go
@@ -131,10 +131,9 @@
if analyzer.Analyzer == nil {
return nil, nil, errors.Errorf("unexpected analyzer: %s", analyzerName)
}
- if !analyzer.Enabled {
+ if !analyzer.Enabled(s) {
return nil, nil, errors.Errorf("disabled analyzer: %s", analyzerName)
}
-
act, err := s.actionHandle(ctx, packageID(pkgID), analyzer.Analyzer)
if err != nil {
return nil, nil, err
diff --git a/internal/lsp/cmd/cmd.go b/internal/lsp/cmd/cmd.go
index 299bc59..f667c46 100644
--- a/internal/lsp/cmd/cmd.go
+++ b/internal/lsp/cmd/cmd.go
@@ -263,7 +263,6 @@
params.InitializationOptions = map[string]interface{}{
"matcher": matcherString[opts.Matcher],
}
-
if _, err := c.Server.Initialize(ctx, params); err != nil {
return err
}
@@ -374,8 +373,13 @@
env[l[0]] = l[1]
}
results[i] = map[string]interface{}{
- "env": env,
- "go-diff": true,
+ "env": env,
+ "analyses": map[string]bool{
+ "fillreturns": true,
+ "nonewvars": true,
+ "noresultvalues": true,
+ "undeclaredname": true,
+ },
}
}
return results, nil
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index 1d3fd86..0b38d1d 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -54,10 +54,16 @@
options := tests.DefaultOptions()
session.SetOptions(options)
options.Env = datum.Config.Env
- v, _, err := session.NewView(ctx, datum.Config.Dir, span.URIFromPath(datum.Config.Dir), options)
+ v, snapshot, err := session.NewView(ctx, datum.Config.Dir, span.URIFromPath(datum.Config.Dir), options)
if err != nil {
t.Fatal(err)
}
+
+ // Enable type error analyses for tests.
+ // TODO(golang/go#38212): Delete this once they are enabled by default.
+ tests.EnableAllAnalyzers(snapshot, &options)
+ v.SetOptions(ctx, options)
+
// Check to see if the -modfile flag is available, this is basically a check
// to see if the go version >= 1.14. Otherwise, the modfile specific tests
// will always fail if this flag is not available.
diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go
index b5b0444..2c97cba 100644
--- a/internal/lsp/source/diagnostics.go
+++ b/internal/lsp/source/diagnostics.go
@@ -274,14 +274,8 @@
func analyses(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][]Diagnostic, ph PackageHandle, analyses map[string]Analyzer) error {
var analyzers []*analysis.Analyzer
- for name, a := range analyses {
- if enabled, ok := snapshot.View().Options().UserEnabledAnalyses[name]; ok {
- if enabled {
- analyzers = append(analyzers, a.Analyzer)
- }
- continue
- }
- if !a.Enabled {
+ for _, a := range analyses {
+ if !a.Enabled(snapshot) {
continue
}
analyzers = append(analyzers, a.Analyzer)
diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go
index 98d408d..6b33d63 100644
--- a/internal/lsp/source/options.go
+++ b/internal/lsp/source/options.go
@@ -10,6 +10,7 @@
"regexp"
"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"
@@ -198,6 +199,10 @@
TypeErrorAnalyzers map[string]Analyzer
}
+func (o Options) AddDefaultAnalyzer(a *analysis.Analyzer) {
+ o.DefaultAnalyzers[a.Name] = Analyzer{Analyzer: a, enabled: true}
+}
+
type ExperimentalOptions struct {
// WARNING: This configuration will be changed in the future.
// It only exists while this feature is under development.
@@ -492,49 +497,49 @@
func typeErrorAnalyzers() map[string]Analyzer {
return map[string]Analyzer{
- fillreturns.Analyzer.Name: {Analyzer: fillreturns.Analyzer, Enabled: true},
- nonewvars.Analyzer.Name: {Analyzer: nonewvars.Analyzer, Enabled: true},
- noresultvalues.Analyzer.Name: {Analyzer: noresultvalues.Analyzer, Enabled: true},
- undeclaredname.Analyzer.Name: {Analyzer: undeclaredname.Analyzer, Enabled: true},
+ fillreturns.Analyzer.Name: {Analyzer: fillreturns.Analyzer, enabled: false},
+ nonewvars.Analyzer.Name: {Analyzer: nonewvars.Analyzer, enabled: false},
+ noresultvalues.Analyzer.Name: {Analyzer: noresultvalues.Analyzer, enabled: false},
+ undeclaredname.Analyzer.Name: {Analyzer: undeclaredname.Analyzer, enabled: false},
}
}
func defaultAnalyzers() map[string]Analyzer {
return map[string]Analyzer{
// The traditional vet suite:
- asmdecl.Analyzer.Name: {Analyzer: asmdecl.Analyzer, Enabled: true},
- assign.Analyzer.Name: {Analyzer: assign.Analyzer, Enabled: true},
- atomic.Analyzer.Name: {Analyzer: atomic.Analyzer, Enabled: true},
- atomicalign.Analyzer.Name: {Analyzer: atomicalign.Analyzer, Enabled: true},
- bools.Analyzer.Name: {Analyzer: bools.Analyzer, Enabled: true},
- buildtag.Analyzer.Name: {Analyzer: buildtag.Analyzer, Enabled: true},
- cgocall.Analyzer.Name: {Analyzer: cgocall.Analyzer, Enabled: true},
- composite.Analyzer.Name: {Analyzer: composite.Analyzer, Enabled: true},
- copylock.Analyzer.Name: {Analyzer: copylock.Analyzer, Enabled: true},
- errorsas.Analyzer.Name: {Analyzer: errorsas.Analyzer, Enabled: true},
- httpresponse.Analyzer.Name: {Analyzer: httpresponse.Analyzer, Enabled: true},
- loopclosure.Analyzer.Name: {Analyzer: loopclosure.Analyzer, Enabled: true},
- lostcancel.Analyzer.Name: {Analyzer: lostcancel.Analyzer, Enabled: true},
- nilfunc.Analyzer.Name: {Analyzer: nilfunc.Analyzer, Enabled: true},
- printf.Analyzer.Name: {Analyzer: printf.Analyzer, Enabled: true},
- shift.Analyzer.Name: {Analyzer: shift.Analyzer, Enabled: true},
- stdmethods.Analyzer.Name: {Analyzer: stdmethods.Analyzer, Enabled: true},
- structtag.Analyzer.Name: {Analyzer: structtag.Analyzer, Enabled: true},
- tests.Analyzer.Name: {Analyzer: tests.Analyzer, Enabled: true},
- unmarshal.Analyzer.Name: {Analyzer: unmarshal.Analyzer, Enabled: true},
- unreachable.Analyzer.Name: {Analyzer: unreachable.Analyzer, Enabled: true},
- unsafeptr.Analyzer.Name: {Analyzer: unsafeptr.Analyzer, Enabled: true},
- unusedresult.Analyzer.Name: {Analyzer: unusedresult.Analyzer, Enabled: true},
+ asmdecl.Analyzer.Name: {Analyzer: asmdecl.Analyzer, enabled: true},
+ assign.Analyzer.Name: {Analyzer: assign.Analyzer, enabled: true},
+ atomic.Analyzer.Name: {Analyzer: atomic.Analyzer, enabled: true},
+ atomicalign.Analyzer.Name: {Analyzer: atomicalign.Analyzer, enabled: true},
+ bools.Analyzer.Name: {Analyzer: bools.Analyzer, enabled: true},
+ buildtag.Analyzer.Name: {Analyzer: buildtag.Analyzer, enabled: true},
+ cgocall.Analyzer.Name: {Analyzer: cgocall.Analyzer, enabled: true},
+ composite.Analyzer.Name: {Analyzer: composite.Analyzer, enabled: true},
+ copylock.Analyzer.Name: {Analyzer: copylock.Analyzer, enabled: true},
+ errorsas.Analyzer.Name: {Analyzer: errorsas.Analyzer, enabled: true},
+ httpresponse.Analyzer.Name: {Analyzer: httpresponse.Analyzer, enabled: true},
+ loopclosure.Analyzer.Name: {Analyzer: loopclosure.Analyzer, enabled: true},
+ lostcancel.Analyzer.Name: {Analyzer: lostcancel.Analyzer, enabled: true},
+ nilfunc.Analyzer.Name: {Analyzer: nilfunc.Analyzer, enabled: true},
+ printf.Analyzer.Name: {Analyzer: printf.Analyzer, enabled: true},
+ shift.Analyzer.Name: {Analyzer: shift.Analyzer, enabled: true},
+ stdmethods.Analyzer.Name: {Analyzer: stdmethods.Analyzer, enabled: true},
+ structtag.Analyzer.Name: {Analyzer: structtag.Analyzer, enabled: true},
+ tests.Analyzer.Name: {Analyzer: tests.Analyzer, enabled: true},
+ unmarshal.Analyzer.Name: {Analyzer: unmarshal.Analyzer, enabled: true},
+ unreachable.Analyzer.Name: {Analyzer: unreachable.Analyzer, enabled: true},
+ unsafeptr.Analyzer.Name: {Analyzer: unsafeptr.Analyzer, enabled: true},
+ unusedresult.Analyzer.Name: {Analyzer: unusedresult.Analyzer, enabled: true},
// Non-vet analyzers
- deepequalerrors.Analyzer.Name: {Analyzer: deepequalerrors.Analyzer, Enabled: true},
- sortslice.Analyzer.Name: {Analyzer: sortslice.Analyzer, Enabled: true},
- testinggoroutine.Analyzer.Name: {Analyzer: testinggoroutine.Analyzer, Enabled: true},
- unusedparams.Analyzer.Name: {Analyzer: unusedparams.Analyzer, Enabled: false},
+ deepequalerrors.Analyzer.Name: {Analyzer: deepequalerrors.Analyzer, enabled: true},
+ sortslice.Analyzer.Name: {Analyzer: sortslice.Analyzer, enabled: true},
+ testinggoroutine.Analyzer.Name: {Analyzer: testinggoroutine.Analyzer, enabled: true},
+ unusedparams.Analyzer.Name: {Analyzer: unusedparams.Analyzer, enabled: false},
// gofmt -s suite:
- simplifycompositelit.Analyzer.Name: {Analyzer: simplifycompositelit.Analyzer, Enabled: true, HighConfidence: true},
- simplifyrange.Analyzer.Name: {Analyzer: simplifyrange.Analyzer, Enabled: true, HighConfidence: true},
- simplifyslice.Analyzer.Name: {Analyzer: simplifyslice.Analyzer, Enabled: true, HighConfidence: true},
+ simplifycompositelit.Analyzer.Name: {Analyzer: simplifycompositelit.Analyzer, enabled: true, HighConfidence: true},
+ simplifyrange.Analyzer.Name: {Analyzer: simplifyrange.Analyzer, enabled: true, HighConfidence: true},
+ simplifyslice.Analyzer.Name: {Analyzer: simplifyslice.Analyzer, enabled: true, HighConfidence: true},
}
}
diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go
index 15a1300..d8933e4 100644
--- a/internal/lsp/source/source_test.go
+++ b/internal/lsp/source/source_test.go
@@ -51,10 +51,14 @@
session := cache.NewSession(ctx)
options := tests.DefaultOptions()
options.Env = datum.Config.Env
- view, _, err := session.NewView(ctx, "source_test", span.URIFromPath(datum.Config.Dir), options)
+ view, snapshot, err := session.NewView(ctx, "source_test", span.URIFromPath(datum.Config.Dir), options)
if err != nil {
t.Fatal(err)
}
+ // Enable type error analyses for tests.
+ // TODO(golang/go#38212): Delete this once they are enabled by default.
+ tests.EnableAllAnalyzers(snapshot, &options)
+ view.SetOptions(ctx, options)
r := &runner{
view: view,
data: datum,
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index ab9e666..eba54e6 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -367,13 +367,20 @@
// that let the user know how to use the analyzer.
type Analyzer struct {
Analyzer *analysis.Analyzer
- Enabled bool
+ enabled bool
// If this is true, then we can apply the suggested fixes
// as part of a source.FixAll codeaction.
HighConfidence bool
}
+func (a Analyzer) Enabled(snapshot Snapshot) bool {
+ if enabled, ok := snapshot.View().Options().UserEnabledAnalyses[a.Analyzer.Name]; ok {
+ return enabled
+ }
+ return a.enabled
+}
+
// Package represents a Go package that has been type-checked. It maintains
// only the relevant fields of a *go/packages.Package.
type Package interface {
diff --git a/internal/lsp/tests/util.go b/internal/lsp/tests/util.go
index 8d2fe05..e6de57f 100644
--- a/internal/lsp/tests/util.go
+++ b/internal/lsp/tests/util.go
@@ -503,3 +503,19 @@
}
return folder
}
+
+func EnableAllAnalyzers(snapshot source.Snapshot, opts *source.Options) {
+ if opts.UserEnabledAnalyses == nil {
+ opts.UserEnabledAnalyses = make(map[string]bool)
+ }
+ for _, a := range opts.DefaultAnalyzers {
+ if !a.Enabled(snapshot) {
+ opts.UserEnabledAnalyses[a.Analyzer.Name] = true
+ }
+ }
+ for _, a := range opts.TypeErrorAnalyzers {
+ if !a.Enabled(snapshot) {
+ opts.UserEnabledAnalyses[a.Analyzer.Name] = true
+ }
+ }
+}
