Repository: mgechev/revive
Pull request #909 (2023-09-26 01:34:50)

fix: literal calls case
related with issue #902

fix is simple, we should add `case *ast.FuncLit` in rule: rule/unconditional-recursion.go , and return nil, then test pass. 

but at the end I dont know if I should add some additional code as in previous cases for ex: `w.updateFuncStatus(n.Body)` but if test pass it should be ok :P 

chavacava: Hi @mfederowicz, thanks for the PR.
I added a new test case:
```go
func nr902() {
	go func() {
		nr902() // MATCH /unconditional recursive call/
	})()
}
```

In this case, the function literal contains an actual unconditional recursive call and the rule should spot it.
To cope with this case, and still avoid false positives as the initial case of this issue, the rule should skip analyzing func literals used as *function call arguments*. 
mfederowicz: ok @chavacava with that last one test was a problem with checking line number in: `if p.Position.Start.Line != in.Line {` statement and signature used to identifate current function:

`w.currentFunc = &funcStatus{&funcDesc{rec, n.Name}, false}`

after some tweaks, all tests passes

of course you can propose other solution, maye my changes were not optimal :P

FYI: I made cleanup in commits (rebase on branch), so please git pull on your copy

Patch:
diff --git a/rule/unconditional-recursion.go b/rule/unconditional-recursion.go
index bad9075..7e73b72 100644
--- a/rule/unconditional-recursion.go
+++ b/rule/unconditional-recursion.go
@@ -45,8 +45,9 @@ type funcStatus struct {
 }
 
 type lintUnconditionalRecursionRule struct {
-	onFailure   func(lint.Failure)
-	currentFunc *funcStatus
+	onFailure     func(lint.Failure)
+	currentFunc   *funcStatus
+	inGoStatement bool
 }
 
 // Visit will traverse the file AST.
@@ -68,9 +69,13 @@ func (w lintUnconditionalRecursionRule) Visit(node ast.Node) ast.Visitor {
 		default:
 			rec = n.Recv.List[0].Names[0]
 		}
-
 		w.currentFunc = &funcStatus{&funcDesc{rec, n.Name}, false}
 	case *ast.CallExpr:
+		// check if call arguments has a recursive call
+		for _, arg := range n.Args {
+			ast.Walk(w, arg)
+		}
+
 		var funcID *ast.Ident
 		var selector *ast.Ident
 		switch c := n.Fun.(type) {
@@ -84,6 +89,9 @@ func (w lintUnconditionalRecursionRule) Visit(node ast.Node) ast.Visitor {
 				return nil
 			}
 			funcID = c.Sel
+		case *ast.FuncLit:
+			ast.Walk(w, c.Body) // analyze the body of the function literal
+			return nil
 		default:
 			return w
 		}
@@ -93,11 +101,12 @@ func (w lintUnconditionalRecursionRule) Visit(node ast.Node) ast.Visitor {
 			w.currentFunc.funcDesc.equal(&funcDesc{selector, funcID}) {
 			w.onFailure(lint.Failure{
 				Category:   "logic",
-				Confidence: 1,
+				Confidence: 0.8,
 				Node:       n,
 				Failure:    "unconditional recursive call",
 			})
 		}
+		return nil
 	case *ast.IfStmt:
 		w.updateFuncStatus(n.Body)
 		w.updateFuncStatus(n.Else)
@@ -115,16 +124,21 @@ func (w lintUnconditionalRecursionRule) Visit(node ast.Node) ast.Visitor {
 		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
+		w.inGoStatement = true
+		ast.Walk(w, n.Call)
+		w.inGoStatement = false
+		return nil
 	case *ast.ForStmt:
 		if n.Cond != nil {
 			return nil
 		}
 		// unconditional loop
 		return w
+	case *ast.FuncLit:
+		if w.inGoStatement {
+			return w
+		}
+		return nil // literal call (closure) is not necessarily an issue
 	}
 
 	return w


Test patch:
diff --git a/testdata/unconditional-recursion.go b/testdata/unconditional-recursion.go
index b626769..a35e9cc 100644
--- a/testdata/unconditional-recursion.go
+++ b/testdata/unconditional-recursion.go
@@ -134,8 +134,8 @@ func ur10() {
 	ur10()
 }
 
-func ur11() {
-	go ur11()
+func ur11() { // this pattern produces "infinite" number of goroutines
+	go ur11() // MATCH /unconditional recursive call/
 }
 
 func ur12() {
@@ -187,3 +187,15 @@ func (*fooType) BarFunc() {
 func (_ *fooType) BazFunc() {
 	BazFunc()
 }
+
+// Tests for #902
+func falsePositiveFuncLiteral() {
+	_ = foo(func() {
+		falsePositiveFuncLiteral()
+	})
+}
+func nr902() {
+	go func() {
+		nr902() // MATCH /unconditional recursive call/
+	}()
+}