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: add support for the File filters in Where clause #83

Merged
merged 1 commit into from
Oct 8, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion analyzer/testdata/src/filtertest/f1.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package filtertest

import "time"
import (
"os"
"time"
)

type implementsAll struct{}

Expand Down Expand Up @@ -103,3 +106,9 @@ func detectParensFilter() {
var err error
parensFilterTest(err, "type is error") // want `YES`
}

func fileFilters1() {
// No matches as this file doesn't import "path/filepath".
importsTest(os.PathSeparator, "path/filepath")
importsTest(os.PathListSeparator, "path/filepath")
}
14 changes: 14 additions & 0 deletions analyzer/testdata/src/filtertest/imports_filepath.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package filtertest

import (
"os"
"path/filepath"
)

func fileFilters2() {
importsTest(os.PathSeparator, "path/filepath") // want `YES`
importsTest(filepath.Separator, "path/filepath")

importsTest(os.PathListSeparator, "path/filepath") // want `YES`
importsTest(filepath.ListSeparator, "path/filepath")
}
8 changes: 8 additions & 0 deletions analyzer/testdata/src/filtertest/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,12 @@ func _(m fluent.Matcher) {
m.Match(`textTest($x, "doesn't match [A-Z]")`).Where(!m["x"].Text.Matches(`[A-Z]`)).Report(`YES`)

m.Match(`parensFilterTest($x, "type is error")`).Where((m["x"].Type.Is(`error`))).Report(`YES`)

m.Match(`importsTest(os.PathSeparator, "path/filepath")`).
Where(m.File().Imports("path/filepath")).
Report(`YES`)

m.Match(`importsTest(os.PathListSeparator, "path/filepath")`).
Where(m.File().Imports("path/filepath")).
Report(`YES`)
}
1 change: 1 addition & 0 deletions analyzer/testdata/src/filtertest/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ func typeTest(args ...interface{}) {}
func pureTest(args ...interface{}) {}
func textTest(args ...interface{}) {}
func parensFilterTest(args ...interface{}) {}
func importsTest(args ...interface{}) {}

