Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 40 additions & 43 deletions cmd/depguard/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package main

import (
"embed"
"io/fs"
"testing"

"github.com/OpenPeeDeeP/depguard/v2"
Expand All @@ -16,6 +17,7 @@ var expectedConfigStruct = &depguard.LinterSettings{
Files: []string{"$all", "!$test"},
Allow: []string{"$gostd", "github.com/"},
Deny: map[string]string{
"github.com/**/test": "No test packages allowed",
"reflect": "Who needs reflection",
"github.com/OpenPeeDeeP": "Use Something Else",
},
Expand All @@ -29,50 +31,45 @@ var expectedConfigStruct = &depguard.LinterSettings{
},
}

func TestJsonConfigurator(t *testing.T) {
con := &jsonConfigurator{}
f, err := testfiles.Open("testfiles/.depguard.json")
if err != nil {
t.Fatal("could not read embedded file")
func TestConfigurators(t *testing.T) {
mustGetFile := func(name string) fs.File {
f, err := testfiles.Open(name)
if err != nil {
t.Fatal("could not read embedded file")
}
return f
}
set, err := con.parse(f)
if err != nil {
t.Fatalf("file is not a valid json file: %s", err)
}
diff := cmp.Diff(expectedConfigStruct, set)
if diff != "" {
t.Errorf("did not create expected config\n%s", diff)
}
}

func TestYamlConfigurator(t *testing.T) {
con := &yamlConfigurator{}
f, err := testfiles.Open("testfiles/.depguard.yaml")
if err != nil {
t.Fatal("could not read embedded file")
}
set, err := con.parse(f)
if err != nil {
t.Fatalf("file is not a valid yaml file: %s", err)
}
diff := cmp.Diff(expectedConfigStruct, set)
if diff != "" {
t.Errorf("did not create expected config\n%s", diff)
}
}

func TestTomlConfigurator(t *testing.T) {
con := &tomlConfigurator{}
f, err := testfiles.Open("testfiles/.depguard.toml")
if err != nil {
t.Fatal("could not read embedded file")
}
set, err := con.parse(f)
if err != nil {
t.Fatalf("file is not a valid toml file: %s", err)
cases := []struct {
name string
inputFile fs.File
configurator configurator
}{
{
name: "json",
inputFile: mustGetFile("testfiles/.depguard.json"),
configurator: &jsonConfigurator{},
},
{
name: "yaml",
inputFile: mustGetFile("testfiles/.depguard.yaml"),
configurator: &yamlConfigurator{},
},
{
name: "toml",
inputFile: mustGetFile("testfiles/.depguard.toml"),
configurator: &tomlConfigurator{},
},
}
diff := cmp.Diff(expectedConfigStruct, set)
if diff != "" {
t.Errorf("did not create expected config\n%s", diff)
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
set, err := c.configurator.parse(c.inputFile)
if err != nil {
t.Fatalf("file is not a valid json file: %s", err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File may not be json.

}
diff := cmp.Diff(expectedConfigStruct, set)
if diff != "" {
t.Errorf("did not create expected config\n%s", diff)
}
})
}
}
3 changes: 2 additions & 1 deletion cmd/depguard/testfiles/.depguard.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
],
"deny": {
"reflect": "Who needs reflection",
"github.com/OpenPeeDeeP": "Use Something Else"
"github.com/OpenPeeDeeP": "Use Something Else",
"github.com/**/test": "No test packages allowed"
}
},
"tests": {
Expand Down
1 change: 1 addition & 0 deletions cmd/depguard/testfiles/.depguard.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ allow = [
[main.deny]
reflect = "Who needs reflection"
"github.com/OpenPeeDeeP" = "Use Something Else"
"github.com/**/test" = "No test packages allowed"

[tests]
files = [
Expand Down
1 change: 1 addition & 0 deletions cmd/depguard/testfiles/.depguard.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ main:
deny:
reflect: Who needs reflection
github.com/OpenPeeDeeP: Use Something Else
github.com/**/test: No test packages allowed
tests:
files:
- "$test"
Expand Down
12 changes: 2 additions & 10 deletions internal/utils/variables_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package utils

import (
"errors"
"slices"
"strings"
"testing"

Expand Down Expand Up @@ -61,7 +62,7 @@ func TestGoStdExpander(t *testing.T) {
t.Fatal("expected more than 1 expansion")
}
// Just make sure a few are in there
if !contains(pre, "os") && !contains(pre, "strings") {
if !slices.Contains(pre, "os") && !slices.Contains(pre, "strings") {
t.Error("could not find some of the expected packages")
}
}
Expand Down Expand Up @@ -197,12 +198,3 @@ func TestExpandMap(t *testing.T) {
}
})
}

func contains(sl []string, str string) bool {
for _, s := range sl {
if s == str {
return true
}
}
return false
}
93 changes: 53 additions & 40 deletions settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ type list struct {
name string
files []glob.Glob
negFiles []glob.Glob
allow []string
deny []string
allow []glob.Glob
deny []glob.Glob
suggestions []string
}

Expand Down Expand Up @@ -65,11 +65,18 @@ func (l *List) compile() (*list, error) {
if err != nil {
errs = append(errs, err)
}
sort.Strings(l.Allow)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This modifies the input slice which can be unexpected behavior to the consumer (if there are any). That is why I had it copied to an internal slice before sort.


// Sort Allow
li.allow = make([]string, len(l.Allow))
copy(li.allow, l.Allow)
sort.Strings(li.allow)
li.allow = make([]glob.Glob, 0, len(l.Allow))
for _, pkg := range l.Allow {
glob, err := inputPatternToGlob(pkg)
if err != nil {
errs = append(errs, err)
continue
}
li.allow = append(li.allow, glob)
}
}

if l.Deny != nil {
Expand All @@ -80,18 +87,24 @@ func (l *List) compile() (*list, error) {
}

// Split Deny Into Package Slice
li.deny = make([]string, 0, len(l.Deny))
for pkg := range l.Deny {
li.deny = append(li.deny, pkg)
}

// Sort Deny
sort.Strings(li.deny)
// sort before compiling as globs are opaque
pkgs := make([]string, 0, len(l.Deny))
for k := range l.Deny {
pkgs = append(pkgs, k)
}
sort.Strings(pkgs)

// Populate Suggestions to match the Deny order
li.suggestions = make([]string, 0, len(li.deny))
for _, dp := range li.deny {
li.suggestions = append(li.suggestions, strings.TrimSpace(l.Deny[dp]))
li.deny = make([]glob.Glob, 0, len(pkgs))
li.suggestions = make([]string, 0, len(pkgs))
for _, pkg := range pkgs {
glob, err := inputPatternToGlob(pkg)
if err != nil {
errs = append(errs, err)
continue
}
li.deny = append(li.deny, glob)
li.suggestions = append(li.suggestions, strings.TrimSpace(l.Deny[pkg]))
}
}

Expand All @@ -107,17 +120,24 @@ func (l *List) compile() (*list, error) {
}

func (l *list) fileMatch(fileName string) bool {
inAllowed := len(l.files) == 0 || strInGlobList(fileName, l.files)
inDenied := strInGlobList(fileName, l.negFiles)
return inAllowed && !inDenied
inDenied, _ := strInGlobList(fileName, l.negFiles)
if inDenied {
return false
}
if len(l.files) == 0 {
// No allow list matches all
return true
}
inAllowed, _ := strInGlobList(fileName, l.files)
return inAllowed
}

func (l *list) importAllowed(imp string) (bool, string) {
inAllowed := len(l.allow) == 0
if !inAllowed {
inAllowed, _ = strInPrefixList(imp, l.allow)
inAllowed, _ = strInGlobList(imp, l.allow)
}
inDenied, suggIdx := strInPrefixList(imp, l.deny)
inDenied, suggIdx := strInGlobList(imp, l.deny)
sugg := ""
if inDenied && suggIdx != -1 {
sugg = l.suggestions[suggIdx]
Expand Down Expand Up @@ -179,29 +199,22 @@ func (ls linterSettings) whichLists(fileName string) []*list {
return matches
}

func strInGlobList(str string, globList []glob.Glob) bool {
for _, g := range globList {
func strInGlobList(str string, globList []glob.Glob) (bool, int) {
for idx, g := range globList {
if g.Match(str) {
return true
return true, idx
}
}
return false
return false, 0
}

func strInPrefixList(str string, prefixList []string) (bool, int) {
// Idx represents where in the prefix slice the passed in string would go
// when sorted. -1 Just means that it would be at the very front of the slice.
idx := sort.Search(len(prefixList), func(i int) bool {
return strings.TrimRight(prefixList[i], "$") > str
}) - 1
// This means that the string passed in has no way to be prefixed by anything
// in the prefix list as it is already smaller then everything
if idx == -1 {
return false, idx
}
ioc := prefixList[idx]
if ioc[len(ioc)-1] == '$' {
return str == ioc[:len(ioc)-1], idx
}
return strings.HasPrefix(str, prefixList[idx]), idx
func inputPatternToGlob(pattern string) (glob.Glob, error) {
lastIdx := len(pattern) - 1
if pattern[lastIdx] == '$' {
pattern = pattern[:lastIdx]
} else {
pattern += "**"
}
return glob.Compile(pattern, '/')

}
Loading