diff --git a/.circleci/config.yml b/.circleci/config.yml index 49aacbd..4eec796 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -3,11 +3,11 @@ version: 2 jobs: build: docker: - - image: cimg/go:1.17.7 + - image: cimg/go:1.19.5 steps: - checkout - - run: sudo apt-get update && sudo apt-get -y -qq install python pip - - run: pip install pre-commit + - run: sudo apt-get update && sudo apt-get -y -qq install python3 python3-pip + - run: pip3 install pre-commit - run: SKIP=no-commit-to-branch pre-commit run -a - run: go test ./... - run: make test diff --git a/.golangci.yml b/.golangci.yml index 2d89202..9688026 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,3 +1,4 @@ linters: enable: - prealloc + - nakedret diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 925f136..45dcf46 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,5 +1,5 @@ repos: - repo: https://github.com/golangci/golangci-lint - rev: v1.44.2 + rev: v1.50.1 hooks: - id: golangci-lint diff --git a/Makefile b/Makefile index 8d17ac8..191c886 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,12 @@ SHELL=bash +setup: + pre-commit install + test: cd examples && diff <(sed 's|CURDIR|$(CURDIR)|' expected_results.txt) <(go run .. 2>&1 | sed '/^go: downloading/d') + +lint: + pre-commit run --all-files + +.PHONY: lint test diff --git a/README.md b/README.md index b4bbf58..ca50a2c 100644 --- a/README.md +++ b/README.md @@ -16,20 +16,121 @@ If no patterns are specified, the default pattern of `^(fmt\.Print.*|print|print functions (and whole files), that are identifies as Godoc examples (https://blog.golang.org/examples) are excluded from checking. -A larger set of interesting patterns might include: +By default, patterns get matched against the actual expression as it appears in +the source code. The effect is that ``^fmt\.Print.*$` will not match when that +package gets imported with `import fmt2 "fmt"` and then the function gets +called with `fmt2.Print`. + +This makes it hard to match packages that may get imported under a variety of +different names, for example because there is no established convention or the +name is so generic that import aliases have to be used. To solve this, +forbidigo also supports a more advanced mode where it uses type information to +identify what an expression references. This needs to be enabled through the +`analyze_types` command line parameter. Beware this may have a performance +impact because additional information is required for the analysis. + +Replacing the literal source code works for items in a package as in the +`fmt2.Print` example above and also for struct fields and methods. For those, +`..` replaces the source code +text. `` is what the package declares in its `package` statement, +which may be different from last part of the import path: + + import "example.com/some/pkg" // pkg uses `package somepkg` + s := somepkg.SomeStruct{} + s.SomeMethod() // -> somepkg.SomeStruct.SomeMethod + +Pointers are treated like the type they point to: + + var cf *spew.ConfigState = ... + cf.Dump() // -> spew.ConfigState.Dump + +When a type is an alias for a type in some other package, the name of that +other package will be used. + +An imported identifier gets replaced as if it had been imported without `import .` +*and* also gets matched literally, so in this example both `^ginkgo.FIt$` +and `^FIt$` would catch the usage of `FIt`: + + import . "github.com/onsi/ginkgo/v2" + FIt(...) // -> ginkgo.FIt, FIt + +Beware that looking up the package name has limitations. When a struct embeds +some other type, references to the inherited fields or methods get resolved +with the outer struct as type: + + package foo + + type InnerStruct { + SomeField int + } + + func (i innerStruct) SomeMethod() {} + + type OuterStruct { + InnerStruct + } -* `^fmt\.Print.*$` -- forbid use of Print statements because they are likely just for debugging -* `^fmt\.Errorf$` -- forbid Errorf in favor of using github.com/pkg/errors -* `^ginkgo\.F[A-Z].*$` -- forbid ginkgo focused commands (used for debug issues) -* `^spew\.Dump$` -- forbid dumping detailed data to stdout -* `^fmt\.Errorf(# please use github\.com/pkg/errors)?$` -- forbid Errorf, with a custom message + s := OuterStruct{} + s.SomeMethod() // -> foo.OuterStruct.SomeMethod + i := s.SomeField // -> foo.OuterStruct.SomeField + +When a method gets called via some interface, that invocation also only +gets resolved to the interface, not the underlying implementation: + + // innerStruct as above + + type myInterface interface { + SomeMethod() + } + + var i myInterface = InnerStruct{} + i.SomeMethod() // -> foo.myInterface.SomeMethod + +Using the package name is simple, but the name is not necessarily unique. For +more advanced cases, it is possible to specify more complex patterns. Such +patterns are strings that contain JSON or YAML for a struct. + +The full pattern struct has the following fields: + +* `msg`: an additional comment that gets added to the error message when a + pattern matches. +* `pattern`: the regular expression that matches the source code or expanded + expression, depending on the global flag. +* `package`: a regular expression for the full package import path. The package + path includes the package version if the package has a version >= 2. This is + only supported when `analyze_types` is enabled. + +To distinguish such patterns from traditional regular expression patterns, the +encoding must start with a `{` or contain line breaks. When using just JSON +encoding, backslashes must get quoted inside strings. When using YAML, this +isn't necessary. The following pattern strings are equivalent: + + {p: "^fmt\\.Println$", msg: "do not write to stdout"} + + {p: ^fmt\.Println$, + msg: do not write to stdout, + } + + {p: ^fmt\.Println$, msg: do not write to stdout} + + p: ^fmt\.Println$ + msg: do not write to stdout + +A larger set of interesting patterns might include: -Note that the linter has no knowledge of what packages were actually imported, so aliased imports will match these patterns. +-* `^fmt\.Print.*$` -- forbid use of Print statements because they are likely just for debugging +-* `^fmt\.Errorf$` -- forbid Errorf in favor of using github.com/pkg/errors +-* `^ginkgo\.F[A-Z].*$` -- forbid ginkgo focused commands (used for debug issues) +-* `^spew\.Dump$` -- forbid dumping detailed data to stdout +-* `^spew.ConfigState\.Dump$` -- also forbid it via a `ConfigState` +-* `^fmt\.Errorf(# please use github\.com/pkg/errors)?$` -- forbid Errorf, with a custom message +-* `{p: ^fmt\.Errorf$, msg: please use github.com/pkg/errors}` -- the same with separate msg field ### Flags - **-set_exit_status** (default false) - Set exit status to 1 if any issues are found. - **-exclude_godoc_examples** (default true) - Controls whether godoc examples are identified and excluded - **-tests** (default true) - Controls whether tests are included +- **-analyze_types** (default false) - Replace literal source code before matching ## Purpose diff --git a/forbidigo/forbidigo.go b/forbidigo/forbidigo.go index 9b7d46b..9b37654 100644 --- a/forbidigo/forbidigo.go +++ b/forbidigo/forbidigo.go @@ -7,6 +7,7 @@ import ( "go/ast" "go/printer" "go/token" + "go/types" "log" "regexp" "strings" @@ -97,19 +98,43 @@ type visitor struct { linter *Linter comments []*ast.CommentGroup - fset *token.FileSet - issues []Issue + runConfig RunConfig + issues []Issue } +// Deprecated: Run was the original entrypoint before RunWithConfig was introduced to support +// additional match patterns that need additional information. func (l *Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) { - var issues []Issue + return l.RunWithConfig(RunConfig{Fset: fset}, nodes...) +} + +// RunConfig provides information that the linter needs for different kinds +// of match patterns. Ideally, all fields should get set. More fields may get +// added in the future as needed. +type RunConfig struct { + // FSet is required. + Fset *token.FileSet + + // TypesInfo is needed for expanding source code expressions. + // Nil disables that step, i.e. patterns match the literal source code. + TypesInfo *types.Info + + // DebugLog is used to print debug messages. May be nil. + DebugLog func(format string, args ...interface{}) +} + +func (l *Linter) RunWithConfig(config RunConfig, nodes ...ast.Node) ([]Issue, error) { + if config.DebugLog == nil { + config.DebugLog = func(format string, args ...interface{}) {} + } + var issues []Issue for _, node := range nodes { var comments []*ast.CommentGroup isTestFile := false isWholeFileExample := false if file, ok := node.(*ast.File); ok { comments = file.Comments - fileName := fset.Position(file.Pos()).Filename + fileName := config.Fset.Position(file.Pos()).Filename isTestFile = strings.HasSuffix(fileName, "_test.go") // From https://blog.golang.org/examples, a "whole file example" is: @@ -145,7 +170,7 @@ func (l *Linter) Run(fset *token.FileSet, nodes ...ast.Node) ([]Issue, error) { cfg: l.cfg, isTestFile: isTestFile, linter: l, - fset: fset, + runConfig: config, comments: comments, } ast.Walk(&visitor, node) @@ -163,41 +188,169 @@ func (v *visitor) Visit(node ast.Node) ast.Visitor { return nil } return v + // The following two are handled below. case *ast.SelectorExpr: case *ast.Ident: + // Everything else isn't. default: return v } + + // The text as it appears in the source is always used because issues + // use that. It's used for matching unless usage of type information + // is enabled. + srcText := v.textFor(node) + matchTexts, pkgText := v.expandMatchText(node, srcText) + v.runConfig.DebugLog("%s: match %v, package %q", v.runConfig.Fset.Position(node.Pos()), matchTexts, pkgText) for _, p := range v.linter.patterns { - if p.pattern.MatchString(v.textFor(node)) && !v.permit(node) { + if p.matches(matchTexts) && + (p.Package == "" || p.pkgRe.MatchString(pkgText)) && + !v.permit(node) { v.issues = append(v.issues, UsedIssue{ - identifier: v.textFor(node), - pattern: p.pattern.String(), + identifier: srcText, // Always report the expression as it appears in the source code. + pattern: p.re.String(), pos: node.Pos(), - position: v.fset.Position(node.Pos()), - customMsg: p.msg, + position: v.runConfig.Fset.Position(node.Pos()), + customMsg: p.Msg, }) } } return nil } +// textFor returns the expression as it appears in the source code (for +// example, .). func (v *visitor) textFor(node ast.Node) string { buf := new(bytes.Buffer) - if err := printer.Fprint(buf, v.fset, node); err != nil { - log.Fatalf("ERROR: unable to print node at %s: %s", v.fset.Position(node.Pos()), err) + if err := printer.Fprint(buf, v.runConfig.Fset, node); err != nil { + log.Fatalf("ERROR: unable to print node at %s: %s", v.runConfig.Fset.Position(node.Pos()), err) } return buf.String() } +// expandMatchText expands the selector in a selector expression to the full package +// name and (for variables) the type: +// +// - example.com/some/pkg.Function +// - example.com/some/pkg.CustomType.Method +// +// It updates the text to match against and fills the package string if possible, +// otherwise it just returns. +func (v *visitor) expandMatchText(node ast.Node, srcText string) (matchTexts []string, pkgText string) { + // The text to match against is the literal source code if we cannot + // come up with something different. + matchText := srcText + + if v.runConfig.TypesInfo == nil { + return []string{matchText}, pkgText + } + + location := v.runConfig.Fset.Position(node.Pos()) + + switch node := node.(type) { + case *ast.Ident: + object, ok := v.runConfig.TypesInfo.Uses[node] + if !ok { + // No information about the identifier. Should + // not happen, but perhaps there were compile + // errors? + v.runConfig.DebugLog("%s: unknown identifier %q", location, srcText) + return []string{matchText}, pkgText + } + if pkg := object.Pkg(); pkg != nil { + pkgText = pkg.Path() + v.runConfig.DebugLog("%s: identifier: %q -> %q in package %q", location, srcText, matchText, pkgText) + // match either with or without package name + return []string{pkg.Name() + "." + srcText, srcText}, pkgText + } else { + v.runConfig.DebugLog("%s: identifier: %q -> %q without package", location, srcText, matchText) + } + return []string{matchText}, pkgText + case *ast.SelectorExpr: + selector := node.X + field := node.Sel.Name + + // If we are lucky, the entire selector expression has a known + // type. We don't care about the value. + selectorText := v.textFor(node) + if typeAndValue, ok := v.runConfig.TypesInfo.Types[selector]; ok { + m, p, ok := pkgFromType(typeAndValue.Type) + if !ok { + v.runConfig.DebugLog("%s: selector %q with supported type %T", location, selectorText, typeAndValue.Type) + } + matchText = m + "." + field + pkgText = p + v.runConfig.DebugLog("%s: selector %q with supported type %q: %q -> %q, package %q", location, selectorText, typeAndValue.Type.String(), srcText, matchText, pkgText) + return []string{matchText}, pkgText + } + // Some expressions need special treatment. + switch selector := selector.(type) { + case *ast.Ident: + object, ok := v.runConfig.TypesInfo.Uses[selector] + if !ok { + // No information about the identifier. Should + // not happen, but perhaps there were compile + // errors? + v.runConfig.DebugLog("%s: unknown selector identifier %q", location, selectorText) + return []string{matchText}, pkgText + } + switch object := object.(type) { + case *types.PkgName: + pkgText = object.Imported().Path() + matchText = object.Imported().Name() + "." + field + v.runConfig.DebugLog("%s: selector %q is package: %q -> %q, package %q", location, selectorText, srcText, matchText, pkgText) + return []string{matchText}, pkgText + case *types.Var: + m, p, ok := pkgFromType(object.Type()) + if !ok { + v.runConfig.DebugLog("%s: selector %q is variable with unsupported type %T", location, selectorText, object.Type()) + } + matchText = m + "." + field + pkgText = p + v.runConfig.DebugLog("%s: selector %q is variable of type %q: %q -> %q, package %q", location, selectorText, object.Type().String(), srcText, matchText, pkgText) + default: + // Something else? + v.runConfig.DebugLog("%s: selector %q is identifier with unsupported type %T", location, selectorText, object) + } + default: + v.runConfig.DebugLog("%s: selector %q of unsupported type %T", location, selectorText, selector) + } + return []string{matchText}, pkgText + default: + v.runConfig.DebugLog("%s: unsupported type %T", location, node) + return []string{matchText}, pkgText + } +} + +// pkgFromType tries to determine `.` and the full +// package path. This only needs to work for types of a selector in a selector +// expression. +func pkgFromType(t types.Type) (typeStr, pkgStr string, ok bool) { + if ptr, ok := t.(*types.Pointer); ok { + t = ptr.Elem() + } + + switch t := t.(type) { + case *types.Named: + obj := t.Obj() + pkg := obj.Pkg() + if pkg == nil { + return "", "", false + } + return pkg.Name() + "." + obj.Name(), pkg.Path(), true + default: + return "", "", false + } +} + func (v *visitor) permit(node ast.Node) bool { if v.cfg.IgnorePermitDirectives { return false } - nodePos := v.fset.Position(node.Pos()) + nodePos := v.runConfig.Fset.Position(node.Pos()) nolint := regexp.MustCompile(fmt.Sprintf(`^//\s?permit:%s\b`, regexp.QuoteMeta(v.textFor(node)))) for _, c := range v.comments { - commentPos := v.fset.Position(c.Pos()) + commentPos := v.runConfig.Fset.Position(c.Pos()) if commentPos.Line == nodePos.Line && len(c.List) > 0 && nolint.MatchString(c.List[0].Text) { return true } diff --git a/forbidigo/forbidigo_test.go b/forbidigo/forbidigo_test.go index d8b0f8d..f93132a 100644 --- a/forbidigo/forbidigo_test.go +++ b/forbidigo/forbidigo_test.go @@ -1,17 +1,21 @@ package forbidigo import ( - "go/parser" - "go/token" + "go/ast" + "os" + "path" + "regexp" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/tools/go/packages" ) func TestForbiddenIdentifiers(t *testing.T) { t.Run("it finds forbidden identifiers", func(t *testing.T) { linter, _ := NewLinter([]string{`fmt\.Printf`}) - expectIssues(t, linter, ` + expectIssues(t, linter, false, ` package bar func foo() { @@ -19,9 +23,21 @@ func foo() { }`, "use of `fmt.Printf` forbidden by pattern `fmt\\.Printf` at testing.go:5:2") }) + t.Run("it finds forbidden, renamed identifiers", func(t *testing.T) { + linter, _ := NewLinter([]string{`fmt\.Printf`}) + expectIssues(t, linter, true, ` +package bar + +import renamed "fmt" + +func foo() { + renamed.Printf("here i am") +}`, "use of `renamed.Printf` forbidden by pattern `fmt\\.Printf` at testing.go:7:2") + }) + t.Run("displays custom messages", func(t *testing.T) { linter, _ := NewLinter([]string{`^fmt\.Printf(# a custom message)?$`}) - expectIssues(t, linter, ` + expectIssues(t, linter, false, ` package bar func foo() { @@ -31,7 +47,7 @@ func foo() { t.Run("it doesn't require a package on the identifier", func(t *testing.T) { linter, _ := NewLinter([]string{`Printf`}) - expectIssues(t, linter, ` + expectIssues(t, linter, false, ` package bar func foo() { @@ -41,7 +57,7 @@ func foo() { t.Run("allows explicitly permitting otherwise forbidden identifiers", func(t *testing.T) { linter, _ := NewLinter([]string{`fmt\.Printf`}) - expectIssues(t, linter, ` + expectIssues(t, linter, false, ` package bar func foo() { @@ -51,7 +67,7 @@ func foo() { t.Run("allows old notation for explicitly permitting otherwise forbidden identifiers", func(t *testing.T) { linter, _ := NewLinter([]string{`fmt\.Printf`}) - expectIssues(t, linter, ` + expectIssues(t, linter, false, ` package bar func foo() { @@ -61,7 +77,7 @@ func foo() { t.Run("has option to ignore permit directives", func(t *testing.T) { linter, _ := NewLinter([]string{`fmt\.Printf`}, OptionIgnorePermitDirectives(true)) - issues := parseFile(t, linter, "file.go", ` + issues := parseFile(t, linter, false, "file.go", ` package bar func foo() { @@ -72,7 +88,7 @@ func foo() { t.Run("examples are excluded by default in test files", func(t *testing.T) { linter, _ := NewLinter([]string{`fmt\.Printf`}) - issues := parseFile(t, linter, "file_test.go", ` + issues := parseFile(t, linter, false, "file_test.go", ` package bar func ExampleFoo() { @@ -83,7 +99,7 @@ func ExampleFoo() { t.Run("whole file examples are excluded by default", func(t *testing.T) { linter, _ := NewLinter([]string{`fmt\.Printf`}) - issues := parseFile(t, linter, "file_test.go", ` + issues := parseFile(t, linter, false, "file_test.go", ` package bar func Foo() { @@ -98,7 +114,7 @@ func Example() { t.Run("Test functions prevent a file from being considered a whole file example", func(t *testing.T) { linter, _ := NewLinter([]string{`fmt\.Printf`}) - issues := parseFile(t, linter, "file_test.go", ` + issues := parseFile(t, linter, false, "file_test.go", ` package bar func TestFoo() { @@ -112,7 +128,7 @@ func Example() { t.Run("Benchmark functions prevent a file from being considered a whole file example", func(t *testing.T) { linter, _ := NewLinter([]string{`fmt\.Printf`}) - issues := parseFile(t, linter, "file_test.go", ` + issues := parseFile(t, linter, false, "file_test.go", ` package bar func BenchmarkFoo() { @@ -126,7 +142,7 @@ func Example() { t.Run("examples can be included", func(t *testing.T) { linter, _ := NewLinter([]string{`fmt\.Printf`}, OptionExcludeGodocExamples(false)) - issues := parseFile(t, linter, "file.go", ` + issues := parseFile(t, linter, false, "file.go", ` package bar func ExampleFoo() { @@ -134,24 +150,92 @@ func ExampleFoo() { }`) assert.NotEmpty(t, issues) }) + + t.Run("import renames not detected without type information", func(t *testing.T) { + linter, _ := NewLinter([]string{`fmt\.Printf`}, OptionExcludeGodocExamples(false)) + issues := parseFile(t, linter, false, "file.go", ` +package bar + +import fmt2 "fmt" + +func ExampleFoo() { + fmt2.Printf("here i am") +}`) + assert.Empty(t, issues) + }) + + t.Run("import renames detected with type information", func(t *testing.T) { + linter, err := NewLinter([]string{`^fmt\.Printf`}, OptionExcludeGodocExamples(false)) + require.NoError(t, err) + expectIssues(t, linter, true, ` +package bar + +import fmt2 "fmt" + +func ExampleFoo() { + fmt2.Printf("here i am") +}`, "use of `fmt2.Printf` forbidden by pattern `^fmt\\.Printf` at testing.go:7:2") + }) + } -func expectIssues(t *testing.T, linter *Linter, contents string, issues ...string) { - actualIssues := parseFile(t, linter, "testing.go", contents) +// sourcePath matches "at /tmp/TestForbiddenIdentifiersdisplays_custom_messages4260088387/001/testing.go". +var sourcePath = regexp.MustCompile(`at .*/([[:alnum:]]+.go)`) + +func expectIssues(t *testing.T, linter *Linter, expand bool, contents string, issues ...string) { + actualIssues := parseFile(t, linter, expand, "testing.go", contents) actualIssueStrs := make([]string, 0, len(actualIssues)) for _, i := range actualIssues { - actualIssueStrs = append(actualIssueStrs, i.String()) + str := i.String() + str = sourcePath.ReplaceAllString(str, "at $1") + actualIssueStrs = append(actualIssueStrs, str) } assert.ElementsMatch(t, issues, actualIssueStrs) } -func parseFile(t *testing.T, linter *Linter, fileName, contents string) []Issue { - fset := token.NewFileSet() - expr, err := parser.ParseFile(fset, fileName, contents, parser.ParseComments) +func parseFile(t *testing.T, linter *Linter, expand bool, fileName, contents string) []Issue { + // We can use packages.Load if we put a single file into a separate + // directory and parse it with Go modules of. We have to be in that + // directory to use "." as pattern, parsing it via the absolute path + // from the forbidigo project doesn't work ("cannot import absolute + // path"). + tmpDir := t.TempDir() + if err := os.WriteFile(path.Join(tmpDir, fileName), []byte(contents), 0644); err != nil { + t.Fatalf("could not write source file: %v", err) + } + env := os.Environ() + env = append(env, "GO111MODULE=off") + cfg := packages.Config{ + Mode: packages.NeedSyntax | packages.NeedName | packages.NeedFiles | packages.NeedTypes, + Env: env, + Tests: true, + } + if expand { + cfg.Mode |= packages.NeedTypesInfo | packages.NeedDeps + } + pwd, err := os.Getwd() + require.NoError(t, err) + defer func() { + _ = os.Chdir(pwd) + }() + err = os.Chdir(tmpDir) + require.NoError(t, err) + pkgs, err := packages.Load(&cfg, ".") if err != nil { - t.Fatalf("unable to parse file contents: %s", err) + t.Fatalf("could not load packages: %v", err) + } + var issues []Issue + for _, p := range pkgs { + nodes := make([]ast.Node, 0, len(p.Syntax)) + for _, n := range p.Syntax { + nodes = append(nodes, n) + } + newIssues, err := linter.RunWithConfig(RunConfig{Fset: p.Fset, TypesInfo: p.TypesInfo, DebugLog: t.Logf}, nodes...) + if err != nil { + t.Fatalf("failed: %s", err) + } + issues = append(issues, newIssues...) } - issues, err := linter.Run(fset, expr) if err != nil { t.Fatalf("unable to parse file: %s", err) } diff --git a/forbidigo/patterns.go b/forbidigo/patterns.go index 9dc70ec..2692dcd 100644 --- a/forbidigo/patterns.go +++ b/forbidigo/patterns.go @@ -5,24 +5,108 @@ import ( "regexp" "regexp/syntax" "strings" + + "gopkg.in/yaml.v2" ) +// pattern matches code that is not supposed to be used. type pattern struct { - pattern *regexp.Regexp - msg string + re, pkgRe *regexp.Regexp + + // Pattern is the regular expression string that is used for matching. + // It gets matched against the literal source code text or the expanded + // text, depending on the mode in which the analyzer runs. + Pattern string `yaml:"p"` + + // Package is a regular expression for the full package path of + // an imported item. Ignored unless the analyzer is configured to + // determine that information. + Package string `yaml:"pkg,omitempty"` + + // Msg gets printed in addition to the normal message if a match is + // found. + Msg string `yaml:"msg,omitempty"` } +// A yamlPattern pattern in a YAML string may be represented either by a string +// (the traditional regular expression syntax) or a struct (for more complex +// patterns). +type yamlPattern pattern + +func (p *yamlPattern) UnmarshalYAML(unmarshal func(interface{}) error) error { + // Try struct first. It's unlikely that a regular expression string + // is valid YAML for a struct. + var ptrn pattern + if err := unmarshal(&ptrn); err != nil { + errStr := err.Error() + // Didn't work, try plain string. + var ptrn string + if err := unmarshal(&ptrn); err != nil { + return fmt.Errorf("pattern is neither a regular expression string (%s) nor a Pattern struct (%s)", err.Error(), errStr) + } + p.Pattern = ptrn + } else { + *p = yamlPattern(ptrn) + } + return ((*pattern)(p)).validate() +} + +var _ yaml.Unmarshaler = &yamlPattern{} + +// parse accepts a regular expression or, if the string starts with { or contains a line break, a +// JSON or YAML representation of a Pattern. func parse(ptrn string) (*pattern, error) { - ptrnRe, err := regexp.Compile(ptrn) + pattern := &pattern{} + + if strings.HasPrefix(strings.TrimSpace(ptrn), "{") || + strings.Contains(ptrn, "\n") { + // Embedded JSON or YAML. We can decode both with the YAML decoder. + if err := yaml.UnmarshalStrict([]byte(ptrn), pattern); err != nil { + return nil, fmt.Errorf("parsing as JSON or YAML failed: %v", err) + } + } else { + pattern.Pattern = ptrn + } + + if err := pattern.validate(); err != nil { + return nil, err + } + return pattern, nil +} + +func (p *pattern) validate() error { + ptrnRe, err := regexp.Compile(p.Pattern) if err != nil { - return nil, fmt.Errorf("unable to compile pattern `%s`: %s", ptrn, err) + return fmt.Errorf("unable to compile source code pattern `%s`: %s", p.Pattern, err) } - re, err := syntax.Parse(ptrn, syntax.Perl) + re, err := syntax.Parse(p.Pattern, syntax.Perl) if err != nil { - return nil, fmt.Errorf("unable to parse pattern `%s`: %s", ptrn, err) + return fmt.Errorf("unable to parse source code pattern `%s`: %s", p.Pattern, err) } msg := extractComment(re) - return &pattern{pattern: ptrnRe, msg: msg}, nil + if msg != "" { + p.Msg = msg + } + p.re = ptrnRe + + if p.Package != "" { + pkgRe, err := regexp.Compile(p.Package) + if err != nil { + return fmt.Errorf("unable to compile package pattern `%s`: %s", p.Package, err) + } + p.pkgRe = pkgRe + } + + return nil +} + +func (p *pattern) matches(matchTexts []string) bool { + for _, text := range matchTexts { + if p.re.MatchString(text) { + return true + } + } + return false } // Traverse the leaf submatches in the regex tree and extract a comment, if any diff --git a/forbidigo/patterns_test.go b/forbidigo/patterns_test.go index ce4e617..dbea9d1 100644 --- a/forbidigo/patterns_test.go +++ b/forbidigo/patterns_test.go @@ -5,6 +5,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "gopkg.in/yaml.v2" ) func TestParseValidPatterns(t *testing.T) { @@ -12,6 +13,8 @@ func TestParseValidPatterns(t *testing.T) { name string ptrn string expectedComment string + expectedPattern string + expectedPackage string }{ { name: "simple expression, no comment", @@ -41,12 +44,43 @@ func TestParseValidPatterns(t *testing.T) { ptrn: `^fmt\.Println(# Please don't use this!)?$`, expectedComment: "Please don't use this!", }, + { + name: "match import", + ptrn: `{p: "^fmt\\.Println$"}`, + expectedPattern: `^fmt\.Println$`, + }, + { + name: "match import with YAML", + ptrn: `{msg: hello world, +p: ^fmt\.Println$ +}`, + expectedComment: "hello world", + expectedPattern: `^fmt\.Println$`, + }, + { + name: "match import with YAML, no line breaks", + ptrn: `{p: ^fmt\.Println$}`, + expectedPattern: `^fmt\.Println$`, + }, + { + name: "simple YAML", + ptrn: `p: ^fmt\.Println$ +`, + expectedPattern: `^fmt\.Println$`, + }, } { t.Run(tc.name, func(t *testing.T) { ptrn, err := parse(tc.ptrn) require.Nil(t, err) - assert.Equal(t, tc.ptrn, ptrn.pattern.String()) - assert.Equal(t, tc.expectedComment, ptrn.msg) + expectedPattern := tc.expectedPattern + if expectedPattern == "" { + expectedPattern = tc.ptrn + } + assert.Equal(t, expectedPattern, ptrn.re.String(), "pattern") + if assert.Equal(t, tc.expectedPackage, ptrn.Package, "package") && tc.expectedPackage != "" { + assert.Equal(t, tc.expectedPackage, ptrn.pkgRe.String(), "package RE") + } + assert.Equal(t, tc.expectedComment, ptrn.Msg, "comment") }) } } @@ -55,3 +89,70 @@ func TestParseInvalidPattern_ReturnsError(t *testing.T) { _, err := parse(`fmt\`) assert.NotNil(t, err) } + +func TestUnmarshalYAML(t *testing.T) { + for _, tc := range []struct { + name string + yaml string + expectedErr string + expectedComment string + expectedPattern string + }{ + { + name: "string: simple expression, no comment", + yaml: `fmt\.Errorf`, + }, + { + name: "string: contains multiple subexpression, with comment", + yaml: `(f)mt\.Errorf(# a comment)?`, + expectedComment: "a comment", + }, + { + name: "struct: simple expression, no comment", + yaml: `p: fmt\.Errorf`, + expectedPattern: `fmt\.Errorf`, + }, + { + name: "match import with YAML", + yaml: `p: ^fmt\.Println$ +`, + expectedPattern: `^fmt\.Println$`, + }, + { + name: "string: invalid regexp", + yaml: `fmt\`, + expectedErr: "unable to compile source code pattern `fmt\\`: error parsing regexp: trailing backslash at end of expression: ``", + }, + { + name: "struct: invalid regexp", + yaml: `p: fmt\ +`, + expectedErr: "unable to compile source code pattern `fmt\\`: error parsing regexp: trailing backslash at end of expression: ``", + }, + { + name: "invalid struct", + yaml: `Foo: bar`, + expectedErr: `pattern is neither a regular expression string (yaml: unmarshal errors: + line 1: cannot unmarshal !!map into string) nor a Pattern struct (yaml: unmarshal errors: + line 1: field Foo not found in type forbidigo.pattern)`, + }, + } { + t.Run(tc.name, func(t *testing.T) { + var p yamlPattern + err := yaml.UnmarshalStrict([]byte(tc.yaml), &p) + if tc.expectedErr == "" { + require.NoError(t, err) + } else { + require.Error(t, err) + require.Equal(t, tc.expectedErr, err.Error()) + return + } + expectedPattern := tc.expectedPattern + if expectedPattern == "" { + expectedPattern = tc.yaml + } + assert.Equal(t, expectedPattern, p.re.String(), "pattern") + assert.Equal(t, tc.expectedComment, p.Msg, "comment") + }) + } +} diff --git a/go.mod b/go.mod index 45bc675..d2f4e16 100644 --- a/go.mod +++ b/go.mod @@ -5,5 +5,6 @@ go 1.12 require ( github.com/pkg/errors v0.9.1 github.com/stretchr/testify v1.4.0 - golang.org/x/tools v0.0.0-20190916130336-e45ffcd953cc + golang.org/x/tools v0.3.0 + gopkg.in/yaml.v2 v2.2.2 ) diff --git a/go.sum b/go.sum index 545861c..306b6f5 100644 --- a/go.sum +++ b/go.sum @@ -4,23 +4,42 @@ github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/stretchr/objx v0.1.0 h1:4G4v2dO3VZwixGIRoQ5Lfboy6nUhCyYzaqnIAPPhYs4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= -golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2 h1:VklqNMn3ovrHsnt90PveolxSbWFaJdECFbxSq0Mqo2M= +github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= -golang.org/x/net v0.0.0-20190620200207-3b0461eec859 h1:R/3boaszxrf1GEUWTVDzSKVwLmSJpwZ1yqXm8j0v2QI= +golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= +golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= +golang.org/x/mod v0.7.0 h1:LapD9S96VoQRhi/GrNTqeBJFrUjs5UHCAtTlgwA5oZA= +golang.org/x/mod v0.7.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= -golang.org/x/sync v0.0.0-20190423024810-112230192c58 h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU= +golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= +golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= +golang.org/x/net v0.2.0/go.mod h1:KqCZLdyyvdV855qA2rE3GC2aiw5xGR5TEjj8smXukLY= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a h1:1BGLXjeY4akVXGgbC9HugT3Jv3hCI0z56oJR5vAMgBU= +golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o= +golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= -golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg= +golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.2.0 h1:ljd4t30dBnAvMZaQCevtY0xLLD0A+bRZXbgLMLU1F/A= +golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= +golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= +golang.org/x/term v0.2.0/go.mod h1:TVmDHMZPmdnySmBfhjOoOdhjzdE1h4u1VwSiw2l1Nuc= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= -golang.org/x/tools v0.0.0-20190916130336-e45ffcd953cc h1:+GB9/q0gCzmtaIl6WdoJFMS3lPwrR6rpcMyY6jfQHAw= -golang.org/x/tools v0.0.0-20190916130336-e45ffcd953cc/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= -golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7 h1:9zdDQZ7Thm29KFXgAX/+yaf3eVbP7djjWp/dXAppNCc= +golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= +golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= +golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= +golang.org/x/tools v0.3.0 h1:SrNbZl6ECOS1qFzgTdQfWXZM9XBkiA6tkFrH9YSTPHM= +golang.org/x/tools v0.3.0/go.mod h1:/rWhSS2+zyEVwoJf8YAX6L2f0ntZ7Kn/mGgAWcipA5k= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/main.go b/main.go index 568940b..daaca76 100644 --- a/main.go +++ b/main.go @@ -16,6 +16,7 @@ func main() { setExitStatus := flag.Bool("set_exit_status", false, "Set exit status to 1 if any issues are found") includeTests := flag.Bool("tests", true, "Include tests") excludeGodocExamples := flag.Bool("exclude_godoc_examples", true, "Exclude code in godoc examples") + expand := flag.Bool("analyze_types", false, "Replace the literal source code based on the semantic of the code before matching against patterns") flag.Parse() var patterns = []string(nil) @@ -32,22 +33,27 @@ func main() { if patterns == nil { patterns = forbidigo.DefaultPatterns() } + options := []forbidigo.Option{ + forbidigo.OptionExcludeGodocExamples(*excludeGodocExamples), + } + linter, err := forbidigo.NewLinter(patterns, options...) + if err != nil { + log.Fatalf("Could not create linter: %s", err) + } cfg := packages.Config{ Mode: packages.NeedSyntax | packages.NeedName | packages.NeedFiles | packages.NeedTypes, Tests: *includeTests, } + + if *expand { + cfg.Mode |= packages.NeedTypesInfo | packages.NeedDeps + } + pkgs, err := packages.Load(&cfg, flag.Args()[firstPkg:]...) if err != nil { log.Fatalf("Could not load packages: %s", err) } - options := []forbidigo.Option{ - forbidigo.OptionExcludeGodocExamples(*excludeGodocExamples), - } - linter, err := forbidigo.NewLinter(patterns, options...) - if err != nil { - log.Fatalf("Could not create linter: %s", err) - } var issues []forbidigo.Issue for _, p := range pkgs { @@ -55,7 +61,7 @@ func main() { for _, n := range p.Syntax { nodes = append(nodes, n) } - newIssues, err := linter.Run(p.Fset, nodes...) + newIssues, err := linter.RunWithConfig(forbidigo.RunConfig{Fset: p.Fset, TypesInfo: p.TypesInfo}, nodes...) if err != nil { log.Fatalf("failed: %s", err) } diff --git a/pkg/analyzer/analyzer.go b/pkg/analyzer/analyzer.go index a14ba18..43e557f 100644 --- a/pkg/analyzer/analyzer.go +++ b/pkg/analyzer/analyzer.go @@ -29,6 +29,8 @@ type analyzer struct { patterns []string usePermitDirective bool includeExamples bool + expand bool + debugLog func(format string, args ...interface{}) } // NewAnalyzer returns a go/analysis-compatible analyzer @@ -36,14 +38,21 @@ type analyzer struct { // Set "-examples" to analyze godoc examples // Set "-permit=false" to ignore "//permit:" directives. func NewAnalyzer() *analysis.Analyzer { + return newAnalyzer(nil /* no debug output */) +} + +func newAnalyzer(debugLog func(format string, args ...interface{})) *analysis.Analyzer { var flags flag.FlagSet a := analyzer{ usePermitDirective: true, includeExamples: true, + debugLog: debugLog, } + flags.Var(&listVar{values: &a.patterns}, "p", "pattern") flags.BoolVar(&a.includeExamples, "examples", false, "check godoc examples") flags.BoolVar(&a.usePermitDirective, "permit", true, `when set, lines with "//permit" directives will be ignored`) + flags.BoolVar(&a.expand, "analyze_types", false, `when set, expressions get expanded instead of matching the literal source code`) return &analysis.Analyzer{ Name: "forbidigo", Doc: "forbid identifiers", @@ -67,7 +76,11 @@ func (a *analyzer) runAnalysis(pass *analysis.Pass) (interface{}, error) { for _, f := range pass.Files { nodes = append(nodes, f) } - issues, err := linter.Run(pass.Fset, nodes...) + config := forbidigo.RunConfig{Fset: pass.Fset, DebugLog: a.debugLog} + if a.expand { + config.TypesInfo = pass.TypesInfo + } + issues, err := linter.RunWithConfig(config, nodes...) if err != nil { return nil, err } diff --git a/pkg/analyzer/analyzer_test.go b/pkg/analyzer/analyzer_test.go index 6d521cc..b314f8f 100644 --- a/pkg/analyzer/analyzer_test.go +++ b/pkg/analyzer/analyzer_test.go @@ -1,14 +1,50 @@ -package analyzer_test +package analyzer import ( "testing" - "github.com/ashanbrown/forbidigo/pkg/analyzer" + "github.com/ashanbrown/forbidigo/forbidigo" "golang.org/x/tools/go/analysis/analysistest" ) -func TestAnalyzer(t *testing.T) { +func TestLiteralAnalyzer(t *testing.T) { testdata := analysistest.TestData() - a := analyzer.NewAnalyzer() - analysistest.Run(t, testdata, a, "") + patterns := append(forbidigo.DefaultPatterns(), + `^pkg\.Forbidden$`, + `^Shiny`, + `^AlsoShiny`, + `^renamed\.Forbidden`, + ) + a := newAnalyzer(t.Logf) + for _, pattern := range patterns { + if err := a.Flags.Set("p", pattern); err != nil { + t.Fatalf("unexpected error when setting pattern: %v", err) + } + } + analysistest.Run(t, testdata, a, "matchtext") +} + +func TestExpandAnalyzer(t *testing.T) { + testdata := analysistest.TestData() + patterns := append(forbidigo.DefaultPatterns(), + `{p: ^pkg\.Forbidden$, pkg: ^example.com/some/pkg$}`, + `{p: ^pkg\.CustomType.*Forbidden.*$, pkg: ^example.com/some/pkg$}`, + `{p: ^pkg\.CustomInterface.*Forbidden$, pkg: ^example.com/some/pkg$}`, + `{p: ^Shiny, pkg: ^example.com/some/thing$}`, + `{p: ^AlsoShiny}`, + `{p: myCustomStruct\..*Forbidden, pkg: ^expandtext$}`, + `{p: myCustomInterface\.AlsoForbidden, pkg: ^expandtext$}`, + `{p: renamed\.Forbidden, pkg: ^example.com/some/renamedpkg$}`, + `{p: renamed\.Struct.Forbidden, pkg: ^example.com/some/renamedpkg$}`, + ) + a := newAnalyzer(t.Logf) + for _, pattern := range patterns { + if err := a.Flags.Set("p", pattern); err != nil { + t.Fatalf("unexpected error when setting pattern: %v", err) + } + } + if err := a.Flags.Set("analyze_types", "true"); err != nil { + t.Fatalf("unexpected error when enabling expression expansion: %v", err) + } + analysistest.Run(t, testdata, a, "expandtext") } diff --git a/pkg/analyzer/testdata/forbidden.go b/pkg/analyzer/testdata/forbidden.go deleted file mode 100644 index 7966ae7..0000000 --- a/pkg/analyzer/testdata/forbidden.go +++ /dev/null @@ -1,10 +0,0 @@ -package testdata - -import "fmt" - -func Foo() { - fmt.Println("here I am") // want "forbidden by pattern" - fmt.Printf("this is ok") //permit:fmt.Printf // this is ok - print("not ok") // want "forbidden by pattern" - println("also not ok") // want "forbidden by pattern" -} diff --git a/pkg/analyzer/testdata/src/example.com/another/pkg/pkg2.go b/pkg/analyzer/testdata/src/example.com/another/pkg/pkg2.go new file mode 100644 index 0000000..ff37bc0 --- /dev/null +++ b/pkg/analyzer/testdata/src/example.com/another/pkg/pkg2.go @@ -0,0 +1,5 @@ +package pkg + +import "example.com/some/pkg" + +type CustomTypeAlias = pkg.CustomType diff --git a/pkg/analyzer/testdata/src/example.com/some/pkg/pkg.go b/pkg/analyzer/testdata/src/example.com/some/pkg/pkg.go new file mode 100644 index 0000000..99d20ad --- /dev/null +++ b/pkg/analyzer/testdata/src/example.com/some/pkg/pkg.go @@ -0,0 +1,18 @@ +package pkg + +func Forbidden() { +} + +func NewCustom() CustomType { + return CustomType{} +} + +type CustomType struct { + ForbiddenField int +} + +func (c CustomType) AlsoForbidden() {} + +type CustomInterface interface { + StillForbidden() +} diff --git a/pkg/analyzer/testdata/src/example.com/some/renamedpkg/renamed.go b/pkg/analyzer/testdata/src/example.com/some/renamedpkg/renamed.go new file mode 100644 index 0000000..67078f9 --- /dev/null +++ b/pkg/analyzer/testdata/src/example.com/some/renamedpkg/renamed.go @@ -0,0 +1,11 @@ +// Package renamed intentionally uses a package name that does not match the +// import path to test this situation. +package renamed // import "example.com/some/renamedpkg" + +func ForbiddenFunc() { +} + +type Struct struct{} + +func (s Struct) ForbiddenMethod() { +} diff --git a/pkg/analyzer/testdata/src/example.com/some/thing/thing.go b/pkg/analyzer/testdata/src/example.com/some/thing/thing.go new file mode 100644 index 0000000..0e00312 --- /dev/null +++ b/pkg/analyzer/testdata/src/example.com/some/thing/thing.go @@ -0,0 +1,4 @@ +package thing + +var Shiny int +var AlsoShiny int diff --git a/pkg/analyzer/testdata/src/expandtext/expandtext.go b/pkg/analyzer/testdata/src/expandtext/expandtext.go new file mode 100644 index 0000000..039e39e --- /dev/null +++ b/pkg/analyzer/testdata/src/expandtext/expandtext.go @@ -0,0 +1,78 @@ +package testdata + +import ( + "fmt" + alias "fmt" + + anotherpkg "example.com/another/pkg" + somepkg "example.com/some/pkg" + "example.com/some/renamedpkg" // Package name is "renamed". + . "example.com/some/thing" +) + +func myCustom() somepkg.CustomType { + return somepkg.CustomType{} +} + +var myCustomFunc = myCustom + +type myCustomStruct struct { + somepkg.CustomType +} + +type myCustomInterface interface { + AlsoForbidden() +} + +var forbiddenFunctionRef = somepkg.Forbidden // want "somepkg.Forbidden.*forbidden by pattern .*\\^pkg.*Forbidden" + +var forbiddenVariableRef = Shiny // want "Shiny.*forbidden by pattern.*\\^Shiny" +var forbiddenVariableRef2 = AlsoShiny // want "Shiny.*forbidden by pattern.*\\^AlsoShiny" + +func Foo() { + fmt.Println("here I am") // want "forbidden by pattern" + fmt.Printf("this is ok") //permit:fmt.Printf // this is ok + print("not ok") // want "forbidden by pattern" + println("also not ok") // want "forbidden by pattern" + alias.Println("hello") // want "forbidden by pattern" + somepkg.Forbidden() // want "somepkg.Forbidden.*forbidden by pattern .*\\^pkg.*Forbidden" + + c := somepkg.CustomType{} + c.AlsoForbidden() // want "c.AlsoForbidden.*forbidden by pattern.*\\^pkg..CustomType.*Forbidden" + + // Selector expression with result of function call in package. + somepkg.NewCustom().AlsoForbidden() // want "somepkg.NewCustom...AlsoForbidden.*forbidden by pattern.*\\^pkg..CustomType.*Forbidden" + + // Selector expression with result of normal function call. + myCustom().AlsoForbidden() // want "myCustom...AlsoForbidden.*forbidden by pattern.*\\^pkg..CustomType.*Forbidden" + + // Selector expression with result of normal function call. + myCustomFunc().AlsoForbidden() // want "myCustomFunc...AlsoForbidden.*forbidden by pattern.*\\^pkg..CustomType.*Forbidden" + + // Type alias and pointer. + c2 := &anotherpkg.CustomTypeAlias{} + c2.AlsoForbidden() // want "c2.AlsoForbidden.*forbidden by pattern.*\\^pkg..CustomType.*Forbidden" + + // Interface. + var ci somepkg.CustomInterface + ci.StillForbidden() // want "ci.StillForbidden.*forbidden by pattern.*\\^pkg..CustomInterface..Forbidden" + + // Forbidden embedded inside another: must be forbidden separately! + myCustomStruct{}.AlsoForbidden() // want "myCustomStruct...AlsoForbidden.*forbidden by pattern.*myCustomStruct" + _ = myCustomStruct{}.ForbiddenField // want "myCustomStruct...ForbiddenField.*forbidden by pattern.*myCustomStruct" + + // Forbidden method called via interface: must be forbidden separately! + var ci2 myCustomInterface = somepkg.CustomType{} + ci2.AlsoForbidden() // want "ci2.AlsoForbidden.*forbidden by pattern.*myCustomInterface" + + // Package name != import path. + renamed.ForbiddenFunc() // want "renamed.Forbidden.* by pattern .*renamed..Forbidden" + renamed.Struct{}.ForbiddenMethod() // want "renamed.Struct...ForbiddenMethod.* by pattern .*renamed.*Struct.*Forbidden" +} + +func Bar() string { + fmt := struct { + Println string + }{} + return fmt.Println // not matched because it expands to `struct{Println string}.Println` +} diff --git a/pkg/analyzer/testdata/src/matchtext/matchtext.go b/pkg/analyzer/testdata/src/matchtext/matchtext.go new file mode 100644 index 0000000..58470ae --- /dev/null +++ b/pkg/analyzer/testdata/src/matchtext/matchtext.go @@ -0,0 +1,78 @@ +package testdata + +import ( + "fmt" + alias "fmt" + + anotherpkg "example.com/another/pkg" + somepkg "example.com/some/pkg" + renamed "example.com/some/renamedpkg" // Package name is "renamed". + . "example.com/some/thing" +) + +func myCustom() somepkg.CustomType { + return somepkg.CustomType{} +} + +var myCustomFunc = myCustom + +type myCustomStruct struct { + somepkg.CustomType +} + +type myCustomInterface interface { + AlsoForbidden() +} + +var forbiddenFunctionRef = somepkg.Forbidden + +var forbiddenVariableRef = Shiny // want "Shiny.*forbidden by pattern.*\\^Shiny" +var forbiddenVariableRef2 = AlsoShiny // want "Shiny.*forbidden by pattern.*\\^AlsoShiny" + +func Foo() { + fmt.Println("here I am") // want "forbidden by pattern" + fmt.Printf("this is ok") //permit:fmt.Printf // this is ok + print("not ok") // want "forbidden by pattern" + println("also not ok") // want "forbidden by pattern" + alias.Println("hello") // not matched by default pattern fmt.Println + somepkg.Forbidden() + + c := somepkg.CustomType{} + c.AlsoForbidden() + + // Selector expression with result of function call in package. + somepkg.NewCustom().AlsoForbidden() + + // Selector expression with result of normal function call. + myCustom().AlsoForbidden() + + // Selector expression with result of normal function call. + myCustomFunc().AlsoForbidden() + + // Type alias and pointer. + c2 := &anotherpkg.CustomTypeAlias{} + c2.AlsoForbidden() + + // Interface. + var ci somepkg.CustomInterface + ci.StillForbidden() + + // Struct embedded inside another: must be forbidden separately! + myCustomStruct{}.AlsoForbidden() + _ = myCustomStruct{}.ForbiddenField + + // Forbidden method called via interface: must be forbidden separately! + var ci2 myCustomInterface = somepkg.CustomType{} + ci2.AlsoForbidden() + + // Package name != import path. + renamed.ForbiddenFunc() // want "renamed.Forbidden.* by pattern .*renamed..Forbidden" + renamed.Struct{}.ForbiddenMethod() +} + +func Bar() string { + fmt := struct { + Println string + }{} + return fmt.Println // want "forbidden by pattern" +}