Repository: mgechev/revive

Issue:
rule to propose using `wg.Go`
Go 1.25 adds the method `func (wg *WaitGroup) Go(f func())`
This method allows rewriting code like
```go
wg.Add(1)
go func() {
	defer wg.Done()
	doSomething()
}()
```

into 

```go
wg.Go(func() {
	doSomething()
})
```

revive could spot code fragments where the new method can be used to replace the more verbose (and sometimes fragile) legacy idiom.

rule name: `use-wg-go` (or `use-waitgroup-go`)

A prototype implementation of the rule can be found at the [`sandbox` branch](https://github.com/mgechev/revive/tree/sandbox)
(the rulename in the branch is `deleteme`)  
You can run the rule with `revive -config deleteme.toml ./...`

Discussion:

Solution patch:
diff --git a/README.md b/README.md
index 21d8056..1317ce9 100644
--- a/README.md
+++ b/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  |
diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md
index d1c2e51..23328a1 100644
--- a/RULES_DESCRIPTIONS.md
+++ b/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,
diff --git a/config/config.go b/config/config.go
index a926726..0ffaaab 100644
--- a/config/config.go
+++ b/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.
diff --git a/lint/package.go b/lint/package.go
index 34bd5d2..cb78cb4 100644
--- a/lint/package.go
+++ b/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.
diff --git a/rule/struct_tag.go b/rule/struct_tag.go
index 73d0d0e..edb6f95 100644
--- a/rule/struct_tag.go
+++ b/rule/struct_tag.go
@@ -180,6 +180,10 @@ func (w lintStructTagRule) checkTaggedField(checkCtx *checkContext, f *ast.Field
 			w.addFailureWithTagKey(f.Tag, msg, tag.Key)
 		}
 
+		if msg, ok := checkOptionsOnIgnoredField(tag); !ok {
+			w.addFailureWithTagKey(f.Tag, msg, tag.Key)
+		}
+
 		checker, ok := w.tagCheckers[tagKey(tag.Key)]
 		if !ok {
 			continue // we don't have a checker for the tag
@@ -561,13 +565,6 @@ func checkYAMLTag(checkCtx *checkContext, tag *structtag.Tag, _ ast.Expr) (messa
 }
 
 func checkSpannerTag(checkCtx *checkContext, tag *structtag.Tag, _ ast.Expr) (message string, succeeded bool) {
-	if tag.Name == "-" {
-		if len(tag.Options) > 0 {
-			return fmt.Sprintf("useless option(s) %s for ignored field", strings.Join(tag.Options, ",")), false
-		}
-		return "", true
-	}
-
 	for _, opt := range tag.Options {
 		if !checkCtx.isUserDefined(keySpanner, opt) {
 			return fmt.Sprintf(msgUnknownOption, opt), false
@@ -577,6 +574,28 @@ func checkSpannerTag(checkCtx *checkContext, tag *structtag.Tag, _ ast.Expr) (me
 	return "", true
 }
 
+// checkOptionsOnIgnoredField checks if an ignored struct field (tag name "-") has any options specified.
+// It returns a message and false if there are useless options present, or an empty message and true if valid.
+func checkOptionsOnIgnoredField(tag *structtag.Tag) (message string, succeeded bool) {
+	if tag.Name != "-" {
+		return "", true
+	}
+
+	switch len(tag.Options) {
+	case 0:
+		return "", true
+	case 1:
+		opt := strings.TrimSpace(tag.Options[0])
+		if opt == "" {
+			return "", true // accept "-," as options
+		}
+
+		return fmt.Sprintf("useless option %s for ignored field", opt), false
+	default:
+		return fmt.Sprintf("useless options %s for ignored field", strings.Join(tag.Options, ",")), false
+	}
+}
+
 func checkValidateOptionsAlternatives(checkCtx *checkContext, alternatives []string) (message string, succeeded bool) {
 	for _, alternative := range alternatives {
 		alternative := strings.TrimSpace(alternative)
diff --git a/rule/use_waitgroup_go.go b/rule/use_waitgroup_go.go
new file mode 100644
index 0000000..9dd80d7
--- /dev/null
+++ b/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 patch:
diff --git a/test/use_waitgroup_go_test.go b/test/use_waitgroup_go_test.go
new file mode 100644
index 0000000..482c23c
--- /dev/null
+++ b/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{})
+}
diff --git a/testdata/go1.25/go.mod b/testdata/go1.25/go.mod
new file mode 100644
index 0000000..abc1bbf
--- /dev/null
+++ b/testdata/go1.25/go.mod
@@ -0,0 +1,3 @@
+module github.com/mgechev/revive/testdata
+
+go 1.25
diff --git a/testdata/go1.25/use_waitgroup_go.go b/testdata/go1.25/use_waitgroup_go.go
new file mode 100644
index 0000000..a827a49
--- /dev/null
+++ b/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()
+}
diff --git a/testdata/struct_tag.go b/testdata/struct_tag.go
index 2382008..432fc51 100644
--- a/testdata/struct_tag.go
+++ b/testdata/struct_tag.go
@@ -192,7 +192,7 @@ type PropertiesTags struct {
 type SpannerUser struct {
 	ID        int       `spanner:"user_id"`
 	Name      string    `spanner:"full_name"`
-	Email     string    `spanner:"-"`                    // Valid: ignore field
+	Email     string    `spanner:"-"` // Valid: ignore field
 	CreatedAt time.Time `spanner:"created_at"`
-	UpdatedAt time.Time `spanner:"updated_at,unknown"`   // MATCH /unknown option "unknown" in spanner tag/
-}
\ No newline at end of file
+	UpdatedAt time.Time `spanner:"updated_at,unknown"` // MATCH /unknown option "unknown" in spanner tag/
+}
diff --git a/testdata/struct_tag_user_options.go b/testdata/struct_tag_user_options.go
index 76f8600..32bd830 100644
--- a/testdata/struct_tag_user_options.go
+++ b/testdata/struct_tag_user_options.go
@@ -1,5 +1,7 @@
 package fixtures
 
+import "time"
+
 type RangeAllocation struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
@@ -49,6 +51,49 @@ type TomlUser struct {
 
 type SpannerUserOptions struct {
 	ID   int    `spanner:"user_id,mySpannerOption"`
-	A    int    `spanner:"-,mySpannerOption"`       // MATCH /useless option(s) mySpannerOption for ignored field in spanner tag/
+	A    int    `spanner:"-,mySpannerOption"`       // MATCH /useless option mySpannerOption for ignored field in spanner tag/
 	Name string `spanner:"full_name,unknownOption"` // MATCH /unknown option "unknownOption" in spanner tag/
 }
+
+type uselessOptions struct {
+	A  int       `bson:"-,"`
+	B  int       `bson:"-,omitempty"`           // MATCH /useless option omitempty for ignored field in bson tag/
+	C  int       `bson:"-,omitempty,omitempty"` // MATCH /useless options omitempty,omitempty for ignored field in bson tag/
+	D  int       `datastore:"-,"`
+	E  int       `datastore:"-,omitempty"`           // MATCH /useless option omitempty for ignored field in datastore tag/
+	F  int       `datastore:"-,omitempty,omitempty"` // MATCH /useless options omitempty,omitempty for ignored field in datastore tag/
+	G  int       `json:"-,"`
+	H  int       `json:"-,omitempty"`           // MATCH /useless option omitempty for ignored field in json tag/
+	I  int       `json:"-,omitempty,omitempty"` // MATCH /useless options omitempty,omitempty for ignored field in json tag/
+	J  int       `mapstructure:"-,"`
+	K  int       `mapstructure:"-,squash"`              // MATCH /useless option squash for ignored field in mapstructure tag/
+	L  int       `mapstructure:"-,omitempty,omitempty"` // MATCH /useless options omitempty,omitempty for ignored field in mapstructure tag/
+	M  int       `properties:"-,"`
+	N  int       `properties:"-,default=15"`                           // MATCH /useless option default=15 for ignored field in properties tag/
+	O  time.Time `properties:"-,layout=2006-01-02,default=2006-01-02"` // MATCH /useless options layout=2006-01-02,default=2006-01-02 for ignored field in properties tag/
+	P  int       `spanner:"-,"`
+	Q  int       `spanner:"-,mySpannerOption"`                 // MATCH /useless option mySpannerOption for ignored field in spanner tag/
+	R  int       `spanner:"-,mySpannerOption,mySpannerOption"` // MATCH /useless options mySpannerOption,mySpannerOption for ignored field in spanner tag/
+	S  int       `toml:"-,"`
+	T  int       `toml:"-,omitempty"`           // MATCH /useless option omitempty for ignored field in toml tag/
+	U  int       `toml:"-,omitempty,omitempty"` // MATCH /useless options omitempty,omitempty for ignored field in toml tag/
+	V  int       `url:"-,"`
+	W  int       `url:"-,omitempty"`           // MATCH /useless option omitempty for ignored field in url tag/
+	X  int       `url:"-,omitempty,omitempty"` // MATCH /useless options omitempty,omitempty for ignored field in url tag/
+	Y  int       `xml:"-,"`
+	Z  int       `xml:"-,omitempty"`           // MATCH /useless option omitempty for ignored field in xml tag/
+	Aa int       `xml:"-,omitempty,omitempty"` // MATCH /useless options omitempty,omitempty for ignored field in xml tag/
+	Ba int       `yaml:"-,"`
+	Ca int       `yaml:"-,omitempty"`           // MATCH /useless option omitempty for ignored field in yaml tag/
+	Da int       `yaml:"-,omitempty,omitempty"` // MATCH /useless options omitempty,omitempty for ignored field in yaml tag/
+
+	// MATCH:59 /unknown option "" in bson tag/
+	// MATCH:62 /unknown option "" in datastore tag/
+	// MATCH:68 /unknown option "" in mapstructure tag/
+	// MATCH:71 /unknown or malformed option "" in properties tag/
+	// MATCH:74 /unknown option "" in spanner tag/
+	// MATCH:77 /unknown option "" in toml tag/
+	// MATCH:80 /unknown option "" in url tag/
+	// MATCH:83 /unknown option "" in xml tag/
+	// MATCH:86 /unknown option "" in yaml tag/
+}
diff --git a/testdata/use_waitgroup_go.go b/testdata/use_waitgroup_go.go
new file mode 100644
index 0000000..4270b43
--- /dev/null
+++ b/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()
+}