From bc759185c5ee9e1a2b7ab0a2e32e376babb08927 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Sun, 9 Jul 2023 11:18:06 +0200 Subject: [PATCH] simple: reverse "len(x) > 0" as "len(x) == 0" Rather than "len(x) <= 0", which is technically correct, but not actually what most Go programmers would write, since funcs like len or cap can never return negative integers. Do the same for cap and copy, which never return negative ints either. Fixes #1422. --- simple/lint.go | 13 ++++++++++--- .../src/example.com/CheckIfReturn/if-return.go | 7 +++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/simple/lint.go b/simple/lint.go index 8458f7be6..3880b5d36 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 a2d6944b6..2823c0b6f 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 +}