From c9e2547e7a2a3ac50323becd622562101d1b4110 Mon Sep 17 00:00:00 2001 From: ckaznocha Date: Sun, 10 Nov 2024 18:46:29 -0800 Subject: [PATCH] FInd and suggest fixes for geq/leq with int literals --- .golangci.yml | 1 - intrange.go | 97 +++++++++++++++++++++++---- testdata/main.go | 145 ++++++++++++++++++++++++++++++++++++++++ testdata/main.go.golden | 145 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 375 insertions(+), 13 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 2ad830d..ae1d2db 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -68,7 +68,6 @@ linters: - gofmt - gofumpt - goimports - - gomnd - goprintffuncname - gosec - gosimple diff --git a/intrange.go b/intrange.go index 92b80a9..229c847 100644 --- a/intrange.go +++ b/intrange.go @@ -99,10 +99,13 @@ func checkForStmt(pass *analysis.Pass, forStmt *ast.ForStmt) { return } - var operand ast.Expr + var ( + operand ast.Expr + hasEquivalentOperator bool + ) switch cond.Op { - case token.LSS: // ;i < n; + case token.LSS, token.LEQ: // ;i < n; || ;i <= n; x, ok := cond.X.(*ast.Ident) if !ok { return @@ -112,8 +115,9 @@ func checkForStmt(pass *analysis.Pass, forStmt *ast.ForStmt) { return } + hasEquivalentOperator = cond.Op == token.LEQ operand = cond.Y - case token.GTR: // ;n > i; + case token.GTR, token.GEQ: // ;n > i; || ;n >= i; y, ok := cond.Y.(*ast.Ident) if !ok { return @@ -123,6 +127,7 @@ func checkForStmt(pass *analysis.Pass, forStmt *ast.ForStmt) { return } + hasEquivalentOperator = cond.Op == token.GEQ operand = cond.X default: return @@ -240,7 +245,18 @@ func checkForStmt(pass *analysis.Pass, forStmt *ast.ForStmt) { return } - rangeX := operandToString(pass, initIdent, operand) + operandIsNumberLit := isNumberLit(operand) + + if hasEquivalentOperator && !operandIsNumberLit { + return + } + + rangeX := operandToString( + pass, + initIdent, + operand, + hasEquivalentOperator && operandIsNumberLit, + ) var replacement string if bc.accessed { @@ -387,7 +403,10 @@ func findNExpr(expr ast.Expr) ast.Expr { } } -func recursiveOperandToString(expr ast.Expr) string { +func recursiveOperandToString( + expr ast.Expr, + incrementInt bool, +) string { switch e := expr.(type) { case *ast.CallExpr: args := "" @@ -397,20 +416,29 @@ func recursiveOperandToString(expr ast.Expr) string { args += ", " } - args += recursiveOperandToString(v) + args += recursiveOperandToString(v, incrementInt && len(e.Args) == 1) } - return recursiveOperandToString(e.Fun) + "(" + args + ")" + return recursiveOperandToString(e.Fun, false) + "(" + args + ")" case *ast.BasicLit: + if incrementInt && e.Kind == token.INT { + v, err := strconv.Atoi(e.Value) + if err == nil { + return strconv.Itoa(v + 1) + } + + return e.Value + } + return e.Value case *ast.Ident: return e.Name case *ast.SelectorExpr: - return recursiveOperandToString(e.X) + "." + recursiveOperandToString(e.Sel) + return recursiveOperandToString(e.X, false) + "." + recursiveOperandToString(e.Sel, false) case *ast.IndexExpr: - return recursiveOperandToString(e.X) + "[" + recursiveOperandToString(e.Index) + "]" + return recursiveOperandToString(e.X, false) + "[" + recursiveOperandToString(e.Index, false) + "]" case *ast.BinaryExpr: - return recursiveOperandToString(e.X) + " " + e.Op.String() + " " + recursiveOperandToString(e.Y) + return recursiveOperandToString(e.X, false) + " " + e.Op.String() + " " + recursiveOperandToString(e.Y, false) default: return "" } @@ -487,6 +515,46 @@ func (b *bodyChecker) check(n ast.Node) bool { return true } +func isNumberLit(exp ast.Expr) bool { + switch lit := exp.(type) { + case *ast.BasicLit: + if lit.Kind == token.INT { + return true + } + + return false + case *ast.CallExpr: + switch fun := lit.Fun.(type) { + case *ast.Ident: + switch fun.Name { + case + "int", + "int8", + "int16", + "int32", + "int64", + "uint", + "uint8", + "uint16", + "uint32", + "uint64": + default: + return false + } + default: + return false + } + + if len(lit.Args) != 1 { + return false + } + + return isNumberLit(lit.Args[0]) + default: + return false + } +} + func compareNumberLit(exp ast.Expr, val int) bool { switch lit := exp.(type) { case *ast.BasicLit: @@ -534,8 +602,13 @@ func compareNumberLit(exp ast.Expr, val int) bool { } } -func operandToString(pass *analysis.Pass, i *ast.Ident, operand ast.Expr) string { - s := recursiveOperandToString(operand) +func operandToString( + pass *analysis.Pass, + i *ast.Ident, + operand ast.Expr, + increment bool, +) string { + s := recursiveOperandToString(operand, increment) t := pass.TypesInfo.TypeOf(i) if t == types.Typ[types.Int] { diff --git a/testdata/main.go b/testdata/main.go index 43c28ca..3690bcf 100644 --- a/testdata/main.go +++ b/testdata/main.go @@ -37,6 +37,13 @@ func main() { for i := 0; i < 10; i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := 0; i <= 9; i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for i := 0; i <= 9; i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + for i := 0; i < int(10); i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } @@ -44,6 +51,13 @@ func main() { for i := 0; i < int(10); i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := 0; i <= int(9); i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for i := 0; i <= int(9); i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + for i := int32(0); i < calculate(10); i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } @@ -51,6 +65,13 @@ func main() { for i := int32(0); i < calculate(10); i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := int32(0); i <= calculate(9); i++ { + fmt.Println(i) + } + + for i := int32(0); i <= calculate(9); i++ { + } + for i := uint32(0); i < 10; i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } @@ -58,6 +79,13 @@ func main() { for i := uint32(0); i < 10; i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := uint32(0); i <= 9; i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for i := uint32(0); i <= 9; i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + for i := 0x0; i < 10; i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } @@ -65,6 +93,13 @@ func main() { for i := 0x0; i < 10; i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := 0x0; i <= 9; i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for i := 0x0; i <= 9; i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + for i := 0; i < 10; i += 1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } @@ -72,6 +107,13 @@ func main() { for i := 0; i < 10; i += 1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := 0; i <= 9; i += 1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for i := 0; i <= 9; i += 1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + for i := 0; i < 10; i += 0x1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } @@ -79,6 +121,13 @@ func main() { for i := 0; i < 10; i += 0x1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := 0; i <= 9; i += 0x1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for i := 0; i <= 9; i += 0x1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + for i := 0; i < 10; i = i + 1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } @@ -86,6 +135,13 @@ func main() { for i := 0; i < 10; i = i + 1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := 0; i <= 9; i = i + 1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for i := 0; i <= 9; i = i + 1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + for i := 0; i < 10; i = i + 0x1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } @@ -93,6 +149,13 @@ func main() { for i := 0; i < 10; i = i + 0x1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := 0; i <= 9; i = i + 0x1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for i := 0; i <= 9; i = i + 0x1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + for i := 0; i < 10; i = 1 + i { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } @@ -100,6 +163,13 @@ func main() { for i := 0; i < 10; i = 1 + i { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := 0; i <= 9; i = 1 + i { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for i := 0; i <= 9; i = 1 + i { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + for i := 0; i < 10; i = 0x1 + i { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } @@ -107,6 +177,13 @@ func main() { for i := 0; i < 10; i = 0x1 + i { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := 0; i <= 9; i = 0x1 + i { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for i := 0; i <= 9; i = 0x1 + i { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + for i := 0; 10 > i; i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } @@ -114,6 +191,13 @@ func main() { for i := 0; 10 > i; i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := 0; 9 >= i; i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for i := 0; 9 >= i; i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + for i := 0x0; 10 > i; i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } @@ -121,6 +205,13 @@ func main() { for i := 0x0; 10 > i; i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := 0x0; 9 >= i; i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for i := 0x0; 9 >= i; i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + for i := 0; 10 > i; i += 1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } @@ -128,6 +219,13 @@ func main() { for i := 0; 10 > i; i += 1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := 0; 9 >= i; i += 1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for i := 0; 9 >= i; i += 1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + for i := 0; 10 > i; i += 0x1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } @@ -135,6 +233,13 @@ func main() { for i := 0; 10 > i; i += 0x1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := 0; 9 >= i; i += 0x1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for i := 0; 9 >= i; i += 0x1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + for i := 0; 10 > i; i = i + 1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } @@ -142,6 +247,13 @@ func main() { for i := 0; 10 > i; i = i + 1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := 0; 9 >= i; i = i + 1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for i := 0; 9 >= i; i = i + 1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + for i := 0; 10 > i; i = i + 0x1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } @@ -149,6 +261,13 @@ func main() { for i := 0; 10 > i; i = i + 0x1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := 0; 9 >= i; i = i + 0x1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for i := 0; 9 >= i; i = i + 0x1 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + for i := 0; 10 > i; i = 1 + i { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } @@ -156,6 +275,13 @@ func main() { for i := 0; 10 > i; i = 1 + i { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := 0; 9 >= i; i = 1 + i { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for i := 0; 9 >= i; i = 1 + i { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + for i := 0; 10 > i; i = 0x1 + i { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } @@ -163,6 +289,13 @@ func main() { for i := 0; 10 > i; i = 0x1 + i { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := 0; 9 >= i; i = 0x1 + i { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for i := 0; 9 >= i; i = 0x1 + i { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + const x = 10 for i := 2; i < x; i++ { @@ -171,6 +304,9 @@ func main() { for i := 0; i < x; i += 2 { } + for i := 0; i <= x; i++ { + } + for i := 0; i < x; i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } @@ -248,6 +384,15 @@ func main() { for i := 0; i < x; i = 0x1 + i { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := 2; x > i; i++ { + } + + for i := 0; x > i; i += 2 { + } + + for i := 0; x >= i; i++ { + } + for i := 0; x > i; i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } diff --git a/testdata/main.go.golden b/testdata/main.go.golden index 16c1ad6..8f3c268 100644 --- a/testdata/main.go.golden +++ b/testdata/main.go.golden @@ -44,6 +44,20 @@ func main() { for range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + + for i := range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + for i := range calculate(10) { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } @@ -51,6 +65,13 @@ func main() { for range calculate(10) { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := int32(0); i <= calculate(9); i++ { + fmt.Println(i) + } + + for i := int32(0); i <= calculate(9); i++ { + } + for i := range uint32(10) { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } @@ -58,6 +79,69 @@ func main() { for range uint32(10) { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := range uint32(10) { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for range uint32(10) { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + + for i := range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + + for i := range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + + for i := range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + + for i := range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + + for i := range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + + for i := range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + + for i := range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + + for i := range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + for i := range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } @@ -163,6 +247,55 @@ func main() { for range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + + for i := range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + + for i := range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + + for i := range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + + for i := range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + + for i := range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + + for i := range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + fmt.Println(i) + } + + for range 10 { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + } + const x = 10 for i := 2; i < x; i++ { @@ -171,6 +304,9 @@ func main() { for i := 0; i < x; i += 2 { } + for i := 0; i <= x; i++ { + } + for i := range x { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } @@ -248,6 +384,15 @@ func main() { for range x { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } + for i := 2; x > i; i++ { + } + + for i := 0; x > i; i += 2 { + } + + for i := 0; x >= i; i++ { + } + for i := range x { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) }