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

ruleguard: use a separate token.FileSet for gogrep parsing #275

Merged
merged 1 commit into from Oct 12, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 12 additions & 9 deletions ruleguard/engine.go
Expand Up @@ -4,6 +4,7 @@ import (
"errors"
"fmt"
"go/ast"
"go/token"
"go/types"
"io"
"io/ioutil"
Expand Down Expand Up @@ -55,11 +56,12 @@ func (e *engine) Load(ctx *LoadContext, filename string, r io.Reader) error {
return err
}
config := irLoaderConfig{
state: e.state,
pkg: pkg,
ctx: ctx,
importer: imp,
itab: typematch.NewImportsTab(stdinfo.Packages),
state: e.state,
pkg: pkg,
ctx: ctx,
importer: imp,
itab: typematch.NewImportsTab(stdinfo.Packages),
gogrepFset: token.NewFileSet(),
}
l := newIRLoader(config)
rset, err := l.LoadFile(filename, irfile)
Expand Down Expand Up @@ -87,10 +89,11 @@ func (e *engine) LoadFromIR(ctx *LoadContext, filename string, f *ir.File) error
debugPrint: ctx.DebugPrint,
})
config := irLoaderConfig{
state: e.state,
ctx: ctx,
importer: imp,
itab: typematch.NewImportsTab(stdinfo.Packages),
state: e.state,
ctx: ctx,
importer: imp,
itab: typematch.NewImportsTab(stdinfo.Packages),
gogrepFset: token.NewFileSet(),
}
l := newIRLoader(config)
rset, err := l.LoadFile(filename, f)
Expand Down
21 changes: 13 additions & 8 deletions ruleguard/ir_loader.go
Expand Up @@ -30,6 +30,8 @@ type irLoaderConfig struct {

pkg *types.Package

gogrepFset *token.FileSet

prefix string
importedPkg string
}
Expand All @@ -41,7 +43,8 @@ type irLoader struct {

pkg *types.Package

file *ir.File
file *ir.File
gogrepFset *token.FileSet

filename string
res *goRuleSet
Expand All @@ -58,12 +61,13 @@ type irLoader struct {

func newIRLoader(config irLoaderConfig) *irLoader {
return &irLoader{
state: config.state,
ctx: config.ctx,
importer: config.importer,
itab: config.itab,
pkg: config.pkg,
prefix: config.prefix,
state: config.state,
ctx: config.ctx,
importer: config.importer,
itab: config.itab,
pkg: config.pkg,
prefix: config.prefix,
gogrepFset: config.gogrepFset,
}
}

Expand Down Expand Up @@ -154,6 +158,7 @@ func (l *irLoader) loadExternFile(prefix, pkgPath, filename string) (*goRuleSet,
pkg: pkg,
importedPkg: pkgPath,
itab: l.itab,
gogrepFset: l.gogrepFset,
}
rset, err := newIRLoader(config).LoadFile(filename, irfile)
if err != nil {
Expand Down Expand Up @@ -297,7 +302,7 @@ func (l *irLoader) loadCommentRule(resultProto goRule, rule ir.Rule) error {
func (l *irLoader) loadSyntaxRule(resultProto goRule, rule ir.Rule) error {
result := resultProto

pat, err := gogrep.Compile(l.ctx.Fset, rule.SyntaxPattern, false)
pat, err := gogrep.Compile(l.gogrepFset, rule.SyntaxPattern, false)
if err != nil {
return l.errorf(rule.Line, err, "parse match pattern")
}
Expand Down
29 changes: 15 additions & 14 deletions ruleguard/ruleguard_error_test.go
Expand Up @@ -4,6 +4,7 @@ import (
"errors"
"fmt"
"go/token"
"regexp"
"strings"
"testing"
)
Expand Down Expand Up @@ -189,57 +190,57 @@ func TestParseRuleError(t *testing.T) {
}{
{
`m.Match("$x").Where(m["x"].Object.Is("abc")).Report("")`,
`abc is not a valid go/types object name`,
`\Qabc is not a valid go/types object name`,
},

{
`m.Match("$x").MatchComment("").Report("")`,
`Match() and MatchComment() can't be combined`,
`\QMatch() and MatchComment() can't be combined`,
},

{
`m.MatchComment("").Match("$x").Report("")`,
`Match() and MatchComment() can't be combined`,
`\QMatch() and MatchComment() can't be combined`,
},

{
`m.Where(m.File().Imports("strings")).Report("no match call")`,
`missing Match() or MatchComment() call`,
`\Qmissing Match() or MatchComment() call`,
},

{
`m.Match("$x").Where(m["x"].Pure)`,
`missing Report() or Suggest() call`,
`\Qmissing Report() or Suggest() call`,
},

{
`m.Match("$x").Match("$x")`,
`Match() can't be repeated`,
`\QMatch() can't be repeated`,
},

{
`m.MatchComment("").MatchComment("")`,
`MatchComment() can't be repeated`,
`\QMatchComment() can't be repeated`,
},

{
`m.Match().Report("$$")`,
`too few arguments in call to m.Match`,
`(?:too few|not enough) arguments in call to m\.Match`,
},

{
`m.MatchComment().Report("$$")`,
`too few arguments in call to m.MatchComment`,
`(?:too few|not enough) arguments in call to m\.MatchComment`,
},

{
`m.MatchComment("(").Report("")`,
`error parsing regexp: missing closing )`,
`\Qerror parsing regexp: missing closing )`,
},

{
`m.Match("func[]").Report("$$")`,
`parse match pattern: cannot parse expr: 1:5: expected '(', found '['`,
`\Qparse match pattern: cannot parse expr: 1:5: expected '(', found '['`,
},
}

Expand All @@ -261,9 +262,9 @@ func TestParseRuleError(t *testing.T) {
continue
}
have := err.Error()
want := test.err
if !strings.Contains(have, want) {
t.Errorf("parse %s: errors mismatch:\nhave: %s\nwant: %s", test.expr, have, want)
wantRE := regexp.MustCompile(test.err)
if !wantRE.MatchString(have) {
t.Errorf("parse %s: errors mismatch:\nhave: %s\nwant: %s", test.expr, have, test.err)
continue
}
}
Expand Down