From fb746c4b04c3a2f2f55400d22d40192b7e659b3b Mon Sep 17 00:00:00 2001
From: Ludovic Fernandez <ldez@users.noreply.github.com>
Date: Sun, 23 Apr 2023 17:56:47 +0200
Subject: [PATCH] depguard: migrate to v2 (#3795)
---
.golangci.reference.yml | 75 +++---
.golangci.yml | 9 +
go.mod | 2 +-
go.sum | 5 +-
pkg/commands/run.go | 9 -
pkg/config/linters_settings.go | 18 +-
pkg/golinters/depguard.go | 214 +++---------------
test/testdata/configs/depguard.yml | 15 +-
.../configs/depguard_additional_guards.yml | 26 +--
.../configs/depguard_ignore_file_rules.yml | 19 +-
test/testdata/depguard.go | 7 +-
test/testdata/depguard_additional_guards.go | 11 +-
test/testdata/depguard_ignore_file_rules.go | 3 +
13 files changed, 135 insertions(+), 278 deletions(-)
diff --git a/.golangci.reference.yml b/.golangci.reference.yml
index 9cd3507b..7e167edf 100644
--- a/.golangci.reference.yml
+++ b/.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.
diff --git a/.golangci.yml b/.golangci.yml
index ea4bc9ca..d34c3c83 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -1,5 +1,6 @@
linters-settings:
depguard:
+ # old configuration. TODO(ldez): must be removed
list-type: denylist
packages:
# logging is allowed only by logutils.Log, logrus
@@ -7,6 +8,14 @@ linters-settings:
- 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:
diff --git a/go.mod b/go.mod
index 0041f94b..a9a44d00 100644
--- a/go.mod
+++ b/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
diff --git a/go.sum b/go.sum
index 1230f7e8..fbf7a3e3 100644
--- a/go.sum
+++ b/go.sum
@@ -58,8 +58,8 @@ github.com/GaijinEntertainment/go-exhaustruct/v2 v2.3.0 h1:+r1rSv4gvYn0wmRjC8X7I
github.com/GaijinEntertainment/go-exhaustruct/v2 v2.3.0/go.mod h1:b3g59n2Y+T5xmcxJL+UEG2f8cQploZm1mR/v6BW0mU0=
github.com/Masterminds/semver v1.5.0 h1:H65muMkzWKEuNDnfl9d70GUjFniHKHRbFPGBuZ3QEww=
github.com/Masterminds/semver v1.5.0/go.mod h1:MB6lktGJrhw8PrUyiEoblNEGEQ+RzHPF078ddwwvV3Y=
-github.com/OpenPeeDeeP/depguard v1.1.1 h1:TSUznLjvp/4IUP+OQ0t/4jF4QUyxIcVX8YnghZdunyA=
-github.com/OpenPeeDeeP/depguard v1.1.1/go.mod h1:JtAMzWkmFEzDPyAd+W0NHl1lvpQKTvT9jnRVsohBKpc=
+github.com/OpenPeeDeeP/depguard/v2 v2.0.1 h1:yr9ZswukmNxl/hmJHEoLEjCF1d+f2pQrC0m1jzVljAE=
+github.com/OpenPeeDeeP/depguard/v2 v2.0.1/go.mod h1:gwSk4XDpowOuQSsMWNK5F7+C3kMz7QIexKRkD/GL4GU=
github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
@@ -803,7 +803,6 @@ golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
-golang.org/x/tools v0.0.0-20180525024113-a5b4c53f6e8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20190226205152-f727befe758c/go.mod h1:9Yl7xja0Znq3iFh3HoIrodX9oNMXvdceNzlUR8zjMvY=
diff --git a/pkg/commands/run.go b/pkg/commands/run.go
index e5d75afe..9149b177 100644
--- a/pkg/commands/run.go
+++ b/pkg/commands/run.go
@@ -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")
diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go
index 3a2833fc..9386b463 100644
--- a/pkg/config/linters_settings.go
+++ b/pkg/config/linters_settings.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 {
diff --git a/pkg/golinters/depguard.go b/pkg/golinters/depguard.go
index 09b0e579..db0df7b4 100644
--- a/pkg/golinters/depguard.go
+++ b/pkg/golinters/depguard.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
+ conf := depguard.LinterSettings{}
- analyzer := &analysis.Analyzer{
- Name: depguardName,
- Doc: goanalysis.TheOnlyanalyzerDoc,
- Run: goanalysis.DummyRun,
+ if settings != nil {
+ for s, rule := range settings.Rules {
+ list := &depguard.List{
+ Files: rule.Files,
+ Allow: rule.Allow,
+ }
+
+ // 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
+
+ deny := map[string]string{}
+ for _, r := range rule.Deny {
+ deny[r.Pkg] = r.Desc
+ }
+ list.Deny = deny
+
+ conf[s] = list
+ }
+ }
+
+ a, err := depguard.NewAnalyzer(&conf)
+ if err != nil {
+ linterLogger.Fatalf("depguard: create analyzer: %v", err)
}
return goanalysis.NewLinter(
- depguardName,
- "Go linter that checks if package imports are in a list of acceptable packages",
- []*analysis.Analyzer{analyzer},
+ a.Name,
+ a.Doc,
+ []*analysis.Analyzer{a},
nil,
- ).WithContextSetter(func(lintCtx *linter.Context) {
- dg, err := newDepGuard(settings)
-
- analyzer.Run = func(pass *analysis.Pass) (any, error) {
- if err != nil {
- return nil, err
- }
-
- issues, errRun := dg.run(pass)
- if errRun != nil {
- return nil, errRun
- }
-
- 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
- }
-
- 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: 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"
- }
-
- 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
+ ).WithLoadMode(goanalysis.LoadModeSyntax)
}
diff --git a/test/testdata/configs/depguard.yml b/test/testdata/configs/depguard.yml
index aa06e3ee..133b9adf 100644
--- a/test/testdata/configs/depguard.yml
+++ b/test/testdata/configs/depguard.yml
@@ -1,7 +1,12 @@
linters-settings:
depguard:
- include-go-root: true
- packages:
- - compress/*
- packages-with-error-message:
- log: "don't use log"
+ rules:
+ main:
+ deny:
+ - pkg: compress
+ desc: "nope"
+ - pkg: log
+ desc: "don't use log"
+ - pkg: "golang.org/x/tools/go/analysis"
+ desc: "example import with dot"
+
diff --git a/test/testdata/configs/depguard_additional_guards.yml b/test/testdata/configs/depguard_additional_guards.yml
index 099edc46..0894d2ee 100644
--- a/test/testdata/configs/depguard_additional_guards.yml
+++ b/test/testdata/configs/depguard_additional_guards.yml
@@ -1,16 +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"
+ rules:
+ main:
+ deny:
+ - pkg: fmt
+ desc: "nope"
+ - pkg: strings
+ desc: "nope"
+ - pkg: compress
+ desc: "nope"
+ - pkg: log
+ desc: "don't use log"
+ - pkg: "golang.org/x/tools/go/analysis"
+ desc: "example import with dot"
diff --git a/test/testdata/configs/depguard_ignore_file_rules.yml b/test/testdata/configs/depguard_ignore_file_rules.yml
index 2fe4b023..c71f7895 100644
--- a/test/testdata/configs/depguard_ignore_file_rules.yml
+++ b/test/testdata/configs/depguard_ignore_file_rules.yml
@@ -1,10 +1,13 @@
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"
+ rules:
+ main:
+ files:
+ - "!**/*_ignore_file_rules.go"
+ deny:
+ - pkg: compress
+ desc: "nope"
+ - pkg: log
+ desc: "don't use log"
+ - pkg: "golang.org/x/tools/go/analysis"
+ desc: "example import with dot"
diff --git a/test/testdata/depguard.go b/test/testdata/depguard.go
index b1b1946b..5aa20fde 100644
--- a/test/testdata/depguard.go
+++ b/test/testdata/depguard.go
@@ -3,10 +3,13 @@
package testdata
import (
- "compress/gzip" // want "`compress/gzip` is in the denylist"
- "log" // want "`log` is in the denylist: don't use log"
+ "compress/gzip" // want "import 'compress/gzip' is not allowed from list 'main': nope"
+ "log" // want "import 'log' is not allowed from list 'main': don't use log"
+
+ "golang.org/x/tools/go/analysis" // want "import 'golang.org/x/tools/go/analysis' is not allowed from list 'main': example import with dot"
)
func SpewDebugInfo() {
log.Println(gzip.BestCompression)
+ _ = analysis.Analyzer{}
}
diff --git a/test/testdata/depguard_additional_guards.go b/test/testdata/depguard_additional_guards.go
index 46f2f43e..01e85c01 100644
--- a/test/testdata/depguard_additional_guards.go
+++ b/test/testdata/depguard_additional_guards.go
@@ -3,14 +3,17 @@
package testdata
import (
- "compress/gzip" // want "`compress/gzip` is in the denylist"
- "fmt" // want "`fmt` is in the denylist"
- "log" // want "`log` is in the denylist: don't use log"
- "strings" // want "`strings` is in the denylist: disallowed in additional guard"
+ "compress/gzip" // want "import 'compress/gzip' is not allowed from list 'main': nope"
+ "fmt" // want "import 'fmt' is not allowed from list 'main': nope"
+ "log" // want "import 'log' is not allowed from list 'main': don't use log"
+ "strings" // want "import 'strings' is not allowed from list 'main': nope"
+
+ "golang.org/x/tools/go/analysis" // want "import 'golang.org/x/tools/go/analysis' is not allowed from list 'main': example import with dot"
)
func SpewDebugInfo() {
log.Println(gzip.BestCompression)
log.Println(fmt.Sprintf("SpewDebugInfo"))
log.Println(strings.ToLower("SpewDebugInfo"))
+ _ = analysis.Analyzer{}
}
diff --git a/test/testdata/depguard_ignore_file_rules.go b/test/testdata/depguard_ignore_file_rules.go
index 76729237..d7b9401d 100644
--- a/test/testdata/depguard_ignore_file_rules.go
+++ b/test/testdata/depguard_ignore_file_rules.go
@@ -7,8 +7,11 @@ package testdata
import (
"compress/gzip"
"log"
+
+ "golang.org/x/tools/go/analysis"
)
func SpewDebugInfo() {
log.Println(gzip.BestCompression)
+ _ = analysis.Analyzer{}
}