depguard: migrate to v2 (#3795)

Author: ldez

.golangci.reference.yml

@@ -171,49 +171,38 @@ linters-settings:
     disable-dec-num-check: false
 
   depguard:
-    # Kind of list is passed in.
-    # Allowed values: allowlist|denylist
-    # Default: denylist
-    list-type: allowlist
-
-    # Check the list against standard lib.
-    # Default: false
-    include-go-root: true
-
-    # A list of packages for the list type specified.
-    # Can accept both string prefixes and string glob patterns.
-    # Default: []
-    packages:
-      - github.com/sirupsen/logrus
-      - allow/**/pkg
-
-    # A list of packages for the list type specified.
-    # Specify an error message to output when a denied package is used.
-    # Default: []
-    packages-with-error-message:
-      - github.com/sirupsen/logrus: 'logging is allowed only by logutils.Log'
-
-    # Specify rules by which the linter ignores certain files for consideration.
-    # Can accept both string prefixes and string glob patterns.
-    # The ! character in front of the rule is a special character
-    # which signals that the linter should negate the rule.
-    # This allows for more precise control, but it is only available for glob patterns.
-    # Default: []
-    ignore-file-rules:
-      - "ignore/**/*.go"
-      - "!**/*_test.go"
-
-    # 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"
+    # Rules to apply.
+    #
+    # Variables:
+    # - File Variables
+    #   you can still use and exclamation mark ! in front of a variable to say not to use it.
+    #   Example !$test will match any file that is not a go test file.
+    #
+    #   `$all` - matches all go files
+    #   `$test` - matches all go test files
+    #
+    # - Package Variables
+    #
+    #  `$gostd` - matches all of go's standard library (Pulled from `GOROOT`)
+    #
+    # Default: no rules.
+    rules:
+      # Name of a rule.
+      main:
+        # List of file globs that will match this list of settings to compare against.
+        # Default: $all
+        files:
+          - "!**/*_a _file.go"
+        # List of allowed packages.
+        allow:
+          - $gostd
+          - github.com/OpenPeeDeeP
+        # Packages that are not allowed where the value is a suggestion.
+        deny:
+          - pkg: "github.com/sirupsen/logrus"
+            desc: not allowed
+          - pkg: "github.com/pkg/errors"
+            desc: Should be replaced by standard lib errors package
 
   dogsled:
     # Checks assignments with too many blank identifiers.

.golangci.yml

@@ -1,12 +1,21 @@
 linters-settings:
   depguard:
+    # old configuration. TODO(ldez): must be removed
     list-type: denylist
     packages:
       # logging is allowed only by logutils.Log, logrus
       # is allowed to use only in logutils package
       - github.com/sirupsen/logrus
     packages-with-error-message:
       - github.com/sirupsen/logrus: "logging is allowed only by logutils.Log"
+    # new configuration
+    rules:
+      logger:
+        deny:
+          # logging is allowed only by logutils.Log,
+          # logrus is allowed to use only in logutils package.
+          - pkg: "github.com/sirupsen/logrus"
+            desc: logging is allowed only by logutils.Log
   dupl:
     threshold: 100
   funlen:

go.mod

@@ -12,7 +12,7 @@ require (
 	github.com/BurntSushi/toml v1.2.1
 	github.com/Djarvur/go-err113 v0.0.0-20210108212216-aea10b59be24
 	github.com/GaijinEntertainment/go-exhaustruct/v2 v2.3.0
-	github.com/OpenPeeDeeP/depguard v1.1.1
+	github.com/OpenPeeDeeP/depguard/v2 v2.0.1
 	github.com/alexkohler/nakedret/v2 v2.0.1
 	github.com/alexkohler/prealloc v1.0.0
 	github.com/alingse/asasalint v0.0.11

go.sum

@@ -191,15 +191,6 @@ func initFlagSet(fs *pflag.FlagSet, cfg *config.Config, m *lintersdb.Manager, is
 		true, "Goconst: ignore when constant is not used as function argument")
 	hideFlag("goconst.ignore-calls")
 
-	// (@dixonwille) These flag is only used for testing purposes.
-	fs.StringSliceVar(&lsc.Depguard.Packages, "depguard.packages", nil,
-		"Depguard: packages to add to the list")
-	hideFlag("depguard.packages")
-
-	fs.BoolVar(&lsc.Depguard.IncludeGoRoot, "depguard.include-go-root", false,
-		"Depguard: check list against standard lib")
-	hideFlag("depguard.include-go-root")
-
 	fs.IntVar(&lsc.Lll.TabWidth, "lll.tab-width", 1,
 		"Lll: tab width in spaces")
 	hideFlag("lll.tab-width")

pkg/commands/run.go

@@ -253,12 +253,18 @@ 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"`
-	IgnoreFileRules          []string           `mapstructure:"ignore-file-rules"`
-	AdditionalGuards         []DepGuardSettings `mapstructure:"additional-guards"`
+	Rules map[string]*DepGuardList `mapstructure:"rules"`
+}
+
+type DepGuardList struct {
+	Files []string       `mapstructure:"files"`
+	Allow []string       `mapstructure:"allow"`
+	Deny  []DepGuardDeny `mapstructure:"deny"`
+}
+
+type DepGuardDeny struct {
+	Pkg  string `mapstructure:"pkg"`
+	Desc string `mapstructure:"desc"`
 }
 
 type DecorderSettings struct {

pkg/config/linters_settings.go

@@ -1,200 +1,46 @@
 package golinters
 
 import (
-	"fmt"
-	"strings"
-	"sync"
-
-	"github.com/OpenPeeDeeP/depguard"
+	"github.com/OpenPeeDeeP/depguard/v2"
 	"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/fsutils"
 	"github.com/golangci/golangci-lint/pkg/golinters/goanalysis"
-	"github.com/golangci/golangci-lint/pkg/lint/linter"
-	"github.com/golangci/golangci-lint/pkg/result"
 )
 
-const depguardName = "depguard"
-
 func NewDepguard(settings *config.DepGuardSettings) *goanalysis.Linter {
-	var mu sync.Mutex
-	var resIssues []goanalysis.Issue
-
-	analyzer := &analysis.Analyzer{
-		Name: depguardName,
-		Doc:  goanalysis.TheOnlyanalyzerDoc,
-		Run:  goanalysis.DummyRun,
-	}
-
-	return goanalysis.NewLinter(
-		depguardName,
-		"Go linter that checks if package imports are in a list of acceptable packages",
-		[]*analysis.Analyzer{analyzer},
-		nil,
-	).WithContextSetter(func(lintCtx *linter.Context) {
-		dg, err := newDepGuard(settings)
-
-		analyzer.Run = func(pass *analysis.Pass) (any, error) {
-			if err != nil {
-				return nil, err
-			}
+	conf := depguard.LinterSettings{}
 
-			issues, errRun := dg.run(pass)
-			if errRun != nil {
-				return nil, errRun
+	if settings != nil {
+		for s, rule := range settings.Rules {
+			list := &depguard.List{
+				Files: rule.Files,
+				Allow: rule.Allow,
 			}
 
-			mu.Lock()
-			resIssues = append(resIssues, issues...)
-			mu.Unlock()
-
-			return nil, nil
-		}
-	}).WithIssuesReporter(func(*linter.Context) []goanalysis.Issue {
-		return resIssues
-	}).WithLoadMode(goanalysis.LoadModeSyntax)
-}
-
-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) {
-	var ignoreFileRules []string
-	for _, rule := range settings.IgnoreFileRules {
-		ignoreFileRules = append(ignoreFileRules, fsutils.NormalizePathInRegex(rule))
-	}
-
-	dg := &depguard.Depguard{
-		Packages:        settings.Packages,
-		IncludeGoRoot:   settings.IncludeGoRoot,
-		IgnoreFileRules: ignoreFileRules,
-	}
-
-	var err error
-	dg.ListType, err = getDepGuardListType(settings.ListType)
-	if err != nil {
-		return nil, err
-	}
-
-	// if the list type was a denylist the packages with error messages should be included in the denylist package list
-	if dg.ListType == depguard.LTBlacklist {
-		noMessagePackages := make(map[string]bool)
-		for _, pkg := range dg.Packages {
-			noMessagePackages[pkg] = true
-		}
+			// because of bug with Viper parsing (split on dot) we use a list of struct instead of a map.
+			// https://github.com/spf13/viper/issues/324
+			// https://github.com/golangci/golangci-lint/issues/3749#issuecomment-1492536630
 
-		for pkg := range settings.PackagesWithErrorMessage {
-			if _, ok := noMessagePackages[pkg]; !ok {
-				dg.Packages = append(dg.Packages, pkg)
+			deny := map[string]string{}
+			for _, r := range rule.Deny {
+				deny[r.Pkg] = r.Desc
 			}
-		}
-	}
-
-	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: depguardName,
-			}, pass),
-		)
-	}
-
-	return res, nil
-}
-
-func (g guardian) createMsg(pkgName string) string {
-	msgSuffix := "is in the denylist"
-	if g.ListType == depguard.LTWhitelist {
-		msgSuffix = "is not in the allowlist"
-	}
+			list.Deny = deny
 
-	var userSuppliedMsgSuffix string
-	if g.pkgsWithErrorMessage != nil {
-		userSuppliedMsgSuffix = g.pkgsWithErrorMessage[pkgName]
-		if userSuppliedMsgSuffix != "" {
-			userSuppliedMsgSuffix = ": " + userSuppliedMsgSuffix
+			conf[s] = list
 		}
 	}
 
-	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)
+	a, err := depguard.NewAnalyzer(&conf)
+	if err != nil {
+		linterLogger.Fatalf("depguard: create analyzer: %v", err)
 	}
 
-	return listT, nil
+	return goanalysis.NewLinter(
+		a.Name,
+		a.Doc,
+		[]*analysis.Analyzer{a},
+		nil,
+	).WithLoadMode(goanalysis.LoadModeSyntax)
 }