Skip to content

Commit

Permalink
fix: Infinite recursion in assignment finder (fixes #57)
Browse files Browse the repository at this point in the history
  • Loading branch information
polyfloyd committed Sep 8, 2023
1 parent 2a65e64 commit 700fba3
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 6 deletions.
19 changes: 13 additions & 6 deletions errorlint/allowed.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package errorlint
import (
"fmt"
"go/ast"
"go/types"
"strings"
)

Expand Down Expand Up @@ -110,7 +111,7 @@ func isAllowedErrorComparison(pass *TypesInfoExt, binExpr *ast.BinaryExpr) bool
case *ast.Ident:
// Identifier, most likely to be the `err` variable or whatever
// produces it.
callExprs = assigningCallExprs(pass, t)
callExprs = assigningCallExprs(pass, t, map[types.Object]bool{})
case *ast.CallExpr:
callExprs = append(callExprs, t)
}
Expand Down Expand Up @@ -149,15 +150,21 @@ func isAllowedErrorComparison(pass *TypesInfoExt, binExpr *ast.BinaryExpr) bool

// assigningCallExprs finds all *ast.CallExpr nodes that are part of an
// *ast.AssignStmt that assign to the subject identifier.
func assigningCallExprs(pass *TypesInfoExt, subject *ast.Ident) []*ast.CallExpr {
func assigningCallExprs(pass *TypesInfoExt, subject *ast.Ident, visitedObjects map[types.Object]bool) []*ast.CallExpr {
if subject.Obj == nil {
return nil
}

// Find other identifiers that reference this same object. Make sure to
// exclude the subject identifier as it will cause an infinite recursion
// and is being used in a read operation anyway.
// Find other identifiers that reference this same object.
sobj := pass.TypesInfo.ObjectOf(subject)

if visitedObjects[sobj] {
return nil
}
visitedObjects[sobj] = true

// Make sure to exclude the subject identifier as it will cause an infinite recursion and is
// being used in a read operation anyway.
identifiers := []*ast.Ident{}
for _, ident := range pass.IdentifiersForObject[sobj] {
if subject.Pos() != ident.Pos() {
Expand Down Expand Up @@ -196,7 +203,7 @@ func assigningCallExprs(pass *TypesInfoExt, subject *ast.Ident) []*ast.CallExpr
continue
}
// The subject was the result of assigning from another identifier.
callExprs = append(callExprs, assigningCallExprs(pass, assignT)...)
callExprs = append(callExprs, assigningCallExprs(pass, assignT, visitedObjects)...)
default:
// TODO: inconclusive?
}
Expand Down
20 changes: 20 additions & 0 deletions errorlint/testdata/src/issues/github-57.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package issues

import (
"errors"
"fmt"
)

var err1 = errors.New("1")

func Issue57() {
err := err1
var authErr error

authErr = err
err = authErr

if err == err1 { // want `comparing with == will fail on wrapped errors. Use errors.Is to check for a specific error`
fmt.Println("err1")
}
}

0 comments on commit 700fba3

Please sign in to comment.