Skip to content

Commit

Permalink
ruleguard,analyzer: implement rules debugging
Browse files Browse the repository at this point in the history
When ruleguard is called with -debug-group=<string> arg,
it'll print rejected matches explanations to the stderr
for the specified rules group.

By default, this argument is an empty string that means "no debug".

Explanation includes:

	* Submatch nodes (with types)
	* Rejection reason (sometimes approx)
	* Rejected node location

Here is an example of how explanations can look like:

	example.go:4: rejected ($s type filter)
	  $s [10]int: a

	example2.go:9: rejected ($s2 is const)
	  $s1 string: v1
	  $s2 string: ""

	example2.go:7: rejected ($s1 is not const)
	  $s1 string: v1
	  $s2 string: v2

The debug output format is experimental and can change over time.
Any feedback is appreciated.

Refs #98

Signed-off-by: Iskander Sharipov <quasilyte@gmail.com>
  • Loading branch information
quasilyte committed Oct 30, 2020
1 parent b030e75 commit acf031a
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 0 deletions.
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ example.go:5:10: !(v1 != v2)

It automatically inserts `Report("$$")` into the specified pattern.

For named functions (rule groups) you can use `-debug-group <name>` flag to see explanations
on why some rules rejected the match (e.g. which `Where()` condition failed).

## How does it work?

`ruleguard` parses [gorules](docs/gorules.md) (e.g. `rules.go`) during the start to load the rule set.
Expand Down
7 changes: 7 additions & 0 deletions analyzer/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"go/ast"
"go/token"
"io/ioutil"
"os"
"path/filepath"
"strings"

Expand All @@ -23,11 +24,13 @@ var Analyzer = &analysis.Analyzer{
var (
flagRules string
flagE string
flagDebug string
)

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 named rules group")
}

type parseRulesResult struct {
Expand All @@ -47,6 +50,10 @@ func runAnalyzer(pass *analysis.Pass) (interface{}, error) {
multiFile := parseResult.multiFile

ctx := &ruleguard.Context{
Debug: flagDebug,
DebugPrint: func(s string) {
fmt.Fprintln(os.Stderr, s)
},
Pkg: pass.Pkg,
Types: pass.TypesInfo,
Sizes: pass.TypesSizes,
Expand Down
1 change: 1 addition & 0 deletions ruleguard/gorule.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ type scopedGoRuleSet struct {
}

type goRule struct {
group string
filename string
severity string
pat *gogrep.Pattern
Expand Down
4 changes: 4 additions & 0 deletions ruleguard/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (

type rulesParser struct {
filename string
group string
fset *token.FileSet
res *GoRuleSet
types *types.Info
Expand Down Expand Up @@ -236,6 +237,8 @@ func (p *rulesParser) parseRuleGroup(f *ast.FuncDecl) error {
// TODO(quasilyte): do an actual matcher param type check?
matcher := params[0].Names[0].Name

p.group = f.Name.Name

p.itab.EnterScope()
defer p.itab.LeaveScope()

Expand Down Expand Up @@ -337,6 +340,7 @@ func (p *rulesParser) parseRule(matcher string, call *ast.CallExpr) error {
dst := p.res.universal
proto := goRule{
filename: p.filename,
group: p.group,
filter: matchFilter{
sub: map[string]submatchFilter{},
},
Expand Down
6 changes: 6 additions & 0 deletions ruleguard/ruleguard.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ import (
)

type Context struct {
Debug string
DebugPrint func(string)

Types *types.Info
Sizes types.Sizes
Fset *token.FileSet
Expand All @@ -33,6 +36,9 @@ func RunRules(ctx *Context, f *ast.File, rules *GoRuleSet) error {
type GoRuleInfo struct {
// Filename is a file that defined this rule.
Filename string

// Group is a function name that contained this rule.
Group string
}

type GoRuleSet struct {
Expand Down
42 changes: 42 additions & 0 deletions ruleguard/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package ruleguard

import (
"bytes"
"fmt"
"go/ast"
"go/printer"
"io/ioutil"
Expand Down Expand Up @@ -90,9 +91,41 @@ func (rr *rulesRunner) run(f *ast.File) error {
return nil
}

func (rr *rulesRunner) reject(rule goRule, reason, sub string, m gogrep.MatchData) {
// Note: we accept reason and sub args instead of formatted or
// concatenated string so it's cheaper for us to call this
// function is debugging is not enabled.

if rule.group != rr.ctx.Debug {
return // This rule is not being debugged
}

pos := rr.ctx.Fset.Position(m.Node.Pos())
if sub != "" {
reason = "$" + sub + " " + reason
}
rr.ctx.DebugPrint(fmt.Sprintf("%s:%d: rejected (%s)", pos.Filename, pos.Line, reason))
for name, node := range m.Values {
var expr ast.Expr
switch node := node.(type) {
case ast.Expr:
expr = node
case *ast.ExprStmt:
expr = node.X
default:
continue
}

typ := rr.ctx.Types.TypeOf(expr)
s := strings.ReplaceAll(sprintNode(rr.ctx.Fset, expr), "\n", `\n`)
rr.ctx.DebugPrint(fmt.Sprintf(" $%s %s: %s", name, typ, s))
}
}

func (rr *rulesRunner) handleMatch(rule goRule, m gogrep.MatchData) bool {
for _, neededImport := range rule.filter.fileImports {
if _, ok := rr.imports[neededImport]; !ok {
rr.reject(rule, "file imports filter", "", m)
return false
}
}
Expand All @@ -104,6 +137,7 @@ func (rr *rulesRunner) handleMatch(rule goRule, m gogrep.MatchData) bool {
// filters, so we don't loose much here, but we can optimize
// this file filters in the future.
if rule.filter.filenamePred != nil && !rule.filter.filenamePred(rr.filename) {
rr.reject(rule, "file name filter", "", m)
return false
}

Expand All @@ -126,41 +160,49 @@ func (rr *rulesRunner) handleMatch(rule goRule, m gogrep.MatchData) bool {
typ := rr.ctx.Types.TypeOf(expr)
q := typeQuery{x: typ, ctx: rr.ctx}
if !filter.typePred(q) {
rr.reject(rule, "type filter", name, m)
return false
}
}
if filter.textPred != nil {
if !filter.textPred(string(rr.nodeText(expr))) {
rr.reject(rule, "text filter", name, m)
return false
}
}
switch filter.addressable {
case bool3true:
if !isAddressable(rr.ctx.Types, expr) {
rr.reject(rule, "is not addressable", name, m)
return false
}
case bool3false:
if isAddressable(rr.ctx.Types, expr) {
rr.reject(rule, "is addressable", name, m)
return false
}
}
switch filter.pure {
case bool3true:
if !isPure(rr.ctx.Types, expr) {
rr.reject(rule, "is not pure", name, m)
return false
}
case bool3false:
if isPure(rr.ctx.Types, expr) {
rr.reject(rule, "is pure", name, m)
return false
}
}
switch filter.constant {
case bool3true:
if !isConstant(rr.ctx.Types, expr) {
rr.reject(rule, "is not const", name, m)
return false
}
case bool3false:
if isConstant(rr.ctx.Types, expr) {
rr.reject(rule, "is const", name, m)
return false
}
}
Expand Down

0 comments on commit acf031a

Please sign in to comment.