diff --git a/simple/lint.go b/simple/lint.go index 8458f7be..3880b5d3 100644 --- a/simple/lint.go +++ b/simple/lint.go @@ -546,7 +546,7 @@ func CheckIfReturn(pass *analysis.Pass) (interface{}, error) { cond := m1.State["cond"].(ast.Expr) origCond := cond if ret1.Name == "false" { - cond = negate(cond) + cond = negate(pass, cond) } report.Report(pass, n1, fmt.Sprintf("should use 'return %s' instead of 'if %s { return %s }; return %s'", @@ -558,7 +558,7 @@ func CheckIfReturn(pass *analysis.Pass) (interface{}, error) { return nil, nil } -func negate(expr ast.Expr) ast.Expr { +func negate(pass *analysis.Pass, expr ast.Expr) ast.Expr { switch expr := expr.(type) { case *ast.BinaryExpr: out := *expr @@ -568,7 +568,14 @@ func negate(expr ast.Expr) ast.Expr { case token.LSS: out.Op = token.GEQ case token.GTR: - out.Op = token.LEQ + // Some builtins never return negative ints; "len(x) <= 0" should be "len(x) == 0". + if call, ok := expr.X.(*ast.CallExpr); ok && + code.IsCallToAny(pass, call, "len", "cap", "copy") && + code.IsIntegerLiteral(pass, expr.Y, constant.MakeInt64(0)) { + out.Op = token.EQL + } else { + out.Op = token.LEQ + } case token.NEQ: out.Op = token.EQL case token.LEQ: diff --git a/simple/testdata/src/example.com/CheckIfReturn/if-return.go b/simple/testdata/src/example.com/CheckIfReturn/if-return.go index a2d6944b..2823c0b6 100644 --- a/simple/testdata/src/example.com/CheckIfReturn/if-return.go +++ b/simple/testdata/src/example.com/CheckIfReturn/if-return.go @@ -153,3 +153,10 @@ func fn21(x bool) bool { } return false } + +func fn22(x string) bool { + if len(x) > 0 { //@ diag(`should use 'return len(x) == 0'`) + return false + } + return true +}