Skip to content

Commit

Permalink
ruleguard: allow FQN in Implements() filter
Browse files Browse the repository at this point in the history
  • Loading branch information
quasilyte committed Jan 13, 2021
1 parent 1b24401 commit 0cc118a
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 50 deletions.
14 changes: 14 additions & 0 deletions _test/regress/issue103/expected2.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/root/target.go:20:2: logrus: logger.WithError(err).Error(...) (rules2.go:10)
20 logger.Error(err)
/root/target.go:22:2: logrus: logger.WithError(fmt.Errorf("wrap: %w", err)).Error(...) (rules2.go:10)
22 logger.Error(fmt.Errorf("wrap: %w", err))
/root/target.go:25:2: loggerType: testLoggerType called with *logrus.Logger (rules2.go:21)
25 testLoggerType(logger)
/root/target.go:29:2: logrus: loggerEntry.WithError(err).Error(...) (rules2.go:10)
29 loggerEntry.Error(err)
/root/target.go:31:2: logrus: loggerEntry.WithError(fmt.Errorf("wrap: %w", err)).Error(...) (rules2.go:10)
31 loggerEntry.Error(fmt.Errorf("wrap: %w", err))
/root/target.go:38:2: logrus: loggerIface.WithError(err).Error(...) (rules2.go:10)
38 loggerIface.Error(err)
/root/target.go:40:2: logrus: loggerIface.WithError(fmt.Errorf("wrap: %w", err)).Error(...) (rules2.go:10)
40 loggerIface.Error(fmt.Errorf("wrap: %w", err))
24 changes: 24 additions & 0 deletions _test/regress/issue103/rules2.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// +build ignore

package gorules

import (
"github.com/quasilyte/go-ruleguard/dsl"
)

func logrus(m dsl.Matcher) {
m.Match(
`$log.Error($*_, $err, $*_)`,
`$log.Errorf($*_, $err, $*_)`,
).
Where(m["err"].Type.Is(`error`) && m["log"].Type.Implements(`github.com/sirupsen/logrus.FieldLogger`)).
Report(`$log.WithError($err).Error(...)`)
}

func loggerType(m dsl.Matcher) {
m.Import("github.com/sirupsen/logrus")

m.Match(`testLoggerType($x)`).
Where(m["x"].Type.Is(`*logrus.Logger`)).
Report("testLoggerType called with *logrus.Logger")
}
3 changes: 3 additions & 0 deletions _test/regress/issue103/test.bash
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,7 @@ go get -v -u github.com/sirupsen/logrus
./ruleguard -c 0 -rules /root/rules.go /root/target.go &> actual.txt || true
diff -u actual.txt /root/expected.txt

./ruleguard -c 0 -rules /root/rules2.go /root/target.go &> actual.txt || true
diff -u actual.txt /root/expected2.txt

echo SUCCESS
14 changes: 13 additions & 1 deletion dsl/dsl.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func (ExprType) ConvertibleTo(typ string) bool { return boolResult }

// Implements reports whether a type implements a given interface.
// See https://golang.org/pkg/go/types/#Implements.
func (ExprType) Implements(typ string) bool { return boolResult }
func (ExprType) Implements(typ typeName) bool { return boolResult }

