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 +} 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/ + } + } + } + } +}