gopls/internal/settings: enable 'waitgroup' analyzer

This analyzer (equivalent to staticcheck SA2000) will appear
in gopls in advance of its appearance in cmd/vet after go1.24.

+ Test, relnote

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

Author: adonovan

gopls/doc/analyzers.md

@@ -976,6 +976,37 @@ Default: off. Enable by setting `"analyses": {"useany": true}`.
 
 Package documentation: [useany](https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/useany)
 
+<a id='waitgroup'></a>
+## `waitgroup`: check for misuses of sync.WaitGroup
+
+
+This analyzer detects mistaken calls to the (*sync.WaitGroup).Add
+method from inside a new goroutine, causing Add to race with Wait:
+
+	// WRONG
+	var wg sync.WaitGroup
+	go func() {
+	        wg.Add(1) // "WaitGroup.Add called from inside new goroutine"
+	        defer wg.Done()
+	        ...
+	}()
+	wg.Wait() // (may return prematurely before new goroutine starts)
+
+The correct code calls Add before starting the goroutine:
+
+	// RIGHT
+	var wg sync.WaitGroup
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		...
+	}()
+	wg.Wait()
+
+Default: on.
+
+Package documentation: [waitgroup](https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/waitgroup)
+
 <a id='yield'></a>
 ## `yield`: report calls to yield where the result is ignored
 

gopls/doc/release/v0.17.0.md

@@ -94,6 +94,15 @@ The new `yield` analyzer detects mistakes using the `yield` function
 in a Go 1.23 iterator, such as failure to check its boolean result and
 break out of a loop.
 
+## `waitgroup` analyzer
+
+The new `waitgroup` analyzer detects calls to the `Add` method of
+`sync.WaitGroup` that are (mistakenly) made within the new goroutine,
+causing `Add` to race with `Wait`.
+(This check is equivalent to
+[staticcheck's SA2000](https://staticcheck.dev/docs/checks#SA2000),
+but is enabled by default.)
+
 ## Add test for function or method
 
 If the selected chunk of code is part of a function or method declaration F,

gopls/internal/doc/api.json

@@ -609,6 +609,11 @@
 							"Doc": "check for constraints that could be simplified to \"any\"",
 							"Default": "false"
 						},
+						{
+							"Name": "\"waitgroup\"",
+							"Doc": "check for misuses of sync.WaitGroup\n\nThis analyzer detects mistaken calls to the (*sync.WaitGroup).Add\nmethod from inside a new goroutine, causing Add to race with Wait:\n\n\t// WRONG\n\tvar wg sync.WaitGroup\n\tgo func() {\n\t        wg.Add(1) // \"WaitGroup.Add called from inside new goroutine\"\n\t        defer wg.Done()\n\t        ...\n\t}()\n\twg.Wait() // (may return prematurely before new goroutine starts)\n\nThe correct code calls Add before starting the goroutine:\n\n\t// RIGHT\n\tvar wg sync.WaitGroup\n\twg.Add(1)\n\tgo func() {\n\t\tdefer wg.Done()\n\t\t...\n\t}()\n\twg.Wait()",
+							"Default": "true"
+						},
 						{
 							"Name": "\"yield\"",
 							"Doc": "report calls to yield where the result is ignored\n\nAfter a yield function returns false, the caller should not call\nthe yield function again; generally the iterator should return\npromptly.\n\nThis example fails to check the result of the call to yield,\ncausing this analyzer to report a diagnostic:\n\n\tyield(1) // yield may be called again (on L2) after returning false\n\tyield(2)\n\nThe corrected code is either this:\n\n\tif yield(1) { yield(2) }\n\nor simply:\n\n\t_ = yield(1) \u0026\u0026 yield(2)\n\nIt is not always a mistake to ignore the result of yield.\nFor example, this is a valid single-element iterator:\n\n\tyield(1) // ok to ignore result\n\treturn\n\nIt is only a mistake when the yield call that returned false may be\nfollowed by another call.",
@@ -1298,6 +1303,12 @@
 			"URL": "https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/useany",
 			"Default": false
 		},
+		{
+			"Name": "waitgroup",
+			"Doc": "check for misuses of sync.WaitGroup\n\nThis analyzer detects mistaken calls to the (*sync.WaitGroup).Add\nmethod from inside a new goroutine, causing Add to race with Wait:\n\n\t// WRONG\n\tvar wg sync.WaitGroup\n\tgo func() {\n\t        wg.Add(1) // \"WaitGroup.Add called from inside new goroutine\"\n\t        defer wg.Done()\n\t        ...\n\t}()\n\twg.Wait() // (may return prematurely before new goroutine starts)\n\nThe correct code calls Add before starting the goroutine:\n\n\t// RIGHT\n\tvar wg sync.WaitGroup\n\twg.Add(1)\n\tgo func() {\n\t\tdefer wg.Done()\n\t\t...\n\t}()\n\twg.Wait()",
+			"URL": "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/waitgroup",
+			"Default": true
+		},
 		{
 			"Name": "yield",
 			"Doc": "report calls to yield where the result is ignored\n\nAfter a yield function returns false, the caller should not call\nthe yield function again; generally the iterator should return\npromptly.\n\nThis example fails to check the result of the call to yield,\ncausing this analyzer to report a diagnostic:\n\n\tyield(1) // yield may be called again (on L2) after returning false\n\tyield(2)\n\nThe corrected code is either this:\n\n\tif yield(1) { yield(2) }\n\nor simply:\n\n\t_ = yield(1) \u0026\u0026 yield(2)\n\nIt is not always a mistake to ignore the result of yield.\nFor example, this is a valid single-element iterator:\n\n\tyield(1) // ok to ignore result\n\treturn\n\nIt is only a mistake when the yield call that returned false may be\nfollowed by another call.",

gopls/internal/settings/analysis.go

@@ -45,6 +45,7 @@ import (
 	"golang.org/x/tools/go/analysis/passes/unsafeptr"
 	"golang.org/x/tools/go/analysis/passes/unusedresult"
 	"golang.org/x/tools/go/analysis/passes/unusedwrite"
+	"golang.org/x/tools/go/analysis/passes/waitgroup"
 	"golang.org/x/tools/gopls/internal/analysis/deprecated"
 	"golang.org/x/tools/gopls/internal/analysis/embeddirective"
 	"golang.org/x/tools/gopls/internal/analysis/fillreturns"
@@ -154,6 +155,7 @@ func init() {
 		{analyzer: yield.Analyzer, enabled: true},   // uses go/ssa
 		{analyzer: sortslice.Analyzer, enabled: true},
 		{analyzer: embeddirective.Analyzer, enabled: true},
+		{analyzer: waitgroup.Analyzer, enabled: true}, // to appear in cmd/[email protected]
 
 		// disabled due to high false positives
 		{analyzer: shadow.Analyzer, enabled: false}, // very noisy

gopls/internal/test/marker/testdata/diagnostics/analyzers.txt

@@ -66,6 +66,14 @@ func _() {
 	slog.Info("msg", 1) //@diag("1", re`slog.Info arg "1" should be a string or a slog.Attr`)
 }
 
+// waitgroup
+func _() {
+	var wg sync.WaitGroup
+	go func() {
+		wg.Add(1) //@diag("(", re"WaitGroup.Add called from inside new goroutine")
+	}()
+}
+
 -- cgocall/cgocall.go --
 package cgocall