Skip to content

Commit

Permalink
Fixes issue #619 imports-blacklist support regex (#684)
Browse files Browse the repository at this point in the history
* Fixes issue #619 imports-blacklist support regex

* refactors method name and error message

* restores original test cases

Co-authored-by: chavacava <salvadorcavadini+github@gmail.com>
  • Loading branch information
ydah and chavacava authored Apr 21, 2022
1 parent a67ecdd commit e10678f
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 14 deletions.
4 changes: 2 additions & 2 deletions RULES_DESCRIPTIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -432,13 +432,13 @@ _Configuration_: N/A

_Description_: Warns when importing black-listed packages.

_Configuration_: black-list of package names
_Configuration_: black-list of package names (or regular expression package names).

Example:

```toml
[imports-blacklist]
arguments =["crypto/md5", "crypto/sha1"]
arguments =["crypto/md5", "crypto/sha1", "crypto/**/pkix"]
```
## import-shadowing

Expand Down
29 changes: 21 additions & 8 deletions rule/imports-blacklist.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,35 +2,48 @@ package rule

import (
"fmt"
"regexp"
"sync"

"github.com/mgechev/revive/lint"
)

// ImportsBlacklistRule lints given else constructs.
type ImportsBlacklistRule struct {
blacklist map[string]bool
blacklist []*regexp.Regexp
sync.Mutex
}

var replaceRegexp = regexp.MustCompile(`/?\*\*/?`)

func (r *ImportsBlacklistRule) configure(arguments lint.Arguments) {
r.Lock()
defer r.Unlock()

if r.blacklist == nil {
r.blacklist = make(map[string]bool, len(arguments))
r.blacklist = make([]*regexp.Regexp, 0)

for _, arg := range arguments {
argStr, ok := arg.(string)
if !ok {
panic(fmt.Sprintf("Invalid argument to the imports-blacklist rule. Expecting a string, got %T", arg))
}
// we add quotes if not present, because when parsed, the value of the AST node, will be quoted
if len(argStr) > 2 && argStr[0] != '"' && argStr[len(argStr)-1] != '"' {
argStr = fmt.Sprintf(`%q`, argStr)
regStr, err := regexp.Compile(fmt.Sprintf(`(?m)"%s"$`, replaceRegexp.ReplaceAllString(argStr, `(\W|\w)*`)))
if err != nil {
panic(fmt.Sprintf("Invalid argument to the imports-blacklist rule. Expecting %q to be a valid regular expression, got: %v", argStr, err))
}
r.blacklist[argStr] = true
r.blacklist = append(r.blacklist, regStr)
}
}
}

func (r *ImportsBlacklistRule) isBlacklisted(path string) bool {
for _, regex := range r.blacklist {
if regex.MatchString(path) {
return true
}
}
r.Unlock()
return false
}

// Apply applies the rule to given file.
Expand All @@ -45,7 +58,7 @@ func (r *ImportsBlacklistRule) Apply(file *lint.File, arguments lint.Arguments)

for _, is := range file.AST.Imports {
path := is.Path
if path != nil && r.blacklist[path.Value] {
if path != nil && r.isBlacklisted(path.Value) {
failures = append(failures, lint.Failure{
Confidence: 1,
Failure: "should not use the following blacklisted import: " + path.Value,
Expand Down
12 changes: 10 additions & 2 deletions test/import-blacklist_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,24 @@ import (
"github.com/mgechev/revive/rule"
)

func TestImportsBlacklist(t *testing.T) {
func TestImportsBlacklistOriginal(t *testing.T) {
args := []interface{}{"crypto/md5", "crypto/sha1"}

testRule(t, "imports-blacklist-original", &rule.ImportsBlacklistRule{}, &lint.RuleConfig{
Arguments: args,
})
}

func TestImportsBlacklist(t *testing.T) {
args := []interface{}{"github.com/full/match", "wildcard/**/between", "wildcard/backward/**", "**/wildcard/forward", "full"}

testRule(t, "imports-blacklist", &rule.ImportsBlacklistRule{}, &lint.RuleConfig{
Arguments: args,
})
}

func BenchmarkImportsBlacklist(b *testing.B) {
args := []interface{}{"crypto/md5", "crypto/sha1"}
args := []interface{}{"github.com/full/match", "wildcard/**/between", "wildcard/backward/**", "**/wildcard/forward", "full"}
var t *testing.T
for i := 0; i <= b.N; i++ {
testRule(t, "imports-blacklist", &rule.ImportsBlacklistRule{}, &lint.RuleConfig{
Expand Down
8 changes: 8 additions & 0 deletions testdata/imports-blacklist-original.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package fixtures

import (
"crypto/md5" // MATCH /should not use the following blacklisted import: "crypto/md5"/
"crypto/sha1" // MATCH /should not use the following blacklisted import: "crypto/sha1"/
"strings"
)

16 changes: 14 additions & 2 deletions testdata/imports-blacklist.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,19 @@
package fixtures

import (
"crypto/md5" // MATCH /should not use the following blacklisted import: "crypto/md5"/
"crypto/sha1" // MATCH /should not use the following blacklisted import: "crypto/sha1"/
"github.com/full/match" // MATCH /should not use the following blacklisted import: "github.com/full/match"/
"bithub.com/full/match"
"github.com/full/matche"
"wildcard/between" // MATCH /should not use the following blacklisted import: "wildcard/between"/
"wildcard/pkg1/between" // MATCH /should not use the following blacklisted import: "wildcard/pkg1/between"/
"wildcard/pkg1/pkg2/between" // MATCH /should not use the following blacklisted import: "wildcard/pkg1/pkg2/between"/
"wildcard/backward" // MATCH /should not use the following blacklisted import: "wildcard/backward"/
"wildcard/backward/pkg" // MATCH /should not use the following blacklisted import: "wildcard/backward/pkg"/
"wildcard/backward/pkg/pkg1" // MATCH /should not use the following blacklisted import: "wildcard/backward/pkg/pkg1"/
"wildcard/forward" // MATCH /should not use the following blacklisted import: "wildcard/forward"/
"pkg/wildcard/forward" // MATCH /should not use the following blacklisted import: "pkg/wildcard/forward"/
"pkg/pkg1/wildcard/forward" // MATCH /should not use the following blacklisted import: "pkg/pkg1/wildcard/forward"/
"full" // MATCH /should not use the following blacklisted import: "full"/
"github.com/partical/match/fully"
"strings"
)

0 comments on commit e10678f

Please sign in to comment.