Skip to content

Commit

Permalink
checkers: ignore labeled continue in select statement (#1158)
Browse files Browse the repository at this point in the history
Fixes #1130
  • Loading branch information
tony2001 committed Jan 4, 2022
1 parent 411c227 commit 0326f2b
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 15 deletions.
32 changes: 23 additions & 9 deletions checkers/internal/lintutil/astfind.go
Expand Up @@ -7,21 +7,35 @@ import (
)

// FindNode applies pred for root and all it's childs until it returns true.
// If followFunc is defined, it's called before following any node to check whether it needs to be followed.
// followFunc has to return true in order to continuing traversing the node and return false otherwise.
// Matched node is returned.
// If none of the nodes matched predicate, nil is returned.
func FindNode(root ast.Node, pred func(ast.Node) bool) ast.Node {
var found ast.Node
astutil.Apply(root, nil, func(cur *astutil.Cursor) bool {
if pred(cur.Node()) {
found = cur.Node()
return false
func FindNode(root ast.Node, followFunc, pred func(ast.Node) bool) ast.Node {
var (
found ast.Node
preFunc func(*astutil.Cursor) bool
)

if followFunc != nil {
preFunc = func(cur *astutil.Cursor) bool {
return followFunc(cur.Node())
}
return true
})
}

astutil.Apply(root,
preFunc,
func(cur *astutil.Cursor) bool {
if pred(cur.Node()) {
found = cur.Node()
return false
}
return true
})
return found
}

// ContainsNode reports whether `FindNode(root, pred)!=nil`.
func ContainsNode(root ast.Node, pred func(ast.Node) bool) bool {
return FindNode(root, pred) != nil
return FindNode(root, nil, pred) != nil
}
13 changes: 13 additions & 0 deletions checkers/testdata/unlabelStmt/negative_tests.go
Expand Up @@ -147,3 +147,16 @@ outer2:
_ = x
}
}

func twoLoopsWithSelect(c chan int) {
outer2:
for {
println("foo")
for {
select {
case <-c:
continue outer2
}
}
}
}
2 changes: 1 addition & 1 deletion checkers/typeSwitchVar_checker.go
Expand Up @@ -74,7 +74,7 @@ func (c *typeSwitchVarChecker) checkTypeSwitch(root *ast.TypeSwitchStmt) {
// Create artificial node just for matching.
assert1 := ast.TypeAssertExpr{X: expr, Type: clause.List[0]}
for _, stmt := range clause.Body {
assert2 := lintutil.FindNode(stmt, func(x ast.Node) bool {
assert2 := lintutil.FindNode(stmt, nil, func(x ast.Node) bool {
return astequal.Node(&assert1, x)
})
if object == c.ctx.TypesInfo.ObjectOf(identOf(assert2)) {
Expand Down
21 changes: 16 additions & 5 deletions checkers/unlabelStmt_checker.go
Expand Up @@ -87,6 +87,7 @@ func (c *unlabelStmtChecker) VisitStmt(stmt ast.Stmt) {
// Only for loops: if last stmt in list is a loop
// that contains labeled "continue" to the outer loop label,
// it can be refactored to use "break" instead.
// Exceptions: select statements with a labeled "continue" are ignored.
if c.isLoop(labeled.Stmt) {
body := c.blockStmtOf(labeled.Stmt)
if len(body.List) == 0 {
Expand All @@ -96,11 +97,21 @@ func (c *unlabelStmtChecker) VisitStmt(stmt ast.Stmt) {
if !c.isLoop(last) {
return
}
br := lintutil.FindNode(c.blockStmtOf(last), func(n ast.Node) bool {
br, ok := n.(*ast.BranchStmt)
return ok && br.Label != nil &&
br.Label.Name == name && br.Tok == token.CONTINUE
})
br := lintutil.FindNode(c.blockStmtOf(last),
func(n ast.Node) bool {
switch n.(type) {
case *ast.SelectStmt:
return false
default:
return true
}
},
func(n ast.Node) bool {
br, ok := n.(*ast.BranchStmt)
return ok && br.Label != nil &&
br.Label.Name == name && br.Tok == token.CONTINUE
})

if br != nil {
c.warnLabeledContinue(br, name)
}
Expand Down

0 comments on commit 0326f2b

Please sign in to comment.