Repository: mgechev/revive
Pull request #967 (2024-01-27 15:44:01)

Adds rule `max-control-nesting`
This PR adds rule `max-control-nesting`, the GO implementation of https://sonarsource.atlassian.net/browse/RSPEC-134


denisvmedia: Thanks for the new rule @chavacava. 

What about select/case - should we (do we?) support it as well?

Another thing: can we have test cases for the code placed in closures? 

Patch:
diff --git a/README.md b/README.md
index 4461de9..89e4350 100644
--- a/README.md
+++ b/README.md
@@ -538,6 +538,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | [`enforce-map-style`](./RULES_DESCRIPTIONS.md#enforce-map-style) |  string (defaults to "any")  |  Enforces consistent usage of `make(map[type]type)` or `map[type]type{}` for map initialization. Does not affect `make(map[type]type, size)` constructions. |    no    |  no   |
 | [`enforce-slice-style`](./RULES_DESCRIPTIONS.md#enforce-slice-style) |  string (defaults to "any")  |  Enforces consistent usage of `make([]type, 0)` or `[]type{}` for slice initialization. Does not affect `make(map[type]type, non_zero_len, or_non_zero_cap)` constructions. |    no    |  no   |
 | [`enforce-repeated-arg-type-style`](./RULES_DESCRIPTIONS.md#enforce-repeated-arg-type-style) |  string (defaults to "any")  |  Enforces consistent style for repeated argument and/or return value types. |    no    |  no   |
+| [`max-control-nesting`](./RULES_DESCRIPTIONS.md#max-control-nesting) |  int (defaults to 5)  | Sets restriction for maximum nesting of control structures. |    no    |  no   |
 
 
 ## Configurable rules
diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md
index f4ffc72..e1b63b6 100644
--- a/RULES_DESCRIPTIONS.md
+++ b/RULES_DESCRIPTIONS.md
@@ -48,6 +48,7 @@ List of all available rules.
   - [increment-decrement](#increment-decrement)
   - [indent-error-flow](#indent-error-flow)
   - [line-length-limit](#line-length-limit)
+  - [max-control-nesting](#max-control-nesting)
   - [max-public-structs](#max-public-structs)
   - [modifies-parameter](#modifies-parameter)
   - [modifies-value-receiver](#modifies-value-receiver)
@@ -620,6 +621,19 @@ Example:
   arguments =[80]
 ```
 
+## max-control-nesting
+_Description_: Warns if nesting level of control structures (`if-then-else`, `for`, `switch`) exceeds a given maximum.
+
+_Configuration_: (int) maximum accepted nesting level of control structures (defaults to 5)
+
+Example:
+
+```toml
+[max-control-nesting]
+  arguments =[3]
+```
+
+
 ## max-public-structs
 
 _Description_: Packages declaring too many public structs can be hard to understand/use,
diff --git a/config/config.go b/config/config.go
index f37f6ef..a0e0ff6 100644
--- a/config/config.go
+++ b/config/config.go
@@ -94,6 +94,7 @@ var allRules = append([]lint.Rule{
 	&rule.ImportAliasNamingRule{},
 	&rule.EnforceMapStyleRule{},
 	&rule.EnforceSliceStyleRule{},
+	&rule.MaxControlNestingRule{},
 }, defaultRules...)
 
 var allFormatters = []lint.Formatter{
diff --git a/rule/max-control-nesting.go b/rule/max-control-nesting.go
new file mode 100644
index 0000000..c4eb361
--- /dev/null
+++ b/rule/max-control-nesting.go
@@ -0,0 +1,128 @@
+package rule
+
+import (
+	"fmt"
+	"go/ast"
+	"sync"
+
+	"github.com/mgechev/revive/lint"
+)
+
+// MaxControlNestingRule lints given else constructs.
+type MaxControlNestingRule struct {
+	max int64
+	sync.Mutex
+}
+
+const defaultMaxControlNesting = 5
+
+// Apply applies the rule to given file.
+func (r *MaxControlNestingRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure {
+	r.configure(arguments)
+
+	var failures []lint.Failure
+
+	fileAst := file.AST
+
+	walker := &lintMaxControlNesting{
+		onFailure: func(failure lint.Failure) {
+			failures = append(failures, failure)
+		},
+		max: int(r.max),
+	}
+
+	ast.Walk(walker, fileAst)
+
+	return failures
+}
+
+// Name returns the rule name.
+func (*MaxControlNestingRule) Name() string {
+	return "max-control-nesting"
+}
+
+type lintMaxControlNesting struct {
+	max             int
+	onFailure       func(lint.Failure)
+	nestingLevelAcc int
+	lastCtrlStmt    ast.Node
+}
+
+func (w *lintMaxControlNesting) Visit(n ast.Node) ast.Visitor {
+	if w.nestingLevelAcc > w.max { // we are visiting a node beyond the max nesting level
+		w.onFailure(lint.Failure{
+			Failure:    fmt.Sprintf("control flow nesting exceeds %d", w.max),
+			Confidence: 1,
+			Node:       w.lastCtrlStmt,
+			Category:   "complexity",
+		})
+		return nil // stop visiting deeper
+	}
+
+	switch v := n.(type) {
+	case *ast.IfStmt:
+		w.lastCtrlStmt = v
+		w.walkControlledBlock(v.Body) // "then" branch block
+		if v.Else != nil {
+			w.walkControlledBlock(v.Else) // "else" branch block
+		}
+		return nil // stop re-visiting nesting blocks (already visited by w.walkControlledBlock)
+
+	case *ast.ForStmt:
+		w.lastCtrlStmt = v
+		w.walkControlledBlock(v.Body)
+		return nil // stop re-visiting nesting blocks (already visited by w.walkControlledBlock)
+
+	case *ast.CaseClause: // switch case
+		w.lastCtrlStmt = v
+		for _, s := range v.Body { // visit each statement in the case clause
+			w.walkControlledBlock(s)
+		}
+		return nil // stop re-visiting nesting blocks (already visited by w.walkControlledBlock)
+
+	case *ast.CommClause: // select case
+		w.lastCtrlStmt = v
+		for _, s := range v.Body { // visit each statement in the select case clause
+			w.walkControlledBlock(s)
+		}
+		return nil // stop re-visiting nesting blocks (already visited by w.walkControlledBlock)
+
+	case *ast.FuncLit:
+		walker := &lintMaxControlNesting{
+			onFailure: w.onFailure,
+			max:       w.max,
+		}
+		ast.Walk(walker, v.Body)
+		return nil
+	}
+
+	return w
+}
+
+func (w *lintMaxControlNesting) walkControlledBlock(b ast.Node) {
+	oldNestingLevel := w.nestingLevelAcc
+	w.nestingLevelAcc++
+	ast.Walk(w, b)
+	w.nestingLevelAcc = oldNestingLevel
+}
+
+func (r *MaxControlNestingRule) configure(arguments lint.Arguments) {
+	r.Lock()
+	defer r.Unlock()
+	if !(r.max < 1) {
+		return // max already set
+	}
+
+	if len(arguments) < 1 {
+		r.max = defaultMaxControlNesting
+		return
+	}
+
+	checkNumberOfArguments(1, arguments, r.Name())
+
+	max, ok := arguments[0].(int64) // Alt. non panicking version
+	if !ok {
+		panic(`invalid value passed as argument number to the "max-control-nesting" rule`)
+	}
+	r.max = max
+}


Test patch:
diff --git a/test/max-control-nesting_test.go b/test/max-control-nesting_test.go
new file mode 100644
index 0000000..cbb0beb
--- /dev/null
+++ b/test/max-control-nesting_test.go
@@ -0,0 +1,14 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/mgechev/revive/lint"
+	"github.com/mgechev/revive/rule"
+)
+
+func TestMaxControlNesting(t *testing.T) {
+	testRule(t, "max-control-nesting", &rule.MaxControlNestingRule{}, &lint.RuleConfig{
+		Arguments: []any{int64(2)}},
+	)
+}
diff --git a/testdata/max-control-nesting.go b/testdata/max-control-nesting.go
new file mode 100644
index 0000000..f85878d
--- /dev/null
+++ b/testdata/max-control-nesting.go
@@ -0,0 +1,67 @@
+package fixtures
+
+func mcn() {
+	if true {
+		if true {
+			if true { // MATCH /control flow nesting exceeds 2/
+
+			}
+		}
+	} else {
+		if true {
+			if true { // MATCH /control flow nesting exceeds 2/
+				if true {
+
+				}
+			}
+		}
+	}
+
+	for {
+		if true {
+			for { // MATCH /control flow nesting exceeds 2/
+			}
+		}
+	}
+
+	switch {
+	case false:
+		if true {
+
+		}
+	case true:
+		if true {
+			for { // MATCH /control flow nesting exceeds 2/
+			}
+		}
+	default:
+	}
+
+	select {
+	case msg1 := <-c1:
+		println("received", msg1)
+	case msg2 := <-c2:
+		if true {
+			for { // MATCH /control flow nesting exceeds 2/
+			}
+		}
+	}
+
+	if true {
+		f1 := func() {
+			if true {
+				for {
+				}
+			}
+		}
+	}
+
+	f1 := func() {
+		for {
+			if true {
+				for { // MATCH /control flow nesting exceeds 2/
+				}
+			}
+		}
+	}
+}