From 84f37f889527e449e574fc806c204afcebeb149d Mon Sep 17 00:00:00 2001 From: Denis Date: Tue, 3 May 2022 13:37:17 +0300 Subject: [PATCH 01/11] draft dynamic interfaces --- analyzer/testdata/src/filtertest/f1.go | 8 +++ analyzer/testdata/src/filtertest/rules.go | 6 ++ ruleguard/ir_loader.go | 75 ++++++++++++++++------- ruleguard/typematch/typematch.go | 4 +- 4 files changed, 69 insertions(+), 24 deletions(-) diff --git a/analyzer/testdata/src/filtertest/f1.go b/analyzer/testdata/src/filtertest/f1.go index 2d54b33..d0bacf3 100644 --- a/analyzer/testdata/src/filtertest/f1.go +++ b/analyzer/testdata/src/filtertest/f1.go @@ -1075,3 +1075,11 @@ type exampleStruct struct { w io.Writer buf *bytes.Buffer } + +type foo string + +func (foo) FooBar(_ string) {} +func dynamicInterface() { + var f foo + f.FooBar("123") // want `dynamic interface` +} diff --git a/analyzer/testdata/src/filtertest/rules.go b/analyzer/testdata/src/filtertest/rules.go index fa13cbf..ed7d85b 100644 --- a/analyzer/testdata/src/filtertest/rules.go +++ b/analyzer/testdata/src/filtertest/rules.go @@ -282,4 +282,10 @@ func testRules(m dsl.Matcher) { m.Match(`newIface("sink is interface{}").($_)`). Where(m["$$"].SinkType.Is(`interface{}`)). Report(`true`) + + m.Match(`$x.FooBar($_)`). + Where(m["x"].Type.Implements(`interface { FooBar(k string); KekFoo(k int, s string) }`)).Report(`dynamic interface`) + + m.Match(`$x.FooBar($_)`). + Where(m["x"].Type.Implements(`interface { FooBar(k string, fields ...int) }`)).Report(`dynamic interface`) } diff --git a/ruleguard/ir_loader.go b/ruleguard/ir_loader.go index c07a19f..bcb3140 100644 --- a/ruleguard/ir_loader.go +++ b/ruleguard/ir_loader.go @@ -478,30 +478,61 @@ func (l *irLoader) unwrapInterfaceExpr(filter ir.FilterExpr) (*types.Interface, if err != nil { return nil, l.errorf(filter.Line, err, "parse %s type expr", typeString) } - qn, ok := n.(*ast.SelectorExpr) - if !ok { + + var iface *types.Interface + switch qn := n.(type) { + case *ast.SelectorExpr: + pkgName, ok := qn.X.(*ast.Ident) + if !ok { + return nil, l.errorf(filter.Line, nil, "invalid package name") + } + pkgPath, ok := l.itab.Lookup(pkgName.Name) + if !ok { + return nil, l.errorf(filter.Line, nil, "package %s is not imported", pkgName.Name) + } + pkg, err := l.importer.Import(pkgPath) + if err != nil { + return nil, l.importErrorf(filter.Line, err, "can't load %s", pkgPath) + } + obj := pkg.Scope().Lookup(qn.Sel.Name) + if obj == nil { + return nil, l.errorf(filter.Line, nil, "%s is not found in %s", qn.Sel.Name, pkgPath) + } + iface, ok = obj.Type().Underlying().(*types.Interface) + if !ok { + return nil, l.errorf(filter.Line, nil, "%s is not an interface type", qn.Sel.Name) + } + case *ast.InterfaceType: + var methods []*types.Func + + for _, method := range qn.Methods.List { + var isVariadic bool + var vars []*types.Var + funcType := method.Type.(*ast.FuncType) + for _, field := range funcType.Params.List { + var ident *ast.Ident + switch p := field.Type.(type) { + case *ast.Ident: + ident = field.Type.(*ast.Ident) + case *ast.Ellipsis: + isVariadic = true + ident = p.Elt.(*ast.Ident) + } + + for _, param := range field.Names { + vars = append(vars, types.NewVar(token.NoPos, nil, param.Name, typematch.BuiltinTypeByName[ident.Name])) + } + } + + signature := types.NewSignature(nil, types.NewTuple(vars...), nil, isVariadic) + methods = append(methods, types.NewFunc(method.Pos(), nil, method.Names[0].Name, signature)) + } + + iface = types.NewInterfaceType(methods, nil) + default: return nil, l.errorf(filter.Line, nil, "can't resolve %s type; try a fully-qualified name", typeString) } - pkgName, ok := qn.X.(*ast.Ident) - if !ok { - return nil, l.errorf(filter.Line, nil, "invalid package name") - } - pkgPath, ok := l.itab.Lookup(pkgName.Name) - if !ok { - return nil, l.errorf(filter.Line, nil, "package %s is not imported", pkgName.Name) - } - pkg, err := l.importer.Import(pkgPath) - if err != nil { - return nil, l.importErrorf(filter.Line, err, "can't load %s", pkgPath) - } - obj := pkg.Scope().Lookup(qn.Sel.Name) - if obj == nil { - return nil, l.errorf(filter.Line, nil, "%s is not found in %s", qn.Sel.Name, pkgPath) - } - iface, ok := obj.Type().Underlying().(*types.Interface) - if !ok { - return nil, l.errorf(filter.Line, nil, "%s is not an interface type", qn.Sel.Name) - } + return iface, nil } diff --git a/ruleguard/typematch/typematch.go b/ruleguard/typematch/typematch.go index b747403..ac9984d 100644 --- a/ruleguard/typematch/typematch.go +++ b/ruleguard/typematch/typematch.go @@ -135,7 +135,7 @@ func Parse(ctx *Context, s string) (*Pattern, error) { } var ( - builtinTypeByName = map[string]types.Type{ + BuiltinTypeByName = map[string]types.Type{ "bool": types.Typ[types.Bool], "int": types.Typ[types.Int], "int8": types.Typ[types.Int8], @@ -167,7 +167,7 @@ var ( func parseExpr(ctx *Context, e ast.Expr) *pattern { switch e := e.(type) { case *ast.Ident: - basic, ok := builtinTypeByName[e.Name] + basic, ok := BuiltinTypeByName[e.Name] if ok { return &pattern{op: opBuiltinType, value: basic} } From da702d4f8944a95c840740bb368bfae3e24eb1ee Mon Sep 17 00:00:00 2001 From: Denis Date: Tue, 3 May 2022 16:08:10 +0300 Subject: [PATCH 02/11] add nil checks --- analyzer/testdata/src/filtertest/rules.go | 5 +- ruleguard/ir_loader.go | 73 ++++++++++++++++------- 2 files changed, 52 insertions(+), 26 deletions(-) diff --git a/analyzer/testdata/src/filtertest/rules.go b/analyzer/testdata/src/filtertest/rules.go index ed7d85b..3741d65 100644 --- a/analyzer/testdata/src/filtertest/rules.go +++ b/analyzer/testdata/src/filtertest/rules.go @@ -284,8 +284,5 @@ func testRules(m dsl.Matcher) { Report(`true`) m.Match(`$x.FooBar($_)`). - Where(m["x"].Type.Implements(`interface { FooBar(k string); KekFoo(k int, s string) }`)).Report(`dynamic interface`) - - m.Match(`$x.FooBar($_)`). - Where(m["x"].Type.Implements(`interface { FooBar(k string, fields ...int) }`)).Report(`dynamic interface`) + Where(m["x"].Type.Implements(`interface { FooBar(k string) }`)).Report(`dynamic interface`) } diff --git a/ruleguard/ir_loader.go b/ruleguard/ir_loader.go index bcb3140..63b17df 100644 --- a/ruleguard/ir_loader.go +++ b/ruleguard/ir_loader.go @@ -503,32 +503,16 @@ func (l *irLoader) unwrapInterfaceExpr(filter ir.FilterExpr) (*types.Interface, return nil, l.errorf(filter.Line, nil, "%s is not an interface type", qn.Sel.Name) } case *ast.InterfaceType: - var methods []*types.Func - + methods := make([]*types.Func, 0, len(qn.Methods.List)) for _, method := range qn.Methods.List { - var isVariadic bool - var vars []*types.Var - funcType := method.Type.(*ast.FuncType) - for _, field := range funcType.Params.List { - var ident *ast.Ident - switch p := field.Type.(type) { - case *ast.Ident: - ident = field.Type.(*ast.Ident) - case *ast.Ellipsis: - isVariadic = true - ident = p.Elt.(*ast.Ident) - } - - for _, param := range field.Names { - vars = append(vars, types.NewVar(token.NoPos, nil, param.Name, typematch.BuiltinTypeByName[ident.Name])) - } + fnType, ok := method.Type.(*ast.FuncType) + if !ok { + continue } - - signature := types.NewSignature(nil, types.NewTuple(vars...), nil, isVariadic) - methods = append(methods, types.NewFunc(method.Pos(), nil, method.Names[0].Name, signature)) + methods = append(methods, MapAstFuncTypeToTypesFunc(method.Names[0].Name, fnType)) } - iface = types.NewInterfaceType(methods, nil) + iface = types.NewInterfaceType(methods, nil).Complete() default: return nil, l.errorf(filter.Line, nil, "can't resolve %s type; try a fully-qualified name", typeString) } @@ -912,6 +896,51 @@ func (l *irLoader) newBinaryExprFilter(filter ir.FilterExpr, info *filterInfo) ( return result, nil } +func MapAstFuncTypeToTypesFunc(name string, funcType *ast.FuncType) *types.Func { + var ( + vars []*types.Var + res []*types.Var + ) + + if funcType.Params != nil { + vars = make([]*types.Var, 0, len(funcType.Params.List)) + for _, param := range funcType.Params.List { + tt := mapAstFieldToTypesType(param) + for _, name := range param.Names { // one param has several names when their type the same + vars = append(vars, types.NewVar(name.Pos(), nil, name.Name, tt)) + } + } + } + + if funcType.Results != nil { + res = make([]*types.Var, 0, len(funcType.Results.List)) + for _, param := range funcType.Results.List { + tt := mapAstFieldToTypesType(param) + for _, name := range param.Names { + res = append(res, types.NewVar(name.Pos(), nil, name.Name, tt)) + } + } + } + + return types.NewFunc(funcType.Pos(), + nil, + name, + types.NewSignature(nil, types.NewTuple(vars...), types.NewTuple(res...), false), + ) +} + +func mapAstFieldToTypesType(param *ast.Field) types.Type { + switch p := param.Type.(type) { + case *ast.Ident: + return typematch.BuiltinTypeByName[p.Name] + case *ast.Ellipsis: + // TODO variadic type + case *ast.ChanType: + + } + panic("unreachable statement") +} + type filterInfo struct { Vars map[string]struct{} From 12e388388b075b958ce042d592e5cd135e26200f Mon Sep 17 00:00:00 2001 From: Denis Date: Tue, 3 May 2022 16:29:50 +0300 Subject: [PATCH 03/11] add \Q --- analyzer/testdata/src/filtertest/f1.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/analyzer/testdata/src/filtertest/f1.go b/analyzer/testdata/src/filtertest/f1.go index d0bacf3..2109ee7 100644 --- a/analyzer/testdata/src/filtertest/f1.go +++ b/analyzer/testdata/src/filtertest/f1.go @@ -1081,5 +1081,5 @@ type foo string func (foo) FooBar(_ string) {} func dynamicInterface() { var f foo - f.FooBar("123") // want `dynamic interface` + f.FooBar("123") // want `\Qdynamic interface` } From bb04d5f09f97ab2a6f54e67bf621a205cad68fbe Mon Sep 17 00:00:00 2001 From: Denis Date: Tue, 3 May 2022 17:33:27 +0300 Subject: [PATCH 04/11] add new expressions to mapper --- ruleguard/ir_loader.go | 86 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 76 insertions(+), 10 deletions(-) diff --git a/ruleguard/ir_loader.go b/ruleguard/ir_loader.go index 63b17df..670af1d 100644 --- a/ruleguard/ir_loader.go +++ b/ruleguard/ir_loader.go @@ -2,6 +2,7 @@ package ruleguard import ( "bytes" + "errors" "fmt" "go/ast" "go/constant" @@ -10,6 +11,7 @@ import ( "go/types" "io/ioutil" "regexp" + "strconv" "github.com/quasilyte/gogrep" "github.com/quasilyte/gogrep/nodetag" @@ -509,7 +511,13 @@ func (l *irLoader) unwrapInterfaceExpr(filter ir.FilterExpr) (*types.Interface, if !ok { continue } - methods = append(methods, MapAstFuncTypeToTypesFunc(method.Names[0].Name, fnType)) + + fn, err := MapAstFuncTypeToTypesFunc(method.Names[0].Name, fnType) + if err != nil { + return nil, fmt.Errorf("on unwrapInterfaceExpr: %w", err) + } + + methods = append(methods, fn) } iface = types.NewInterfaceType(methods, nil).Complete() @@ -896,7 +904,7 @@ func (l *irLoader) newBinaryExprFilter(filter ir.FilterExpr, info *filterInfo) ( return result, nil } -func MapAstFuncTypeToTypesFunc(name string, funcType *ast.FuncType) *types.Func { +func MapAstFuncTypeToTypesFunc(name string, funcType *ast.FuncType) (*types.Func, error) { var ( vars []*types.Var res []*types.Var @@ -905,7 +913,11 @@ func MapAstFuncTypeToTypesFunc(name string, funcType *ast.FuncType) *types.Func if funcType.Params != nil { vars = make([]*types.Var, 0, len(funcType.Params.List)) for _, param := range funcType.Params.List { - tt := mapAstFieldToTypesType(param) + tt, err := mapAstExprToTypesType(param.Type) + if err != nil { + return nil, err + } + for _, name := range param.Names { // one param has several names when their type the same vars = append(vars, types.NewVar(name.Pos(), nil, name.Name, tt)) } @@ -915,7 +927,11 @@ func MapAstFuncTypeToTypesFunc(name string, funcType *ast.FuncType) *types.Func if funcType.Results != nil { res = make([]*types.Var, 0, len(funcType.Results.List)) for _, param := range funcType.Results.List { - tt := mapAstFieldToTypesType(param) + tt, err := mapAstExprToTypesType(param.Type) + if err != nil { + return nil, err + } + for _, name := range param.Names { res = append(res, types.NewVar(name.Pos(), nil, name.Name, tt)) } @@ -926,19 +942,69 @@ func MapAstFuncTypeToTypesFunc(name string, funcType *ast.FuncType) *types.Func nil, name, types.NewSignature(nil, types.NewTuple(vars...), types.NewTuple(res...), false), - ) + ), nil } -func mapAstFieldToTypesType(param *ast.Field) types.Type { - switch p := param.Type.(type) { +func mapAstExprToTypesType(param ast.Expr) (types.Type, error) { + switch p := param.(type) { case *ast.Ident: - return typematch.BuiltinTypeByName[p.Name] + return typematch.BuiltinTypeByName[p.Name], nil + case *ast.StarExpr: + el, err := mapAstExprToTypesType(p.X) + if err != nil { + return nil, err + } + + return types.NewPointer(el), nil case *ast.Ellipsis: - // TODO variadic type + //TODO + return nil, errors.New("on mapAstExprToTypesType: variadic types not supported") case *ast.ChanType: + var dir types.ChanDir + switch { + case p.Dir&ast.SEND != 0 && p.Dir&ast.RECV != 0: + dir = types.SendRecv + case p.Dir&ast.SEND != 0: + dir = types.SendOnly + case p.Dir&ast.RECV != 0: + dir = types.RecvOnly + default: + return nil, nil + } + + v, err := mapAstExprToTypesType(p.Value) + if err != nil { + return nil, err + } + + return types.NewChan(dir, v), nil + case *ast.MapType: + key, err := mapAstExprToTypesType(p.Key) + if err != nil { + return nil, err + } + + val, err := mapAstExprToTypesType(p.Value) + if err != nil { + return nil, err + } + + return types.NewMap(key, val), nil + case *ast.ArrayType: + // TODO add variadic + l, err := strconv.ParseInt(p.Len.(*ast.BasicLit).Value, 10, 64) + if err != nil { + return nil, fmt.Errorf("on mapAstExprToTypesType: %w", err) + } + + val, err := mapAstExprToTypesType(p.Elt) + if err != nil { + return nil, err + } + return types.NewArray(val, l), nil } - panic("unreachable statement") + return nil, errors.New("on mapAstExprToTypesType: unsupported statement") } type filterInfo struct { From 3df58498f97e6b02456932346d177ee9e5b6afe5 Mon Sep 17 00:00:00 2001 From: Denis Date: Tue, 3 May 2022 19:00:55 +0300 Subject: [PATCH 05/11] add supported types and tests --- analyzer/testdata/src/filtertest/f1.go | 11 ++++-- analyzer/testdata/src/filtertest/rules.go | 14 ++++++-- ruleguard/ir_loader.go | 43 ++++++++++++++++------- 3 files changed, 51 insertions(+), 17 deletions(-) diff --git a/analyzer/testdata/src/filtertest/f1.go b/analyzer/testdata/src/filtertest/f1.go index 2109ee7..e5c6345 100644 --- a/analyzer/testdata/src/filtertest/f1.go +++ b/analyzer/testdata/src/filtertest/f1.go @@ -1078,8 +1078,15 @@ type exampleStruct struct { type foo string -func (foo) FooBar(_ string) {} +func (foo) FooString(_ string) {} +func (foo) FooMap(_ map[string]string) {} +func (foo) FooChan(_ chan string) {} +func (foo) FooType(_ io.Closer) {} + func dynamicInterface() { var f foo - f.FooBar("123") // want `\Qdynamic interface` + f.FooString("123") // want `\Qdynamic interface 1` + f.FooMap(nil) // want `\Qdynamic interface 2` + f.FooChan(nil) // want `\Qdynamic interface 3` + f.FooType(nil) // want `\Qdynamic interface 4` } diff --git a/analyzer/testdata/src/filtertest/rules.go b/analyzer/testdata/src/filtertest/rules.go index 3741d65..f5273a1 100644 --- a/analyzer/testdata/src/filtertest/rules.go +++ b/analyzer/testdata/src/filtertest/rules.go @@ -3,7 +3,9 @@ package gorules -import "github.com/quasilyte/go-ruleguard/dsl" +import ( + "github.com/quasilyte/go-ruleguard/dsl" +) func testRules(m dsl.Matcher) { m.Import(`github.com/quasilyte/go-ruleguard/analyzer/testdata/src/filtertest/foolib`) @@ -283,6 +285,12 @@ func testRules(m dsl.Matcher) { Where(m["$$"].SinkType.Is(`interface{}`)). Report(`true`) - m.Match(`$x.FooBar($_)`). - Where(m["x"].Type.Implements(`interface { FooBar(k string) }`)).Report(`dynamic interface`) + m.Match(`$x.FooString($_)`). + Where(m["x"].Type.Implements(`interface { FooString(k string) }`)).Report(`dynamic interface 1`) + m.Match(`$x.FooMap($_)`). + Where(m["x"].Type.Implements(`interface { FooMap(k map[string]string) }`)).Report(`dynamic interface 2`) + m.Match(`$x.FooChan($_)`). + Where(m["x"].Type.Implements(`interface { FooChan(k chan string) }`)).Report(`dynamic interface 3`) + m.Match(`$x.FooType($_)`). + Where(m["x"].Type.Implements(`interface { FooType(k io.Closer) }`)).Report(`dynamic interface 4`) } diff --git a/ruleguard/ir_loader.go b/ruleguard/ir_loader.go index 670af1d..0f11a1b 100644 --- a/ruleguard/ir_loader.go +++ b/ruleguard/ir_loader.go @@ -512,7 +512,7 @@ func (l *irLoader) unwrapInterfaceExpr(filter ir.FilterExpr) (*types.Interface, continue } - fn, err := MapAstFuncTypeToTypesFunc(method.Names[0].Name, fnType) + fn, err := l.mapAstFuncTypeToTypesFunc(method.Names[0].Name, fnType) if err != nil { return nil, fmt.Errorf("on unwrapInterfaceExpr: %w", err) } @@ -904,7 +904,7 @@ func (l *irLoader) newBinaryExprFilter(filter ir.FilterExpr, info *filterInfo) ( return result, nil } -func MapAstFuncTypeToTypesFunc(name string, funcType *ast.FuncType) (*types.Func, error) { +func (l *irLoader) mapAstFuncTypeToTypesFunc(name string, funcType *ast.FuncType) (*types.Func, error) { var ( vars []*types.Var res []*types.Var @@ -913,7 +913,7 @@ func MapAstFuncTypeToTypesFunc(name string, funcType *ast.FuncType) (*types.Func if funcType.Params != nil { vars = make([]*types.Var, 0, len(funcType.Params.List)) for _, param := range funcType.Params.List { - tt, err := mapAstExprToTypesType(param.Type) + tt, err := l.mapAstExprToTypesType(param.Type) if err != nil { return nil, err } @@ -927,7 +927,7 @@ func MapAstFuncTypeToTypesFunc(name string, funcType *ast.FuncType) (*types.Func if funcType.Results != nil { res = make([]*types.Var, 0, len(funcType.Results.List)) for _, param := range funcType.Results.List { - tt, err := mapAstExprToTypesType(param.Type) + tt, err := l.mapAstExprToTypesType(param.Type) if err != nil { return nil, err } @@ -945,12 +945,12 @@ func MapAstFuncTypeToTypesFunc(name string, funcType *ast.FuncType) (*types.Func ), nil } -func mapAstExprToTypesType(param ast.Expr) (types.Type, error) { +func (l *irLoader) mapAstExprToTypesType(param ast.Expr) (types.Type, error) { switch p := param.(type) { case *ast.Ident: return typematch.BuiltinTypeByName[p.Name], nil case *ast.StarExpr: - el, err := mapAstExprToTypesType(p.X) + el, err := l.mapAstExprToTypesType(p.X) if err != nil { return nil, err } @@ -972,19 +972,19 @@ func mapAstExprToTypesType(param ast.Expr) (types.Type, error) { return nil, nil } - v, err := mapAstExprToTypesType(p.Value) + v, err := l.mapAstExprToTypesType(p.Value) if err != nil { return nil, err } return types.NewChan(dir, v), nil case *ast.MapType: - key, err := mapAstExprToTypesType(p.Key) + key, err := l.mapAstExprToTypesType(p.Key) if err != nil { return nil, err } - val, err := mapAstExprToTypesType(p.Value) + val, err := l.mapAstExprToTypesType(p.Value) if err != nil { return nil, err } @@ -992,17 +992,36 @@ func mapAstExprToTypesType(param ast.Expr) (types.Type, error) { return types.NewMap(key, val), nil case *ast.ArrayType: // TODO add variadic - l, err := strconv.ParseInt(p.Len.(*ast.BasicLit).Value, 10, 64) + arrLen, err := strconv.ParseInt(p.Len.(*ast.BasicLit).Value, 10, 64) if err != nil { return nil, fmt.Errorf("on mapAstExprToTypesType: %w", err) } - val, err := mapAstExprToTypesType(p.Elt) + val, err := l.mapAstExprToTypesType(p.Elt) if err != nil { return nil, err } - return types.NewArray(val, l), nil + return types.NewArray(val, arrLen), nil + case *ast.SelectorExpr: + pkgName, ok := p.X.(*ast.Ident) + if !ok { + return nil, l.errorf(int(p.Pos()), nil, "invalid package name") + } + pkgPath, ok := l.itab.Lookup(pkgName.Name) + if !ok { + return nil, l.errorf(int(p.Pos()), nil, "package %s is not imported", pkgName.Name) + } + pkg, err := l.importer.Import(pkgPath) + if err != nil { + return nil, l.importErrorf(int(p.Pos()), err, "can't load %s", pkgPath) + } + obj := pkg.Scope().Lookup(p.Sel.Name) + if obj == nil { + return nil, l.errorf(int(p.Pos()), nil, "%s is not found in %s", p.Sel.Name, pkgPath) + } + + return obj.Type(), nil } return nil, errors.New("on mapAstExprToTypesType: unsupported statement") } From 7da04472d25f13791b8d6a4243b5936d65ba4b9a Mon Sep 17 00:00:00 2001 From: Denis Date: Tue, 3 May 2022 19:06:16 +0300 Subject: [PATCH 06/11] change error return style --- ruleguard/ir_loader.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ruleguard/ir_loader.go b/ruleguard/ir_loader.go index 0f11a1b..973af1f 100644 --- a/ruleguard/ir_loader.go +++ b/ruleguard/ir_loader.go @@ -958,7 +958,7 @@ func (l *irLoader) mapAstExprToTypesType(param ast.Expr) (types.Type, error) { return types.NewPointer(el), nil case *ast.Ellipsis: //TODO - return nil, errors.New("on mapAstExprToTypesType: variadic types not supported") + return nil, l.errorf(int(p.Pos()), nil, "on mapAstExprToTypesType: variadic types not supported") case *ast.ChanType: var dir types.ChanDir switch { @@ -994,7 +994,7 @@ func (l *irLoader) mapAstExprToTypesType(param ast.Expr) (types.Type, error) { // TODO add variadic arrLen, err := strconv.ParseInt(p.Len.(*ast.BasicLit).Value, 10, 64) if err != nil { - return nil, fmt.Errorf("on mapAstExprToTypesType: %w", err) + return nil, l.errorf(int(p.Pos()), nil, "invalid length provided: "+err.Error()) } val, err := l.mapAstExprToTypesType(p.Elt) From b9bd25b1d6036700f198f9046cffddfe036a3fde Mon Sep 17 00:00:00 2001 From: Denis Date: Tue, 3 May 2022 19:10:14 +0300 Subject: [PATCH 07/11] change error return style --- ruleguard/ir_loader.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ruleguard/ir_loader.go b/ruleguard/ir_loader.go index 973af1f..8ac26c3 100644 --- a/ruleguard/ir_loader.go +++ b/ruleguard/ir_loader.go @@ -2,7 +2,6 @@ package ruleguard import ( "bytes" - "errors" "fmt" "go/ast" "go/constant" @@ -1023,7 +1022,7 @@ func (l *irLoader) mapAstExprToTypesType(param ast.Expr) (types.Type, error) { return obj.Type(), nil } - return nil, errors.New("on mapAstExprToTypesType: unsupported statement") + return nil, l.errorf(int(param.Pos()), nil, "unsupported statement provided: %T", param) } type filterInfo struct { From 0ae0e0eb9d6a45faec49e6e1028d35c9615b1a54 Mon Sep 17 00:00:00 2001 From: Denis Date: Tue, 3 May 2022 23:03:23 +0300 Subject: [PATCH 08/11] fix unnamed return, add tests --- analyzer/testdata/src/filtertest/f1.go | 22 ++++++++++++++-------- analyzer/testdata/src/filtertest/rules.go | 6 ++++++ ruleguard/ir_loader.go | 8 ++++++-- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/analyzer/testdata/src/filtertest/f1.go b/analyzer/testdata/src/filtertest/f1.go index e5c6345..1ac9df9 100644 --- a/analyzer/testdata/src/filtertest/f1.go +++ b/analyzer/testdata/src/filtertest/f1.go @@ -1078,15 +1078,21 @@ type exampleStruct struct { type foo string -func (foo) FooString(_ string) {} -func (foo) FooMap(_ map[string]string) {} -func (foo) FooChan(_ chan string) {} -func (foo) FooType(_ io.Closer) {} +func (foo) FooString(_ string) {} +func (foo) FooMap(_ map[string]string) {} +func (foo) FooChan(_ chan string) {} +func (foo) FooType(_ io.Closer) {} +func (foo) FooWithResult(_ string) string { return "" } +func (foo) FooWithResult2(_ string) (io.Closer, error) { return nil, nil } +func (foo) FooWithResult3(_ string) (cl io.Closer, err error) { return } func dynamicInterface() { var f foo - f.FooString("123") // want `\Qdynamic interface 1` - f.FooMap(nil) // want `\Qdynamic interface 2` - f.FooChan(nil) // want `\Qdynamic interface 3` - f.FooType(nil) // want `\Qdynamic interface 4` + f.FooString("123") // want `\Qdynamic interface 1` + f.FooMap(nil) // want `\Qdynamic interface 2` + f.FooChan(nil) // want `\Qdynamic interface 3` + f.FooType(nil) // want `\Qdynamic interface 4` + f.FooWithResult("") // want `\Qdynamic interface 5` + f.FooWithResult2("") // want `\Qdynamic interface 6` + f.FooWithResult3("") // want `\Qdynamic interface 7` } diff --git a/analyzer/testdata/src/filtertest/rules.go b/analyzer/testdata/src/filtertest/rules.go index f5273a1..4f92baa 100644 --- a/analyzer/testdata/src/filtertest/rules.go +++ b/analyzer/testdata/src/filtertest/rules.go @@ -293,4 +293,10 @@ func testRules(m dsl.Matcher) { Where(m["x"].Type.Implements(`interface { FooChan(k chan string) }`)).Report(`dynamic interface 3`) m.Match(`$x.FooType($_)`). Where(m["x"].Type.Implements(`interface { FooType(k io.Closer) }`)).Report(`dynamic interface 4`) + m.Match(`$x.FooWithResult($_)`). + Where(m["x"].Type.Implements(`interface { FooWithResult(k string) string }`)).Report(`dynamic interface 5`) + m.Match(`$x.FooWithResult2($_)`). + Where(m["x"].Type.Implements(`interface { FooWithResult2(k string) (io.Closer, error) }`)).Report(`dynamic interface 6`) + m.Match(`$x.FooWithResult3($_)`). + Where(m["x"].Type.Implements(`interface { FooWithResult3(k string) (cl io.Closer, err error) }`)).Report(`dynamic interface 7`) } diff --git a/ruleguard/ir_loader.go b/ruleguard/ir_loader.go index 8ac26c3..5c0e931 100644 --- a/ruleguard/ir_loader.go +++ b/ruleguard/ir_loader.go @@ -931,8 +931,12 @@ func (l *irLoader) mapAstFuncTypeToTypesFunc(name string, funcType *ast.FuncType return nil, err } - for _, name := range param.Names { - res = append(res, types.NewVar(name.Pos(), nil, name.Name, tt)) + if param.Names != nil { // named return + for _, name := range param.Names { + res = append(res, types.NewVar(name.Pos(), nil, name.Name, tt)) + } + } else { // unnamed + res = append(res, types.NewVar(param.Pos(), nil, "", tt)) } } } From c79736fc4ec3f231b87f4bbd54432c2884481533 Mon Sep 17 00:00:00 2001 From: Denis Date: Tue, 3 May 2022 23:08:07 +0300 Subject: [PATCH 09/11] add tests --- analyzer/testdata/src/filtertest/f1.go | 16 +++++++++------- analyzer/testdata/src/filtertest/rules.go | 2 ++ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/analyzer/testdata/src/filtertest/f1.go b/analyzer/testdata/src/filtertest/f1.go index 1ac9df9..6e0c941 100644 --- a/analyzer/testdata/src/filtertest/f1.go +++ b/analyzer/testdata/src/filtertest/f1.go @@ -1085,14 +1085,16 @@ func (foo) FooType(_ io.Closer) {} func (foo) FooWithResult(_ string) string { return "" } func (foo) FooWithResult2(_ string) (io.Closer, error) { return nil, nil } func (foo) FooWithResult3(_ string) (cl io.Closer, err error) { return } +func (foo) FooGrouped(_, _ io.Closer) {} func dynamicInterface() { var f foo - f.FooString("123") // want `\Qdynamic interface 1` - f.FooMap(nil) // want `\Qdynamic interface 2` - f.FooChan(nil) // want `\Qdynamic interface 3` - f.FooType(nil) // want `\Qdynamic interface 4` - f.FooWithResult("") // want `\Qdynamic interface 5` - f.FooWithResult2("") // want `\Qdynamic interface 6` - f.FooWithResult3("") // want `\Qdynamic interface 7` + f.FooString("") // want `\Qdynamic interface 1` + f.FooMap(nil) // want `\Qdynamic interface 2` + f.FooChan(nil) // want `\Qdynamic interface 3` + f.FooType(nil) // want `\Qdynamic interface 4` + f.FooGrouped(nil, nil) // want `\Qdynamic interface 4.1` + f.FooWithResult("") // want `\Qdynamic interface 5` + f.FooWithResult2("") // want `\Qdynamic interface 6` + f.FooWithResult3("") // want `\Qdynamic interface 7` } diff --git a/analyzer/testdata/src/filtertest/rules.go b/analyzer/testdata/src/filtertest/rules.go index 4f92baa..c4ae4a5 100644 --- a/analyzer/testdata/src/filtertest/rules.go +++ b/analyzer/testdata/src/filtertest/rules.go @@ -293,6 +293,8 @@ func testRules(m dsl.Matcher) { Where(m["x"].Type.Implements(`interface { FooChan(k chan string) }`)).Report(`dynamic interface 3`) m.Match(`$x.FooType($_)`). Where(m["x"].Type.Implements(`interface { FooType(k io.Closer) }`)).Report(`dynamic interface 4`) + m.Match(`$x.FooGrouped($*_)`). + Where(m["x"].Type.Implements(`interface { FooGrouped(k io.Closer, l io.Closer) }`)).Report(`dynamic interface 4.1`) m.Match(`$x.FooWithResult($_)`). Where(m["x"].Type.Implements(`interface { FooWithResult(k string) string }`)).Report(`dynamic interface 5`) m.Match(`$x.FooWithResult2($_)`). From b91988dd23fe6e964420703582f15580bee14ae1 Mon Sep 17 00:00:00 2001 From: Denis Date: Wed, 4 May 2022 09:30:20 +0300 Subject: [PATCH 10/11] simplify mapper, add support for closure param/results, add tests --- analyzer/testdata/src/filtertest/f1.go | 4 +++ analyzer/testdata/src/filtertest/rules.go | 4 +++ ruleguard/ir_loader.go | 42 ++++++++++++++--------- 3 files changed, 34 insertions(+), 16 deletions(-) diff --git a/analyzer/testdata/src/filtertest/f1.go b/analyzer/testdata/src/filtertest/f1.go index 6e0c941..d2e9fb2 100644 --- a/analyzer/testdata/src/filtertest/f1.go +++ b/analyzer/testdata/src/filtertest/f1.go @@ -1086,6 +1086,8 @@ func (foo) FooWithResult(_ string) string { return "" } func (foo) FooWithResult2(_ string) (io.Closer, error) { return nil, nil } func (foo) FooWithResult3(_ string) (cl io.Closer, err error) { return } func (foo) FooGrouped(_, _ io.Closer) {} +func (foo) FooFunc(_ func(x string) error) error { return nil } +func (foo) FooFunc2(func(string) error) error { return nil } func dynamicInterface() { var f foo @@ -1097,4 +1099,6 @@ func dynamicInterface() { f.FooWithResult("") // want `\Qdynamic interface 5` f.FooWithResult2("") // want `\Qdynamic interface 6` f.FooWithResult3("") // want `\Qdynamic interface 7` + f.FooFunc(nil) // want `\Qdynamic interface 8` + f.FooFunc2(nil) // want `\Qdynamic interface 8.1` } diff --git a/analyzer/testdata/src/filtertest/rules.go b/analyzer/testdata/src/filtertest/rules.go index c4ae4a5..73b767a 100644 --- a/analyzer/testdata/src/filtertest/rules.go +++ b/analyzer/testdata/src/filtertest/rules.go @@ -301,4 +301,8 @@ func testRules(m dsl.Matcher) { Where(m["x"].Type.Implements(`interface { FooWithResult2(k string) (io.Closer, error) }`)).Report(`dynamic interface 6`) m.Match(`$x.FooWithResult3($_)`). Where(m["x"].Type.Implements(`interface { FooWithResult3(k string) (cl io.Closer, err error) }`)).Report(`dynamic interface 7`) + m.Match(`$x.FooFunc($_)`). + Where(m["x"].Type.Implements(`interface { FooFunc(x func (x string) error) error }`)).Report(`dynamic interface 8`) + m.Match(`$x.FooFunc2($_)`). + Where(m["x"].Type.Implements(`interface { FooFunc2(func (string) error) error }`)).Report(`dynamic interface 8.1`) } diff --git a/ruleguard/ir_loader.go b/ruleguard/ir_loader.go index 5c0e931..8df2e2b 100644 --- a/ruleguard/ir_loader.go +++ b/ruleguard/ir_loader.go @@ -909,35 +909,38 @@ func (l *irLoader) mapAstFuncTypeToTypesFunc(name string, funcType *ast.FuncType res []*types.Var ) + mapField := func(param *ast.Field, results []*types.Var) ([]*types.Var, error) { + tt, err := l.mapAstExprToTypesType(param.Type) + if err != nil { + return nil, err + } + + if param.Names != nil { + for _, name := range param.Names { // one param has several names when their type the same + results = append(results, types.NewVar(name.Pos(), nil, name.Name, tt)) + } + } else { // unnamed + results = append(results, types.NewVar(param.Pos(), nil, "", tt)) + } + return results, nil + } + + var err error if funcType.Params != nil { vars = make([]*types.Var, 0, len(funcType.Params.List)) for _, param := range funcType.Params.List { - tt, err := l.mapAstExprToTypesType(param.Type) - if err != nil { + if vars, err = mapField(param, vars); err != nil { return nil, err } - - for _, name := range param.Names { // one param has several names when their type the same - vars = append(vars, types.NewVar(name.Pos(), nil, name.Name, tt)) - } } } if funcType.Results != nil { res = make([]*types.Var, 0, len(funcType.Results.List)) for _, param := range funcType.Results.List { - tt, err := l.mapAstExprToTypesType(param.Type) - if err != nil { + if res, err = mapField(param, res); err != nil { return nil, err } - - if param.Names != nil { // named return - for _, name := range param.Names { - res = append(res, types.NewVar(name.Pos(), nil, name.Name, tt)) - } - } else { // unnamed - res = append(res, types.NewVar(param.Pos(), nil, "", tt)) - } } } @@ -959,6 +962,13 @@ func (l *irLoader) mapAstExprToTypesType(param ast.Expr) (types.Type, error) { } return types.NewPointer(el), nil + case *ast.FuncType: + fn, err := l.mapAstFuncTypeToTypesFunc("", p) + if err != nil { + return nil, err + } + + return fn.Type(), nil case *ast.Ellipsis: //TODO return nil, l.errorf(int(p.Pos()), nil, "on mapAstExprToTypesType: variadic types not supported") From 7649b1d49bdf6b73fea52c7b89a3ca207bea3c70 Mon Sep 17 00:00:00 2001 From: Denis Date: Wed, 4 May 2022 09:58:21 +0300 Subject: [PATCH 11/11] add test for array --- analyzer/testdata/src/filtertest/f1.go | 20 +++++++++++--------- analyzer/testdata/src/filtertest/rules.go | 18 ++++++++++-------- ruleguard/ir_loader.go | 1 - 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/analyzer/testdata/src/filtertest/f1.go b/analyzer/testdata/src/filtertest/f1.go index d2e9fb2..c9f8905 100644 --- a/analyzer/testdata/src/filtertest/f1.go +++ b/analyzer/testdata/src/filtertest/f1.go @@ -1080,6 +1080,7 @@ type foo string func (foo) FooString(_ string) {} func (foo) FooMap(_ map[string]string) {} +func (foo) FooArray(_ [32]byte) {} func (foo) FooChan(_ chan string) {} func (foo) FooType(_ io.Closer) {} func (foo) FooWithResult(_ string) string { return "" } @@ -1087,18 +1088,19 @@ func (foo) FooWithResult2(_ string) (io.Closer, error) { return nil, nil func (foo) FooWithResult3(_ string) (cl io.Closer, err error) { return } func (foo) FooGrouped(_, _ io.Closer) {} func (foo) FooFunc(_ func(x string) error) error { return nil } -func (foo) FooFunc2(func(string) error) error { return nil } +func (foo) FooFunc2(func(string) error) error { return nil } func dynamicInterface() { var f foo f.FooString("") // want `\Qdynamic interface 1` f.FooMap(nil) // want `\Qdynamic interface 2` - f.FooChan(nil) // want `\Qdynamic interface 3` - f.FooType(nil) // want `\Qdynamic interface 4` - f.FooGrouped(nil, nil) // want `\Qdynamic interface 4.1` - f.FooWithResult("") // want `\Qdynamic interface 5` - f.FooWithResult2("") // want `\Qdynamic interface 6` - f.FooWithResult3("") // want `\Qdynamic interface 7` - f.FooFunc(nil) // want `\Qdynamic interface 8` - f.FooFunc2(nil) // want `\Qdynamic interface 8.1` + f.FooArray([32]byte{}) // want `\Qdynamic interface 3` + f.FooChan(nil) // want `\Qdynamic interface 4` + f.FooType(nil) // want `\Qdynamic interface 5` + f.FooGrouped(nil, nil) // want `\Qdynamic interface 6` + f.FooWithResult("") // want `\Qdynamic interface 7` + f.FooWithResult2("") // want `\Qdynamic interface 8` + f.FooWithResult3("") // want `\Qdynamic interface 9` + f.FooFunc(nil) // want `\Qdynamic interface 10` + f.FooFunc2(nil) // want `\Qdynamic interface 11` } diff --git a/analyzer/testdata/src/filtertest/rules.go b/analyzer/testdata/src/filtertest/rules.go index 73b767a..d5d6cf1 100644 --- a/analyzer/testdata/src/filtertest/rules.go +++ b/analyzer/testdata/src/filtertest/rules.go @@ -289,20 +289,22 @@ func testRules(m dsl.Matcher) { Where(m["x"].Type.Implements(`interface { FooString(k string) }`)).Report(`dynamic interface 1`) m.Match(`$x.FooMap($_)`). Where(m["x"].Type.Implements(`interface { FooMap(k map[string]string) }`)).Report(`dynamic interface 2`) + m.Match(`$x.FooArray($_)`). + Where(m["x"].Type.Implements(`interface { FooArray(k [32]byte) }`)).Report(`dynamic interface 3`) m.Match(`$x.FooChan($_)`). - Where(m["x"].Type.Implements(`interface { FooChan(k chan string) }`)).Report(`dynamic interface 3`) + Where(m["x"].Type.Implements(`interface { FooChan(k chan string) }`)).Report(`dynamic interface 4`) m.Match(`$x.FooType($_)`). - Where(m["x"].Type.Implements(`interface { FooType(k io.Closer) }`)).Report(`dynamic interface 4`) + Where(m["x"].Type.Implements(`interface { FooType(k io.Closer) }`)).Report(`dynamic interface 5`) m.Match(`$x.FooGrouped($*_)`). - Where(m["x"].Type.Implements(`interface { FooGrouped(k io.Closer, l io.Closer) }`)).Report(`dynamic interface 4.1`) + Where(m["x"].Type.Implements(`interface { FooGrouped(k io.Closer, l io.Closer) }`)).Report(`dynamic interface 6`) m.Match(`$x.FooWithResult($_)`). - Where(m["x"].Type.Implements(`interface { FooWithResult(k string) string }`)).Report(`dynamic interface 5`) + Where(m["x"].Type.Implements(`interface { FooWithResult(k string) string }`)).Report(`dynamic interface 7`) m.Match(`$x.FooWithResult2($_)`). - Where(m["x"].Type.Implements(`interface { FooWithResult2(k string) (io.Closer, error) }`)).Report(`dynamic interface 6`) + Where(m["x"].Type.Implements(`interface { FooWithResult2(k string) (io.Closer, error) }`)).Report(`dynamic interface 8`) m.Match(`$x.FooWithResult3($_)`). - Where(m["x"].Type.Implements(`interface { FooWithResult3(k string) (cl io.Closer, err error) }`)).Report(`dynamic interface 7`) + Where(m["x"].Type.Implements(`interface { FooWithResult3(k string) (cl io.Closer, err error) }`)).Report(`dynamic interface 9`) m.Match(`$x.FooFunc($_)`). - Where(m["x"].Type.Implements(`interface { FooFunc(x func (x string) error) error }`)).Report(`dynamic interface 8`) + Where(m["x"].Type.Implements(`interface { FooFunc(x func (x string) error) error }`)).Report(`dynamic interface 10`) m.Match(`$x.FooFunc2($_)`). - Where(m["x"].Type.Implements(`interface { FooFunc2(func (string) error) error }`)).Report(`dynamic interface 8.1`) + Where(m["x"].Type.Implements(`interface { FooFunc2(func (string) error) error }`)).Report(`dynamic interface 11`) } diff --git a/ruleguard/ir_loader.go b/ruleguard/ir_loader.go index 8df2e2b..9395fef 100644 --- a/ruleguard/ir_loader.go +++ b/ruleguard/ir_loader.go @@ -1004,7 +1004,6 @@ func (l *irLoader) mapAstExprToTypesType(param ast.Expr) (types.Type, error) { return types.NewMap(key, val), nil case *ast.ArrayType: - // TODO add variadic arrLen, err := strconv.ParseInt(p.Len.(*ast.BasicLit).Value, 10, 64) if err != nil { return nil, l.errorf(int(p.Pos()), nil, "invalid length provided: "+err.Error())