Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ruleguard,analyzer: implement rules debugging #104

Merged
merged 1 commit into from
Oct 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
2 changes: 2 additions & 0 deletions ruleguard/gorule.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ type scopedGoRuleSet struct {
}

type goRule struct {
group string
filename string
line int
severity string
pat *gogrep.Pattern
msg string
Expand Down
5 changes: 5 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,8 @@ func (p *rulesParser) parseRule(matcher string, call *ast.CallExpr) error {
dst := p.res.universal
proto := goRule{
filename: p.filename,
line: p.fset.Position(origCall.Pos()).Line,
group: p.group,
filter: matchFilter{
sub: map[string]submatchFilter{},
},
Expand Down
9 changes: 9 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,12 @@ func RunRules(ctx *Context, f *ast.File, rules *GoRuleSet) error {
type GoRuleInfo struct {
// Filename is a file that defined this rule.
Filename string

// Line is a line inside a file that defined this rule.
Line int

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

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

import (
"bytes"
"fmt"
"go/ast"
"go/printer"
"io/ioutil"
"path/filepath"
"strconv"
"strings"

Expand Down Expand Up @@ -90,9 +92,42 @@ 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 by %s:%d (%s)",
pos.Filename, pos.Line, filepath.Base(rule.filename), rule.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 +139,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 +162,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