Skip to content

Commit

Permalink
Custom filters are simple Go functions defined in ruleguard rule files
Browse files Browse the repository at this point in the history
that can be used as Where clause predicates.

They're compiled to a bytecode that is then interpreted when
the matched rule needs to apply its filters. The performance is
good enough for now (faster than `yaegi`) and can be further
improved in the future.

There are various limitations in what can be used in custom
filters and what's not. It's not well-documented right now,
but the compile errors try to be helpful.
What can be compiled will be executed like a normal Go would.

One use case for this new feature can be found in #129

Here is the solution to #129 which is not possible:

```go
	func implementsStringer(ctx *dsl.VarFilterContext) bool {
		stringer := ctx.GetInterface(`fmt.Stringer`)
		return types.Implements(ctx.Type, stringer) ||
			types.Implements(types.NewPointer(ctx.Type), stringer)
	}

	func stringerLiteral(m dsl.Matcher) {
		m.Match(`$x{$*_}`).
			Where(m["x"].Filter(implementsStringer)).
			Report(`$x implements fmt.Stringer`)
	}
```

Custom filters are more flexible than predefined filters, but they
generally require more coding. As a rule of thumb: they should be
used only when there is no matching builtin filter.

Note: right now many simple operations, like integer multiplication,
are not implemented. They can be added trivially, but I wanted to
present a working concept with a minimal amount of code for this first PR.
Upcoming changes that extend the supported features set are going
to be easier to review.

It's also possible to allow calling byte-compiled functions from other byte-compiled functions.
But it's not there yet (I also need examples where it can be applied to get a better
understanding of the subject).

Also fixes #167 and #170
  • Loading branch information
quasilyte committed Jan 8, 2021
1 parent e47d548 commit a276b3f
Show file tree
Hide file tree
Showing 46 changed files with 3,495 additions and 361 deletions.
5 changes: 3 additions & 2 deletions _test/install/binary_gopath/expected.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/root/target.go:8:10: exprUnparen: the parentheses around 1 are superfluous (rules.go:15)
/root/target.go:9:10: exprUnparen: the parentheses around 2 are superfluous (rules.go:15)
/root/target.go:8:10: exprUnparen: the parentheses around 1 are superfluous (rules.go:21)
/root/target.go:9:10: exprUnparen: the parentheses around 2 are superfluous (rules.go:21)
/root/target.go:11:10: boolComparison: omit bool literal in expression (rules1.go:8)
/root/target.go:12:10: boolExprSimplify: suggestion: b (rules2.go:6)
/root/target.go:15:10: interfaceAddr: taking address of interface-typed value (rules.go:27)
12 changes: 12 additions & 0 deletions _test/install/binary_gopath/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,27 @@ package gorules

import (
"github.com/quasilyte/go-ruleguard/dsl"
"github.com/quasilyte/go-ruleguard/dsl/types"
testrules "github.com/quasilyte/ruleguard-rules-test"
)

func init() {
dsl.ImportRules("", testrules.Bundle)
}

func isInterface(ctx *dsl.VarFilterContext) bool {
// Could be written as m["x"].Type.Underlying().Is(`interface{$*_}`) in DSL.
return types.AsInterface(ctx.Type.Underlying()) != nil
}

func exprUnparen(m dsl.Matcher) {
m.Match(`$f($*_, ($x), $*_)`).
Report(`the parentheses around $x are superfluous`).
Suggest(`$f($x)`)
}

func interfaceAddr(m dsl.Matcher) {
m.Match(`&$x`).
Where(m["x"].Filter(isInterface)).
Report(`taking address of interface-typed value`)
}
3 changes: 3 additions & 0 deletions _test/install/binary_gopath/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,7 @@ func test(b bool) {

println(b == true)
println(!!b)

var eface interface{}
println(&eface)
}
9 changes: 5 additions & 4 deletions _test/install/binary_nogopath/expected.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/root/target.go:8:10: exprUnparen: the parentheses around 1 are superfluous (rules.go:15)
/root/target.go:9:10: exprUnparen: the parentheses around 2 are superfluous (rules.go:15)
/root/target.go:11:10: testrules/boolComparison: omit bool literal in expression (rules1.go:8)
/root/target.go:12:10: testrules/boolExprSimplify: suggestion: b (rules2.go:6)
/root/target.go:8:10: exprUnparen: the parentheses around 1 are superfluous (rules.go:21)
/root/target.go:9:10: exprUnparen: the parentheses around 2 are superfluous (rules.go:21)
/root/target.go:11:10: boolComparison: omit bool literal in expression (rules1.go:8)
/root/target.go:12:10: boolExprSimplify: suggestion: b (rules2.go:6)
/root/target.go:15:10: interfaceAddr: taking address of interface-typed value (rules.go:27)
14 changes: 13 additions & 1 deletion _test/install/binary_nogopath/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,27 @@ package gorules

