Skip to content

Commit d3c15ec

Browse files
spencerschrockdominikh
authored andcommitted
simple/s1009: add checks for redundant "x == nil || len(x) < N"
Closes: gh-1605 Closes: gh-1606 [via git-merge-pr]
1 parent 2739fd0 commit d3c15ec

File tree

2 files changed

+63
-7
lines changed

2 files changed

+63
-7
lines changed

simple/s1009/s1009.go

+22-7
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ var Analyzer = SCAnalyzer.Analyzer
4141
// run checks for the following redundant nil-checks:
4242
//
4343
// if x == nil || len(x) == 0 {}
44+
// if x == nil || len(x) < N {} (where N != 0)
45+
// if x == nil || len(x) <= N {}
4446
// if x != nil && len(x) != 0 {}
4547
// if x != nil && len(x) == N {} (where N != 0)
4648
// if x != nil && len(x) > N {}
@@ -99,9 +101,6 @@ func run(pass *analysis.Pass) (interface{}, error) {
99101
if !ok {
100102
return
101103
}
102-
if eqNil && y.Op != token.EQL { // must be len(xx) *==* 0
103-
return
104-
}
105104
yx, ok := y.X.(*ast.CallExpr)
106105
if !ok {
107106
return
@@ -122,15 +121,31 @@ func run(pass *analysis.Pass) (interface{}, error) {
122121
return
123122
}
124123

125-
if eqNil && !code.IsIntegerLiteral(pass, y.Y, constant.MakeInt64(0)) { // must be len(x) == *0*
124+
isConst, isZero := isConstZero(y.Y)
125+
if !isConst {
126126
return
127127
}
128128

129-
if !eqNil {
130-
isConst, isZero := isConstZero(y.Y)
131-
if !isConst {
129+
if eqNil {
130+
switch y.Op {
131+
case token.EQL:
132+
// avoid false positive for "xx == nil || len(xx) == <non-zero>"
133+
if !isZero {
134+
return
135+
}
136+
case token.LEQ:
137+
// ok
138+
case token.LSS:
139+
// avoid false positive for "xx == nil || len(xx) < 0"
140+
if isZero {
141+
return
142+
}
143+
default:
132144
return
133145
}
146+
}
147+
148+
if !eqNil {
134149
switch y.Op {
135150
case token.EQL:
136151
// avoid false positive for "xx != nil && len(xx) == 0"

simple/s1009/testdata/go1.0/CheckRedundantNilCheckWithLen/nil-len.go

+41
Original file line numberDiff line numberDiff line change
@@ -91,3 +91,44 @@ func issue1527() {
9191
if t.pa == nil || len(t.pa) == 0 { // nil check cannot be removed with pointer to an array
9292
}
9393
}
94+
95+
func issue1605() {
96+
var s []int
97+
var m map[int]int
98+
var ch chan int
99+
100+
if s == nil || len(s) <= 0 { //@ diag(`should omit nil check`)
101+
}
102+
if m == nil || len(m) <= 0 { //@ diag(`should omit nil check`)
103+
}
104+
if ch == nil || len(ch) <= 0 { //@ diag(`should omit nil check`)
105+
}
106+
107+
if s == nil || len(s) < 2 { //@ diag(`should omit nil check`)
108+
}
109+
if m == nil || len(m) < 2 { //@ diag(`should omit nil check`)
110+
}
111+
if ch == nil || len(ch) < 2 { //@ diag(`should omit nil check`)
112+
}
113+
114+
if s == nil || len(s) <= 2 { //@ diag(`should omit nil check`)
115+
}
116+
if m == nil || len(m) <= 2 { //@ diag(`should omit nil check`)
117+
}
118+
if ch == nil || len(ch) <= 2 { //@ diag(`should omit nil check`)
119+
}
120+
121+
if s == nil || len(s) < 0 { // nil check is not redundant here (len(s) < 0 is impossible)
122+
}
123+
if m == nil || len(m) < 0 { // nil check is not redundant here (len(m) < 0 is impossible)
124+
}
125+
if ch == nil || len(ch) < 0 { // nil check is not redundant here (len(ch) < 0 is impossible)
126+
}
127+
128+
if s == nil || len(s) > 2 { // nil check is not redundant here
129+
}
130+
if m == nil || len(m) > 2 { // nil check is not redundant here
131+
}
132+
if ch == nil || len(ch) > 2 { // nil check is not redundant here
133+
}
134+
}

0 commit comments

Comments
 (0)