From 138699d60fd827f47a7681ac5e67057163d59e16 Mon Sep 17 00:00:00 2001 From: Tim Kral Date: Sun, 9 Jan 2022 02:48:31 -0800 Subject: [PATCH] depguard: updates configuration (#2467) Co-authored-by: Fernandez Ludovic --- .golangci.example.yml | 15 +- go.mod | 2 +- go.sum | 9 +- pkg/config/linters_settings.go | 6 +- pkg/golinters/depguard.go | 218 ++++++++++++------ .../configs/depguard_additional_guards.yml | 16 ++ .../configs/depguard_ignore_file_rules.yml | 10 + test/testdata/depguard_additional_guards.go | 16 ++ test/testdata/depguard_ignore_file_rules.go | 13 ++ 9 files changed, 226 insertions(+), 79 deletions(-) create mode 100644 test/testdata/configs/depguard_additional_guards.yml create mode 100644 test/testdata/configs/depguard_ignore_file_rules.yml create mode 100644 test/testdata/depguard_additional_guards.go create mode 100644 test/testdata/depguard_ignore_file_rules.go diff --git a/.golangci.example.yml b/.golangci.example.yml index e4f9506ff049..69647fbb324c 100644 --- a/.golangci.example.yml +++ b/.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. diff --git a/go.mod b/go.mod index 7b7492e2a9e3..35eb75c8b5c6 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 3cdbacc3d3f3..ca58c3f44f1c 100644 --- a/go.sum +++ b/go.sum @@ -72,8 +72,8 @@ github.com/Masterminds/semver v1.5.0/go.mod h1:MB6lktGJrhw8PrUyiEoblNEGEQ+RzHPF0 github.com/Masterminds/sprig v2.15.0+incompatible/go.mod h1:y6hNFY5UBTIWBxnzTeuNhlNS5hqE0NB0E6fgfo2Br3o= github.com/Masterminds/sprig v2.22.0+incompatible/go.mod h1:y6hNFY5UBTIWBxnzTeuNhlNS5hqE0NB0E6fgfo2Br3o= github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU= -github.com/OpenPeeDeeP/depguard v1.0.1 h1:VlW4R6jmBIv3/u1JNlawEvJMM4J+dPORPaZasQee8Us= -github.com/OpenPeeDeeP/depguard v1.0.1/go.mod h1:xsIw86fROiiwelg+jB2uM9PiKihMMmUx/1V+TNhjQvM= +github.com/OpenPeeDeeP/depguard v1.1.0 h1:pjK9nLPS1FwQYGGpPxoMYpe7qACHOhAWQMQzV71i49o= +github.com/OpenPeeDeeP/depguard v1.1.0/go.mod h1:JtAMzWkmFEzDPyAd+W0NHl1lvpQKTvT9jnRVsohBKpc= 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= @@ -1067,11 +1067,10 @@ golang.org/x/sys v0.0.0-20211019181941-9d821ace8654/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20211105183446-c75c47738b0c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20211124211545-fe61309f8881/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20211205182925-97ca703d548d/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e h1:fLOSk5Q00efkSvAm+4xcoXD+RRmLmmulPn5I3Y9F2EM= -golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20211210111614-af8b64212486/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20211213223007-03aa0b5f6827 h1:A0Qkn7Z/n8zC1xd9LTw17AiKlBRK64tw3ejWQiEqca0= golang.org/x/sys v0.0.0-20211213223007-03aa0b5f6827/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e h1:fLOSk5Q00efkSvAm+4xcoXD+RRmLmmulPn5I3Y9F2EM= +golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 49f20cf4ddb0..123ebc7feedc 100644 --- a/pkg/config/linters_settings.go +++ b/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 { diff --git a/pkg/golinters/depguard.go b/pkg/golinters/depguard.go index aa372e9568c0..dd6a79772072 100644 --- a/pkg/golinters/depguard.go +++ b/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 +} diff --git a/test/testdata/configs/depguard_additional_guards.yml b/test/testdata/configs/depguard_additional_guards.yml new file mode 100644 index 000000000000..099edc4600b3 --- /dev/null +++ b/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" + diff --git a/test/testdata/configs/depguard_ignore_file_rules.yml b/test/testdata/configs/depguard_ignore_file_rules.yml new file mode 100644 index 000000000000..2fe4b023e8e2 --- /dev/null +++ b/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" diff --git a/test/testdata/depguard_additional_guards.go b/test/testdata/depguard_additional_guards.go new file mode 100644 index 000000000000..56679c090f07 --- /dev/null +++ b/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")) +} diff --git a/test/testdata/depguard_ignore_file_rules.go b/test/testdata/depguard_ignore_file_rules.go new file mode 100644 index 000000000000..e5131cb7b421 --- /dev/null +++ b/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) +}