import (
"github.com/quasilyte/go-ruleguard/dsl"
"github.com/quasilyte/go-ruleguard/dsl/types"
testrules "github.com/quasilyte/ruleguard-rules-test"
)

func init() {
dsl.ImportRules("testrules", testrules.Bundle)
dsl.ImportRules("", testrules.Bundle)
}

func isInterface(ctx *dsl.VarFilterContext) bool {
// Could be written as m["x"].Type.Underlying().Is(`interface{$*_}`) in DSL.
return types.AsInterface(ctx.Type.Underlying()) != nil
}

func exprUnparen(m dsl.Matcher) {
m.Match(`$f($*_, ($x), $*_)`).
Report(`the parentheses around $x are superfluous`).
Suggest(`$f($x)`)
}

func interfaceAddr(m dsl.Matcher) {
m.Match(`&$x`).
Where(m["x"].Filter(isInterface)).
Report(`taking address of interface-typed value`)
}
3 changes: 3 additions & 0 deletions _test/install/binary_nogopath/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,7 @@ func test(b bool) {

println(b == true)
println(!!b)

var eface interface{}
println(&eface)
}
4 changes: 2 additions & 2 deletions _test/install/gitclone/expected.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
/root/target.go:8:10: exprUnparen: the parentheses around 1 are superfluous
/root/target.go:9:10: exprUnparen: the parentheses around 2 are superfluous
/root/target.go:8:10: exprUnparen: the parentheses around 1 are superfluous (rules.go:10)
/root/target.go:9:10: exprUnparen: the parentheses around 2 are superfluous (rules.go:10)
113 changes: 72 additions & 41 deletions analyzer/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"os"
"path/filepath"
"strings"
"sync"

"github.com/quasilyte/go-ruleguard/ruleguard"
"golang.org/x/tools/go/analysis"
Expand All @@ -21,62 +22,73 @@ var Analyzer = &analysis.Analyzer{
Run: runAnalyzer,
}

// ForceNewEngine disables engine cache optimization.
// This should only be useful for analyzer testing.
var ForceNewEngine = false

var (
globalEngineMu sync.Mutex
globalEngine *ruleguard.Engine
globalEngineErrored bool
)

var (
flagRules string
flagE string
flagEnable string
flagDisable string

flagDebug string
flagDebugFilter string
flagDebugImports bool
flagDebugEnableDisable bool
)

func init() {
Analyzer.Flags.StringVar(&flagRules, "rules", "", "comma-separated list of gorule file paths")
Analyzer.Flags.StringVar(&flagE, "e", "", "execute a single rule from a given string")
Analyzer.Flags.StringVar(&flagDebug, "debug-group", "", "enable debug for the specified function")
Analyzer.Flags.StringVar(&flagDebug, "debug-group", "", "enable debug for the specified matcher function")
Analyzer.Flags.StringVar(&flagDebugFilter, "debug-filter", "", "enable debug for the specified filter function")
Analyzer.Flags.StringVar(&flagEnable, "enable", "<all>", "comma-separated list of enabled groups or '<all>' to enable everything")
Analyzer.Flags.StringVar(&flagDisable, "disable", "", "comma-separated list of groups to be disabled")
Analyzer.Flags.BoolVar(&flagDebugImports, "debug-imports", false, "enable debug for rules compile-time package lookups")
Analyzer.Flags.BoolVar(&flagDebugEnableDisable, "debug-enable-disable", false, "enable debug for -enable/-disable related info")
}

type parseRulesResult struct {
rset *ruleguard.GoRuleSet
multiFile bool
}

func debugPrint(s string) {
fmt.Fprintln(os.Stderr, s)
}

