Skip to content

Commit

Permalink
[dot-imports] support allow list of packages
Browse files Browse the repository at this point in the history
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" }]
```
  • Loading branch information
nunnatsa committed Nov 21, 2023
1 parent ccae54e commit 09b6755
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 7 deletions.
12 changes: 11 additions & 1 deletion RULES_DESCRIPTIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
63 changes: 57 additions & 6 deletions rule/dot-imports.go
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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)
Expand All @@ -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",
Expand All @@ -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
}
18 changes: 18 additions & 0 deletions test/dot_imports_test.go
Original file line number Diff line number Diff line change
@@ -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,
})
}
18 changes: 18 additions & 0 deletions testdata/import-dot.go
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 09b6755

Please sign in to comment.