feature: new rule use-waitgroup-go (#1484)

README.md

@@ -599,6 +599,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | [`use-any`](./RULES_DESCRIPTIONS.md#use-any)          |  n/a   |  Proposes to replace `interface{}` with its alias `any` |    no    |  no   |
 | [`use-errors-new`](./RULES_DESCRIPTIONS.md#use-errors-new) | n/a   | Spots calls to `fmt.Errorf` that can be replaced by `errors.New` |   no    |  no   |
 | [`use-fmt-print`](./RULES_DESCRIPTIONS.md#use-fmt-print) | n/a   | Proposes to replace calls to built-in `print` and `println` with their equivalents from `fmt`. |   no    |  no   |
+| [`use-waitgroup-go`](./RULES_DESCRIPTIONS.md#use-waitgroup-go)          |  n/a   |  Proposes to replace `wg.Add ... go {... wg.Done ...}` idiom with `wg.Go` |    no    |  no   |
 | [`useless-break`](./RULES_DESCRIPTIONS.md#useless-break)          |  n/a   |  Warns on useless `break` statements in case clauses |    no    |  no   |
 | [`useless-fallthrough`](./RULES_DESCRIPTIONS.md#useless-fallthrough)  |  n/a   |  Warns on useless `fallthrough` statements in case clauses |    no    |  no   |
 | [`var-declaration`](./RULES_DESCRIPTIONS.md#var-declaration)     |  n/a   | Reduces redundancies around variable declaration.                |   yes    |  yes  |

RULES_DESCRIPTIONS.md

@@ -1421,6 +1421,18 @@ _Description_: This rule proposes to replace calls to built-in `print` and `prin
 
 _Configuration_: N/A
 
+## use-waitgroup-go
+
+_Description_: Since Go 1.25 the `sync` package proposes the [`WaitGroup.Go`](https://pkg.go.dev/sync#WaitGroup.Go) method.
+This method is a shorter and safer replacement for the idiom `wg.Add ... go { ... wg.Done ... }`.
+The rule proposes to replace these legacy idioms with calls to the new method.
+
+_Limitations_: The rule doesn't rely on type information but on variable names to identify waitgroups.
+This means the rule search for `wg` (the defacto standard name for wait groups);
+if the waitgroup variable is named differently than `wg` the rule will skip it.
+
+_Configuration_: N/A
+
 ## useless-break
 
 _Description_: This rule warns on useless `break` statements in case clauses of switch and select statements. Go,

config/config.go

@@ -112,6 +112,7 @@ var allRules = append([]lint.Rule{
 	&rule.IdenticalSwitchBranchesRule{},
 	&rule.UselessFallthroughRule{},
 	&rule.PackageDirectoryMismatchRule{},
+	&rule.UseWaitGroupGoRule{},
 }, defaultRules...)
 
 // allFormatters is a list of all available formatters to output the linting results.

lint/package.go

@@ -42,6 +42,8 @@ var (
 	Go122 = goversion.Must(goversion.NewVersion("1.22"))
 	// Go124 is a constant representing the Go version 1.24.
 	Go124 = goversion.Must(goversion.NewVersion("1.24"))
+	// Go125 is a constant representing the Go version 1.25.
+	Go125 = goversion.Must(goversion.NewVersion("1.25"))
 )
 
 // Files return package's files.

rule/use_waitgroup_go.go

@@ -0,0 +1,158 @@
+package rule
+
+import (
+	"go/ast"
+
+	"github.com/mgechev/revive/internal/astutils"
+	"github.com/mgechev/revive/lint"
+)
+
+// UseWaitGroupGoRule spots Go idioms that might be rewritten using WaitGroup.Go.
+type UseWaitGroupGoRule struct{}
+
+// Apply applies the rule to given file.
+func (*UseWaitGroupGoRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
+	if !file.Pkg.IsAtLeastGoVersion(lint.Go125) {
+		return nil // skip analysis if Go version < 1.25
+	}
+
+	var failures []lint.Failure
+
+	onFailure := func(failure lint.Failure) {
+		failures = append(failures, failure)
+	}
+
+	w := &lintUseWaitGroupGo{
+		onFailure: onFailure,
+	}
+
+	// Iterate over declarations looking for function declarations
+	for _, decl := range file.AST.Decls {
+		fn, ok := decl.(*ast.FuncDecl)
+		if !ok {
+			continue // not a function
+		}
+
+		if fn.Body == nil {
+			continue // external (no-Go) function
+		}
+
+		// Analyze the function body
+		ast.Walk(w, fn.Body)
+	}
+
+	return failures
+}
+
+// Name returns the rule name.
+func (*UseWaitGroupGoRule) Name() string {
+	return "use-waitgroup-go"
+}
+
+type lintUseWaitGroupGo struct {
+	onFailure func(lint.Failure)
+}
+
+func (w *lintUseWaitGroupGo) Visit(node ast.Node) ast.Visitor {
+	// Only interested in blocks of statements
+	block, ok := node.(*ast.BlockStmt)
+	if !ok {
+		return w // not a block of statements
+	}
+
+	w.analyzeBlock(block)
+
+	return w
+}
+
+// analyzeBlock searches AST subtrees with the following form
+// wg.Add(...)
+// ...
+//
+//	go func (...) {
+//	   ...
+//	   wg.Done // or defer wg.Done
+//	   ...
+//	}
+//
+// Warning: the analysis only looks for exactly wg.Add and wg.Done, that means
+// calls to Add and Done on a WaitGroup struct within a variable named differently than wg will be ignored
+// This simplification avoids requiring type information while still makes the rule work in most of the cases.
+// This rule assumes the WaitGroup variable is named 'wg', which is the common convention.
+func (w *lintUseWaitGroupGo) analyzeBlock(b *ast.BlockStmt) {
+	// we will iterate over all statements in search for wg.Add()
+	stmts := b.List
+	for i := 0; i < len(stmts); i++ {
+		stmt := stmts[i]
+		if !w.isCallToWgAdd(stmt) {
+			continue
+		}
+
+		call := stmt
+
+		// Here we have identified a call to wg.Add
+		// Let's iterate over the statements that follow the wg.Add
+		// to see if there is a go statement that runs a goroutine with a wg.Done
+		//
+		// wg.Add is the i-th statement of block.List
+		// we will iterate from the (i+1)-th statement up to the last statement of block.List
+		for i++; i < len(stmts); i++ {
+			stmt := stmts[i]
+			// looking for a go statement
+			goStmt, ok := stmt.(*ast.GoStmt)
+			if !ok {
+				continue // not a go statement
+			}
+
+			// here we found a the go statement
+			// now let's check is the go statement is applied to a function literal that contains a wg.Done
+			if !w.hasCallToWgDone(goStmt) {
+				continue
+			}
+
+			w.onFailure(lint.Failure{
+				Confidence: 1,
+				Node:       call,
+				Category:   lint.FailureCategoryCodeStyle,
+				Failure:    "replace wg.Add()...go {...wg.Done()...} with wg.Go(...)",
+			})
+
+			break
+		}
+	}
+}
+
+// hasCallToWgDone returns true if the given go statement
+// calls to a function literal containing a call to wg.Done, false otherwise.
+func (*lintUseWaitGroupGo) hasCallToWgDone(goStmt *ast.GoStmt) bool {
+	funcLit, ok := goStmt.Call.Fun.(*ast.FuncLit)
+	if !ok {
+		return false // the go statements runs a function defined elsewhere
+	}
+
+	// here we found a go statement running a function literal
+	// now we will look for a wg.Done inside the body of the function literal
+	wgDoneStmt := astutils.SeekNode[ast.Node](funcLit.Body, wgDonePicker)
+
+	return wgDoneStmt != nil
+}
+
+// isCallToWgAdd returns true if the given statement is a call to wg.Add, false otherwise.
+func (*lintUseWaitGroupGo) isCallToWgAdd(stmt ast.Stmt) bool {
+	expr, ok := stmt.(*ast.ExprStmt)
+	if !ok {
+		return false // not an expression statements thus not a function call
+	}
+
+	// Lets check if the expression statement is a call to wg.Add
+	call, ok := expr.X.(*ast.CallExpr)
+
+	return ok && astutils.IsPkgDotName(call.Fun, "wg", "Add")
+}
+
+// function used when calling astutils.SeekNode that search for calls to wg.Done.
+func wgDonePicker(n ast.Node) bool {
+	call, ok := n.(*ast.CallExpr)
+	result := ok && astutils.IsPkgDotName(call.Fun, "wg", "Done")
+	return result
+}

test/use_waitgroup_go_test.go

@@ -0,0 +1,13 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/mgechev/revive/lint"
+	"github.com/mgechev/revive/rule"
+)
+
+func TestUseWaitGroupGo(t *testing.T) {
+	testRule(t, "use_waitgroup_go", &rule.UseWaitGroupGoRule{}, &lint.RuleConfig{})
+	testRule(t, "go1.25/use_waitgroup_go", &rule.UseWaitGroupGoRule{}, &lint.RuleConfig{})
+}

testdata/go1.25/go.mod

@@ -0,0 +1,3 @@
+module github.com/mgechev/revive/testdata
+
+go 1.25

testdata/go1.25/use_waitgroup_go.go

@@ -0,0 +1,53 @@
+package fixtures
+
+import (
+	"sync"
+)
+
+func useWaitGroupGo() {
+	wg := sync.WaitGroup{}
+
+	wg.Add(1) // MATCH /replace wg.Add()...go {...wg.Done()...} with wg.Go(...)/
+	go func() {
+		defer wg.Done()
+		doSomething()
+	}()
+
+	wg.Add(1) // MATCH /replace wg.Add()...go {...wg.Done()...} with wg.Go(...)/
+	go func() {
+		doSomething()
+		wg.Done()
+	}()
+
+	// from golang.org/x/tools/go/packages/packages.go/parseFiles
+	for i, file := range filenames {
+		wg.Add(1) // MATCH /replace wg.Add()...go {...wg.Done()...} with wg.Go(...)/
+		go func(i int, filename string) {
+			parsed[i], errors[i] = ld.parseFile(filename)
+			wg.Done()
+		}(i, file)
+	}
+	wg.Wait()
+
+	// from kubernetes/pkg/kubelet/cm/devicemanager/manager_test.go/TestGetTopologyHintsWithUpdates
+	// notice the rule spots a wg.Add(2) (vs wg.Add(1)) therefore using wg.Go is possible but requires
+	// replacing the wg.Add and the next two go statements with two wg.Go
+
+	wg.Add(2) // MATCH /replace wg.Add()...go {...wg.Done()...} with wg.Go(...)/
+
+	go func() {
+		defer wg.Done()
+		for i := 0; i < test.count; i++ {
+			// simulate the device plugin to send device updates
+			mimpl.genericDeviceUpdateCallback(testResourceName, devs)
+		}
+		updated.Store(true)
+	}()
+	go func() {
+		defer wg.Done()
+		for !updated.Load() {
+			test.testfunc(mimpl)
+		}
+	}()
+	wg.Wait()
+}

testdata/use_waitgroup_go.go

@@ -0,0 +1,53 @@
+package fixtures
+
+import (
+	"sync"
+)
+
+// Rule use-waitgroup-go shall not match because this file is a package with Go version < 1.25
+func useWaitGroupGo() {
+	wg := sync.WaitGroup{}
+
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		doSomething()
+	}()
+
+	wg.Add(1)
+	go func() {
+		doSomething()
+		wg.Done()
+	}()
+
+	// from golang.org/x/tools/go/packages/packages.go/parseFiles
+	for i, file := range filenames {
+		wg.Add(1)
+		go func(i int, filename string) {
+			parsed[i], errors[i] = ld.parseFile(filename)
+			wg.Done()
+		}(i, file)
+	}
+	wg.Wait()
+
+	// from kubernetes/pkg/kubelet/cm/devicemanager/manager_test.go/TestGetTopologyHintsWithUpdates
+	// notice the rule spots a wg.Add(2) (vs wg.Add(1)) therefore using wg.Go is possible but requires
+	// replacing the wg.Add and the next two go statements with two wg.Go
+	wg.Add(2)
+
+	go func() {
+		defer wg.Done()
+		for i := 0; i < test.count; i++ {
+			// simulate the device plugin to send device updates
+			mimpl.genericDeviceUpdateCallback(testResourceName, devs)
+		}
+		updated.Store(true)
+	}()
+	go func() {
+		defer wg.Done()
+		for !updated.Load() {
+			test.testfunc(mimpl)
+		}
+	}()
+	wg.Wait()
+}