New Rule: Avoid spec Pollution

Does not allow variable assignments in container nodes. See here for
more details: https://onsi.github.io/ginkgo/#avoid-spec-pollution-dont-initialize-variables-in-container-nodes

This rule is disabled by default. use the `--forbid-spec-pollution=true`
flag to enable it.

For example, this code will trigger a warning:
```go
var _ = Describe("description", func(){
    var x = 10
    ...
})
```

Instead, use `BeforeEach()`; e.g.
```go
var _ = Describe("description", func (){
    var x int

    BeforeEach(func (){
        x = 10
    })
    ...
})
```

README.md

@@ -227,7 +227,7 @@ ginkgolinter checks the following:
 * If the first parameter is a function with the format of `func(error)bool`, ginkgolinter makes sure that the second 
   parameter exists and its type is string.
 
-### Async timing interval: timeout is shorter than polling interval [Bug]
+### Async timing interval: timeout is shorter than polling interval [BUG]
 ***Note***: Only applied when the `suppress-async-assertion` flag is **not set** *and* the `validate-async-intervals` 
 flag **is** set.
 
@@ -249,6 +249,32 @@ This will probably happen when using the old format:
    Eventually(aFunc, 500 * time.Millisecond /*timeout*/, 10 * time.Second /*polling*/).Should(Succeed())
    ```
 
+### Avoid Spec Pollution: Don't Initialize Variables in Container Nodes [BUG/STYLE]:
+***Note***: Only applied when the `--forbid-spec-pollution=true` flag is set (disabled by default).
+
+According to [ginkgo documentation](https://onsi.github.io/ginkgo/#avoid-spec-pollution-dont-initialize-variables-in-container-nodes), 
+no variable should be assigned within a container node (`Describe`, `Context`, `When` or their `F`, `P` or `X` forms)
+  
+For example:
+```go
+var _ = Describe("description", func(){
+    var x = 10
+    ...
+})
+```
+
+Instead, use `BeforeEach()`; e.g.
+```go
+var _ = Describe("description", func (){
+    var x int
+	
+    BeforeEach(func (){
+        x = 10
+    })
+    ...
+})
+```
+
 ### Wrong Length Assertion [STYLE]
 The linter finds assertion of the golang built-in `len` function, with all kind of matchers, while there are already 
 gomega matchers for these usecases; We want to assert the item, rather than its length.
@@ -403,7 +429,7 @@ This rule support auto fixing.
 
 ***This rule is disabled by default***. Use the `--force-expect-to=true` command line flag to enable it.
 
-### Async timing interval: multiple timeout or polling intervals [Style]
+### Async timing interval: multiple timeout or polling intervals [STYLE]
 ***Note***: Only applied when the `suppress-async-assertion` flag is **not set** *and* the `validate-async-intervals`
 flag **is** set.
 
@@ -421,7 +447,7 @@ Eventually(aFunc).WithTimeout(time.Second * 10).Within(time.Second * 10).WithPol
 Eventually(aFunc, time.Second*10, time.Millisecond * 500).WithPolling(time.Millisecond * 500).Should(BeTrue())
 ```
 
-### Async timing interval: non-time.Duration intervals [Bug]
+### Async timing interval: non-time.Duration intervals [STYLE]
 ***Note***: Only applied when the `suppress-async-assertion` flag is **not set** *and* the `validate-async-intervals`
 flag **is** set.
 

analyzer.go

@@ -43,6 +43,7 @@ func NewAnalyzer() *analysis.Analyzer {
 	a.Flags.Var(&config.ForceExpectTo, "force-expect-to", "force using `Expect` with `To`, `ToNot` or `NotTo`. reject using `Expect` with `Should` or `ShouldNot`; default = false (not forced)")
 	a.Flags.BoolVar(&ignored, "suppress-focus-container", true, "Suppress warning for ginkgo focus containers like FDescribe, FContext, FWhen or FIt. Deprecated and ignored: use --forbid-focus-container instead")
 	a.Flags.Var(&config.ForbidFocus, "forbid-focus-container", "trigger a warning for ginkgo focus containers like FDescribe, FContext, FWhen or FIt; default = false.")
+	a.Flags.Var(&config.ForbidSpecPollution, "forbid-spec-pollution", "trigger a warning for variable assignments in ginkgo containers like Describe, Context and When, instead of in BeforeEach(); default = false.")
 
 	return a
 }

