diff --git a/.golangci.reference.yml b/.golangci.reference.yml index 9cd3507b5389..7e167edf465c 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 ea4bc9ca8bf4..d34c3c83218d 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 0041f94b9d60..a9a44d009b96 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 1230f7e8553d..fbf7a3e3d398 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 e5d75afe0588..9149b177bcee 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 3a2833fc099b..9386b4631e19 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 09b0e5797fc0..db0df7b47f61 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 - - 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) } diff --git a/test/testdata/configs/depguard.yml b/test/testdata/configs/depguard.yml index aa06e3ee9e61..133b9adf2b82 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 099edc4600b3..0894d2eed80a 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 2fe4b023e8e2..c71f7895188c 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 b1b1946b43e4..5aa20fded6ec 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 46f2f43ebab6..01e85c01f58c 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 767292375d88..d7b9401dcf4f 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{} }