depguard: updates configuration (#2467)

Co-authored-by: Fernandez Ludovic <ldez@users.noreply.github.com>

.golangci.example.yml

@@ -507,13 +507,24 @@ linters-settings:
     disable-all: false
 
   depguard:
-    list-type: blacklist
+    list-type: denylist
     include-go-root: false
     packages:
       - github.com/sirupsen/logrus
     packages-with-error-message:
-      # specify an error message to output when a blacklisted package is used
+      # specify an error message to output when a denied package is used
       - github.com/sirupsen/logrus: "logging is allowed only by logutils.Log"
+    # create additional guards that follow the same configuration pattern
+    # results from all guards are aggregated together
+    additional-guards:
+      - list-type: denylist
+        include-go-root: false
+        packages:
+          - github.com/stretchr/testify
+        # specify rules by which the linter ignores certain files for consideration
+        ignore-file-rules:
+          - "**/*_test.go"
+          - "**/mock/**/*.go"
 
   ifshort:
     # Maximum length of variable declaration measured in number of lines, after which linter won't suggest using short syntax.

go.mod

@@ -8,7 +8,7 @@ require (
 	github.com/Antonboom/nilnil v0.1.0
 	github.com/BurntSushi/toml v0.4.1
 	github.com/Djarvur/go-err113 v0.0.0-20210108212216-aea10b59be24
-	github.com/OpenPeeDeeP/depguard v1.0.1
+	github.com/OpenPeeDeeP/depguard v1.1.0
 	github.com/alexkohler/prealloc v1.0.0
 	github.com/ashanbrown/forbidigo v1.2.0
 	github.com/ashanbrown/makezero v0.0.0-20210520155254-b6261585ddde

pkg/config/linters_settings.go

@@ -176,8 +176,10 @@ type Cyclop struct {
 type DepGuardSettings struct {
 	ListType                 string `mapstructure:"list-type"`
 	Packages                 []string
-	IncludeGoRoot            bool              `mapstructure:"include-go-root"`
-	PackagesWithErrorMessage map[string]string `mapstructure:"packages-with-error-message"`
+	IncludeGoRoot            bool               `mapstructure:"include-go-root"`
+	PackagesWithErrorMessage map[string]string  `mapstructure:"packages-with-error-message"`
+	IgnoreFileRules          []string           `mapstructure:"ignore-file-rules"`
+	AdditionalGuards         []DepGuardSettings `mapstructure:"additional-guards"`
 }
 
 type DecorderSettings struct {

pkg/golinters/depguard.go

@@ -9,99 +9,42 @@ import (
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/loader" //nolint:staticcheck // require changes in github.com/OpenPeeDeeP/depguard
 
+	"github.com/golangci/golangci-lint/pkg/config"
 	"github.com/golangci/golangci-lint/pkg/golinters/goanalysis"
 	"github.com/golangci/golangci-lint/pkg/lint/linter"
 	"github.com/golangci/golangci-lint/pkg/result"
 )
 
-func setDepguardListType(dg *depguard.Depguard, lintCtx *linter.Context) error {
-	listType := lintCtx.Settings().Depguard.ListType
-	var found bool
-	dg.ListType, found = depguard.StringToListType[strings.ToLower(listType)]
-	if !found {
-		if listType != "" {
-			return fmt.Errorf("unsure what list type %s is", listType)
-		}
-		dg.ListType = depguard.LTBlacklist
-	}
-
-	return nil
-}
-
-func setupDepguardPackages(dg *depguard.Depguard, lintCtx *linter.Context) {
-	if dg.ListType == depguard.LTBlacklist {
-		// if the list type was a blacklist the packages with error messages should
-		// be included in the blacklist package list
-
-		noMessagePackages := make(map[string]bool)
-		for _, pkg := range dg.Packages {
-			noMessagePackages[pkg] = true
-		}
-
-		for pkg := range lintCtx.Settings().Depguard.PackagesWithErrorMessage {
-			if _, ok := noMessagePackages[pkg]; !ok {
-				dg.Packages = append(dg.Packages, pkg)
-			}
-		}
-	}
-}
+const depguardLinterName = "depguard"
 
 func NewDepguard() *goanalysis.Linter {
-	const linterName = "depguard"
 	var mu sync.Mutex
 	var resIssues []goanalysis.Issue
 
 	analyzer := &analysis.Analyzer{
-		Name: linterName,
+		Name: depguardLinterName,
 		Doc:  goanalysis.TheOnlyanalyzerDoc,
 	}
 	return goanalysis.NewLinter(
-		linterName,
+		depguardLinterName,
 		"Go linter that checks if package imports are in a list of acceptable packages",
 		[]*analysis.Analyzer{analyzer},
 		nil,
 	).WithContextSetter(func(lintCtx *linter.Context) {
-		dgSettings := &lintCtx.Settings().Depguard
-		analyzer.Run = func(pass *analysis.Pass) (interface{}, error) {
-			prog := goanalysis.MakeFakeLoaderProgram(pass)
-			dg := &depguard.Depguard{
-				Packages:      dgSettings.Packages,
-				IncludeGoRoot: dgSettings.IncludeGoRoot,
-			}
-			if err := setDepguardListType(dg, lintCtx); err != nil {
-				return nil, err
-			}
-			setupDepguardPackages(dg, lintCtx)
+		dg, err := newDepGuard(&lintCtx.Settings().Depguard)
 
-			loadConfig := &loader.Config{
-				Cwd:   "",  // fallbacked to os.Getcwd
-				Build: nil, // fallbacked to build.Default
-			}
-			issues, err := dg.Run(loadConfig, prog)
+		analyzer.Run = func(pass *analysis.Pass) (interface{}, error) {
 			if err != nil {
 				return nil, err
 			}
-			if len(issues) == 0 {
-				return nil, nil
-			}
-			msgSuffix := "is in the blacklist"
-			if dg.ListType == depguard.LTWhitelist {
-				msgSuffix = "is not in the whitelist"
-			}
-			res := make([]goanalysis.Issue, 0, len(issues))
-			for _, i := range issues {
-				userSuppliedMsgSuffix := dgSettings.PackagesWithErrorMessage[i.PackageName]
-				if userSuppliedMsgSuffix != "" {
-					userSuppliedMsgSuffix = ": " + userSuppliedMsgSuffix
-				}
-				res = append(res, goanalysis.NewIssue(&result.Issue{
-					Pos:        i.Position,
-					Text:       fmt.Sprintf("%s %s%s", formatCode(i.PackageName, lintCtx.Cfg), msgSuffix, userSuppliedMsgSuffix),
-					FromLinter: linterName,
-				}, pass))
+
+			issues, errRun := dg.run(pass)
+			if errRun != nil {
+				return nil, errRun
 			}
+
 			mu.Lock()
-			resIssues = append(resIssues, res...)
+			resIssues = append(resIssues, issues...)
 			mu.Unlock()
 
 			return nil, nil
@@ -110,3 +53,140 @@ func NewDepguard() *goanalysis.Linter {
 		return resIssues
 	}).WithLoadMode(goanalysis.LoadModeTypesInfo)
 }
+
+type depGuard struct {
+	loadConfig *loader.Config
+	guardians  []*guardian
+}
+
+func newDepGuard(settings *config.DepGuardSettings) (*depGuard, error) {
+	ps, err := newGuardian(settings)
+	if err != nil {
+		return nil, err
+	}
+
+	d := &depGuard{
+		loadConfig: &loader.Config{
+			Cwd:   "",  // fallbacked to os.Getcwd
+			Build: nil, // fallbacked to build.Default
+		},
+		guardians: []*guardian{ps},
+	}
+
+	for _, additional := range settings.AdditionalGuards {
+		add := additional
+		ps, err = newGuardian(&add)
+		if err != nil {
+			return nil, err
+		}
+
+		d.guardians = append(d.guardians, ps)
+	}
+
+	return d, nil
+}
+
+func (d depGuard) run(pass *analysis.Pass) ([]goanalysis.Issue, error) {
+	prog := goanalysis.MakeFakeLoaderProgram(pass)
+
+	var resIssues []goanalysis.Issue
+	for _, g := range d.guardians {
+		issues, errRun := g.run(d.loadConfig, prog, pass)
+		if errRun != nil {
+			return nil, errRun
+		}
+
+		resIssues = append(resIssues, issues...)
+	}
+
+	return resIssues, nil
+}
+
+type guardian struct {
+	*depguard.Depguard
+	pkgsWithErrorMessage map[string]string
+}
+
+func newGuardian(settings *config.DepGuardSettings) (*guardian, error) {
+	dg := &depguard.Depguard{
+		Packages:        settings.Packages,
+		IncludeGoRoot:   settings.IncludeGoRoot,
+		IgnoreFileRules: settings.IgnoreFileRules,
+	}
+
+	var err error
+	dg.ListType, err = getDepGuardListType(settings.ListType)
+	if err != nil {
+		return nil, err
+	}
+
+	// if the list type was a blacklist the packages with error messages should be included in the blacklist package list
+	if dg.ListType == depguard.LTBlacklist {
+		noMessagePackages := make(map[string]bool)
+		for _, pkg := range dg.Packages {
+			noMessagePackages[pkg] = true
+		}
+
+		for pkg := range settings.PackagesWithErrorMessage {
+			if _, ok := noMessagePackages[pkg]; !ok {
+				dg.Packages = append(dg.Packages, pkg)
+			}
+		}
+	}
+
+	return &guardian{
+		Depguard:             dg,
+		pkgsWithErrorMessage: settings.PackagesWithErrorMessage,
+	}, nil
+}
+
+func (g guardian) run(loadConfig *loader.Config, prog *loader.Program, pass *analysis.Pass) ([]goanalysis.Issue, error) {
+	issues, err := g.Run(loadConfig, prog)
+	if err != nil {
+		return nil, err
+	}
+
+	res := make([]goanalysis.Issue, 0, len(issues))
+
+	for _, issue := range issues {
+		res = append(res,
+			goanalysis.NewIssue(&result.Issue{
+				Pos:        issue.Position,
+				Text:       g.createMsg(issue.PackageName),
+				FromLinter: depguardLinterName,
+			}, pass),
+		)
+	}
+
+	return res, nil
+}
+
+func (g guardian) createMsg(pkgName string) string {
+	msgSuffix := "is in the blacklist"
+	if g.ListType == depguard.LTWhitelist {
+		msgSuffix = "is not in the whitelist"
+	}
+
+	var userSuppliedMsgSuffix string
+	if g.pkgsWithErrorMessage != nil {
+		userSuppliedMsgSuffix = g.pkgsWithErrorMessage[pkgName]
+		if userSuppliedMsgSuffix != "" {
+			userSuppliedMsgSuffix = ": " + userSuppliedMsgSuffix
+		}
+	}
+
+	return fmt.Sprintf("%s %s%s", formatCode(pkgName, nil), msgSuffix, userSuppliedMsgSuffix)
+}
+
+func getDepGuardListType(listType string) (depguard.ListType, error) {
+	if listType == "" {
+		return depguard.LTBlacklist, nil
+	}
+
+	listT, found := depguard.StringToListType[strings.ToLower(listType)]
+	if !found {
+		return depguard.LTBlacklist, fmt.Errorf("unsure what list type %s is", listType)
+	}
+
+	return listT, nil
+}

test/testdata/configs/depguard_additional_guards.yml

@@ -0,0 +1,16 @@
+linters-settings:
+  depguard:
+    list-type: denylist
+    include-go-root: true
+    packages: 
+      - compress/*
+    packages-with-error-message:
+      log: "don't use log"
+    additional-guards:
+      - list-type: denylist
+        include-go-root: true
+        packages:
+          - fmt
+        packages-with-error-message:
+          strings: "disallowed in additional guard"
+

test/testdata/configs/depguard_ignore_file_rules.yml

@@ -0,0 +1,10 @@
+linters-settings:
+  depguard:
+    list-type: denylist
+    include-go-root: true
+    packages: 
+      - compress/*
+    packages-with-error-message:
+      log: "don't use log"
+    ignore-file-rules:
+      - "**/*_ignore_file_rules.go"

test/testdata/depguard_additional_guards.go

@@ -0,0 +1,16 @@
+//args: -Edepguard
+//config_path: testdata/configs/depguard_additional_guards.yml
+package testdata
+
+import (
+	"compress/gzip" // ERROR "`compress/gzip` is in the blacklist"
+	"fmt"           // ERROR "`fmt` is in the blacklist"
+	"log"           // ERROR "`log` is in the blacklist: don't use log"
+	"strings"       // ERROR "`strings` is in the blacklist: disallowed in additional guard"
+)
+
+func SpewDebugInfo() {
+	log.Println(gzip.BestCompression)
+	log.Println(fmt.Sprintf("SpewDebugInfo"))
+	log.Println(strings.ToLower("SpewDebugInfo"))
+}

test/testdata/depguard_ignore_file_rules.go

@@ -0,0 +1,13 @@
+//args: -Edepguard
+//config_path: testdata/configs/depguard_ignore_file_rules.yml
+package testdata
+
+// NOTE - No lint errors becuase this file is ignored
+import (
+	"compress/gzip"
+	"log"
+)
+
+func SpewDebugInfo() {
+	log.Println(gzip.BestCompression)
+}