func runAnalyzer(pass *analysis.Pass) (interface{}, error) {
// TODO(quasilyte): parse config under sync.Once and
// create rule sets from it.

parseResult, err := readRules()
engine, err := prepareEngine()
if err != nil {
return nil, fmt.Errorf("load rules: %v", err)
}
rset := parseResult.rset
multiFile := parseResult.multiFile

ctx := &ruleguard.Context{
Debug: flagDebug,
DebugPrint: debugPrint,
Pkg: pass.Pkg,
Types: pass.TypesInfo,
Sizes: pass.TypesSizes,
Fset: pass.Fset,
// This condition will trigger only if we failed to init
// the engine. Return without an error as other analysis
// pass probably reported init error by this moment.
if engine == nil {
return nil, nil
}

printRuleLocation := flagE == ""

ctx := &ruleguard.RunContext{
Debug: flagDebug,
DebugImports: flagDebugImports,
DebugPrint: debugPrint,
Pkg: pass.Pkg,
Types: pass.TypesInfo,
Sizes: pass.TypesSizes,
Fset: pass.Fset,
Report: func(info ruleguard.GoRuleInfo, n ast.Node, msg string, s *ruleguard.Suggestion) {
msg = info.Group + ": " + msg
if multiFile {
msg += fmt.Sprintf(" (%s:%d)", filepath.Base(info.Filename), info.Line)
fullMessage := info.Group + ": " + msg
if printRuleLocation {
fullMessage += fmt.Sprintf(" (%s:%d)", filepath.Base(info.Filename), info.Line)
}
diag := analysis.Diagnostic{
Pos: n.Pos(),
Message: msg,
Message: fullMessage,
}
if s != nil {
diag.SuggestedFixes = []analysis.SuggestedFix{
Expand All @@ -97,15 +109,41 @@ func runAnalyzer(pass *analysis.Pass) (interface{}, error) {
}

for _, f := range pass.Files {
if err := ruleguard.RunRules(ctx, f, rset); err != nil {
if err := engine.Run(ctx, f); err != nil {
return nil, err
}
}

return nil, nil
}

func readRules() (*parseRulesResult, error) {
func prepareEngine() (*ruleguard.Engine, error) {
if ForceNewEngine {
return newEngine()
}

globalEngineMu.Lock()
defer globalEngineMu.Unlock()

if globalEngine != nil {
return globalEngine, nil
}
// If we already failed once, don't try again to avoid #167.
if globalEngineErrored {
return nil, nil
}

engine, err := newEngine()
if err != nil {
globalEngineErrored = true
return nil, err
}
globalEngine = engine
return engine, nil
}

func newEngine() (*ruleguard.Engine, error) {
e := ruleguard.NewEngine()
fset := token.NewFileSet()

disabledGroups := make(map[string]bool)
Expand All @@ -123,6 +161,7 @@ func readRules() (*parseRulesResult, error) {

ctx := &ruleguard.ParseContext{
Fset: fset,
DebugFilter: flagDebugFilter,
DebugImports: flagDebugImports,
DebugPrint: debugPrint,
GroupFilter: func(g string) bool {
Expand All @@ -148,28 +187,17 @@ func readRules() (*parseRulesResult, error) {
switch {
case flagRules != "":
filenames := strings.Split(flagRules, ",")
multifile := len(filenames) > 1
var ruleSets []*ruleguard.GoRuleSet
for _, filename := range filenames {
filename = strings.TrimSpace(filename)
data, err := ioutil.ReadFile(filename)
if err != nil {
return nil, fmt.Errorf("read rules file: %v", err)
}
rset, err := ruleguard.ParseRules(ctx, filename, bytes.NewReader(data))
if err != nil {
if err := e.Load(ctx, filename, bytes.NewReader(data)); err != nil {
return nil, fmt.Errorf("parse rules file: %v", err)
}
if len(rset.Imports) != 0 {
multifile = true
}
ruleSets = append(ruleSets, rset)
}
rset, err := ruleguard.MergeRuleSets(ruleSets)
if err != nil {
return nil, fmt.Errorf("merge rule files: %v", err)
}
return &parseRulesResult{rset: rset, multiFile: multifile}, nil
return e, nil

case flagE != "":
ruleText := fmt.Sprintf(`
Expand All @@ -180,8 +208,11 @@ func readRules() (*parseRulesResult, error) {
}`,
flagE)
r := strings.NewReader(ruleText)
rset, err := ruleguard.ParseRules(ctx, flagRules, r)
return &parseRulesResult{rset: rset}, err
err := e.Load(ctx, "e", r)
if err != nil {
return nil, err
}
return e, nil

default:
return nil, fmt.Errorf("both -e and -rules flags are empty")
Expand Down
2 changes: 2 additions & 0 deletions analyzer/analyzer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ func TestAnalyzer(t *testing.T) {
"golint",
"regression",
"testvendored",
"quasigo",
}

analyzer.ForceNewEngine = true
for _, test := range tests {
t.Run(test, func(t *testing.T) {
testdata := analysistest.TestData()
Expand Down
Loading

0 comments on commit a276b3f

Please sign in to comment.