From b10bc38a81147139ce79c0066fbdbfde14ffa497 Mon Sep 17 00:00:00 2001 From: Iskander Sharipov Date: Wed, 13 Oct 2021 00:31:11 +0300 Subject: [PATCH] ruleguard: make printed IR more compact 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). --- _test/install/binary_gopath/expected3.txt | 8 +++--- _test/install/binary_nogopath/expected3.txt | 8 +++--- ruleguard/ir/ir.go | 9 +++++-- ruleguard/ir_loader.go | 30 ++++++++++++++------- ruleguard/irconv/irconv.go | 24 +++++++++-------- ruleguard/irprint/irprint.go | 16 +++++++++++ 6 files changed, 64 insertions(+), 31 deletions(-) diff --git a/_test/install/binary_gopath/expected3.txt b/_test/install/binary_gopath/expected3.txt index c42d7c17..f5020521 100644 --- a/_test/install/binary_gopath/expected3.txt +++ b/_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) diff --git a/_test/install/binary_nogopath/expected3.txt b/_test/install/binary_nogopath/expected3.txt index 87598a0e..bf9c2f5d 100644 --- a/_test/install/binary_nogopath/expected3.txt +++ b/_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) diff --git a/ruleguard/ir/ir.go b/ruleguard/ir/ir.go index 84818130..6b88c74c 100644 --- a/ruleguard/ir/ir.go +++ b/ruleguard/ir/ir.go @@ -46,8 +46,8 @@ type PackageImport struct { type Rule struct { Line int - SyntaxPattern string - CommentPattern string + SyntaxPatterns []PatternString + CommentPatterns []PatternString ReportTemplate string SuggestTemplate string @@ -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 diff --git a/ruleguard/ir_loader.go b/ruleguard/ir_loader.go index a8f2c751..fa7bf163 100644 --- a/ruleguard/ir_loader.go +++ b/ruleguard/ir_loader.go @@ -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") } @@ -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, diff --git a/ruleguard/irconv/irconv.go b/ruleguard/irconv/irconv.go index 0afa635f..ceb6e816 100644 --- a/ruleguard/irconv/irconv.go +++ b/ruleguard/irconv/irconv.go @@ -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 { diff --git a/ruleguard/irprint/irprint.go b/ruleguard/irprint/irprint.go index ea7ff290..878dcc63 100644 --- a/ruleguard/irprint/irprint.go +++ b/ruleguard/irprint/irprint.go @@ -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++ {