From 6d996d56542adff089bdf7dc8eea8d7783ba9087 Mon Sep 17 00:00:00 2001 From: Iskander Sharipov Date: Fri, 30 Oct 2020 06:47:33 +0300 Subject: [PATCH] ruleguard,analyzer: implement rules debugging When ruleguard is called with -debug-group= 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 by rules.go:6 ($s type filter) $s [10]int: a example2.go:9: rejected rules2.go:8 ($s2 is const) $s1 string: v1 $s2 string: "" example2.go:7: rejected rules2.go:8 ($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. This change also adds Line:int and Group:string fields to the rules info. Line is a rules file line that declared this rule Group is a containing function name Refs #98 Signed-off-by: Iskander Sharipov --- README.md | 3 +++ analyzer/analyzer.go | 7 +++++++ ruleguard/gorule.go | 2 ++ ruleguard/parser.go | 5 +++++ ruleguard/ruleguard.go | 9 +++++++++ ruleguard/runner.go | 44 ++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 70 insertions(+) diff --git a/README.md b/README.md index ed4c81e1..176ee641 100644 --- a/README.md +++ b/README.md @@ -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 ` 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. diff --git a/analyzer/analyzer.go b/analyzer/analyzer.go index 303df1fa..d877f54d 100644 --- a/analyzer/analyzer.go +++ b/analyzer/analyzer.go @@ -6,6 +6,7 @@ import ( "go/ast" "go/token" "io/ioutil" + "os" "path/filepath" "strings" @@ -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 { @@ -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, diff --git a/ruleguard/gorule.go b/ruleguard/gorule.go index 07bbb5c7..e3ad120c 100644 --- a/ruleguard/gorule.go +++ b/ruleguard/gorule.go @@ -13,7 +13,9 @@ type scopedGoRuleSet struct { } type goRule struct { + group string filename string + line int severity string pat *gogrep.Pattern msg string diff --git a/ruleguard/parser.go b/ruleguard/parser.go index ae7fc10f..fc9ed5b4 100644 --- a/ruleguard/parser.go +++ b/ruleguard/parser.go @@ -20,6 +20,7 @@ import ( type rulesParser struct { filename string + group string fset *token.FileSet res *GoRuleSet types *types.Info @@ -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() @@ -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{}, }, diff --git a/ruleguard/ruleguard.go b/ruleguard/ruleguard.go index f6032c86..841bc279 100644 --- a/ruleguard/ruleguard.go +++ b/ruleguard/ruleguard.go @@ -8,6 +8,9 @@ import ( ) type Context struct { + Debug string + DebugPrint func(string) + Types *types.Info Sizes types.Sizes Fset *token.FileSet @@ -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 { diff --git a/ruleguard/runner.go b/ruleguard/runner.go index f0560290..e5b353b3 100644 --- a/ruleguard/runner.go +++ b/ruleguard/runner.go @@ -2,9 +2,11 @@ package ruleguard import ( "bytes" + "fmt" "go/ast" "go/printer" "io/ioutil" + "path/filepath" "strconv" "strings" @@ -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 } } @@ -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 } @@ -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 } }