From 9fd899cde7d8f4ea5d873165d97882f4e4716ecc Mon Sep 17 00:00:00 2001 From: Tim Kral Date: Wed, 24 Nov 2021 08:29:20 -0800 Subject: [PATCH 01/14] Update wrapcheck configuration to include ignoreSignRegexps --- .golangci.example.yml | 6 ++++-- pkg/config/linters_settings.go | 1 + pkg/golinters/wrapcheck.go | 3 +++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/.golangci.example.yml b/.golangci.example.yml index a79b718ce947..e349e5cb1348 100644 --- a/.golangci.example.yml +++ b/.golangci.example.yml @@ -744,9 +744,11 @@ linters-settings: - .WithMessage( - .WithMessagef( - .WithStack( + ignoreSigRegexps: + - \.New.*Error\( ignorePackageGlobs: - - encoding/* - - github.com/pkg/* + - encoding/* + - github.com/pkg/* wsl: # See https://github.com/bombsimon/wsl/blob/master/doc/configuration.md for diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 9a26ca06a4ba..39b2d13a354b 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -519,6 +519,7 @@ type WhitespaceSettings struct { type WrapcheckSettings struct { IgnoreSigs []string `mapstructure:"ignoreSigs"` + IgnoreSigRegexps []string `mapstructure:"ignoreSigRegexps"` IgnorePackageGlobs []string `mapstructure:"ignorePackageGlobs"` } diff --git a/pkg/golinters/wrapcheck.go b/pkg/golinters/wrapcheck.go index 5eaf085d7435..c52bcb740f42 100644 --- a/pkg/golinters/wrapcheck.go +++ b/pkg/golinters/wrapcheck.go @@ -16,6 +16,9 @@ func NewWrapcheck(settings *config.WrapcheckSettings) *goanalysis.Linter { if len(settings.IgnoreSigs) != 0 { cfg.IgnoreSigs = settings.IgnoreSigs } + if len(settings.IgnoreSigRegexps) != 0 { + cfg.IgnoreSigRegexps = settings.IgnoreSigRegexps + } if len(settings.IgnorePackageGlobs) != 0 { cfg.IgnorePackageGlobs = settings.IgnorePackageGlobs } From 3c7bec7de40fbadc8b150d043d75a61cd18ac693 Mon Sep 17 00:00:00 2001 From: Tim Kral Date: Fri, 7 Jan 2022 23:17:29 -0800 Subject: [PATCH 02/14] Upgrade depguard --- go.mod | 2 +- go.sum | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) 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..e9e2dc917cf2 100644 --- a/go.sum +++ b/go.sum @@ -74,6 +74,8 @@ github.com/Masterminds/sprig v2.22.0+incompatible/go.mod h1:y6hNFY5UBTIWBxnzTeuN 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 +1069,11 @@ 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= From f1dc40b2a3a4f901fa586a963c83b8fc25054c48 Mon Sep 17 00:00:00 2001 From: Tim Kral Date: Sat, 8 Jan 2022 01:05:03 -0800 Subject: [PATCH 03/14] Add ignore-file-rules setting to depguard --- pkg/config/linters_settings.go | 1 + pkg/golinters/depguard.go | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 49f20cf4ddb0..88b6ba5e3706 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -178,6 +178,7 @@ type DepGuardSettings struct { Packages []string IncludeGoRoot bool `mapstructure:"include-go-root"` PackagesWithErrorMessage map[string]string `mapstructure:"packages-with-error-message"` + IgnoreFileRules []string `mapstructure:"ignore-file-rules"` } type DecorderSettings struct { diff --git a/pkg/golinters/depguard.go b/pkg/golinters/depguard.go index aa372e9568c0..a8ca8bd4ff3e 100644 --- a/pkg/golinters/depguard.go +++ b/pkg/golinters/depguard.go @@ -65,8 +65,9 @@ func NewDepguard() *goanalysis.Linter { analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { prog := goanalysis.MakeFakeLoaderProgram(pass) dg := &depguard.Depguard{ - Packages: dgSettings.Packages, - IncludeGoRoot: dgSettings.IncludeGoRoot, + Packages: dgSettings.Packages, + IncludeGoRoot: dgSettings.IncludeGoRoot, + IgnoreFileRules: dgSettings.IgnoreFileRules, } if err := setDepguardListType(dg, lintCtx); err != nil { return nil, err From b1ea1787e50f577e5542a8057ff8f5d729e12687 Mon Sep 17 00:00:00 2001 From: Tim Kral Date: Sat, 8 Jan 2022 01:12:43 -0800 Subject: [PATCH 04/14] Allow additional guards in depguard linter --- pkg/config/linters_settings.go | 7 +++ pkg/golinters/depguard.go | 79 +++++++++++++++++++--------------- 2 files changed, 52 insertions(+), 34 deletions(-) diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 88b6ba5e3706..188cad202165 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -179,6 +179,13 @@ type DepGuardSettings struct { IncludeGoRoot bool `mapstructure:"include-go-root"` PackagesWithErrorMessage map[string]string `mapstructure:"packages-with-error-message"` IgnoreFileRules []string `mapstructure:"ignore-file-rules"` + AdditionalGuards []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"` + } `mapstructure:"additional-guards"` } type DecorderSettings struct { diff --git a/pkg/golinters/depguard.go b/pkg/golinters/depguard.go index a8ca8bd4ff3e..913ad5cb7495 100644 --- a/pkg/golinters/depguard.go +++ b/pkg/golinters/depguard.go @@ -63,48 +63,59 @@ func NewDepguard() *goanalysis.Linter { ).WithContextSetter(func(lintCtx *linter.Context) { dgSettings := &lintCtx.Settings().Depguard analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + loadConfig := &loader.Config{ + Cwd: "", // fallbacked to os.Getcwd + Build: nil, // fallbacked to build.Default + } prog := goanalysis.MakeFakeLoaderProgram(pass) - dg := &depguard.Depguard{ + + var dgs []*depguard.Depguard + dgs = append(dgs, &depguard.Depguard{ Packages: dgSettings.Packages, IncludeGoRoot: dgSettings.IncludeGoRoot, IgnoreFileRules: dgSettings.IgnoreFileRules, + }) + for _, additionalGuard := range dgSettings.AdditionalGuards { + dgs = append(dgs, &depguard.Depguard{ + Packages: additionalGuard.Packages, + IncludeGoRoot: additionalGuard.IncludeGoRoot, + IgnoreFileRules: additionalGuard.IgnoreFileRules, + }) } - if err := setDepguardListType(dg, lintCtx); err != nil { - return nil, err - } - setupDepguardPackages(dg, lintCtx) - loadConfig := &loader.Config{ - Cwd: "", // fallbacked to os.Getcwd - Build: nil, // fallbacked to build.Default - } - issues, err := dg.Run(loadConfig, prog) - 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 + for _, dg := range dgs { + if err := setDepguardListType(dg, lintCtx); err != nil { + return nil, err } - 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)) - } - mu.Lock() - resIssues = append(resIssues, res...) - mu.Unlock() + setupDepguardPackages(dg, lintCtx) + issues, err := dg.Run(loadConfig, prog) + 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)) + } + mu.Lock() + resIssues = append(resIssues, res...) + mu.Unlock() + } return nil, nil } }).WithIssuesReporter(func(*linter.Context) []goanalysis.Issue { From 40b6f176938d3f368f26e07a1d34df7548f1745b Mon Sep 17 00:00:00 2001 From: Tim Kral Date: Sat, 8 Jan 2022 01:44:50 -0800 Subject: [PATCH 05/14] Update yml/cli configuration --- .golangci.example.yml | 11 +++++++++++ pkg/commands/run.go | 4 ++++ 2 files changed, 15 insertions(+) diff --git a/.golangci.example.yml b/.golangci.example.yml index e4f9506ff049..d012ac60cbde 100644 --- a/.golangci.example.yml +++ b/.golangci.example.yml @@ -514,6 +514,17 @@ linters-settings: packages-with-error-message: # specify an error message to output when a blacklisted 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: blacklist + 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/pkg/commands/run.go b/pkg/commands/run.go index f75fa82f3bd3..9e358f916fdd 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -193,6 +193,10 @@ func initFlagSet(fs *pflag.FlagSet, cfg *config.Config, m *lintersdb.Manager, is "Depguard: check list against standard lib") hideFlag("depguard.include-go-root") + fs.StringSliceVar(&lsc.Depguard.IgnoreFileRules, "depguard.ignore-file-rules", nil, + "Depguard: rules to ignore certain files for consideration") + hideFlag("depguard.ignore-file-rules") + fs.IntVar(&lsc.Lll.TabWidth, "lll.tab-width", 1, "Lll: tab width in spaces") hideFlag("lll.tab-width") From 16ef21bb25e009ea2fe207e24e3a728a28076ed8 Mon Sep 17 00:00:00 2001 From: Tim Kral Date: Sat, 8 Jan 2022 17:42:17 -0800 Subject: [PATCH 06/14] Update depguard settings parsing for additional guards --- pkg/golinters/depguard.go | 66 +++++++++++++++++++++++++-------------- 1 file changed, 43 insertions(+), 23 deletions(-) diff --git a/pkg/golinters/depguard.go b/pkg/golinters/depguard.go index 913ad5cb7495..3f2d572bf7b0 100644 --- a/pkg/golinters/depguard.go +++ b/pkg/golinters/depguard.go @@ -9,13 +9,44 @@ 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 +func parseDepguardSettings(dgSettings *config.DepGuardSettings) ([]*depguard.Depguard, error) { + var dgs []*depguard.Depguard + dg := &depguard.Depguard{ + Packages: dgSettings.Packages, + IncludeGoRoot: dgSettings.IncludeGoRoot, + IgnoreFileRules: dgSettings.IgnoreFileRules, + } + + if err := setDepguardListType(dg, dgSettings.ListType); err != nil { + return nil, err + } + setupDepguardPackages(dg, dgSettings.PackagesWithErrorMessage) + dgs = append(dgs, dg) + + for _, additionalGuard := range dgSettings.AdditionalGuards { + additionalDg := &depguard.Depguard{ + Packages: additionalGuard.Packages, + IncludeGoRoot: additionalGuard.IncludeGoRoot, + IgnoreFileRules: additionalGuard.IgnoreFileRules, + } + + if err := setDepguardListType(additionalDg, additionalGuard.ListType); err != nil { + return nil, err + } + setupDepguardPackages(additionalDg, additionalGuard.PackagesWithErrorMessage) + dgs = append(dgs, additionalDg) + } + + return dgs, nil +} + +func setDepguardListType(dg *depguard.Depguard, listType string) error { var found bool dg.ListType, found = depguard.StringToListType[strings.ToLower(listType)] if !found { @@ -28,7 +59,10 @@ func setDepguardListType(dg *depguard.Depguard, lintCtx *linter.Context) error { return nil } -func setupDepguardPackages(dg *depguard.Depguard, lintCtx *linter.Context) { +func setupDepguardPackages( + dg *depguard.Depguard, + packagesWithErrorMessage map[string]string, +) { 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 @@ -38,7 +72,7 @@ func setupDepguardPackages(dg *depguard.Depguard, lintCtx *linter.Context) { noMessagePackages[pkg] = true } - for pkg := range lintCtx.Settings().Depguard.PackagesWithErrorMessage { + for pkg := range packagesWithErrorMessage { if _, ok := noMessagePackages[pkg]; !ok { dg.Packages = append(dg.Packages, pkg) } @@ -63,32 +97,18 @@ func NewDepguard() *goanalysis.Linter { ).WithContextSetter(func(lintCtx *linter.Context) { dgSettings := &lintCtx.Settings().Depguard analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + dgs, err := parseDepguardSettings(dgSettings) + if err != nil { + return nil, err + } + loadConfig := &loader.Config{ Cwd: "", // fallbacked to os.Getcwd Build: nil, // fallbacked to build.Default } prog := goanalysis.MakeFakeLoaderProgram(pass) - var dgs []*depguard.Depguard - dgs = append(dgs, &depguard.Depguard{ - Packages: dgSettings.Packages, - IncludeGoRoot: dgSettings.IncludeGoRoot, - IgnoreFileRules: dgSettings.IgnoreFileRules, - }) - for _, additionalGuard := range dgSettings.AdditionalGuards { - dgs = append(dgs, &depguard.Depguard{ - Packages: additionalGuard.Packages, - IncludeGoRoot: additionalGuard.IncludeGoRoot, - IgnoreFileRules: additionalGuard.IgnoreFileRules, - }) - } - for _, dg := range dgs { - if err := setDepguardListType(dg, lintCtx); err != nil { - return nil, err - } - setupDepguardPackages(dg, lintCtx) - issues, err := dg.Run(loadConfig, prog) if err != nil { return nil, err From 20783653768c8a120694e27249a884e3626da8cb Mon Sep 17 00:00:00 2001 From: Tim Kral Date: Sat, 8 Jan 2022 19:21:40 -0800 Subject: [PATCH 07/14] Allow custom error messages in additional rules --- pkg/golinters/depguard.go | 63 ++++++++++++++++++++++++++------------- 1 file changed, 42 insertions(+), 21 deletions(-) diff --git a/pkg/golinters/depguard.go b/pkg/golinters/depguard.go index 3f2d572bf7b0..1fa2dcba7b0c 100644 --- a/pkg/golinters/depguard.go +++ b/pkg/golinters/depguard.go @@ -15,8 +15,8 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) -func parseDepguardSettings(dgSettings *config.DepGuardSettings) ([]*depguard.Depguard, error) { - var dgs []*depguard.Depguard +func parseDepguardSettings(dgSettings *config.DepGuardSettings) (map[*depguard.Depguard]map[string]string, error) { + parsedDgSettings := make(map[*depguard.Depguard]map[string]string) dg := &depguard.Depguard{ Packages: dgSettings.Packages, IncludeGoRoot: dgSettings.IncludeGoRoot, @@ -27,7 +27,11 @@ func parseDepguardSettings(dgSettings *config.DepGuardSettings) ([]*depguard.Dep return nil, err } setupDepguardPackages(dg, dgSettings.PackagesWithErrorMessage) - dgs = append(dgs, dg) + if dgSettings.PackagesWithErrorMessage != nil { + parsedDgSettings[dg] = dgSettings.PackagesWithErrorMessage + } else { + parsedDgSettings[dg] = make(map[string]string) + } for _, additionalGuard := range dgSettings.AdditionalGuards { additionalDg := &depguard.Depguard{ @@ -40,10 +44,37 @@ func parseDepguardSettings(dgSettings *config.DepGuardSettings) ([]*depguard.Dep return nil, err } setupDepguardPackages(additionalDg, additionalGuard.PackagesWithErrorMessage) - dgs = append(dgs, additionalDg) + if additionalGuard.PackagesWithErrorMessage != nil { + parsedDgSettings[additionalDg] = additionalGuard.PackagesWithErrorMessage + } else { + parsedDgSettings[additionalDg] = make(map[string]string) + } } - return dgs, nil + return parsedDgSettings, nil +} + +func postProcessIssue( + issue *depguard.Issue, + dg *depguard.Depguard, + packagesWithErrorMessage map[string]string, + lintCtx *linter.Context, +) *result.Issue { + msgSuffix := "is in the blacklist" + if dg.ListType == depguard.LTWhitelist { + msgSuffix = "is not in the whitelist" + } + + userSuppliedMsgSuffix := packagesWithErrorMessage[issue.PackageName] + if userSuppliedMsgSuffix != "" { + userSuppliedMsgSuffix = ": " + userSuppliedMsgSuffix + } + + return &result.Issue{ + Pos: issue.Position, + Text: fmt.Sprintf("%s %s%s", formatCode(issue.PackageName, lintCtx.Cfg), msgSuffix, userSuppliedMsgSuffix), + FromLinter: linterName, + } } func setDepguardListType(dg *depguard.Depguard, listType string) error { @@ -80,8 +111,9 @@ func setupDepguardPackages( } } +const linterName = "depguard" + func NewDepguard() *goanalysis.Linter { - const linterName = "depguard" var mu sync.Mutex var resIssues []goanalysis.Issue @@ -97,7 +129,7 @@ func NewDepguard() *goanalysis.Linter { ).WithContextSetter(func(lintCtx *linter.Context) { dgSettings := &lintCtx.Settings().Depguard analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { - dgs, err := parseDepguardSettings(dgSettings) + parsedDgSettings, err := parseDepguardSettings(dgSettings) if err != nil { return nil, err } @@ -108,7 +140,7 @@ func NewDepguard() *goanalysis.Linter { } prog := goanalysis.MakeFakeLoaderProgram(pass) - for _, dg := range dgs { + for dg, packagesWithErrorMessage := range parsedDgSettings { issues, err := dg.Run(loadConfig, prog) if err != nil { return nil, err @@ -116,21 +148,10 @@ func NewDepguard() *goanalysis.Linter { 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)) + lintIssue := postProcessIssue(i, dg, packagesWithErrorMessage, lintCtx) + res = append(res, goanalysis.NewIssue(lintIssue, pass)) } mu.Lock() resIssues = append(resIssues, res...) From d809457f2501d8fd22cdf73ec26f9422873fc426 Mon Sep 17 00:00:00 2001 From: Tim Kral Date: Sat, 8 Jan 2022 19:23:55 -0800 Subject: [PATCH 08/14] Add testdata for new depguard configuration --- .../configs/depguard_additional_guards.yml | 14 ++++++++++++++ .../configs/depguard_ignore_file_rules.yml | 9 +++++++++ test/testdata/depguard_additional_guards.go | 16 ++++++++++++++++ test/testdata/depguard_ignore_file_rules.go | 13 +++++++++++++ 4 files changed, 52 insertions(+) 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/test/testdata/configs/depguard_additional_guards.yml b/test/testdata/configs/depguard_additional_guards.yml new file mode 100644 index 000000000000..e4ccb40b0ca2 --- /dev/null +++ b/test/testdata/configs/depguard_additional_guards.yml @@ -0,0 +1,14 @@ +linters-settings: + depguard: + include-go-root: true + packages: + - compress/* + packages-with-error-message: + log: "don't use log" + additional-guards: + - 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..6b7f10f748a1 --- /dev/null +++ b/test/testdata/configs/depguard_ignore_file_rules.yml @@ -0,0 +1,9 @@ +linters-settings: + depguard: + 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) +} From 012f461b99c4d32b39db8331e764d82b3fb106a4 Mon Sep 17 00:00:00 2001 From: Tim Kral Date: Sat, 8 Jan 2022 19:28:54 -0800 Subject: [PATCH 09/14] update to more inclusive language --- .golangci.example.yml | 6 +++--- .golangci.yml | 2 +- test/testdata/configs/depguard_additional_guards.yml | 4 +++- test/testdata/configs/depguard_ignore_file_rules.yml | 1 + 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/.golangci.example.yml b/.golangci.example.yml index d012ac60cbde..69647fbb324c 100644 --- a/.golangci.example.yml +++ b/.golangci.example.yml @@ -507,17 +507,17 @@ 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: blacklist + - list-type: denylist include-go-root: false packages: - github.com/stretchr/testify diff --git a/.golangci.yml b/.golangci.yml index 651a326706a5..d064a084130d 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,6 +1,6 @@ linters-settings: depguard: - list-type: blacklist + list-type: denylist packages: # logging is allowed only by logutils.Log, logrus # is allowed to use only in logutils package diff --git a/test/testdata/configs/depguard_additional_guards.yml b/test/testdata/configs/depguard_additional_guards.yml index e4ccb40b0ca2..099edc4600b3 100644 --- a/test/testdata/configs/depguard_additional_guards.yml +++ b/test/testdata/configs/depguard_additional_guards.yml @@ -1,12 +1,14 @@ linters-settings: depguard: + list-type: denylist include-go-root: true packages: - compress/* packages-with-error-message: log: "don't use log" additional-guards: - - include-go-root: true + - list-type: denylist + include-go-root: true packages: - fmt packages-with-error-message: diff --git a/test/testdata/configs/depguard_ignore_file_rules.yml b/test/testdata/configs/depguard_ignore_file_rules.yml index 6b7f10f748a1..2fe4b023e8e2 100644 --- a/test/testdata/configs/depguard_ignore_file_rules.yml +++ b/test/testdata/configs/depguard_ignore_file_rules.yml @@ -1,5 +1,6 @@ linters-settings: depguard: + list-type: denylist include-go-root: true packages: - compress/* From d7ce5b6bb7ca230d9cb3042ea68b97d37ee3f9ce Mon Sep 17 00:00:00 2001 From: Tim Kral Date: Sat, 8 Jan 2022 20:01:23 -0800 Subject: [PATCH 10/14] Do not short circuit if a guard returns no issues --- pkg/golinters/depguard.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/golinters/depguard.go b/pkg/golinters/depguard.go index 1fa2dcba7b0c..36efbaab6823 100644 --- a/pkg/golinters/depguard.go +++ b/pkg/golinters/depguard.go @@ -145,9 +145,6 @@ func NewDepguard() *goanalysis.Linter { if err != nil { return nil, err } - if len(issues) == 0 { - return nil, nil - } res := make([]goanalysis.Issue, 0, len(issues)) for _, i := range issues { lintIssue := postProcessIssue(i, dg, packagesWithErrorMessage, lintCtx) From d3dd17d96e8512e42e5616e939bd5076e358d046 Mon Sep 17 00:00:00 2001 From: Tim Kral Date: Sat, 8 Jan 2022 20:04:17 -0800 Subject: [PATCH 11/14] Revert to old default list-type --- .golangci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index d064a084130d..651a326706a5 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,6 +1,6 @@ linters-settings: depguard: - list-type: denylist + list-type: blacklist packages: # logging is allowed only by logutils.Log, logrus # is allowed to use only in logutils package From 4d73509b9afa7fa84e9e2c295d09d04a7f8e9aff Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 9 Jan 2022 09:23:12 +0100 Subject: [PATCH 12/14] review: refactor --- go.sum | 3 - pkg/commands/run.go | 4 - pkg/config/linters_settings.go | 14 +-- pkg/golinters/depguard.go | 207 ++++++++++++++++----------------- 4 files changed, 105 insertions(+), 123 deletions(-) diff --git a/go.sum b/go.sum index e9e2dc917cf2..ca58c3f44f1c 100644 --- a/go.sum +++ b/go.sum @@ -72,8 +72,6 @@ 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= @@ -1070,7 +1068,6 @@ golang.org/x/sys v0.0.0-20211105183446-c75c47738b0c/go.mod h1:oPkhp1MJrh7nUepCBc 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-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= diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 9e358f916fdd..f75fa82f3bd3 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -193,10 +193,6 @@ func initFlagSet(fs *pflag.FlagSet, cfg *config.Config, m *lintersdb.Manager, is "Depguard: check list against standard lib") hideFlag("depguard.include-go-root") - fs.StringSliceVar(&lsc.Depguard.IgnoreFileRules, "depguard.ignore-file-rules", nil, - "Depguard: rules to ignore certain files for consideration") - hideFlag("depguard.ignore-file-rules") - 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 188cad202165..123ebc7feedc 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -176,16 +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"` - IgnoreFileRules []string `mapstructure:"ignore-file-rules"` - AdditionalGuards []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"` - } `mapstructure:"additional-guards"` + 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 36efbaab6823..d6018eac624b 100644 --- a/pkg/golinters/depguard.go +++ b/pkg/golinters/depguard.go @@ -15,121 +15,25 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) -func parseDepguardSettings(dgSettings *config.DepGuardSettings) (map[*depguard.Depguard]map[string]string, error) { - parsedDgSettings := make(map[*depguard.Depguard]map[string]string) - dg := &depguard.Depguard{ - Packages: dgSettings.Packages, - IncludeGoRoot: dgSettings.IncludeGoRoot, - IgnoreFileRules: dgSettings.IgnoreFileRules, - } - - if err := setDepguardListType(dg, dgSettings.ListType); err != nil { - return nil, err - } - setupDepguardPackages(dg, dgSettings.PackagesWithErrorMessage) - if dgSettings.PackagesWithErrorMessage != nil { - parsedDgSettings[dg] = dgSettings.PackagesWithErrorMessage - } else { - parsedDgSettings[dg] = make(map[string]string) - } - - for _, additionalGuard := range dgSettings.AdditionalGuards { - additionalDg := &depguard.Depguard{ - Packages: additionalGuard.Packages, - IncludeGoRoot: additionalGuard.IncludeGoRoot, - IgnoreFileRules: additionalGuard.IgnoreFileRules, - } - - if err := setDepguardListType(additionalDg, additionalGuard.ListType); err != nil { - return nil, err - } - setupDepguardPackages(additionalDg, additionalGuard.PackagesWithErrorMessage) - if additionalGuard.PackagesWithErrorMessage != nil { - parsedDgSettings[additionalDg] = additionalGuard.PackagesWithErrorMessage - } else { - parsedDgSettings[additionalDg] = make(map[string]string) - } - } - - return parsedDgSettings, nil -} - -func postProcessIssue( - issue *depguard.Issue, - dg *depguard.Depguard, - packagesWithErrorMessage map[string]string, - lintCtx *linter.Context, -) *result.Issue { - msgSuffix := "is in the blacklist" - if dg.ListType == depguard.LTWhitelist { - msgSuffix = "is not in the whitelist" - } - - userSuppliedMsgSuffix := packagesWithErrorMessage[issue.PackageName] - if userSuppliedMsgSuffix != "" { - userSuppliedMsgSuffix = ": " + userSuppliedMsgSuffix - } - - return &result.Issue{ - Pos: issue.Position, - Text: fmt.Sprintf("%s %s%s", formatCode(issue.PackageName, lintCtx.Cfg), msgSuffix, userSuppliedMsgSuffix), - FromLinter: linterName, - } -} - -func setDepguardListType(dg *depguard.Depguard, listType string) error { - 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, - packagesWithErrorMessage map[string]string, -) { - 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 packagesWithErrorMessage { - if _, ok := noMessagePackages[pkg]; !ok { - dg.Packages = append(dg.Packages, pkg) - } - } - } -} - -const linterName = "depguard" +const depguardLinterName = "depguard" func NewDepguard() *goanalysis.Linter { 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 + parsedSettings, err := parseDepguardSettings(&lintCtx.Settings().Depguard) + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { - parsedDgSettings, err := parseDepguardSettings(dgSettings) if err != nil { return nil, err } @@ -138,25 +42,116 @@ func NewDepguard() *goanalysis.Linter { Cwd: "", // fallbacked to os.Getcwd Build: nil, // fallbacked to build.Default } + prog := goanalysis.MakeFakeLoaderProgram(pass) - for dg, packagesWithErrorMessage := range parsedDgSettings { - issues, err := dg.Run(loadConfig, prog) - if err != nil { - return nil, err + for dg, pkgsWithErrorMessage := range parsedSettings { + issues, errRun := dg.Run(loadConfig, prog) + if errRun != nil { + return nil, errRun } + res := make([]goanalysis.Issue, 0, len(issues)) for _, i := range issues { - lintIssue := postProcessIssue(i, dg, packagesWithErrorMessage, lintCtx) + lintIssue := postProcessIssue(i, dg, pkgsWithErrorMessage, lintCtx) res = append(res, goanalysis.NewIssue(lintIssue, pass)) } + mu.Lock() resIssues = append(resIssues, res...) mu.Unlock() } + return nil, nil } }).WithIssuesReporter(func(*linter.Context) []goanalysis.Issue { return resIssues }).WithLoadMode(goanalysis.LoadModeTypesInfo) } + +func parseDepguardSettings(settings *config.DepGuardSettings) (map[*depguard.Depguard]map[string]string, error) { + parsedSettings := make(map[*depguard.Depguard]map[string]string) + + err := parseDGSettings(settings, parsedSettings) + if err != nil { + return nil, err + } + + for _, additional := range settings.AdditionalGuards { + add := additional + err := parseDGSettings(&add, parsedSettings) + if err != nil { + return nil, err + } + } + + return parsedSettings, nil +} + +func parseDGSettings(settings *config.DepGuardSettings, parsedSettings map[*depguard.Depguard]map[string]string) 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 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) + } + } + } + + if settings.PackagesWithErrorMessage != nil { + parsedSettings[dg] = settings.PackagesWithErrorMessage + } else { + parsedSettings[dg] = make(map[string]string) + } + + return nil +} + +func postProcessIssue(issue *depguard.Issue, dg *depguard.Depguard, + pkgsWithErrorMessage map[string]string, lintCtx *linter.Context) *result.Issue { + msgSuffix := "is in the blacklist" + if dg.ListType == depguard.LTWhitelist { + msgSuffix = "is not in the whitelist" + } + + userSuppliedMsgSuffix := pkgsWithErrorMessage[issue.PackageName] + if userSuppliedMsgSuffix != "" { + userSuppliedMsgSuffix = ": " + userSuppliedMsgSuffix + } + + return &result.Issue{ + Pos: issue.Position, + Text: fmt.Sprintf("%s %s%s", formatCode(issue.PackageName, lintCtx.Cfg), msgSuffix, userSuppliedMsgSuffix), + FromLinter: depguardLinterName, + } +} + +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 +} From f2cff713d692895f2aa4e792896a073d7ebe781a Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 9 Jan 2022 09:59:09 +0100 Subject: [PATCH 13/14] review: refactor --- pkg/golinters/depguard.go | 65 ++++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 29 deletions(-) diff --git a/pkg/golinters/depguard.go b/pkg/golinters/depguard.go index d6018eac624b..2ff813ed7670 100644 --- a/pkg/golinters/depguard.go +++ b/pkg/golinters/depguard.go @@ -31,7 +31,7 @@ func NewDepguard() *goanalysis.Linter { []*analysis.Analyzer{analyzer}, nil, ).WithContextSetter(func(lintCtx *linter.Context) { - parsedSettings, err := parseDepguardSettings(&lintCtx.Settings().Depguard) + parsedSettings, err := parseDepGuardSettings(&lintCtx.Settings().Depguard) analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { if err != nil { @@ -46,19 +46,13 @@ func NewDepguard() *goanalysis.Linter { prog := goanalysis.MakeFakeLoaderProgram(pass) for dg, pkgsWithErrorMessage := range parsedSettings { - issues, errRun := dg.Run(loadConfig, prog) + issues, errRun := runDepGuard(dg, pkgsWithErrorMessage, loadConfig, prog, pass) if errRun != nil { return nil, errRun } - res := make([]goanalysis.Issue, 0, len(issues)) - for _, i := range issues { - lintIssue := postProcessIssue(i, dg, pkgsWithErrorMessage, lintCtx) - res = append(res, goanalysis.NewIssue(lintIssue, pass)) - } - mu.Lock() - resIssues = append(resIssues, res...) + resIssues = append(resIssues, issues...) mu.Unlock() } @@ -69,7 +63,7 @@ func NewDepguard() *goanalysis.Linter { }).WithLoadMode(goanalysis.LoadModeTypesInfo) } -func parseDepguardSettings(settings *config.DepGuardSettings) (map[*depguard.Depguard]map[string]string, error) { +func parseDepGuardSettings(settings *config.DepGuardSettings) (map[*depguard.Depguard]map[string]string, error) { parsedSettings := make(map[*depguard.Depguard]map[string]string) err := parseDGSettings(settings, parsedSettings) @@ -124,25 +118,6 @@ func parseDGSettings(settings *config.DepGuardSettings, parsedSettings map[*depg return nil } -func postProcessIssue(issue *depguard.Issue, dg *depguard.Depguard, - pkgsWithErrorMessage map[string]string, lintCtx *linter.Context) *result.Issue { - msgSuffix := "is in the blacklist" - if dg.ListType == depguard.LTWhitelist { - msgSuffix = "is not in the whitelist" - } - - userSuppliedMsgSuffix := pkgsWithErrorMessage[issue.PackageName] - if userSuppliedMsgSuffix != "" { - userSuppliedMsgSuffix = ": " + userSuppliedMsgSuffix - } - - return &result.Issue{ - Pos: issue.Position, - Text: fmt.Sprintf("%s %s%s", formatCode(issue.PackageName, lintCtx.Cfg), msgSuffix, userSuppliedMsgSuffix), - FromLinter: depguardLinterName, - } -} - func getDepGuardListType(listType string) (depguard.ListType, error) { if listType == "" { return depguard.LTBlacklist, nil @@ -155,3 +130,35 @@ func getDepGuardListType(listType string) (depguard.ListType, error) { return listT, nil } + +func runDepGuard(dg *depguard.Depguard, pkgsWithErrorMessage map[string]string, + loadConfig *loader.Config, prog *loader.Program, pass *analysis.Pass) ([]goanalysis.Issue, error) { + issues, err := dg.Run(loadConfig, prog) + if err != nil { + return nil, err + } + + res := make([]goanalysis.Issue, 0, len(issues)) + + for _, issue := range issues { + msgSuffix := "is in the blacklist" + if dg.ListType == depguard.LTWhitelist { + msgSuffix = "is not in the whitelist" + } + + userSuppliedMsgSuffix := pkgsWithErrorMessage[issue.PackageName] + if userSuppliedMsgSuffix != "" { + userSuppliedMsgSuffix = ": " + userSuppliedMsgSuffix + } + + res = append(res, + goanalysis.NewIssue(&result.Issue{ + Pos: issue.Position, + Text: fmt.Sprintf("%s %s%s", formatCode(issue.PackageName, nil), msgSuffix, userSuppliedMsgSuffix), + FromLinter: depguardLinterName, + }, pass), + ) + } + + return res, nil +} From 781253f79dcd3ca0830018ecb15b4d630657ac14 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 9 Jan 2022 11:34:35 +0100 Subject: [PATCH 14/14] review: refactor --- pkg/golinters/depguard.go | 144 +++++++++++++++++++++++--------------- 1 file changed, 86 insertions(+), 58 deletions(-) diff --git a/pkg/golinters/depguard.go b/pkg/golinters/depguard.go index 2ff813ed7670..dd6a79772072 100644 --- a/pkg/golinters/depguard.go +++ b/pkg/golinters/depguard.go @@ -31,30 +31,21 @@ func NewDepguard() *goanalysis.Linter { []*analysis.Analyzer{analyzer}, nil, ).WithContextSetter(func(lintCtx *linter.Context) { - parsedSettings, err := parseDepGuardSettings(&lintCtx.Settings().Depguard) + dg, err := newDepGuard(&lintCtx.Settings().Depguard) analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { if err != nil { return nil, err } - loadConfig := &loader.Config{ - Cwd: "", // fallbacked to os.Getcwd - Build: nil, // fallbacked to build.Default + issues, errRun := dg.run(pass) + if errRun != nil { + return nil, errRun } - prog := goanalysis.MakeFakeLoaderProgram(pass) - - for dg, pkgsWithErrorMessage := range parsedSettings { - issues, errRun := runDepGuard(dg, pkgsWithErrorMessage, loadConfig, prog, pass) - if errRun != nil { - return nil, errRun - } - - mu.Lock() - resIssues = append(resIssues, issues...) - mu.Unlock() - } + mu.Lock() + resIssues = append(resIssues, issues...) + mu.Unlock() return nil, nil } @@ -63,26 +54,60 @@ func NewDepguard() *goanalysis.Linter { }).WithLoadMode(goanalysis.LoadModeTypesInfo) } -func parseDepGuardSettings(settings *config.DepGuardSettings) (map[*depguard.Depguard]map[string]string, error) { - parsedSettings := make(map[*depguard.Depguard]map[string]string) +type depGuard struct { + loadConfig *loader.Config + guardians []*guardian +} - err := parseDGSettings(settings, parsedSettings) +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 - err := parseDGSettings(&add, parsedSettings) + 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 parsedSettings, nil + return resIssues, nil } -func parseDGSettings(settings *config.DepGuardSettings, parsedSettings map[*depguard.Depguard]map[string]string) error { +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, @@ -92,10 +117,10 @@ func parseDGSettings(settings *config.DepGuardSettings, parsedSettings map[*depg var err error dg.ListType, err = getDepGuardListType(settings.ListType) if err != nil { - return err + return nil, err } - // if the list type was a blacklist the packages with error messages should be included in the blacklist package list + // 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 { @@ -109,31 +134,14 @@ func parseDGSettings(settings *config.DepGuardSettings, parsedSettings map[*depg } } - if settings.PackagesWithErrorMessage != nil { - parsedSettings[dg] = settings.PackagesWithErrorMessage - } else { - parsedSettings[dg] = make(map[string]string) - } - - return nil + return &guardian{ + Depguard: dg, + pkgsWithErrorMessage: settings.PackagesWithErrorMessage, + }, nil } -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 -} - -func runDepGuard(dg *depguard.Depguard, pkgsWithErrorMessage map[string]string, - loadConfig *loader.Config, prog *loader.Program, pass *analysis.Pass) ([]goanalysis.Issue, error) { - issues, err := dg.Run(loadConfig, prog) +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 } @@ -141,20 +149,10 @@ func runDepGuard(dg *depguard.Depguard, pkgsWithErrorMessage map[string]string, res := make([]goanalysis.Issue, 0, len(issues)) for _, issue := range issues { - msgSuffix := "is in the blacklist" - if dg.ListType == depguard.LTWhitelist { - msgSuffix = "is not in the whitelist" - } - - userSuppliedMsgSuffix := pkgsWithErrorMessage[issue.PackageName] - if userSuppliedMsgSuffix != "" { - userSuppliedMsgSuffix = ": " + userSuppliedMsgSuffix - } - res = append(res, goanalysis.NewIssue(&result.Issue{ Pos: issue.Position, - Text: fmt.Sprintf("%s %s%s", formatCode(issue.PackageName, nil), msgSuffix, userSuppliedMsgSuffix), + Text: g.createMsg(issue.PackageName), FromLinter: depguardLinterName, }, pass), ) @@ -162,3 +160,33 @@ func runDepGuard(dg *depguard.Depguard, pkgsWithErrorMessage map[string]string, 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 +}