diff --git a/intrange.go b/intrange.go index 9173bdf..dc7d33a 100644 --- a/intrange.go +++ b/intrange.go @@ -265,6 +265,15 @@ func checkForStmt(pass *analysis.Pass, forStmt *ast.ForStmt) { replacement = fmt.Sprintf("range %s", rangeX) } + if isFunctionOrMethodCall(operand) { + pass.Report(analysis.Diagnostic{ + Pos: forStmt.Pos(), + Message: msg + "\nBecause the key is returned by a function or method, take care to consider side effects.", + }) + + return + } + pass.Report(analysis.Diagnostic{ Pos: forStmt.Pos(), Message: msg, @@ -315,12 +324,11 @@ func checkRangeStmt(pass *analysis.Pass, rangeStmt *ast.RangeStmt) { return } - fn, ok := x.Fun.(*ast.Ident) - if !ok { + if _, ok = x.Fun.(*ast.Ident); !ok { return } - if fn.Name != "len" || len(x.Args) != 1 { + if !isLen(x) { return } @@ -385,7 +393,7 @@ func checkRangeStmt(pass *analysis.Pass, rangeStmt *ast.RangeStmt) { func findNExpr(expr ast.Expr) ast.Expr { switch e := expr.(type) { case *ast.CallExpr: - if fun, ok := e.Fun.(*ast.Ident); ok && fun.Name == "len" && len(e.Args) == 1 { + if isLen(e) { return findNExpr(e.Args[0]) } @@ -528,19 +536,7 @@ func isNumberLit(exp ast.Expr) bool { 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: + if !isIntCast(fun) { return false } default: @@ -575,19 +571,7 @@ func compareNumberLit(exp ast.Expr, val int) bool { 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: + if !isIntCast(fun) { return false } default: @@ -627,3 +611,49 @@ func operandToString( return t.String() + "(" + s + ")" } + +func isFunctionOrMethodCall(expr ast.Expr) bool { + e, ok := expr.(*ast.CallExpr) + if !ok { + return false + } + + fun, ok := e.Fun.(*ast.Ident) + if !ok { + return true + } + + if isLen(e) || isIntCast(fun) { + return false + } + + return true +} + +func isIntCast(ident *ast.Ident) bool { + switch ident.Name { + case + "int", + "int8", + "int16", + "int32", + "int64", + "uint", + "uint8", + "uint16", + "uint32", + "uint64": + return true + default: + return false + } +} + +func isLen(exp *ast.CallExpr) bool { + fun, ok := exp.Fun.(*ast.Ident) + if !ok { + return false + } + + return fun.Name == "len" && len(exp.Args) == 1 +} diff --git a/testdata/main.go b/testdata/main.go index b7c2077..7b0d21b 100644 --- a/testdata/main.go +++ b/testdata/main.go @@ -664,6 +664,19 @@ func issue33() { } } +func issue48() { + foo := struct { + Len func() int + }{ + Len: func() int { + return 10 + }, + } + + for i := 0; i < foo.Len(); i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)\nBecause the key is returned by a function or method, take care to consider side effects.` + } +} + func issue50() { x := 10 i := &x diff --git a/testdata/main.go.golden b/testdata/main.go.golden index 6e03687..ed4e1a9 100644 --- a/testdata/main.go.golden +++ b/testdata/main.go.golden @@ -58,11 +58,11 @@ func main() { 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\+\)` + 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) } - for range calculate(10) { // 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\+\)` } for i := int32(0); i <= calculate(9); i++ { @@ -321,11 +321,11 @@ func main() { for range 1*x { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } - for i := range calculate(1*x) { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + for i := int32(0); i < calculate(1*x); i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` fmt.Println(i) } - for range calculate(1*x) { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + for i := int32(0); i < calculate(1*x); i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` } for i := range uint32(x) { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` @@ -532,7 +532,7 @@ func main() { // https://github.com/ckaznocha/intrange/issues/16 func issue16(service protoreflect.ServiceDescriptor) { - for i := range service.Methods().Len() { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` + for i := 0; i < service.Methods().Len(); i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)` print(i) } } @@ -664,6 +664,19 @@ func issue33() { } } +func issue48() { + foo := struct { + Len func() int + }{ + Len: func() int { + return 10 + }, + } + + for i := 0; i < foo.Len(); i++ { // want `for loop can be changed to use an integer range \(Go 1\.22\+\)\nBecause the key is returned by a function or method, take care to consider side effects.` + } +} + func issue50() { x := 10 i := &x