Merge pull request #413 from chavacava/unconditional-recursion

Unconditional recursion

README.md

@@ -351,6 +351,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | [`cognitive-complexity`](./RULES_DESCRIPTIONS.md#cognitive-complexity)          |  int   | Sets restriction for maximum Cognitive complexity.              |    no    |  no   |
 | [`string-of-int`](./RULES_DESCRIPTIONS.md#string-of-int)          |  n/a   | Warns on suspicious casts from int to string            |    no    |  yes   |
 | [`early-return`](./RULES_DESCRIPTIONS.md#early-return)          |  n/a   | Spots if-then-else statements that can be refactored to simplify code reading            |    no    |  no   |
+| [`unconditional-recursion`](./RULES_DESCRIPTIONS.md#unconditional-recursion)          |  n/a   | Warns on function calls that will lead to (direct) infinite recursion |    no    |  no   |
 | [`identical-branches`](./RULES_DESCRIPTIONS.md#identical-branches)          |  n/a   | Spots if-then-else statements with identical `then` and `else` branches       |    no    |  no   |
 
 ## Configurable rules

RULES_DESCRIPTIONS.md

@@ -55,6 +55,7 @@ List of all available rules.
   - [time-naming](#time-naming)
   - [var-naming](#var-naming)
   - [var-declaration](#var-declaration)
+  - [unconditional-recursion](#unconditional-recursion)
   - [unexported-return](#unexported-return)
   - [unhandled-error](#unhandled-error)
   - [unnecessary-stmt](#unnecessary-stmt)
@@ -489,6 +490,12 @@ _Description_: This rule proposes simplifications of variable declarations.
 
 _Configuration_: N/A
 
+## unconditional-recursion
+
+_Description_: Unconditional recursive calls will produce infinite recursion, thus program stack overflow. This rule detects and warns about unconditional (direct) recursive calls.
+
+_Configuration_: N/A
+
 ## unexported-return
 
 _Description_: This rule warns when an exported function or method returns a value of an un-exported type.

config.go

@@ -85,6 +85,7 @@ var allRules = append([]lint.Rule{
 	&rule.CognitiveComplexityRule{},
 	&rule.StringOfIntRule{},
 	&rule.EarlyReturnRule{},
+	&rule.UnconditionalRecursionRule{},
 	&rule.IdenticalBranchesRule{},
 }, defaultRules...)
 

rule/unconditional-recursion.go

@@ -0,0 +1,183 @@
+package rule
+
+import (
+	"go/ast"
+
+	"github.com/mgechev/revive/lint"
+)
+
+// UnconditionalRecursionRule lints given else constructs.
+type UnconditionalRecursionRule struct{}
+
+// Apply applies the rule to given file.
+func (r *UnconditionalRecursionRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
+	var failures []lint.Failure
+
+	onFailure := func(failure lint.Failure) {
+		failures = append(failures, failure)
+	}
+
+	w := lintUnconditionalRecursionRule{onFailure: onFailure}
+	ast.Walk(w, file.AST)
+	return failures
+}
+
+// Name returns the rule name.
+func (r *UnconditionalRecursionRule) Name() string {
+	return "unconditional-recursion"
+}
+
+type funcDesc struct {
+	reciverId *ast.Ident
+	id        *ast.Ident
+}
+
+func (fd *funcDesc) equal(other *funcDesc) bool {
+	receiversAreEqual := (fd.reciverId == nil && other.reciverId == nil) || fd.reciverId != nil && other.reciverId != nil && fd.reciverId.Name == other.reciverId.Name
+	idsAreEqual := (fd.id == nil && other.id == nil) || fd.id.Name == other.id.Name
+
+	return receiversAreEqual && idsAreEqual
+}
+
+type funcStatus struct {
+	funcDesc            *funcDesc
+	seenConditionalExit bool
+}
+
+type lintUnconditionalRecursionRule struct {
+	onFailure   func(lint.Failure)
+	currentFunc *funcStatus
+}
+
+// Visit will traverse the file AST.
+// The rule is based in the following algorithm: inside each function body we search for calls to the function itself.
+// We do not search inside conditional control structures (if, for, switch, ...) because any recursive call inside them is conditioned
+// We do search inside conditional control structures are statements that will take the control out of the function (return, exit, panic)
+// If we find conditional control exits, it means the function is NOT unconditionally-recursive
+// If we find a recursive call before finding any conditional exit, a failure is generated
+// In resume: if we found a recursive call control-dependant from the entry point of the function then we raise a failure.
+func (w lintUnconditionalRecursionRule) Visit(node ast.Node) ast.Visitor {
+	switch n := node.(type) {
+	case *ast.FuncDecl:
+		var rec *ast.Ident
+		switch {
+		case n.Recv == nil || n.Recv.NumFields() < 1 || len(n.Recv.List[0].Names) < 1:
+			rec = nil
+		default:
+			rec = n.Recv.List[0].Names[0]
+		}
+
+		w.currentFunc = &funcStatus{&funcDesc{rec, n.Name}, false}
+	case *ast.CallExpr:
+		var funcId *ast.Ident
+		var selector *ast.Ident
+		switch c := n.Fun.(type) {
+		case *ast.Ident:
+			selector = nil
+			funcId = c
+		case *ast.SelectorExpr:
+			var ok bool
+			selector, ok = c.X.(*ast.Ident)
+			if !ok { // a.b....Foo()
+				return nil
+			}
+			funcId = c.Sel
+		default:
+			return w
+		}
+
+		if w.currentFunc != nil && // not in a func body
+			!w.currentFunc.seenConditionalExit && // there is a conditional exit in the function
+			w.currentFunc.funcDesc.equal(&funcDesc{selector, funcId}) {
+			w.onFailure(lint.Failure{
+				Category:   "logic",
+				Confidence: 1,
+				Node:       n,
+				Failure:    "unconditional recursive call",
+			})
+		}
+	case *ast.IfStmt:
+		w.updateFuncStatus(n.Body)
+		w.updateFuncStatus(n.Else)
+		return nil
+	case *ast.SelectStmt:
+		w.updateFuncStatus(n.Body)
+		return nil
+	case *ast.RangeStmt:
+		w.updateFuncStatus(n.Body)
+		return nil
+	case *ast.TypeSwitchStmt:
+		w.updateFuncStatus(n.Body)
+		return nil
+	case *ast.SwitchStmt:
+		w.updateFuncStatus(n.Body)
+		return nil
+	case *ast.GoStmt:
+		for _, a := range n.Call.Args {
+			ast.Walk(w, a) // check if arguments have a recursive call
+		}
+		return nil // recursive async call is not an issue
+	case *ast.ForStmt:
+		if n.Cond != nil {
+			return nil
+		}
+		// unconditional loop
+		return w
+	}
+
+	return w
+}
+
+func (w *lintUnconditionalRecursionRule) updateFuncStatus(node ast.Node) {
+	if node == nil || w.currentFunc == nil || w.currentFunc.seenConditionalExit {
+		return
+	}
+
+	w.currentFunc.seenConditionalExit = w.hasControlExit(node)
+}
+
+var exitFunctions = map[string]map[string]bool{
+	"os":      map[string]bool{"Exit": true},
+	"syscall": map[string]bool{"Exit": true},
+	"log": map[string]bool{
+		"Fatal":   true,
+		"Fatalf":  true,
+		"Fatalln": true,
+		"Panic":   true,
+		"Panicf":  true,
+		"Panicln": true,
+	},
+}
+
+func (w *lintUnconditionalRecursionRule) hasControlExit(node ast.Node) bool {
+	// isExit returns true if the given node makes control exit the function
+	isExit := func(node ast.Node) bool {
+		switch n := node.(type) {
+		case *ast.ReturnStmt:
+			return true
+		case *ast.CallExpr:
+			if isIdent(n.Fun, "panic") {
+				return true
+			}
+			se, ok := n.Fun.(*ast.SelectorExpr)
+			if !ok {
+				return false
+			}
+
+			id, ok := se.X.(*ast.Ident)
+			if !ok {
+				return false
+			}
+
+			fn := se.Sel.Name
+			pkg := id.Name
+			if exitFunctions[pkg] != nil && exitFunctions[pkg][fn] { // it's a call to an exit function
+				return true
+			}
+		}
+
+		return false
+	}
+
+	return len(pick(node, isExit, nil)) != 0
+}

test/unconditional-recursion_test.go

@@ -0,0 +1,11 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/mgechev/revive/rule"
+)
+
+func TestUnconditionalRecursion(t *testing.T) {
+	testRule(t, "unconditional-recursion", &rule.UnconditionalRecursionRule{})
+}

testdata/unconditional-recursion.go

@@ -0,0 +1,180 @@
+package fixtures
+
+import (
+	"log"
+	"os"
+	"time"
+)
+
+func ur1() {
+	ur1() // MATCH /unconditional recursive call/
+}
+
+func ur1bis() {
+	if true {
+		print()
+	} else {
+		switch {
+		case true:
+			println()
+		default:
+			for i := 0; i < 10; i++ {
+				print()
+			}
+		}
+
+	}
+
+	ur1bis() // MATCH /unconditional recursive call/
+}
+
+func ur2tris() {
+	for {
+		println()
+		ur2tris() // MATCH /unconditional recursive call/
+	}
+}
+
+func ur2() {
+	if true {
+		return
+	}
+
+	ur2()
+}
+
+func ur3() {
+	ur1()
+}
+
+func urn4() {
+	if true {
+		print()
+	} else if false {
+		return
+	}
+
+	ur4()
+}
+
+func urn5() {
+	if true {
+		return
+	}
+
+	if true {
+		println()
+	}
+
+	ur5()
+}
+
+func ur2tris() {
+	for true == false {
+		println()
+		ur2tris()
+	}
+}
+
+type myType struct {
+	foo int
+	bar int
+}
+
+func (mt *myType) Foo() int {
+	return mt.Foo() // MATCH /unconditional recursive call/
+}
+
+func (mt *myType) Bar() int {
+	return mt.bar
+}
+
+func ur6() {
+	switch {
+	case true:
+		return
+	default:
+		println()
+	}
+
+	ur6()
+}
+
+func ur7(a interface{}) {
+	switch a.(type) {
+	case int:
+		return
+	default:
+		println()
+	}
+
+	ur7()
+}
+
+func ur8(a []int) {
+	for _, i := range a {
+		return
+	}
+
+	ur8()
+}
+
+func ur9(a []int) {
+	for _, i := range a {
+		ur9()
+	}
+}
+
+func ur10() {
+	select {
+	case <-aChannel:
+	case <-time.After(2 * time.Second):
+		return
+	}
+	ur10()
+}
+
+func ur11() {
+	go ur11()
+}
+
+func ur12() {
+	go foo(ur12())                   // MATCH /unconditional recursive call/
+	go bar(1, "string", ur12(), 1.0) // MATCH /unconditional recursive call/
+	go foo(bar())
+}
+
+func urn13() {
+	if true {
+		panic("")
+	}
+	urn13()
+}
+
+func urn14() {
+	if true {
+		os.Exit(1)
+	}
+	urn14()
+}
+
+func urn15() {
+	if true {
+		log.Panic("")
+	}
+	urn15()
+}
+
+func urn16(ch chan int) {
+	for range ch {
+		log.Panic("")
+	}
+	urn16(ch)
+}
+
+func urn17(ch chan int) {
+	for range ch {
+		print("")
+	}
+	urn17(ch) // MATCH /unconditional recursive call/
+}