diff --git a/rule/unconditional-recursion.go b/rule/unconditional-recursion.go index 3ed438a..9656769 100644 --- a/rule/unconditional-recursion.go +++ b/rule/unconditional-recursion.go @@ -53,35 +53,15 @@ type lintUnconditionalRecursionRule struct { 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.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 case *ast.FuncDecl: var rec *ast.Ident switch { @@ -120,6 +100,33 @@ func (w lintUnconditionalRecursionRule) Visit(node ast.Node) ast.Visitor { 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 @@ -133,11 +140,48 @@ func (w *lintUnconditionalRecursionRule) updateFuncStatus(node ast.Node) { 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 { - filter := func(n ast.Node) bool { - _, ok := n.(*ast.ReturnStmt) - return ok + // 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, filter, nil)) != 0 + return len(pick(node, isExit, nil)) != 0 } diff --git a/testdata/unconditional-recursion.go b/testdata/unconditional-recursion.go index 5f17d0f..078b289 100644 --- a/testdata/unconditional-recursion.go +++ b/testdata/unconditional-recursion.go @@ -1,6 +1,10 @@ package fixtures -import "time" +import ( + "log" + "os" + "time" +) func ur1() { ur1() // MATCH /unconditional recursive call/ @@ -139,3 +143,24 @@ func ur12() { 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() +}