diff --git a/analyzer/testdata/src/filtertest/f1.go b/analyzer/testdata/src/filtertest/f1.go index 043a39c0..953efa1a 100644 --- a/analyzer/testdata/src/filtertest/f1.go +++ b/analyzer/testdata/src/filtertest/f1.go @@ -1,6 +1,9 @@ package filtertest -import "time" +import ( + "os" + "time" +) type implementsAll struct{} @@ -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") +} diff --git a/analyzer/testdata/src/filtertest/imports_filepath.go b/analyzer/testdata/src/filtertest/imports_filepath.go new file mode 100644 index 00000000..af65a7b1 --- /dev/null +++ b/analyzer/testdata/src/filtertest/imports_filepath.go @@ -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") +} diff --git a/analyzer/testdata/src/filtertest/rules.go b/analyzer/testdata/src/filtertest/rules.go index 616baf9d..46419e73 100644 --- a/analyzer/testdata/src/filtertest/rules.go +++ b/analyzer/testdata/src/filtertest/rules.go @@ -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`) } diff --git a/analyzer/testdata/src/filtertest/utils.go b/analyzer/testdata/src/filtertest/utils.go index 695f12ce..73c9ce6f 100644 --- a/analyzer/testdata/src/filtertest/utils.go +++ b/analyzer/testdata/src/filtertest/utils.go @@ -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 diff --git a/dsl/fluent/dsl.go b/dsl/fluent/dsl.go index ca7d8075..5f98cf21 100644 --- a/dsl/fluent/dsl.go +++ b/dsl/fluent/dsl.go @@ -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. @@ -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 } diff --git a/dslgen/dsl_sources.go b/dslgen/dsl_sources.go index 3ba584d3..6c4439c5 100644 --- a/dslgen/dsl_sources.go +++ b/dslgen/dsl_sources.go @@ -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 $.\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 $.\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") diff --git a/ruleguard/gorule.go b/ruleguard/gorule.go index 1192d849..5ab89c8f 100644 --- a/ruleguard/gorule.go +++ b/ruleguard/gorule.go @@ -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 { diff --git a/ruleguard/parser.go b/ruleguard/parser.go index 396410e0..85a9e0c0 100644 --- a/ruleguard/parser.go +++ b/ruleguard/parser.go @@ -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 @@ -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 } } @@ -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 @@ -430,21 +431,21 @@ 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 } } @@ -452,10 +453,22 @@ func (p *rulesParser) walkFilter(dst map[string]submatchFilter, e ast.Expr, nega 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)) @@ -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 { @@ -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 { @@ -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 { @@ -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 { @@ -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 { @@ -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 @@ -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 diff --git a/ruleguard/runner.go b/ruleguard/runner.go index 966a345f..6fcf53de 100644 --- a/ruleguard/runner.go +++ b/ruleguard/runner.go @@ -5,6 +5,7 @@ import ( "go/ast" "go/printer" "io/ioutil" + "strconv" "strings" "github.com/quasilyte/go-ruleguard/internal/mvdan.cc/gogrep" @@ -15,6 +16,7 @@ type rulesRunner struct { rules *GoRuleSet filename string + imports map[string]struct{} src []byte } @@ -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) { @@ -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) { @@ -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 } @@ -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, "$$") { diff --git a/rules.go b/rules.go index 6b00eaf4..fb4bce56 100644 --- a/rules.go +++ b/rules.go @@ -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