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

all: added support for multi-file rule sets #64

Merged
merged 1 commit into from
Aug 12, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
54 changes: 38 additions & 16 deletions analyzer/analyzer.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package analyzer

import (
"bytes"
"fmt"
"go/ast"
"go/token"
"io"
"os"
"io/ioutil"
"path/filepath"
"strings"

"github.com/quasilyte/go-ruleguard/ruleguard"
Expand All @@ -25,25 +26,35 @@ var (
)

func init() {
Analyzer.Flags.StringVar(&flagRules, "rules", "", "path to a gorules file")
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")
}

type parseRulesResult struct {
rset *ruleguard.GoRuleSet
multiFile bool
}

func runAnalyzer(pass *analysis.Pass) (interface{}, error) {
// TODO(quasilyte): parse config under sync.Once and
// create rule sets from it.

rset, err := readRules()
parseResult, err := readRules()
if err != nil {
return nil, fmt.Errorf("load rules: %v", err)
}
rset := parseResult.rset
multiFile := parseResult.multiFile

ctx := &ruleguard.Context{
Pkg: pass.Pkg,
Types: pass.TypesInfo,
Sizes: pass.TypesSizes,
Fset: pass.Fset,
Report: func(n ast.Node, msg string, s *ruleguard.Suggestion) {
Report: func(info ruleguard.GoRuleInfo, n ast.Node, msg string, s *ruleguard.Suggestion) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I believe this might be a breaking change. For example it breaks:
https://github.com/go-critic/go-critic/blob/master/checkers/ruleguard_checker.go#L81

not sure what your policy is for breaking changes and semantic versioning, but it will break users of packages relying on golangci, go-critic etc and who use go get -u for their dependencies

if multiFile {
msg += fmt.Sprintf(" (%s)", filepath.Base(info.Filename))
}
diag := analysis.Diagnostic{
Pos: n.Pos(),
Message: msg,
Expand Down Expand Up @@ -75,20 +86,31 @@ func runAnalyzer(pass *analysis.Pass) (interface{}, error) {
return nil, nil
}

func readRules() (*ruleguard.GoRuleSet, error) {
var r io.Reader
func readRules() (*parseRulesResult, error) {
fset := token.NewFileSet()

switch {
case flagRules != "":
if flagRules == "" {
return nil, fmt.Errorf("-rules values is empty")
}
f, err := os.Open(flagRules)
if err != nil {
return nil, fmt.Errorf("open rules file: %v", err)
filenames := strings.Split(flagRules, ",")
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(filename, fset, bytes.NewReader(data))
if err != nil {
return nil, fmt.Errorf("parse rules file: %v", err)
}
ruleSets = append(ruleSets, rset)
}
defer f.Close()
r = f
rset := ruleguard.MergeRuleSets(ruleSets)
return &parseRulesResult{rset: rset, multiFile: len(filenames) > 1}, nil

case flagE != "":
ruleText := fmt.Sprintf(`
package gorules
Expand All @@ -97,11 +119,11 @@ func readRules() (*ruleguard.GoRuleSet, error) {
%s.Report("$$")
}`,
flagE)
r = strings.NewReader(ruleText)
r := strings.NewReader(ruleText)
rset, err := ruleguard.ParseRules(flagRules, fset, r)
return &parseRulesResult{rset: rset}, err

default:
return nil, fmt.Errorf("both -e and -rules flags are empty")
}

fset := token.NewFileSet()
return ruleguard.ParseRules(flagRules, fset, r)
}
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 {
filename string
severity string
pat *gogrep.Pattern
msg string
Expand Down
24 changes: 24 additions & 0 deletions ruleguard/merge.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package ruleguard

func mergeRuleSets(toMerge []*GoRuleSet) *GoRuleSet {
out := &GoRuleSet{
local: &scopedGoRuleSet{},
universal: &scopedGoRuleSet{},
}

for _, x := range toMerge {
out.local = appendScopedRuleSet(out.local, x.local)
out.universal = appendScopedRuleSet(out.universal, x.universal)
}

return out
}

func appendScopedRuleSet(dst, src *scopedGoRuleSet) *scopedGoRuleSet {
dst.uncategorized = append(dst.uncategorized, src.uncategorized...)
for cat, rules := range src.rulesByCategory {
dst.rulesByCategory[cat] = append(dst.rulesByCategory[cat], rules...)
dst.categorizedNum += len(rules)
}
return dst
}
11 changes: 7 additions & 4 deletions ruleguard/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ import (
)

type rulesParser struct {
fset *token.FileSet
res *GoRuleSet
types *types.Info
filename string
fset *token.FileSet
res *GoRuleSet
types *types.Info

itab *typematch.ImportsTab
dslImporter types.Importer
Expand Down Expand Up @@ -183,6 +184,7 @@ func newRulesParser() *rulesParser {
}

func (p *rulesParser) ParseFile(filename string, fset *token.FileSet, r io.Reader) (*GoRuleSet, error) {
p.filename = filename
p.fset = fset
p.res = &GoRuleSet{
local: &scopedGoRuleSet{},
Expand Down Expand Up @@ -319,7 +321,8 @@ func (p *rulesParser) parseRule(matcher string, call *ast.CallExpr) error {
dst := p.res.universal
filters := map[string]submatchFilter{}
proto := goRule{
filters: filters,
filename: p.filename,
filters: filters,
}
var alternatives []string

Expand Down
11 changes: 10 additions & 1 deletion ruleguard/ruleguard.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ type Context struct {
Types *types.Info
Sizes types.Sizes
Fset *token.FileSet
Report func(n ast.Node, msg string, s *Suggestion)
Report func(rule GoRuleInfo, n ast.Node, msg string, s *Suggestion)
Pkg *types.Package
}

Expand All @@ -30,7 +30,16 @@ func RunRules(ctx *Context, f *ast.File, rules *GoRuleSet) error {
return newRulesRunner(ctx, rules).run(f)
}

type GoRuleInfo struct {
// Filename is a file that defined this rule.
Filename string
}

type GoRuleSet struct {
universal *scopedGoRuleSet
local *scopedGoRuleSet
}

func MergeRuleSets(toMerge []*GoRuleSet) *GoRuleSet {
return mergeRuleSets(toMerge)
}
5 changes: 4 additions & 1 deletion ruleguard/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,10 @@ func (rr *rulesRunner) handleMatch(rule goRule, m gogrep.MatchData) bool {
To: node.End(),
}
}
rr.ctx.Report(node, message, suggestion)
info := GoRuleInfo{
Filename: rule.filename,
}
rr.ctx.Report(info, node, message, suggestion)
return true
}

Expand Down