From fd2773648c58823d74946a090423c3ca91c595c3 Mon Sep 17 00:00:00 2001 From: "Iskander (Alex) Sharipov" Date: Tue, 28 Dec 2021 23:16:09 +0300 Subject: [PATCH] ruleguard,analyzer: change Report() callback argument (#327) Pass a structure with a set of fields instead of passing them as separate arguments. This makes it easier to add more (potentially experimental) match context arguments without breaking the API. But this time we have to break the API once. --- analyzer/analyzer.go | 13 +++++++------ analyzer/testanalyzer/testanalyzer.go | 7 ++++--- ruleguard/ast_walker.go | 2 ++ ruleguard/debug_test.go | 2 +- ruleguard/gorule.go | 2 ++ ruleguard/perf_test.go | 2 +- ruleguard/ruleguard.go | 26 +++++++++++++++++++++----- ruleguard/runner.go | 18 ++++++++++++++++-- 8 files changed, 54 insertions(+), 18 deletions(-) diff --git a/analyzer/analyzer.go b/analyzer/analyzer.go index 2bd5ba21..4dd0f8a0 100644 --- a/analyzer/analyzer.go +++ b/analyzer/analyzer.go @@ -3,7 +3,6 @@ package analyzer import ( "bytes" "fmt" - "go/ast" "go/token" "io/ioutil" "os" @@ -105,17 +104,19 @@ func runAnalyzer(pass *analysis.Pass) (interface{}, error) { Sizes: pass.TypesSizes, Fset: pass.Fset, GoVersion: goVersion, - Report: func(info ruleguard.GoRuleInfo, n ast.Node, msg string, s *ruleguard.Suggestion) { - fullMessage := msg + Report: func(data *ruleguard.ReportData) { + fullMessage := data.Message + info := data.RuleInfo if printRuleLocation { fullMessage = fmt.Sprintf("%s: %s (%s:%d)", - info.Group.Name, msg, filepath.Base(info.Group.Filename), info.Line) + info.Group.Name, data.Message, filepath.Base(info.Group.Filename), info.Line) } diag := analysis.Diagnostic{ - Pos: n.Pos(), + Pos: data.Node.Pos(), Message: fullMessage, } - if s != nil { + if data.Suggestion != nil { + s := data.Suggestion diag.SuggestedFixes = []analysis.SuggestedFix{ { Message: "suggested replacement", diff --git a/analyzer/testanalyzer/testanalyzer.go b/analyzer/testanalyzer/testanalyzer.go index 32380184..8052bc08 100644 --- a/analyzer/testanalyzer/testanalyzer.go +++ b/analyzer/testanalyzer/testanalyzer.go @@ -2,7 +2,6 @@ package testanalyzer import ( "fmt" - "go/ast" "go/token" "path/filepath" @@ -26,11 +25,13 @@ func New(ruleSet *ir.File) *analysis.Analyzer { Types: pass.TypesInfo, Sizes: pass.TypesSizes, Fset: pass.Fset, - Report: func(info ruleguard.GoRuleInfo, n ast.Node, msg string, s *ruleguard.Suggestion) { + Report: func(data *ruleguard.ReportData) { + info := data.RuleInfo + msg := data.Message fullMessage := fmt.Sprintf("%s: %s (%s:%d)", info.Group.Name, msg, filepath.Base(info.Group.Filename), info.Line) pass.Report(analysis.Diagnostic{ - Pos: n.Pos(), + Pos: data.Node.Pos(), Message: fullMessage, }) }, diff --git a/ruleguard/ast_walker.go b/ruleguard/ast_walker.go index e3d7ea70..c16f9436 100644 --- a/ruleguard/ast_walker.go +++ b/ruleguard/ast_walker.go @@ -306,6 +306,7 @@ func (w *astWalker) walk(n ast.Node) { if n.Doc != nil { w.walk(n.Doc) } + prevFunc := w.filterParams.currentFunc if n.Recv != nil { w.walk(n.Recv) } @@ -314,6 +315,7 @@ func (w *astWalker) walk(n ast.Node) { if n.Body != nil { w.walk(n.Body) } + w.filterParams.currentFunc = prevFunc case *ast.File: w.walk(n.Name) diff --git a/ruleguard/debug_test.go b/ruleguard/debug_test.go index 32e2de24..805eceaf 100644 --- a/ruleguard/debug_test.go +++ b/ruleguard/debug_test.go @@ -254,7 +254,7 @@ func newDebugTestRunner(input string) (*debugTestRunner, error) { Types: &info, Sizes: types.SizesFor("gc", runtime.GOARCH), Fset: fset, - Report: func(info GoRuleInfo, n ast.Node, msg string, s *Suggestion) { + Report: func(data *ReportData) { // Do nothing. }, } diff --git a/ruleguard/gorule.go b/ruleguard/gorule.go index 90963f43..7c213b44 100644 --- a/ruleguard/gorule.go +++ b/ruleguard/gorule.go @@ -67,6 +67,8 @@ type filterParams struct { deadcode bool + currentFunc *ast.FuncDecl + // varname is set only for custom filters before bytecode function is called. varname string } diff --git a/ruleguard/perf_test.go b/ruleguard/perf_test.go index c700f5a0..283c3017 100644 --- a/ruleguard/perf_test.go +++ b/ruleguard/perf_test.go @@ -49,7 +49,7 @@ func benchRunContext(b *testing.B, src string) (*RunContext, *ast.File) { Types: typesInfo, Sizes: types.SizesFor("gc", runtime.GOARCH), Fset: fset, - Report: func(info GoRuleInfo, n ast.Node, msg string, s *Suggestion) { + Report: func(data *ReportData) { // Do nothing. }, } diff --git a/ruleguard/ruleguard.go b/ruleguard/ruleguard.go index 7e0f40f3..2642bb7b 100644 --- a/ruleguard/ruleguard.go +++ b/ruleguard/ruleguard.go @@ -93,15 +93,31 @@ type RunContext struct { DebugImports bool DebugPrint func(string) - Types *types.Info - Sizes types.Sizes - Fset *token.FileSet - Report func(rule GoRuleInfo, n ast.Node, msg string, s *Suggestion) - Pkg *types.Package + Types *types.Info + Sizes types.Sizes + Fset *token.FileSet + Pkg *types.Package + + // Report is a function that is called for every successful ruleguard match. + // The pointer to ReportData is reused, it should not be kept. + // If you want to keep it after Report() returns, make a copy. + Report func(*ReportData) GoVersion GoVersion } +type ReportData struct { + RuleInfo GoRuleInfo + Node ast.Node + Message string + Suggestion *Suggestion + + // Experimental: fields below are part of the experiment. + // They'll probably be removed or changed over time. + + Func *ast.FuncDecl +} + type Suggestion struct { From token.Pos To token.Pos diff --git a/ruleguard/runner.go b/ruleguard/runner.go index 4f1aabdf..8b7824db 100644 --- a/ruleguard/runner.go +++ b/ruleguard/runner.go @@ -23,6 +23,8 @@ type rulesRunner struct { ctx *RunContext rules *goRuleSet + reportData ReportData + gogrepState gogrep.MatcherState importer *goImporter @@ -291,7 +293,12 @@ func (rr *rulesRunner) handleCommentMatch(rule goCommentRule, m commentMatchData Group: rule.base.group, Line: rule.base.line, } - rr.ctx.Report(info, node, message, suggestion) + rr.reportData.RuleInfo = info + rr.reportData.Node = node + rr.reportData.Message = message + rr.reportData.Suggestion = suggestion + + rr.ctx.Report(&rr.reportData) return true } @@ -322,7 +329,14 @@ func (rr *rulesRunner) handleMatch(rule goRule, m gogrep.MatchData) bool { Group: rule.group, Line: rule.line, } - rr.ctx.Report(info, node, message, suggestion) + rr.reportData.RuleInfo = info + rr.reportData.Node = node + rr.reportData.Message = message + rr.reportData.Suggestion = suggestion + + rr.reportData.Func = rr.filterParams.currentFunc + + rr.ctx.Report(&rr.reportData) return true }