gopls/internal/lsp/source: enable nilness Analyzer in gopls
This change enables the nilness Analyser in gopls, which
was previously disabled due to the high asymptotic cost
of SSA construction in the buildssa Analyzer. That problem
has since been fixed by a series of changes to go/ssa
to avoid the need to call CreatePackage for indirect
dependencies.
There is still work to do to reduce the cost of SSA-based
analyzers that use facts (not that we have any today),
as these must be run on the entire workspace, and the
cost of "deep" fact encoding is a hotspot; see CL 513375.
There is also still work to do to port the changes to
go/ssa and buildssa into ir and buildir, the forks of
these packages used by honnef.co/staticcheck. It does
use facts.
Still, progress.
Plus a test.
Change-Id: I61bb045752a53ef19ae4651deee25c6d948ce2fc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/538802
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
diff --git a/go/analysis/validate.go b/go/analysis/validate.go
index 9da5692..4f2c404 100644
--- a/go/analysis/validate.go
+++ b/go/analysis/validate.go
@@ -19,6 +19,8 @@
// that the Requires graph is acyclic;
// that analyzer fact types are unique;
// that each fact type is a pointer.
+//
+// Analyzer names need not be unique, though this may be confusing.
func Validate(analyzers []*Analyzer) error {
// Map each fact type to its sole generating analyzer.
factTypes := make(map[reflect.Type]*Analyzer)
diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md
index 55c199c..ef14490 100644
--- a/gopls/doc/analyzers.md
+++ b/gopls/doc/analyzers.md
@@ -386,7 +386,7 @@
panic(p)
}
-**Disabled by default. Enable it by setting `"analyses": {"nilness": true}`.**
+**Enabled by default.**
## **printf**
diff --git a/gopls/internal/lsp/cache/analysis.go b/gopls/internal/lsp/cache/analysis.go
index 6004440..ae666ba 100644
--- a/gopls/internal/lsp/cache/analysis.go
+++ b/gopls/internal/lsp/cache/analysis.go
@@ -442,7 +442,7 @@
if !ok {
// Although this 'skip' operation is logically sound,
// it is nonetheless surprising that its absence should
- // cause #60909 since none of the analyzers added for
+ // cause #60909 since none of the analyzers currently added for
// requirements (e.g. ctrlflow, inspect, buildssa)
// is capable of reporting diagnostics.
if summary := root.summary.Actions[stableNames[a]]; summary != nil {
diff --git a/gopls/internal/lsp/source/api_json.go b/gopls/internal/lsp/source/api_json.go
index 429453e..b6e5fbd 100644
--- a/gopls/internal/lsp/source/api_json.go
+++ b/gopls/internal/lsp/source/api_json.go
@@ -338,7 +338,7 @@
{
Name: "\"nilness\"",
Doc: "check for redundant or impossible nil comparisons\n\nThe nilness checker inspects the control-flow graph of each function in\na package and reports nil pointer dereferences, degenerate nil\npointers, and panics with nil values. A degenerate comparison is of the form\nx==nil or x!=nil where x is statically known to be nil or non-nil. These are\noften a mistake, especially in control flow related to errors. Panics with nil\nvalues are checked because they are not detectable by\n\n\tif r := recover(); r != nil {\n\nThis check reports conditions such as:\n\n\tif f == nil { // impossible condition (f is a function)\n\t}\n\nand:\n\n\tp := &v\n\t...\n\tif p != nil { // tautological condition\n\t}\n\nand:\n\n\tif p == nil {\n\t\tprint(*p) // nil dereference\n\t}\n\nand:\n\n\tif p == nil {\n\t\tpanic(p)\n\t}",
- Default: "false",
+ Default: "true",
},
{
Name: "\"printf\"",
@@ -1093,9 +1093,10 @@
Default: true,
},
{
- Name: "nilness",
- Doc: "check for redundant or impossible nil comparisons\n\nThe nilness checker inspects the control-flow graph of each function in\na package and reports nil pointer dereferences, degenerate nil\npointers, and panics with nil values. A degenerate comparison is of the form\nx==nil or x!=nil where x is statically known to be nil or non-nil. These are\noften a mistake, especially in control flow related to errors. Panics with nil\nvalues are checked because they are not detectable by\n\n\tif r := recover(); r != nil {\n\nThis check reports conditions such as:\n\n\tif f == nil { // impossible condition (f is a function)\n\t}\n\nand:\n\n\tp := &v\n\t...\n\tif p != nil { // tautological condition\n\t}\n\nand:\n\n\tif p == nil {\n\t\tprint(*p) // nil dereference\n\t}\n\nand:\n\n\tif p == nil {\n\t\tpanic(p)\n\t}",
- URL: "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/nilness",
+ Name: "nilness",
+ Doc: "check for redundant or impossible nil comparisons\n\nThe nilness checker inspects the control-flow graph of each function in\na package and reports nil pointer dereferences, degenerate nil\npointers, and panics with nil values. A degenerate comparison is of the form\nx==nil or x!=nil where x is statically known to be nil or non-nil. These are\noften a mistake, especially in control flow related to errors. Panics with nil\nvalues are checked because they are not detectable by\n\n\tif r := recover(); r != nil {\n\nThis check reports conditions such as:\n\n\tif f == nil { // impossible condition (f is a function)\n\t}\n\nand:\n\n\tp := &v\n\t...\n\tif p != nil { // tautological condition\n\t}\n\nand:\n\n\tif p == nil {\n\t\tprint(*p) // nil dereference\n\t}\n\nand:\n\n\tif p == nil {\n\t\tpanic(p)\n\t}",
+ URL: "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/nilness",
+ Default: true,
},
{
Name: "printf",
diff --git a/gopls/internal/lsp/source/options.go b/gopls/internal/lsp/source/options.go
index cb8f081..74f9bed 100644
--- a/gopls/internal/lsp/source/options.go
+++ b/gopls/internal/lsp/source/options.go
@@ -1632,7 +1632,7 @@
atomicalign.Analyzer.Name: {Analyzer: atomicalign.Analyzer, Enabled: true},
deepequalerrors.Analyzer.Name: {Analyzer: deepequalerrors.Analyzer, Enabled: true},
fieldalignment.Analyzer.Name: {Analyzer: fieldalignment.Analyzer, Enabled: false},
- nilness.Analyzer.Name: {Analyzer: nilness.Analyzer, Enabled: false},
+ nilness.Analyzer.Name: {Analyzer: nilness.Analyzer, Enabled: true},
shadow.Analyzer.Name: {Analyzer: shadow.Analyzer, Enabled: false},
sortslice.Analyzer.Name: {Analyzer: sortslice.Analyzer, Enabled: true},
testinggoroutine.Analyzer.Name: {Analyzer: testinggoroutine.Analyzer, Enabled: true},
diff --git a/gopls/internal/regtest/marker/testdata/diagnostics/analyzers.txt b/gopls/internal/regtest/marker/testdata/diagnostics/analyzers.txt
index e98674b..837a116 100644
--- a/gopls/internal/regtest/marker/testdata/diagnostics/analyzers.txt
+++ b/gopls/internal/regtest/marker/testdata/diagnostics/analyzers.txt
@@ -1,5 +1,5 @@
Test of warning diagnostics from various analyzers:
-copylocks, printf, slog, tests, and timeformat.
+copylocks, printf, slog, tests, timeformat, and nilness.
-- go.mod --
module example.com
@@ -49,3 +49,9 @@
fmt.Println(now.Format("2006-02-01")) //@diag("2006-02-01", re"2006-02-01 should be 2006-01-02")
}
+// nilness
+func _(ptr *int) {
+ if ptr == nil {
+ _ = *ptr //@diag("*ptr", re"nil dereference in load")
+ }
+}
diff --git a/gopls/internal/regtest/marker/testdata/references/issue60676.txt b/gopls/internal/regtest/marker/testdata/references/issue60676.txt
index cacf6fd..98f608e 100644
--- a/gopls/internal/regtest/marker/testdata/references/issue60676.txt
+++ b/gopls/internal/regtest/marker/testdata/references/issue60676.txt
@@ -62,8 +62,9 @@
}
x.G = "hi" //@refs("G", GDef, "G")
_ = x.E //@refs("E", EDef, "E")
+}
- var y b.BI
+func _(y b.BI) {
_ = y.M //@refs("M", MDef, "M")
_ = y.N //@refs("N", NDef, "N")
}