analyzer_test.go

@@ -156,6 +156,19 @@ func TestFlags(t *testing.T) {
 			testData: []string{"a/timing"},
 			flags:    map[string]string{"validate-async-intervals": "true"},
 		},
+		{
+			testName: "vars in containers",
+			testData: []string{"a/vars-in-containers"},
+			flags:    map[string]string{"forbid-spec-pollution": "true"},
+		},
+		{
+			testName: "vars in containers + focus containers",
+			testData: []string{"a/containers-vas-and-focus"},
+			flags: map[string]string{
+				"forbid-spec-pollution":  "true",
+				"forbid-focus-container": "true",
+			},
+		},
 	} {
 		t.Run(tc.testName, func(tt *testing.T) {
 			analyzer := ginkgolinter.NewAnalyzer()

doc.go

@@ -30,6 +30,20 @@ For example:
 This will probably happen when using the old format:
 	Eventually(aFunc, 500 * time.Millisecond, 10 * time.Second).Should(Succeed())
 
+* reject variable assignments in ginkgo containers [Bug/Style]:
+For example:
+	var _ = Describe("description", func(){
+		var x = 10
+	})
+
+Should use BeforeEach instead; e.g.
+	var _ = Describe("description", func(){
+		var x int
+		BeforeEach(func(){
+			x = 10
+		})
+	})
+
 * wrong length assertions. We want to assert the item rather than its length. [Style]
 For example:
 	Expect(len(x)).Should(Equal(1))

internal/ginkgohandler/handler.go

@@ -15,6 +15,7 @@ const (
 // in imported with "." name, custom name or without any name.
 type Handler interface {
 	GetFocusContainerName(*ast.CallExpr) (bool, *ast.Ident)
+	IsWrapContainer(*ast.CallExpr) bool
 	IsFocusSpec(ident ast.Expr) bool
 }
 
@@ -49,6 +50,13 @@ func (h dotHandler) GetFocusContainerName(exp *ast.CallExpr) (bool, *ast.Ident)
 	return false, nil
 }
 
+func (h dotHandler) IsWrapContainer(exp *ast.CallExpr) bool {
+	if fun, ok := exp.Fun.(*ast.Ident); ok {
+		return IsWrapContainer(fun.Name)
+	}
+	return false
+}
+
 func (h dotHandler) IsFocusSpec(exp ast.Expr) bool {
 	id, ok := exp.(*ast.Ident)
 	return ok && id.Name == focusSpec
@@ -70,6 +78,16 @@ func (h nameHandler) GetFocusContainerName(exp *ast.CallExpr) (bool, *ast.Ident)
 	return false, nil
 }
 
+func (h nameHandler) IsWrapContainer(exp *ast.CallExpr) bool {
+	if sel, ok := exp.Fun.(*ast.SelectorExpr); ok {
+		if id, ok := sel.X.(*ast.Ident); ok && id.Name == string(h) {
+			return IsWrapContainer(sel.Sel.Name)
+		}
+	}
+	return false
+
+}
+
 func (h nameHandler) IsFocusSpec(exp ast.Expr) bool {
 	if selExp, ok := exp.(*ast.SelectorExpr); ok {
 		if x, ok := selExp.X.(*ast.Ident); ok && x.Name == string(h) {
@@ -88,10 +106,24 @@ func isFocusContainer(name string) bool {
 	return false
 }
 
-func IsContainer(id *ast.Ident) bool {
-	switch id.Name {
-	case "It", "When", "Context", "Describe", "DescribeTable", "Entry":
+func IsContainer(name string) bool {
+	switch name {
+	case "It", "When", "Context", "Describe", "DescribeTable", "Entry",
+		"PIt", "PWhen", "PContext", "PDescribe", "PDescribeTable", "PEntry",
+		"XIt", "XWhen", "XContext", "XDescribe", "XDescribeTable", "XEntry":
+		return true
+	}
+	return isFocusContainer(name)
+}
+
+func IsWrapContainer(name string) bool {
+	switch name {
+	case "When", "Context", "Describe",
+		"FWhen", "FContext", "FDescribe",
+		"PWhen", "PContext", "PDescribe",
+		"XWhen", "XContext", "XDescribe":
 		return true
 	}
-	return isFocusContainer(id.Name)
+
+	return false
 }

internal/types/config.go

@@ -27,6 +27,7 @@ type Config struct {
 	AllowHaveLen0          Boolean
 	ForceExpectTo          Boolean
 	ValidateAsyncIntervals Boolean
+	ForbidSpecPollution    Boolean
 }
 
 func (s *Config) AllTrue() bool {
@@ -45,6 +46,7 @@ func (s *Config) Clone() Config {
 		AllowHaveLen0:          s.AllowHaveLen0,
 		ForceExpectTo:          s.ForceExpectTo,
 		ValidateAsyncIntervals: s.ValidateAsyncIntervals,
+		ForbidSpecPollution:    s.ForbidSpecPollution,
 	}
 }
 

linter/ginkgo_linter.go

@@ -47,6 +47,7 @@ const (
 	matchErrorRedundantArg        = "redundant MatchError arguments; consider removing them"
 	matchErrorNoFuncDescription   = "The second parameter of MatchError must be the function description (string)"
 	forceExpectToTemplate         = "must not use Expect with %s"
+	useBeforeEachTemplate         = "use BeforeEach() to assign variable %s"
 )
 
 const ( // gomega matchers
@@ -120,17 +121,25 @@ func (l *GinkgoLinter) Run(pass *analysis.Pass) (interface{}, error) {
 		}
 
 		ast.Inspect(file, func(n ast.Node) bool {
-			if ginkgoHndlr != nil && fileConfig.ForbidFocus {
+			if ginkgoHndlr != nil {
+				goDeeper := false
 				spec, ok := n.(*ast.ValueSpec)
 				if ok {
 					for _, val := range spec.Values {
 						if exp, ok := val.(*ast.CallExpr); ok {
-							if checkFocusContainer(pass, ginkgoHndlr, exp) {
-								return true
+							if bool(fileConfig.ForbidFocus) && checkFocusContainer(pass, ginkgoHndlr, exp) {
+								goDeeper = true
+							}
+
+							if bool(fileConfig.ForbidSpecPollution) && checkAssignmentsInContainer(pass, ginkgoHndlr, exp) {
+								goDeeper = true
 							}
 						}
 					}
 				}
+				if goDeeper {
+					return true
+				}
 			}
 
 			stmt, ok := n.(*ast.ExprStmt)
@@ -150,8 +159,15 @@ func (l *GinkgoLinter) Run(pass *analysis.Pass) (interface{}, error) {
 				return true
 			}
 
-			if ginkgoHndlr != nil && bool(config.ForbidFocus) {
-				if checkFocusContainer(pass, ginkgoHndlr, assertionExp) {
+			if ginkgoHndlr != nil {
+				goDeeper := false
+				if bool(config.ForbidFocus) && checkFocusContainer(pass, ginkgoHndlr, assertionExp) {
+					goDeeper = true
+				}
+				if bool(config.ForbidSpecPollution) && checkAssignmentsInContainer(pass, ginkgoHndlr, assertionExp) {
+					goDeeper = true
+				}
+				if goDeeper {
 					return true
 				}
 			}
@@ -184,6 +200,83 @@ func (l *GinkgoLinter) Run(pass *analysis.Pass) (interface{}, error) {
 	return nil, nil
 }
 
+func checkAssignmentsInContainer(pass *analysis.Pass, ginkgoHndlr ginkgohandler.Handler, exp *ast.CallExpr) bool {
+	foundSomething := false
+	if ginkgoHndlr.IsWrapContainer(exp) {
+		for _, arg := range exp.Args {
+			if fn, ok := arg.(*ast.FuncLit); ok {
+				if fn.Body != nil {
+					if checkAssignments(pass, fn.Body.List) {
+						foundSomething = true
+					}
+					break
+				}
+			}
+		}
+	}
+
+	return foundSomething
+}
+
+func checkAssignments(pass *analysis.Pass, list []ast.Stmt) bool {
+	foundSomething := false
+	for _, stmt := range list {
+		switch st := stmt.(type) {
+		case *ast.DeclStmt:
+			if gen, ok := st.Decl.(*ast.GenDecl); ok {
+				if gen.Tok != token.VAR {
+					continue
+				}
+				for _, spec := range gen.Specs {
+					if valSpec, ok := spec.(*ast.ValueSpec); ok {
+						if checkAssignmentsValues(pass, valSpec.Names, valSpec.Values) {
+							foundSomething = true
+						}
+					}
+				}
+			}
+
+		case *ast.AssignStmt:
+			for i, val := range st.Rhs {
+				if _, isFunc := val.(*ast.FuncLit); !isFunc {
+					if id, isIdent := st.Lhs[i].(*ast.Ident); isIdent {
+						reportNoFix(pass, id.Pos(), useBeforeEachTemplate, id.Name)
+						foundSomething = true
+					}
+				}
+			}
+
+		case *ast.IfStmt:
+			if st.Body != nil {
+				if checkAssignments(pass, st.Body.List) {
+					foundSomething = true
+				}
+			}
+			if st.Else != nil {
+				if block, isBlock := st.Else.(*ast.BlockStmt); isBlock {
+					if checkAssignments(pass, block.List) {
+						foundSomething = true
+					}
+				}
+			}
+		}
+	}
+
+	return foundSomething
+}
+
+func checkAssignmentsValues(pass *analysis.Pass, names []*ast.Ident, values []ast.Expr) bool {
+	foundSomething := false
+	for i, val := range values {
+		if _, isFunc := val.(*ast.FuncLit); !isFunc {
+			reportNoFix(pass, names[i].Pos(), useBeforeEachTemplate, names[i].Name)
+			foundSomething = true
+		}
+	}
+
+	return foundSomething
+}
+
 func checkFocusContainer(pass *analysis.Pass, ginkgoHndlr ginkgohandler.Handler, exp *ast.CallExpr) bool {
 	foundFocus := false
 	isFocus, id := ginkgoHndlr.GetFocusContainerName(exp)
@@ -192,7 +285,7 @@ func checkFocusContainer(pass *analysis.Pass, ginkgoHndlr ginkgohandler.Handler,
 		foundFocus = true
 	}
 
-	if id != nil && ginkgohandler.IsContainer(id) {
+	if id != nil && ginkgohandler.IsContainer(id.Name) {
 		for _, arg := range exp.Args {
 			if ginkgoHndlr.IsFocusSpec(arg) {
 				reportNoFix(pass, arg.Pos(), focusSpecFound)

testdata/src/a/containers-vas-and-focus/containers.go

@@ -0,0 +1,33 @@
+package vars_in_containers
+
+import (
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+var _ = FDescribe("When's", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code. Consider to replace with "Describe"`
+	FWhen("test FWhen", func() { // want `Focus container found. This is used only for local debug and should not be part of the actual source code. Consider to replace with "When"`
+		var x = 1 // want `use BeforeEach\(\) to assign variable x`
+		It("use x", func() {
+			Expect(x).To(Equal(1))
+		})
+	})
+})
+
+var _ = FWhen("Context's", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code. Consider to replace with "When"`
+	FContext("test FContext", func() { // want `Focus container found. This is used only for local debug and should not be part of the actual source code. Consider to replace with "Context"`
+		var x = 1 // want `use BeforeEach\(\) to assign variable x`
+		It("use x", func() {
+			Expect(x).To(Equal(1))
+		})
+	})
+})
+
+var _ = FContext("Describe's", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code. Consider to replace with "Context"`
+	FDescribe("test FDescribe", func() { // want `Focus container found. This is used only for local debug and should not be part of the actual source code. Consider to replace with "Describe"`
+		var x = 1 // want `use BeforeEach\(\) to assign variable x`
+		It("use x", func() {
+			Expect(x).To(Equal(1))
+		})
+	})
+})