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

add custom filter functions initial support #173

Merged
merged 1 commit into from
Jan 8, 2021
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
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