diff --git a/simple/s1009/s1009.go b/simple/s1009/s1009.go index 606c7c65..270215c7 100644 --- a/simple/s1009/s1009.go +++ b/simple/s1009/s1009.go @@ -41,6 +41,8 @@ var Analyzer = SCAnalyzer.Analyzer // run checks for the following redundant nil-checks: // // if x == nil || len(x) == 0 {} +// if x == nil || len(x) < N {} (where N != 0) +// if x == nil || len(x) <= N {} // if x != nil && len(x) != 0 {} // if x != nil && len(x) == N {} (where N != 0) // if x != nil && len(x) > N {} @@ -99,9 +101,6 @@ func run(pass *analysis.Pass) (interface{}, error) { if !ok { return } - if eqNil && y.Op != token.EQL { // must be len(xx) *==* 0 - return - } yx, ok := y.X.(*ast.CallExpr) if !ok { return @@ -122,15 +121,31 @@ func run(pass *analysis.Pass) (interface{}, error) { return } - if eqNil && !code.IsIntegerLiteral(pass, y.Y, constant.MakeInt64(0)) { // must be len(x) == *0* + isConst, isZero := isConstZero(y.Y) + if !isConst { return } - if !eqNil { - isConst, isZero := isConstZero(y.Y) - if !isConst { + if eqNil { + switch y.Op { + case token.EQL: + // avoid false positive for "xx == nil || len(xx) == " + if !isZero { + return + } + case token.LEQ: + // ok + case token.LSS: + // avoid false positive for "xx == nil || len(xx) < 0" + if isZero { + return + } + default: return } + } + + if !eqNil { switch y.Op { case token.EQL: // avoid false positive for "xx != nil && len(xx) == 0" diff --git a/simple/s1009/testdata/go1.0/CheckRedundantNilCheckWithLen/nil-len.go b/simple/s1009/testdata/go1.0/CheckRedundantNilCheckWithLen/nil-len.go index b11c088a..5e94a3c7 100644 --- a/simple/s1009/testdata/go1.0/CheckRedundantNilCheckWithLen/nil-len.go +++ b/simple/s1009/testdata/go1.0/CheckRedundantNilCheckWithLen/nil-len.go @@ -91,3 +91,44 @@ func issue1527() { if t.pa == nil || len(t.pa) == 0 { // nil check cannot be removed with pointer to an array } } + +func issue1605() { + var s []int + var m map[int]int + var ch chan int + + if s == nil || len(s) <= 0 { //@ diag(`should omit nil check`) + } + if m == nil || len(m) <= 0 { //@ diag(`should omit nil check`) + } + if ch == nil || len(ch) <= 0 { //@ diag(`should omit nil check`) + } + + if s == nil || len(s) < 2 { //@ diag(`should omit nil check`) + } + if m == nil || len(m) < 2 { //@ diag(`should omit nil check`) + } + if ch == nil || len(ch) < 2 { //@ diag(`should omit nil check`) + } + + if s == nil || len(s) <= 2 { //@ diag(`should omit nil check`) + } + if m == nil || len(m) <= 2 { //@ diag(`should omit nil check`) + } + if ch == nil || len(ch) <= 2 { //@ diag(`should omit nil check`) + } + + if s == nil || len(s) < 0 { // nil check is not redundant here (len(s) < 0 is impossible) + } + if m == nil || len(m) < 0 { // nil check is not redundant here (len(m) < 0 is impossible) + } + if ch == nil || len(ch) < 0 { // nil check is not redundant here (len(ch) < 0 is impossible) + } + + if s == nil || len(s) > 2 { // nil check is not redundant here + } + if m == nil || len(m) > 2 { // nil check is not redundant here + } + if ch == nil || len(ch) > 2 { // nil check is not redundant here + } +}