func random() int {
return 42
Expand Down
9 changes: 9 additions & 0 deletions dsl/fluent/dsl.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ func (m Matcher) At(v Var) Matcher {
return m
}

// File returns the current file context.
func (m Matcher) File() File { return File{} }

// Var is a pattern variable that describes a named submatch.
type Var struct {
// Pure reports whether expr matched by var is side-effect-free.
Expand Down Expand Up @@ -104,3 +107,9 @@ type MatchedText string

// Matches reports whether the text matches the given regexp pattern.
func (MatchedText) Matches(pattern string) bool { return boolResult }

// File represents the current Go source file.
type File struct{}

// Imports reports whether the current file imports the given path.
func (File) Imports(path string) bool { return boolResult }
2 changes: 1 addition & 1 deletion dslgen/dsl_sources.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
package dslgen

var Fluent = []byte("package fluent\n\n// Matcher is a main API group-level entry point.\n// It's used to define and configure the group rules.\n// It also represents a map of all rule-local variables.\ntype Matcher map[string]Var\n\n// Import loads given package path into a rule group imports table.\n//\n// That table is used during the rules compilation.\n//\n// The table has the following effect on the rules:\n//\t* For type expressions, it's used to resolve the\n//\t full package paths of qualified types, like `foo.Bar`.\n//\t If Import(`a/b/foo`) is called, `foo.Bar` will match\n//\t `a/b/foo.Bar` type during the pattern execution.\nfunc (m Matcher) Import(pkgPath string) {}\n\n// Match specifies a set of patterns that match a rule being defined.\n// Pattern matching succeeds if at least 1 pattern matches.\n//\n// If none of the given patterns matched, rule execution stops.\nfunc (m Matcher) Match(pattern string, alternatives ...string) Matcher {\n\treturn m\n}\n\n// Where applies additional constraint to a match.\n// If a given cond is not satisfied, a match is rejected and\n// rule execution stops.\nfunc (m Matcher) Where(cond bool) Matcher {\n\treturn m\n}\n\n// Report prints a message if associated rule match is successful.\n//\n// A message is a string that can contain interpolated expressions.\n// For every matched variable it's possible to interpolate\n// their printed representation into the message text with $<name>.\n// An entire match can be addressed with $$.\nfunc (m Matcher) Report(message string) Matcher {\n\treturn m\n}\n\n// Suggest assigns a quickfix suggestion for the matched code.\nfunc (m Matcher) Suggest(suggestion string) Matcher {\n\treturn m\n}\n\n// At binds the reported node to a named submatch.\n// If no explicit location is given, the outermost node ($$) is used.\nfunc (m Matcher) At(v Var) Matcher {\n\treturn m\n}\n\n// Var is a pattern variable that describes a named submatch.\ntype Var struct {\n\t// Pure reports whether expr matched by var is side-effect-free.\n\tPure bool\n\n\t// Const reports whether expr matched by var is a constant value.\n\tConst bool\n\n\t// Addressable reports whether the corresponding expression is addressable.\n\t// See https://golang.org/ref/spec#Address_operators.\n\tAddressable bool\n\n\t// Type is a type of a matched expr.\n\tType ExprType\n\n\t// Test is a captured node text as in the source code.\n\tText MatchedText\n}\n\n// ExprType describes a type of a matcher expr.\ntype ExprType struct {\n\t// Size represents expression type size in bytes.\n\tSize int\n}\n\n// AssignableTo reports whether a type is assign-compatible with a given type.\n// See https://golang.org/pkg/go/types/#AssignableTo.\nfunc (ExprType) AssignableTo(typ string) bool { return boolResult }\n\n// ConvertibleTo reports whether a type is conversible to a given type.\n// See https://golang.org/pkg/go/types/#ConvertibleTo.\nfunc (ExprType) ConvertibleTo(typ string) bool { return boolResult }\n\n// Implements reports whether a type implements a given interface.\n// See https://golang.org/pkg/go/types/#Implements.\nfunc (ExprType) Implements(typ string) bool { return boolResult }\n\n// Is reports whether a type is identical to a given type.\nfunc (ExprType) Is(typ string) bool { return boolResult }\n\n// MatchedText represents a source text associated with a matched node.\ntype MatchedText string\n\n// Matches reports whether the text matches the given regexp pattern.\nfunc (MatchedText) Matches(pattern string) bool { return boolResult }\n\n\n\nvar boolResult bool\n\n")
var Fluent = []byte("package fluent\n\n// Matcher is a main API group-level entry point.\n// It's used to define and configure the group rules.\n// It also represents a map of all rule-local variables.\ntype Matcher map[string]Var\n\n// Import loads given package path into a rule group imports table.\n//\n// That table is used during the rules compilation.\n//\n// The table has the following effect on the rules:\n//\t* For type expressions, it's used to resolve the\n//\t full package paths of qualified types, like `foo.Bar`.\n//\t If Import(`a/b/foo`) is called, `foo.Bar` will match\n//\t `a/b/foo.Bar` type during the pattern execution.\nfunc (m Matcher) Import(pkgPath string) {}\n\n// Match specifies a set of patterns that match a rule being defined.\n// Pattern matching succeeds if at least 1 pattern matches.\n//\n// If none of the given patterns matched, rule execution stops.\nfunc (m Matcher) Match(pattern string, alternatives ...string) Matcher {\n\treturn m\n}\n\n// Where applies additional constraint to a match.\n// If a given cond is not satisfied, a match is rejected and\n// rule execution stops.\nfunc (m Matcher) Where(cond bool) Matcher {\n\treturn m\n}\n\n// Report prints a message if associated rule match is successful.\n//\n// A message is a string that can contain interpolated expressions.\n// For every matched variable it's possible to interpolate\n// their printed representation into the message text with $<name>.\n// An entire match can be addressed with $$.\nfunc (m Matcher) Report(message string) Matcher {\n\treturn m\n}\n\n// Suggest assigns a quickfix suggestion for the matched code.\nfunc (m Matcher) Suggest(suggestion string) Matcher {\n\treturn m\n}\n\n// At binds the reported node to a named submatch.\n// If no explicit location is given, the outermost node ($$) is used.\nfunc (m Matcher) At(v Var) Matcher {\n\treturn m\n}\n\n// File returns the current file context.\nfunc (m Matcher) File() File { return File{} }\n\n// Var is a pattern variable that describes a named submatch.\ntype Var struct {\n\t// Pure reports whether expr matched by var is side-effect-free.\n\tPure bool\n\n\t// Const reports whether expr matched by var is a constant value.\n\tConst bool\n\n\t// Addressable reports whether the corresponding expression is addressable.\n\t// See https://golang.org/ref/spec#Address_operators.\n\tAddressable bool\n\n\t// Type is a type of a matched expr.\n\tType ExprType\n\n\t// Text is a captured node text as in the source code.\n\tText MatchedText\n}\n\n// ExprType describes a type of a matcher expr.\ntype ExprType struct {\n\t// Size represents expression type size in bytes.\n\tSize int\n}\n\n// AssignableTo reports whether a type is assign-compatible with a given type.\n// See https://golang.org/pkg/go/types/#AssignableTo.\nfunc (ExprType) AssignableTo(typ string) bool { return boolResult }\n\n// ConvertibleTo reports whether a type is conversible to a given type.\n// See https://golang.org/pkg/go/types/#ConvertibleTo.\nfunc (ExprType) ConvertibleTo(typ string) bool { return boolResult }\n\n// Implements reports whether a type implements a given interface.\n// See https://golang.org/pkg/go/types/#Implements.\nfunc (ExprType) Implements(typ string) bool { return boolResult }\n\n// Is reports whether a type is identical to a given type.\nfunc (ExprType) Is(typ string) bool { return boolResult }\n\n// MatchedText represents a source text associated with a matched node.\ntype MatchedText string\n\n// Matches reports whether the text matches the given regexp pattern.\nfunc (MatchedText) Matches(pattern string) bool { return boolResult }\n\n// File represents the current Go source file.\ntype File struct{}\n\n// Imports reports whether the current file imports the given path.\nfunc (File) Imports(path string) bool { return boolResult }\n\n\n\nvar boolResult bool\n\n")
7 changes: 6 additions & 1 deletion ruleguard/gorule.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@ type goRule struct {
msg string
location string
suggestion string
filters map[string]submatchFilter
filter matchFilter
}

type matchFilter struct {
fileImports []string
sub map[string]submatchFilter
}

type submatchFilter struct {
Expand Down
64 changes: 47 additions & 17 deletions ruleguard/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,10 +319,11 @@ func (p *rulesParser) parseRule(matcher string, call *ast.CallExpr) error {
}

dst := p.res.universal
filters := map[string]submatchFilter{}
proto := goRule{
filename: p.filename,
filters: filters,
filter: matchFilter{
sub: map[string]submatchFilter{},
},
}
var alternatives []string

Expand All @@ -338,7 +339,7 @@ func (p *rulesParser) parseRule(matcher string, call *ast.CallExpr) error {
}

if whereArgs != nil {
if err := p.walkFilter(filters, (*whereArgs)[0], false); err != nil {
if err := p.walkFilter(&proto.filter, (*whereArgs)[0], false); err != nil {
return err
}
}
Expand Down Expand Up @@ -395,7 +396,7 @@ func (p *rulesParser) parseRule(matcher string, call *ast.CallExpr) error {
return nil
}

func (p *rulesParser) walkFilter(dst map[string]submatchFilter, e ast.Expr, negate bool) error {
func (p *rulesParser) walkFilter(dst *matchFilter, e ast.Expr, negate bool) error {
typeAnd := func(x, y func(typeQuery) bool) func(typeQuery) bool {
if x == nil {
return y
Expand Down Expand Up @@ -430,32 +431,44 @@ func (p *rulesParser) walkFilter(dst map[string]submatchFilter, e ast.Expr, nega
y := p.types.Types[e.Y].Value
expectedResult := !negate
if operand.path == "Type.Size" && y != nil {
filter := dst[operand.varName]
filter := dst.sub[operand.varName]
filter.typePred = typeAnd(filter.typePred, func(q typeQuery) bool {
x := constant.MakeInt64(q.ctx.Sizes.Sizeof(q.x))
return expectedResult == constant.Compare(x, e.Op, y)
})
dst[operand.varName] = filter
dst.sub[operand.varName] = filter
return nil
}
if operand.path == "Text" && y != nil {
filter := dst[operand.varName]
filter := dst.sub[operand.varName]
filter.textPred = textAnd(filter.textPred, func(s string) bool {
x := constant.MakeString(s)
return expectedResult == constant.Compare(x, e.Op, y)
})
dst[operand.varName] = filter
dst.sub[operand.varName] = filter
return nil
}
}
case *ast.ParenExpr:
return p.walkFilter(dst, e.X, negate)
}

// File-related filters.
fileOperand := p.toFileFilterOperand(e)
switch fileOperand.path {
case "Imports":
pkgPath, ok := p.toStringValue(fileOperand.args[0])
if !ok {
return p.errorf(fileOperand.args[0], "expected a string literal argument")
}
dst.fileImports = append(dst.fileImports, pkgPath)
return nil
}

// TODO(quasilyte): refactor and extend.
operand := p.toFilterOperand(e)
args := operand.args
filter := dst[operand.varName]
filter := dst.sub[operand.varName]
switch operand.path {
default:
return p.errorf(e, "%s is not a valid filter expression", sprintNode(p.fset, e))
Expand All @@ -465,21 +478,21 @@ func (p *rulesParser) walkFilter(dst map[string]submatchFilter, e ast.Expr, nega
} else {
filter.pure = bool3true
}
dst[operand.varName] = filter
dst.sub[operand.varName] = filter
case "Const":
if negate {
filter.constant = bool3false
} else {
filter.constant = bool3true
}
dst[operand.varName] = filter
dst.sub[operand.varName] = filter
case "Addressable":
if negate {
filter.addressable = bool3false
} else {
filter.addressable = bool3true
}
dst[operand.varName] = filter
dst.sub[operand.varName] = filter
case "Text.Matches":
patternString, ok := p.toStringValue(args[0])
if !ok {
Expand All @@ -493,7 +506,7 @@ func (p *rulesParser) walkFilter(dst map[string]submatchFilter, e ast.Expr, nega
filter.textPred = textAnd(filter.textPred, func(s string) bool {
return wantMatched == re.MatchString(s)
})
dst[operand.varName] = filter
dst.sub[operand.varName] = filter
case "Type.Is":
typeString, ok := p.toStringValue(args[0])
if !ok {
Expand All @@ -508,7 +521,7 @@ func (p *rulesParser) walkFilter(dst map[string]submatchFilter, e ast.Expr, nega
filter.typePred = typeAnd(filter.typePred, func(q typeQuery) bool {
return wantIdentical == pat.MatchIdentical(q.x)
})
dst[operand.varName] = filter
dst.sub[operand.varName] = filter
case "Type.ConvertibleTo":
typeString, ok := p.toStringValue(args[0])
if !ok {
Expand All @@ -525,7 +538,7 @@ func (p *rulesParser) walkFilter(dst map[string]submatchFilter, e ast.Expr, nega
filter.typePred = typeAnd(filter.typePred, func(q typeQuery) bool {
return wantConvertible == types.ConvertibleTo(q.x, y)
})
dst[operand.varName] = filter
dst.sub[operand.varName] = filter
case "Type.AssignableTo":
typeString, ok := p.toStringValue(args[0])
if !ok {
Expand All @@ -542,7 +555,7 @@ func (p *rulesParser) walkFilter(dst map[string]submatchFilter, e ast.Expr, nega
filter.typePred = typeAnd(filter.typePred, func(q typeQuery) bool {
return wantAssignable == types.AssignableTo(q.x, y)
})
dst[operand.varName] = filter
dst.sub[operand.varName] = filter
case "Type.Implements":
typeString, ok := p.toStringValue(args[0])
if !ok {
Expand Down Expand Up @@ -583,7 +596,7 @@ func (p *rulesParser) walkFilter(dst map[string]submatchFilter, e ast.Expr, nega
filter.typePred = typeAnd(filter.typePred, func(q typeQuery) bool {
return wantImplemented == types.Implements(q.x, iface)
})
dst[operand.varName] = filter
dst.sub[operand.varName] = filter
}

return nil
Expand Down Expand Up @@ -621,6 +634,23 @@ func (p *rulesParser) toStringValue(x ast.Node) (string, bool) {
return "", false
}

func (p *rulesParser) toFileFilterOperand(e ast.Expr) filterOperand {
var o filterOperand

call, ok := e.(*ast.CallExpr)
if !ok {
return o
}
selector, ok := call.Fun.(*ast.SelectorExpr)
if !ok {
return o
}

o.args = call.Args
o.path = selector.Sel.Name
return o
}

func (p *rulesParser) toFilterOperand(e ast.Expr) filterOperand {
var o filterOperand

Expand Down
23 changes: 22 additions & 1 deletion ruleguard/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"go/ast"
"go/printer"
"io/ioutil"
"strconv"
"strings"

"github.com/quasilyte/go-ruleguard/internal/mvdan.cc/gogrep"
Expand All @@ -15,6 +16,7 @@ type rulesRunner struct {
rules *GoRuleSet

filename string
imports map[string]struct{}
src []byte
}

Expand Down Expand Up @@ -61,6 +63,7 @@ func (rr *rulesRunner) run(f *ast.File) error {
// TODO(quasilyte): run local rules as well.

rr.filename = rr.ctx.Fset.Position(f.Pos()).Filename
rr.collectImports(f)

for _, rule := range rr.rules.universal.uncategorized {
rule.pat.Match(f, func(m gogrep.MatchData) {
Expand Down Expand Up @@ -88,6 +91,12 @@ func (rr *rulesRunner) run(f *ast.File) error {
}

func (rr *rulesRunner) handleMatch(rule goRule, m gogrep.MatchData) bool {
for _, neededImport := range rule.filter.fileImports {
if _, ok := rr.imports[neededImport]; !ok {
return false
}
}

for name, node := range m.Values {
var expr ast.Expr
switch node := node.(type) {
Expand All @@ -98,7 +107,8 @@ func (rr *rulesRunner) handleMatch(rule goRule, m gogrep.MatchData) bool {
default:
continue
}
filter, ok := rule.filters[name]

filter, ok := rule.filter.sub[name]
if !ok {
continue
}
Expand Down Expand Up @@ -170,6 +180,17 @@ func (rr *rulesRunner) handleMatch(rule goRule, m gogrep.MatchData) bool {
return true
}

func (rr *rulesRunner) collectImports(f *ast.File) {
rr.imports = make(map[string]struct{}, len(f.Imports))
for _, spec := range f.Imports {
s, err := strconv.Unquote(spec.Path.Value)
if err != nil {
continue
}
rr.imports[s] = struct{}{}
}
}

func (rr *rulesRunner) renderMessage(msg string, n ast.Node, nodes map[string]ast.Node, truncate bool) string {
var buf strings.Builder
if strings.Contains(msg, "$$") {
Expand Down
10 changes: 10 additions & 0 deletions rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,16 @@ func _(m fluent.Matcher) {
Suggest(`$a+$b`)
}

func osFilepath(m fluent.Matcher) {
m.Match(`os.PathSeparator`).
Where(m.File().Imports("path/filepath")).
Suggest(`filepath.Separator`)

m.Match(`os.PathListSeparator`).
Where(m.File().Imports("path/filepath")).
Suggest(`filepath.ListSeparator`)
}

// See https://twitter.com/dgryski/status/1281348103505768449
func useMathBits(m fluent.Matcher) {
// RotateLeft
Expand Down