diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index dad372b55..c6096e45a 100644 --- a/RULES_DESCRIPTIONS.md +++ b/RULES_DESCRIPTIONS.md @@ -282,7 +282,17 @@ _Description_: Importing with `.` makes the programs much harder to understand b More information [here](https://github.com/golang/go/wiki/CodeReviewComments#import-dot) -_Configuration_: N/A +_Configuration_: + +* `allowedPackages`: (list of strings) comma-separated list of allowed dot import packages + +Example: + +```toml +[rule.dot-imports] + arguments = [{ allowedPackages = ["github.com/onsi/ginkgo/v2","github.com/onsi/gomega"] }] +``` + ## duplicated-imports diff --git a/rule/dot-imports.go b/rule/dot-imports.go index db78f1957..56c9ad3c3 100644 --- a/rule/dot-imports.go +++ b/rule/dot-imports.go @@ -1,16 +1,23 @@ package rule import ( + "fmt" "go/ast" + "sync" "github.com/mgechev/revive/lint" ) // DotImportsRule lints given else constructs. -type DotImportsRule struct{} +type DotImportsRule struct { + sync.Mutex + allowedPackages allowPackages +} // Apply applies the rule to given file. -func (*DotImportsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (r *DotImportsRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { + r.configure(arguments) + var failures []lint.Failure fileAst := file.AST @@ -20,6 +27,7 @@ func (*DotImportsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { onFailure: func(failure lint.Failure) { failures = append(failures, failure) }, + allowPackages: r.allowedPackages, } ast.Walk(walker, fileAst) @@ -32,15 +40,47 @@ func (*DotImportsRule) Name() string { return "dot-imports" } +func (r *DotImportsRule) configure(arguments lint.Arguments) { + r.Lock() + defer r.Unlock() + + if r.allowedPackages == nil { + r.allowedPackages = make(allowPackages) + if len(arguments) > 0 { + args, ok := arguments[0].(map[string]any) + if !ok { + panic(fmt.Sprintf("Invalid argument to the dot-imports rule. Expecting a k,v map, got %T", arguments[0])) + } + for k, v := range args { + if k == "allowedPackages" { + + if pkgs, ok := v.([]any); ok { + for _, p := range pkgs { + if pkg, ok := p.(string); ok { + r.allowedPackages.add(pkg) + } else { + panic(fmt.Sprintf("Invalid argument to the dot-imports rule, string expected. Got '%v' (%T)", p, p)) + } + } + } else { + panic(fmt.Sprintf("Invalid argument to the dot-imports rule, []string expected. Got '%v' (%T)", v, v)) + } + } + } + } + } +} + type lintImports struct { - file *lint.File - fileAst *ast.File - onFailure func(lint.Failure) + file *lint.File + fileAst *ast.File + onFailure func(lint.Failure) + allowPackages allowPackages } func (w lintImports) Visit(_ ast.Node) ast.Visitor { for _, is := range w.fileAst.Imports { - if is.Name != nil && is.Name.Name == "." { + if is.Name != nil && is.Name.Name == "." && !w.allowPackages.isAllowedPackage(is.Path.Value) { w.onFailure(lint.Failure{ Confidence: 1, Failure: "should not use dot imports", @@ -51,3 +91,14 @@ func (w lintImports) Visit(_ ast.Node) ast.Visitor { } return nil } + +type allowPackages map[string]struct{} + +func (ap allowPackages) add(pkg string) { + ap[fmt.Sprintf(`"%s"`, pkg)] = struct{}{} // import path strings are with double quotes +} + +func (ap allowPackages) isAllowedPackage(pkg string) bool { + _, allowed := ap[pkg] + return allowed +} diff --git a/test/dot_imports_test.go b/test/dot_imports_test.go new file mode 100644 index 000000000..31cd4bd44 --- /dev/null +++ b/test/dot_imports_test.go @@ -0,0 +1,18 @@ +package test + +import ( + "testing" + + "github.com/mgechev/revive/lint" + "github.com/mgechev/revive/rule" +) + +func TestDotImports(t *testing.T) { + args := []any{map[string]any{ + "allowedPackages": []any{"errors", "context", "github.com/BurntSushi/toml"}, + }} + + testRule(t, "import-dot", &rule.DotImportsRule{}, &lint.RuleConfig{ + Arguments: args, + }) +} diff --git a/testdata/import-dot.go b/testdata/import-dot.go new file mode 100644 index 000000000..debdf7648 --- /dev/null +++ b/testdata/import-dot.go @@ -0,0 +1,18 @@ +// Test that dot imports are flagged. + +// Package pkg ... +package pkg + +import ( + . "context" // in allowedPackages (standard library => just the name without full path) + . "errors" // in allowedPackages (standard library => just the name without full path) + . "fmt" // MATCH /should not use dot imports/ + + . "github.com/BurntSushi/toml" // in allowedPackages (not in the standard library) +) + +var _ Stringer // from "fmt" +var _ = New("fake error") +var _ = Background() + +var _ = Position{} // check non a package not in the standard library