Skip to content

Commit

Permalink
ruleguard: make printed IR more compact (#276)
Browse files Browse the repository at this point in the history
This is useful in gocritic where we have more and more
precompiled rules. This change makes generated IR files
2-3 times smaller (less Go code lines).
  • Loading branch information
quasilyte committed Oct 12, 2021
1 parent c59efdc commit 6bbba1d
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 31 deletions.
8 changes: 4 additions & 4 deletions _test/install/binary_gopath/expected3.txt
@@ -1,14 +1,14 @@
/usr/local/go/src/encoding/gob/decode.go:1086:19: interfaceAddr: taking address of interface-typed value (rules.go:27)
/usr/local/go/src/encoding/gob/decode.go:1182:19: interfaceAddr: taking address of interface-typed value (rules.go:27)
/usr/local/go/src/encoding/gob/enc_helpers.go:66:6: boolComparison: omit bool literal in expression (rules1.go:8)
/usr/local/go/src/encoding/gob/enc_helpers.go:66:6: boolComparison: omit bool literal in expression (rules1.go:11)
/usr/local/go/src/encoding/gob/encode.go:691:19: interfaceAddr: taking address of interface-typed value (rules.go:27)
/usr/local/go/src/encoding/json/stream.go:456:25: interfaceAddr: taking address of interface-typed value (rules.go:27)
/usr/local/go/src/encoding/asn1/marshal_test.go:281:28: interfaceAddr: taking address of interface-typed value (rules.go:27)
/usr/local/go/src/encoding/asn1/marshal_test.go:303:28: interfaceAddr: taking address of interface-typed value (rules.go:27)
/usr/local/go/src/encoding/base32/base32_test.go:99:4: exprUnparen: the parentheses around p.encoded[len(p.encoded)-1] == '=' are superfluous (rules.go:21)
/usr/local/go/src/encoding/binary/binary_test.go:447:5: boolComparison: omit bool literal in expression (rules1.go:8)
/usr/local/go/src/encoding/binary/binary_test.go:450:5: boolComparison: omit bool literal in expression (rules1.go:8)
/usr/local/go/src/encoding/gob/codec_test.go:351:6: boolComparison: omit bool literal in expression (rules1.go:8)
/usr/local/go/src/encoding/binary/binary_test.go:447:5: boolComparison: omit bool literal in expression (rules1.go:9)
/usr/local/go/src/encoding/binary/binary_test.go:450:5: boolComparison: omit bool literal in expression (rules1.go:9)
/usr/local/go/src/encoding/gob/codec_test.go:351:6: boolComparison: omit bool literal in expression (rules1.go:9)
/usr/local/go/src/encoding/gob/codec_test.go:1402:23: interfaceAddr: taking address of interface-typed value (rules.go:27)
/usr/local/go/src/encoding/gob/codec_test.go:1415:23: interfaceAddr: taking address of interface-typed value (rules.go:27)
/usr/local/go/src/encoding/gob/encoder_test.go:637:18: interfaceAddr: taking address of interface-typed value (rules.go:27)
Expand Down
8 changes: 4 additions & 4 deletions _test/install/binary_nogopath/expected3.txt
@@ -1,14 +1,14 @@
/usr/local/go/src/encoding/gob/decode.go:1086:19: interfaceAddr: taking address of interface-typed value (rules.go:29)
/usr/local/go/src/encoding/gob/decode.go:1182:19: interfaceAddr: taking address of interface-typed value (rules.go:29)
/usr/local/go/src/encoding/gob/enc_helpers.go:66:6: boolComparison: omit bool literal in expression (rules1.go:8)
/usr/local/go/src/encoding/gob/enc_helpers.go:66:6: boolComparison: omit bool literal in expression (rules1.go:11)
/usr/local/go/src/encoding/gob/encode.go:691:19: interfaceAddr: taking address of interface-typed value (rules.go:29)
/usr/local/go/src/encoding/json/stream.go:456:25: interfaceAddr: taking address of interface-typed value (rules.go:29)
/usr/local/go/src/encoding/asn1/marshal_test.go:281:28: interfaceAddr: taking address of interface-typed value (rules.go:29)
/usr/local/go/src/encoding/asn1/marshal_test.go:303:28: interfaceAddr: taking address of interface-typed value (rules.go:29)
/usr/local/go/src/encoding/base32/base32_test.go:99:4: exprUnparen: the parentheses around p.encoded[len(p.encoded)-1] == '=' are superfluous (rules.go:23)
/usr/local/go/src/encoding/binary/binary_test.go:447:5: boolComparison: omit bool literal in expression (rules1.go:8)
/usr/local/go/src/encoding/binary/binary_test.go:450:5: boolComparison: omit bool literal in expression (rules1.go:8)
/usr/local/go/src/encoding/gob/codec_test.go:351:6: boolComparison: omit bool literal in expression (rules1.go:8)
/usr/local/go/src/encoding/binary/binary_test.go:447:5: boolComparison: omit bool literal in expression (rules1.go:9)
/usr/local/go/src/encoding/binary/binary_test.go:450:5: boolComparison: omit bool literal in expression (rules1.go:9)
/usr/local/go/src/encoding/gob/codec_test.go:351:6: boolComparison: omit bool literal in expression (rules1.go:9)
/usr/local/go/src/encoding/gob/codec_test.go:1402:23: interfaceAddr: taking address of interface-typed value (rules.go:29)
/usr/local/go/src/encoding/gob/codec_test.go:1415:23: interfaceAddr: taking address of interface-typed value (rules.go:29)
/usr/local/go/src/encoding/gob/encoder_test.go:637:18: interfaceAddr: taking address of interface-typed value (rules.go:29)
Expand Down
9 changes: 7 additions & 2 deletions ruleguard/ir/ir.go
Expand Up @@ -46,8 +46,8 @@ type PackageImport struct {
type Rule struct {
Line int

SyntaxPattern string
CommentPattern string
SyntaxPatterns []PatternString
CommentPatterns []PatternString

ReportTemplate string
SuggestTemplate string
Expand All @@ -57,6 +57,11 @@ type Rule struct {
LocationVar string
}

type PatternString struct {
Line int
Value string
}

// stringer -type=FilterOp -trimprefix=Filter

//go:generate go run ./gen_filter_op.go
Expand Down
30 changes: 20 additions & 10 deletions ruleguard/ir_loader.go
Expand Up @@ -277,32 +277,42 @@ func (l *irLoader) loadRule(rule ir.Rule) error {
proto.filter = filter
}

if rule.SyntaxPattern != "" {
return l.loadSyntaxRule(proto, rule)
for _, pat := range rule.SyntaxPatterns {
if err := l.loadSyntaxRule(proto, rule, pat.Value, pat.Line); err != nil {
return err
}
}
for _, pat := range rule.CommentPatterns {
if err := l.loadCommentRule(proto, rule, pat.Value, pat.Line); err != nil {
return err
}
}
return l.loadCommentRule(proto, rule)
return nil
}

func (l *irLoader) loadCommentRule(resultProto goRule, rule ir.Rule) error {
func (l *irLoader) loadCommentRule(resultProto goRule, rule ir.Rule, src string, line int) error {
dst := l.res.universal
pat, err := regexp.Compile(rule.CommentPattern)
pat, err := regexp.Compile(src)
if err != nil {
return l.errorf(rule.Line, err, "compile regexp")
}
resultBase := resultProto
resultBase.line = line
result := goCommentRule{
base: resultProto,
pat: pat,
captureGroups: regexpHasCaptureGroups(rule.CommentPattern),
captureGroups: regexpHasCaptureGroups(src),
}
dst.commentRules = append(dst.commentRules, result)

return nil
}

func (l *irLoader) loadSyntaxRule(resultProto goRule, rule ir.Rule) error {
func (l *irLoader) loadSyntaxRule(resultProto goRule, rule ir.Rule, src string, line int) error {
result := resultProto
result.line = line

pat, err := gogrep.Compile(l.gogrepFset, rule.SyntaxPattern, false)
pat, err := gogrep.Compile(l.gogrepFset, src, false)
if err != nil {
return l.errorf(rule.Line, err, "parse match pattern")
}
Expand All @@ -312,9 +322,9 @@ func (l *irLoader) loadSyntaxRule(resultProto goRule, rule ir.Rule) error {
var dstTags []nodetag.Value
switch tag := pat.NodeTag(); tag {
case nodetag.Unknown:
return l.errorf(rule.Line, nil, "can't infer a tag of %s", rule.SyntaxPattern)
return l.errorf(rule.Line, nil, "can't infer a tag of %s", src)
case nodetag.Node:
return l.errorf(rule.Line, nil, "%s pattern is too general", rule.SyntaxPattern)
return l.errorf(rule.Line, nil, "%s pattern is too general", src)
case nodetag.StmtList:
dstTags = []nodetag.Value{
nodetag.BlockStmt,
Expand Down
24 changes: 13 additions & 11 deletions ruleguard/irconv/irconv.go
Expand Up @@ -354,43 +354,45 @@ func (conv *converter) convertRuleExpr(call *ast.CallExpr) {
}
}

proto := ir.Rule{}
rule := ir.Rule{Line: conv.fset.Position(origCall.Pos()).Line}

if atArgs != nil {
index, ok := (*atArgs)[0].(*ast.IndexExpr)
if !ok {
panic(conv.errorf((*atArgs)[0], "expected %s[`varname`] expression", conv.group.MatcherName))
}
proto.LocationVar = conv.parseStringArg(index.Index)
rule.LocationVar = conv.parseStringArg(index.Index)
}

if whereArgs != nil {
proto.WhereExpr = conv.convertFilterExpr((*whereArgs)[0])
rule.WhereExpr = conv.convertFilterExpr((*whereArgs)[0])
}

if suggestArgs != nil {
proto.SuggestTemplate = conv.parseStringArg((*suggestArgs)[0])
rule.SuggestTemplate = conv.parseStringArg((*suggestArgs)[0])
}

if suggestArgs == nil && reportArgs == nil {
panic(conv.errorf(origCall, "missing Report() or Suggest() call"))
}
if reportArgs == nil {
proto.ReportTemplate = "suggestion: " + proto.SuggestTemplate
rule.ReportTemplate = "suggestion: " + rule.SuggestTemplate
} else {
proto.ReportTemplate = conv.parseStringArg((*reportArgs)[0])
rule.ReportTemplate = conv.parseStringArg((*reportArgs)[0])
}

for i, alt := range alternatives {
rule := proto
rule.Line = alternativeLines[i]
pat := ir.PatternString{
Line: alternativeLines[i],
Value: alt,
}
if matchArgs != nil {
rule.SyntaxPattern = alt
rule.SyntaxPatterns = append(rule.SyntaxPatterns, pat)
} else {
rule.CommentPattern = alt
rule.CommentPatterns = append(rule.CommentPatterns, pat)
}
conv.group.Rules = append(conv.group.Rules, rule)
}
conv.group.Rules = append(conv.group.Rules, rule)
}

func (conv *converter) convertFilterExpr(e ast.Expr) ir.FilterExpr {
Expand Down
16 changes: 16 additions & 0 deletions ruleguard/irprint/irprint.go
Expand Up @@ -67,6 +67,22 @@ func (p *printer) printReflectElem(key string, v reflect.Value) {
return
}

// There are tons of this, print it in a compact way.
if v.Type().Name() == "PatternString" {
v := v.Interface().(ir.PatternString)
p.writef("ir.PatternString{Line: %d, Value: %#v},\n", v.Line, v.Value)
return
}

if v.Type().Name() == "FilterExpr" {
v := v.Interface().(ir.FilterExpr)
if v.Op == ir.FilterStringOp || v.Op == ir.FilterVarPureOp || v.Op == ir.FilterVarTextOp {
p.writef("ir.FilterExpr{Line: %d, Op: ir.Filter%sOp, Src: %#v, Value: %#v},\n",
v.Line, v.Op.String(), v.Src, v.Value.(string))
return
}
}

if v.Type().Kind() == reflect.Struct {
p.writef("%s{\n", v.Type().String())
for i := 0; i < v.NumField(); i++ {
Expand Down

0 comments on commit 6bbba1d

Please sign in to comment.