Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: unconditional-recursion false positive when the function is called right after its declaration (#1212) #1214

Merged
merged 1 commit into from
Jan 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 30 additions & 15 deletions rule/unconditional_recursion.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,35 @@ func (*UnconditionalRecursionRule) Apply(file *lint.File, _ lint.Arguments) []li
failures = append(failures, failure)
}

w := &lintUnconditionalRecursionRule{onFailure: onFailure}
ast.Walk(w, file.AST)
// Range over global declarations of the file to detect func/method declarations and analyze them
for _, decl := range file.AST.Decls {
n, ok := decl.(*ast.FuncDecl)
if !ok {
continue // not a func/method declaration
}

if n.Body == nil {
continue // func/method with empty body => it can not be recursive
}

var rec *ast.Ident
switch {
case n.Recv == nil:
rec = nil
case n.Recv.NumFields() < 1 || len(n.Recv.List[0].Names) < 1:
rec = &ast.Ident{Name: "_"}
default:
rec = n.Recv.List[0].Names[0]
}

w := &lintUnconditionalRecursionRule{
onFailure: onFailure,
currentFunc: &funcStatus{&funcDesc{rec, n.Name}, false},
}

ast.Walk(w, n.Body)
}

return failures
}

Expand Down Expand Up @@ -50,26 +77,14 @@ type lintUnconditionalRecursionRule struct {
inGoStatement bool
}

// 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.
// Visit will traverse function's 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-dependent 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:
rec = nil
case n.Recv.NumFields() < 1 || len(n.Recv.List[0].Names) < 1:
rec = &ast.Ident{Name: "_"}
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 {
Expand Down
7 changes: 7 additions & 0 deletions testdata/unconditional_recursion.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,3 +199,10 @@ func nr902() {
nr902() // MATCH /unconditional recursive call/
}()
}

// Test for issue #1212
func NewFactory() int {
return 0
}

var defaultFactory = NewFactory()