Skip to content

Commit 11711c5

Browse files
semihbkgrchavacava
authored andcommitted
fix false positive in useless-break when a label follows a break (mgechev#1286)
Co-authored-by: chavacava <[email protected]>
1 parent c4a7b4a commit 11711c5

File tree

2 files changed

+39
-24
lines changed

2 files changed

+39
-24
lines changed

rule/useless_break.go

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -47,36 +47,39 @@ func (w *lintUselessBreak) Visit(node ast.Node) ast.Visitor {
4747
w.inLoopBody = false
4848
return nil
4949
case *ast.CommClause:
50-
for _, n := range v.Body {
51-
w.inspectCaseStatement(n)
52-
}
50+
w.inspectCaseStatement(v.Body)
5351
return nil
5452
case *ast.CaseClause:
55-
for _, n := range v.Body {
56-
w.inspectCaseStatement(n)
57-
}
53+
w.inspectCaseStatement(v.Body)
5854
return nil
5955
}
6056
return w
6157
}
6258

63-
func (w *lintUselessBreak) inspectCaseStatement(n ast.Stmt) {
64-
switch s := n.(type) {
65-
case *ast.BranchStmt:
66-
if s.Tok != token.BREAK {
67-
return // not a break statement
68-
}
69-
if s.Label != nil {
70-
return // labeled break statement, usually affects a nesting loop
71-
}
72-
msg := "useless break in case clause"
73-
if w.inLoopBody {
74-
msg += " (WARN: this break statement affects this switch or select statement and not the loop enclosing it)"
75-
}
76-
w.onFailure(lint.Failure{
77-
Confidence: 1,
78-
Node: s,
79-
Failure: msg,
80-
})
59+
func (w *lintUselessBreak) inspectCaseStatement(body []ast.Stmt) {
60+
l := len(body)
61+
if l == 0 {
62+
return // empty body, nothing to do
8163
}
64+
65+
s := body[l-1] // pick the last statement
66+
if !isUnlabelledBreak(s) {
67+
return
68+
}
69+
70+
msg := "useless break in case clause"
71+
if w.inLoopBody {
72+
msg += " (WARN: this break statement affects this switch or select statement and not the loop enclosing it)"
73+
}
74+
75+
w.onFailure(lint.Failure{
76+
Confidence: 1,
77+
Node: s,
78+
Failure: msg,
79+
})
80+
}
81+
82+
func isUnlabelledBreak(stmt ast.Stmt) bool {
83+
s, ok := stmt.(*ast.BranchStmt)
84+
return ok && s.Tok == token.BREAK && s.Label == nil
8285
}

testdata/useless_break.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,4 +87,16 @@ func UselessBreaks() {
8787
default:
8888
s.errorf("range can't iterate over %v", val)
8989
}
90+
91+
// issue #1281
92+
switch 1 {
93+
case 1:
94+
fmt.Println("foo")
95+
break // MATCH /useless break in case clause/
96+
case 2:
97+
fmt.Println("bar")
98+
break
99+
fmt.Println("baz")
100+
case 3:
101+
}
90102
}

0 commit comments

Comments
 (0)