Skip to content

Commit

Permalink
go/analysis/passes/tests: Check malformed fuzz target.
Browse files Browse the repository at this point in the history
This will validate that first letter after FuzzFoo() should be uppercase
Also, following validation will be performed for 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

For golang/go#50198

Change-Id: I540daf635f0fe03d954b010b9b5f8616fd5df47a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/374495
Reviewed-by: Robert Findley <[email protected]>
Trust: Peter Weinberger <[email protected]>
  • Loading branch information
ansaba committed Feb 16, 2022
1 parent 11109f6 commit 2ff4db7
Show file tree
Hide file tree
Showing 5 changed files with 232 additions and 0 deletions.
82 changes: 82 additions & 0 deletions go/analysis/passes/tests/testdata/src/a/go118_test.go
Original file line number Diff line number Diff line change
@@ -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
}
133 changes: 133 additions & 0 deletions go/analysis/passes/tests/tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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") {
Expand All @@ -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)
Expand Down
9 changes: 9 additions & 0 deletions go/analysis/passes/tests/tests_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
5 changes: 5 additions & 0 deletions gopls/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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:])
}
3 changes: 3 additions & 0 deletions internal/analysisinternal/analysis.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 2ff4db7

Please sign in to comment.