diff --git a/go/analysis/passes/tests/testdata/src/a/go118_test.go b/go/analysis/passes/tests/testdata/src/a/go118_test.go new file mode 100644 index 0000000000..059bd089ce --- /dev/null +++ b/go/analysis/passes/tests/testdata/src/a/go118_test.go @@ -0,0 +1,82 @@ +//go:build go1.18 +// +build go1.18 + +package a + +import ( + "testing" +) + +func Fuzzfoo(*testing.F) {} // want "first letter after 'Fuzz' must not be lowercase" + +func FuzzBoo(*testing.F) {} // OK because first letter after 'Fuzz' is Uppercase. + +func FuzzCallDifferentFunc(f *testing.F) { + f.Name() //OK +} + +func FuzzFunc(f *testing.F) { + f.Fuzz(func(t *testing.T) {}) // OK "first argument is of type *testing.T" +} + +func FuzzFuncWithArgs(f *testing.F) { + f.Fuzz(func(t *testing.T, i int, b []byte) {}) // OK "arguments in func are allowed" +} + +func FuzzArgFunc(f *testing.F) { + f.Fuzz(0) // want "argument to Fuzz must be a function" +} + +func FuzzFuncWithReturn(f *testing.F) { + f.Fuzz(func(t *testing.T) bool { return true }) // want "fuzz target must not return any value" +} + +func FuzzFuncNoArg(f *testing.F) { + f.Fuzz(func() {}) // want "fuzz target must have 1 or more argument" +} + +func FuzzFuncFirstArgNotTesting(f *testing.F) { + f.Fuzz(func(i int64) {}) // want "the first parameter of a fuzz target must be \\*testing.T" +} + +func FuzzFuncFirstArgTestingNotT(f *testing.F) { + f.Fuzz(func(t *testing.F) {}) // want "the first parameter of a fuzz target must be \\*testing.T" +} + +func FuzzFuncSecondArgNotAllowed(f *testing.F) { + f.Fuzz(func(t *testing.T, i complex64) {}) // want "fuzzing arguments can only have the following types: string, bool, float32, float64, int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64, \\[\\]byte" +} + +func FuzzFuncSecondArgArrNotAllowed(f *testing.F) { + f.Fuzz(func(t *testing.T, i []int) {}) // want "fuzzing arguments can only have the following types: string, bool, float32, float64, int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64, \\[\\]byte" +} + +func FuzzFuncInner(f *testing.F) { + innerFunc := func(t *testing.T, i float32) {} + f.Fuzz(innerFunc) // ok +} + +func FuzzArrayOfFunc(f *testing.F) { + var funcs = []func(t *testing.T, i int){func(t *testing.T, i int) {}} + f.Fuzz(funcs[0]) // ok +} + +type GenericSlice[T any] []T + +func FuzzGenericFunc(f *testing.F) { + g := GenericSlice[func(t *testing.T, i int)]{func(t *testing.T, i int) {}} + f.Fuzz(g[0]) // ok +} + +type F func(t *testing.T, i int32) + +type myType struct { + myVar F +} + +func FuzzObjectMethod(f *testing.F) { + obj := myType{ + myVar: func(t *testing.T, i int32) {}, + } + f.Fuzz(obj.myVar) // ok +} diff --git a/go/analysis/passes/tests/tests.go b/go/analysis/passes/tests/tests.go index 2c87882496..a0e5edf5ae 100644 --- a/go/analysis/passes/tests/tests.go +++ b/go/analysis/passes/tests/tests.go @@ -16,6 +16,7 @@ import ( "unicode/utf8" "golang.org/x/tools/go/analysis" + "golang.org/x/tools/internal/analysisinternal" "golang.org/x/tools/internal/typeparams" ) @@ -34,6 +35,24 @@ var Analyzer = &analysis.Analyzer{ Run: run, } +var acceptedFuzzTypes = []types.Type{ + types.Typ[types.String], + types.Typ[types.Bool], + types.Typ[types.Float32], + types.Typ[types.Float64], + types.Typ[types.Int], + types.Typ[types.Int8], + types.Typ[types.Int16], + types.Typ[types.Int32], + types.Typ[types.Int64], + types.Typ[types.Uint], + types.Typ[types.Uint8], + types.Typ[types.Uint16], + types.Typ[types.Uint32], + types.Typ[types.Uint64], + types.NewSlice(types.Universe.Lookup("byte").Type()), +} + func run(pass *analysis.Pass) (interface{}, error) { for _, f := range pass.Files { if !strings.HasSuffix(pass.Fset.File(f.Pos()).Name(), "_test.go") { @@ -54,11 +73,125 @@ func run(pass *analysis.Pass) (interface{}, error) { case strings.HasPrefix(fn.Name.Name, "Benchmark"): checkTest(pass, fn, "Benchmark") } + // run fuzz tests diagnostics only for 1.18 i.e. when analysisinternal.DiagnoseFuzzTests is turned on. + if strings.HasPrefix(fn.Name.Name, "Fuzz") && analysisinternal.DiagnoseFuzzTests { + checkTest(pass, fn, "Fuzz") + checkFuzzCall(pass, fn) + } } } return nil, nil } +// Check the arguments of f.Fuzz() calls : +// 1. f.Fuzz() should call a function and it should be of type (*testing.F).Fuzz(). +// 2. The called function in f.Fuzz(func(){}) should not return result. +// 3. First argument of func() should be of type *testing.T +// 4. Second argument onwards should be of type []byte, string, bool, byte, +// rune, float32, float64, int, int8, int16, int32, int64, uint, uint8, uint16, +// uint32, uint64 +func checkFuzzCall(pass *analysis.Pass, fn *ast.FuncDecl) { + ast.Inspect(fn, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if ok { + if !isFuzzTargetDotFuzz(pass, call) { + return true + } + + // Only one argument (func) must be passed to (*testing.F).Fuzz. + if len(call.Args) != 1 { + return true + } + expr := call.Args[0] + if pass.TypesInfo.Types[expr].Type == nil { + return true + } + t := pass.TypesInfo.Types[expr].Type.Underlying() + tSign, argOk := t.(*types.Signature) + // Argument should be a function + if !argOk { + pass.ReportRangef(expr, "argument to Fuzz must be a function") + return true + } + // ff Argument function should not return + if tSign.Results().Len() != 0 { + pass.ReportRangef(expr, "fuzz target must not return any value") + } + // ff Argument function should have 1 or more argument + if tSign.Params().Len() == 0 { + pass.ReportRangef(expr, "fuzz target must have 1 or more argument") + return true + } + validateFuzzArgs(pass, tSign.Params(), expr) + } + return true + }) +} + +// isFuzzTargetDotFuzz reports whether call is (*testing.F).Fuzz(). +func isFuzzTargetDotFuzz(pass *analysis.Pass, call *ast.CallExpr) bool { + if selExpr, ok := call.Fun.(*ast.SelectorExpr); ok { + if !isTestingType(pass.TypesInfo.Types[selExpr.X].Type, "F") { + return false + } + if selExpr.Sel.Name == "Fuzz" { + return true + } + } + return false +} + +// Validate the arguments of fuzz target. +func validateFuzzArgs(pass *analysis.Pass, params *types.Tuple, expr ast.Expr) { + fLit, isFuncLit := expr.(*ast.FuncLit) + exprRange := expr + if !isTestingType(params.At(0).Type(), "T") { + if isFuncLit { + exprRange = fLit.Type.Params.List[0].Type + } + pass.ReportRangef(exprRange, "the first parameter of a fuzz target must be *testing.T") + } + for i := 1; i < params.Len(); i++ { + if !isAcceptedFuzzType(params.At(i).Type()) { + if isFuncLit { + exprRange = fLit.Type.Params.List[i].Type + } + pass.ReportRangef(exprRange, "fuzzing arguments can only have the following types: "+printAcceptedFuzzType()) + } + } +} + +func isTestingType(typ types.Type, testingType string) bool { + ptr, ok := typ.(*types.Pointer) + if !ok { + return false + } + named, ok := ptr.Elem().(*types.Named) + if !ok { + return false + } + return named.Obj().Pkg().Path() == "testing" && named.Obj().Name() == testingType +} + +// Validate that fuzz target function's arguments are of accepted types. +func isAcceptedFuzzType(paramType types.Type) bool { + for _, typ := range acceptedFuzzTypes { + if types.Identical(typ, paramType) { + return true + } + } + return false +} + +func formatAcceptedFuzzType() string { + var acceptedFuzzTypesStrings []string + for _, typ := range acceptedFuzzTypes { + acceptedFuzzTypesStrings = append(acceptedFuzzTypesStrings, typ.String()) + } + acceptedFuzzTypesMsg := strings.Join(acceptedFuzzTypesStrings, ", ") + return acceptedFuzzTypesMsg +} + func isExampleSuffix(s string) bool { r, size := utf8.DecodeRuneInString(s) return size > 0 && unicode.IsLower(r) diff --git a/go/analysis/passes/tests/tests_test.go b/go/analysis/passes/tests/tests_test.go index 740adc5d5e..b0b09dddff 100644 --- a/go/analysis/passes/tests/tests_test.go +++ b/go/analysis/passes/tests/tests_test.go @@ -7,12 +7,21 @@ package tests_test import ( "testing" + "golang.org/x/tools/internal/analysisinternal" + "golang.org/x/tools/go/analysis/analysistest" "golang.org/x/tools/go/analysis/passes/tests" "golang.org/x/tools/internal/typeparams" ) func Test(t *testing.T) { + // In 1.18, diagnostic for Fuzz Tests must not be used by cmd/vet. + // So the code for Fuzz tests diagnostics is guarded behind flag analysisinternal.DiagnoseFuzzTests + // Turn on the flag DiagnoseFuzzTests for analysis tests and then turn it off. + analysisinternal.DiagnoseFuzzTests = true + defer func() { + analysisinternal.DiagnoseFuzzTests = false + }() testdata := analysistest.TestData() pkgs := []string{ "a", // loads "a", "a [a.test]", and "a.test" diff --git a/gopls/main.go b/gopls/main.go index 2e099e7e82..f73eabf576 100644 --- a/gopls/main.go +++ b/gopls/main.go @@ -13,6 +13,7 @@ package main // import "golang.org/x/tools/gopls" import ( "context" + "golang.org/x/tools/internal/analysisinternal" "os" "golang.org/x/tools/gopls/internal/hooks" @@ -21,6 +22,10 @@ import ( ) func main() { + // In 1.18, diagnostics for Fuzz tests must not be used by cmd/vet. + // So the code for Fuzz tests diagnostics is guarded behind flag analysisinternal.DiagnoseFuzzTests + // Turn on analysisinternal.DiagnoseFuzzTests for gopls + analysisinternal.DiagnoseFuzzTests = true ctx := context.Background() tool.Main(ctx, cmd.New("gopls", "", nil, hooks.Options), os.Args[1:]) } diff --git a/internal/analysisinternal/analysis.go b/internal/analysisinternal/analysis.go index 01f6e829f7..78ee2c06be 100644 --- a/internal/analysisinternal/analysis.go +++ b/internal/analysisinternal/analysis.go @@ -17,6 +17,9 @@ import ( "golang.org/x/tools/internal/lsp/fuzzy" ) +// Flag to gate diagnostics for fuzz tests in 1.18. +var DiagnoseFuzzTests bool = false + var ( GetTypeErrors func(p interface{}) []types.Error SetTypeErrors func(p interface{}, errors []types.Error)