Skip to content

Commit

Permalink
ruleguard: use a separate token.FileSet for gogrep parsing (#275)
Browse files Browse the repository at this point in the history
Also adapt the error tests for go1.17 (error messages changed there).
  • Loading branch information
quasilyte committed Oct 12, 2021
1 parent e6a9d2d commit 477f62c
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 31 deletions.
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

0 comments on commit 477f62c

Please sign in to comment.