mirror of
https://github.com/mgechev/revive.git
synced 2025-04-15 11:36:48 +02:00
[dot-imports] support allow list of packages (#939)
Add the ability to allow list of packages to be dot imported. Add a new don-imports configuration: * `allowedPackages`: (string) comma-separated list of allowed dot import packages Example: ```toml [rule.dot-imports] arguments = [{ allowedPackages = "github.com/onsi/ginkgo/v2,github.com/onsi/gomega" }] ```
This commit is contained in:
parent
12dd587d41
commit
9a2eab34f1
@ -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)
|
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
|
## duplicated-imports
|
||||||
|
|
||||||
|
@ -1,16 +1,23 @@
|
|||||||
package rule
|
package rule
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"fmt"
|
||||||
"go/ast"
|
"go/ast"
|
||||||
|
"sync"
|
||||||
|
|
||||||
"github.com/mgechev/revive/lint"
|
"github.com/mgechev/revive/lint"
|
||||||
)
|
)
|
||||||
|
|
||||||
// DotImportsRule lints given else constructs.
|
// DotImportsRule lints given else constructs.
|
||||||
type DotImportsRule struct{}
|
type DotImportsRule struct {
|
||||||
|
sync.Mutex
|
||||||
|
allowedPackages allowPackages
|
||||||
|
}
|
||||||
|
|
||||||
// Apply applies the rule to given file.
|
// 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
|
var failures []lint.Failure
|
||||||
|
|
||||||
fileAst := file.AST
|
fileAst := file.AST
|
||||||
@ -20,6 +27,7 @@ func (*DotImportsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
|
|||||||
onFailure: func(failure lint.Failure) {
|
onFailure: func(failure lint.Failure) {
|
||||||
failures = append(failures, failure)
|
failures = append(failures, failure)
|
||||||
},
|
},
|
||||||
|
allowPackages: r.allowedPackages,
|
||||||
}
|
}
|
||||||
|
|
||||||
ast.Walk(walker, fileAst)
|
ast.Walk(walker, fileAst)
|
||||||
@ -32,15 +40,49 @@ func (*DotImportsRule) Name() string {
|
|||||||
return "dot-imports"
|
return "dot-imports"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (r *DotImportsRule) configure(arguments lint.Arguments) {
|
||||||
|
r.Lock()
|
||||||
|
defer r.Unlock()
|
||||||
|
|
||||||
|
if r.allowedPackages != nil {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
r.allowedPackages = make(allowPackages)
|
||||||
|
if len(arguments) == 0 {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
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]))
|
||||||
|
}
|
||||||
|
|
||||||
|
if allowedPkgArg, ok := args["allowedPackages"]; ok {
|
||||||
|
if pkgs, ok := allowedPkgArg.([]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)", allowedPkgArg, allowedPkgArg))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
type lintImports struct {
|
type lintImports struct {
|
||||||
file *lint.File
|
file *lint.File
|
||||||
fileAst *ast.File
|
fileAst *ast.File
|
||||||
onFailure func(lint.Failure)
|
onFailure func(lint.Failure)
|
||||||
|
allowPackages allowPackages
|
||||||
}
|
}
|
||||||
|
|
||||||
func (w lintImports) Visit(_ ast.Node) ast.Visitor {
|
func (w lintImports) Visit(_ ast.Node) ast.Visitor {
|
||||||
for _, is := range w.fileAst.Imports {
|
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{
|
w.onFailure(lint.Failure{
|
||||||
Confidence: 1,
|
Confidence: 1,
|
||||||
Failure: "should not use dot imports",
|
Failure: "should not use dot imports",
|
||||||
@ -51,3 +93,14 @@ func (w lintImports) Visit(_ ast.Node) ast.Visitor {
|
|||||||
}
|
}
|
||||||
return nil
|
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
|
||||||
|
}
|
||||||
|
18
test/dot_imports_test.go
Normal file
18
test/dot_imports_test.go
Normal file
@ -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,
|
||||||
|
})
|
||||||
|
}
|
22
testdata/import-dot.go
vendored
Normal file
22
testdata/import-dot.go
vendored
Normal file
@ -0,0 +1,22 @@
|
|||||||
|
// Test that dot imports are flagged.
|
||||||
|
|
||||||
|
package fixtures
|
||||||
|
|
||||||
|
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/
|
||||||
|
"math/rand"
|
||||||
|
tmplt "text/template"
|
||||||
|
|
||||||
|
. "github.com/BurntSushi/toml" // in allowedPackages (not in the standard library)
|
||||||
|
)
|
||||||
|
|
||||||
|
var _ Stringer // from "fmt"
|
||||||
|
var _ = New("fake error")
|
||||||
|
var _ = Background()
|
||||||
|
|
||||||
|
var _ = Position{} // check a package not in the standard library
|
||||||
|
|
||||||
|
var _ = rand.Rand{} // check non-alias package
|
||||||
|
var _ = tmplt.Template{} // check alias package
|
Loading…
x
Reference in New Issue
Block a user