// Is reports whether a type is identical to a given type.
func (ExprType) Is(typ string) bool { return boolResult }
Expand Down Expand Up @@ -160,3 +160,15 @@ type File struct {

// Imports reports whether the current file imports the given path.
func (File) Imports(path string) bool { return boolResult }

// typeName is a helper type used to document function params better.
//
// A type name can be:
// - builtin type name: `error`, `string`, etc.
// - qualified name from a standard library: `io.Reader`, etc.
// - fully-qualified type name, like `github.com/username/pkgname.TypeName`
//
// typeName is also affected by a local import table, which can override
// how qualified names are interpreted.
// See `Matcher.Import` for more info.
type typeName = string
9 changes: 2 additions & 7 deletions dsl/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,9 @@ type VarFilterContext struct {
func (*VarFilterContext) SizeOf(x types.Type) int { return 0 }

// GetType finds a type value by a given name.
//
// A name can be:
// - builtin type name, like `error` or `string`
// - fully-qualified type name, like `github.com/username/pkgname.TypeName`
//
// If a type can't be found (or a name is malformed), this function panics.
func (*VarFilterContext) GetType(name string) types.Type { return nil }
func (*VarFilterContext) GetType(name typeName) types.Type { return nil }

// GetInterface finds a type value that represents an interface by a given name.
// Works like `types.AsInterface(ctx.GetType(name))`.
func (*VarFilterContext) GetInterface(name string) *types.Interface { return nil }
func (*VarFilterContext) GetInterface(name typeName) *types.Interface { return nil }
87 changes: 48 additions & 39 deletions ruleguard/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,7 @@ func (p *rulesParser) parseFilterExpr(e ast.Expr) matchFilter {
result.fn = makeCustomVarFilter(result.src, operand.varName, userFn)

case "Type.Is", "Type.Underlying.Is":
// TODO(quasilyte): add FQN support?
typeString, ok := p.toStringValue(args[0])
if !ok {
panic(p.errorf(args[0], "expected a string literal argument"))
Expand All @@ -624,45 +625,7 @@ func (p *rulesParser) parseFilterExpr(e ast.Expr) matchFilter {
result.fn = makeTypeAssignableToFilter(result.src, operand.varName, dstType)

case "Type.Implements":
typeString, ok := p.toStringValue(args[0])
if !ok {
panic(p.errorf(args[0], "expected a string literal argument"))
}
n, err := parser.ParseExpr(typeString)
if err != nil {
panic(p.errorf(args[0], "parse type expr: %v", err))
}
var iface *types.Interface
switch n := n.(type) {
case *ast.Ident:
if n.Name != `error` {
panic(p.errorf(n, "only `error` unqualified type is recognized"))
}
iface = types.Universe.Lookup("error").Type().Underlying().(*types.Interface)
case *ast.SelectorExpr:
pkgName, ok := n.X.(*ast.Ident)
if !ok {
panic(p.errorf(n.X, "invalid package name"))
}
pkgPath, ok := p.itab.Lookup(pkgName.Name)
if !ok {
panic(p.errorf(n.X, "package %s is not imported", pkgName.Name))
}
pkg, err := p.importer.Import(pkgPath)
if err != nil {
panic(p.errorf(n, "can't load %s: %v", pkgPath, err))
}
obj := pkg.Scope().Lookup(n.Sel.Name)
if obj == nil {
panic(p.errorf(n, "%s is not found in %s", n.Sel.Name, pkgPath))
}
iface, ok = obj.Type().Underlying().(*types.Interface)
if !ok {
panic(p.errorf(n, "%s is not an interface type", n.Sel.Name))
}
default:
panic(p.errorf(args[0], "only qualified names (and `error`) are supported"))
}
iface := p.toInterfaceValue(args[0])
result.fn = makeTypeImplementsFilter(result.src, operand.varName, iface)

case "Text.Matches":
Expand All @@ -687,6 +650,52 @@ func (p *rulesParser) parseFilterExpr(e ast.Expr) matchFilter {
return result
}

func (p *rulesParser) toInterfaceValue(x ast.Node) *types.Interface {
typeString, ok := p.toStringValue(x)
if !ok {
panic(p.errorf(x, "expected a string literal argument"))
}

typ, err := p.state.FindType(p.importer, p.pkg, typeString)
if err == nil {
iface, ok := typ.Underlying().(*types.Interface)
if !ok {
panic(p.errorf(x, "%s is not an interface type", typeString))
}
return iface
}

n, err := parser.ParseExpr(typeString)
if err != nil {
panic(p.errorf(x, "parse type expr: %v", err))
}
qn, ok := n.(*ast.SelectorExpr)
if !ok {
panic(p.errorf(x, "can't resolve %s type; try a fully-qualified name", typeString))
}
pkgName, ok := qn.X.(*ast.Ident)
if !ok {
panic(p.errorf(qn.X, "invalid package name"))
}
pkgPath, ok := p.itab.Lookup(pkgName.Name)
if !ok {
panic(p.errorf(qn.X, "package %s is not imported", pkgName.Name))
}
pkg, err := p.importer.Import(pkgPath)
if err != nil {
panic(p.errorf(n, "can't load %s: %v", pkgPath, err))
}
obj := pkg.Scope().Lookup(qn.Sel.Name)
if obj == nil {
panic(p.errorf(n, "%s is not found in %s", qn.Sel.Name, pkgPath))
}
iface, ok := obj.Type().Underlying().(*types.Interface)
if !ok {
panic(p.errorf(n, "%s is not an interface type", qn.Sel.Name))
}
return iface
}

func (p *rulesParser) toStringValue(x ast.Node) (string, bool) {
switch x := x.(type) {
case *ast.BasicLit:
Expand Down
9 changes: 6 additions & 3 deletions ruleguard/ruleguard_error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,12 +257,15 @@ func TestParseFilterError(t *testing.T) {

{
`m["x"].Type.Implements("foo")`,
"only `error` unqualified type is recognized",
`can't resolve foo type; try a fully-qualified name`,
},

{
`m["x"].Type.Implements("func()")`,
"only qualified names (and `error`) are supported",
`can't resolve func() type; try a fully-qualified name`,
},
{
`m["x"].Type.Implements("bytes.Buffer")`,
`bytes.Buffer is not an interface type`,
},

{
Expand Down

0 comments on commit 0cc118a

Please sign in to comment.