From 335e7d8c956ff1119d2a4785640076e9b30bd20e Mon Sep 17 00:00:00 2001 From: Ryan Currah Date: Wed, 13 May 2020 08:59:31 -0400 Subject: [PATCH] added version contraint liniting --- .gomodguard.yaml | 13 +- README.md | 19 +- cmd.go | 28 ++- cmd/gomodguard/main.go | 4 +- cmd_test.go | 7 +- go.mod | 1 + go.sum | 2 + gomodguard.go | 347 +++++++++++++++++------------ gomodguard_test.go | 486 +++++++++++++++++++++++++---------------- 9 files changed, 563 insertions(+), 344 deletions(-) diff --git a/.gomodguard.yaml b/.gomodguard.yaml index c0f061f..38a2f0b 100644 --- a/.gomodguard.yaml +++ b/.gomodguard.yaml @@ -2,7 +2,7 @@ allowed: modules: # List of allowed modules - gopkg.in/yaml.v2 - github.com/go-xmlfmt/xmlfmt - - github.com/phayes/checkstyle + - github.com/Masterminds/semver domains: # List of allowed module domains - golang.org @@ -15,4 +15,13 @@ blocked: - github.com/mitchellh/go-homedir: recommendations: - github.com/ryancurrah/gomodguard - reason: "testing if the linted module is not blocked when it is recommended" + reason: "testing if the current/linted module is not blocked when it is recommended" + - github.com/phayes/checkstyle: + recommendations: + - github.com/someother/module + reason: "testing if module is blocked with recommendation" + + versions: + - github.com/mitchellh/go-homedir: + version: "<= 1.1.0" + reason: "testing if blocked version constraint works." diff --git a/README.md b/README.md index 0589956..f09b5e1 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,8 @@ Alternative modules can be optionally recommended in the blocked modules list. If the linted module imports a blocked module but the linted module is in the recommended modules list the blocked module is ignored. Usually, this means the linted module wraps that blocked module for use by other modules, therefore the import of the blocked module should not be blocked. +Version constraints can be specified for modules as well which lets you block new or old versions of modules or specific versions. + Results are printed to `stdout`. Logging statements are printed to `stderr`. @@ -44,6 +46,10 @@ blocked: recommendations: # Recommended modules that should be used instead (Optional) - golang.org/x/mod reason: "`mod` is the official go.mod parser library." # Reason why the recommended module should be used (Optional) + versions: # List of blocked module version constraints. + - github.com/mitchellh/go-homedir: # Blocked module with version constraint. + version: "<= 1.1.0" # Version constraint, see https://github.com/Masterminds/semver#basic-comparisons. + reason: "testing if blocked version constraint works." # Reason why the version constraint exists. ``` ## Usage @@ -54,17 +60,22 @@ Usage: gomodguard [files...] Also supports package syntax but will use it in relative path, i.e. ./pkg/... Flags: -f string - Report results to the specified file. A report type must also be specified + Report results to the specified file. A report type must also be specified -file string - -h Show this help text + -h Show this help text -help - -n Don't lint test files + -i int + Exit code when issues were found (default 2) + -issues-exit-code int + (default 2) + + -n Don't lint test files -no-test -r string - Report results to one of the following formats: checkstyle. A report file destination must also be specified + Report results to one of the following formats: checkstyle. A report file destination must also be specified -report string ``` diff --git a/cmd.go b/cmd.go index cf50540..652e61f 100644 --- a/cmd.go +++ b/cmd.go @@ -24,19 +24,19 @@ const ( var ( configFile = ".gomodguard.yaml" logger = log.New(os.Stderr, "", 0) - lintErrorRC = 2 errFindingConfigFile = fmt.Errorf("could not find config file") ) -// Run the gomodguard linter. -func Run() { +// Run the gomodguard linter. Returns the exit code to use. +func Run() int { var ( - args []string - help bool - noTest bool - report string - reportFile string - cwd, _ = os.Getwd() + args []string + help bool + noTest bool + report string + reportFile string + issuesExitCode int + cwd, _ = os.Getwd() ) flag.BoolVar(&help, "h", false, "Show this help text") @@ -47,13 +47,15 @@ func Run() { flag.StringVar(&report, "report", "", "") flag.StringVar(&reportFile, "f", "", "Report results to the specified file. A report type must also be specified") flag.StringVar(&reportFile, "file", "", "") + flag.IntVar(&issuesExitCode, "i", 2, "Exit code when issues were found") + flag.IntVar(&issuesExitCode, "issues-exit-code", 2, "") flag.Parse() report = strings.TrimSpace(strings.ToLower(report)) if help { showHelp() - return + return 0 } if report != "" && report != "checkstyle" { @@ -99,8 +101,10 @@ func Run() { } if len(results) > 0 { - os.Exit(lintErrorRC) + return issuesExitCode } + + return 0 } // GetConfig from YAML file. @@ -180,7 +184,7 @@ func GetFilteredFiles(cwd string, skipTests bool, args []string) []string { // showHelp text for command line. func showHelp() { - helpText := `Usage: gomodguard [foundFiles...] + helpText := `Usage: gomodguard [files...] Also supports package syntax but will use it in relative path, i.e. ./pkg/... Flags:` fmt.Println(helpText) diff --git a/cmd/gomodguard/main.go b/cmd/gomodguard/main.go index caff14d..caa3a92 100644 --- a/cmd/gomodguard/main.go +++ b/cmd/gomodguard/main.go @@ -1,9 +1,11 @@ package main import ( + "os" + "github.com/ryancurrah/gomodguard" ) func main() { - gomodguard.Run() + os.Exit(gomodguard.Run()) } diff --git a/cmd_test.go b/cmd_test.go index b505eed..99cfafa 100644 --- a/cmd_test.go +++ b/cmd_test.go @@ -7,5 +7,10 @@ import ( ) func TestCmdRun(t *testing.T) { - gomodguard.Run() + wantExitCode := 2 + exitCode := gomodguard.Run() + + if exitCode != wantExitCode { + t.Errorf("got exit code '%d' want '%d'", exitCode, wantExitCode) + } } diff --git a/go.mod b/go.mod index 17ba2af..15231c9 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/ryancurrah/gomodguard go 1.14 require ( + github.com/Masterminds/semver v1.5.0 github.com/go-xmlfmt/xmlfmt v0.0.0-20191208150333-d5b6f63a941b github.com/mitchellh/go-homedir v1.1.0 github.com/phayes/checkstyle v0.0.0-20170904204023-bfd46e6a821d diff --git a/go.sum b/go.sum index a918cac..55ae4e5 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,5 @@ +github.com/Masterminds/semver v1.5.0 h1:H65muMkzWKEuNDnfl9d70GUjFniHKHRbFPGBuZ3QEww= +github.com/Masterminds/semver v1.5.0/go.mod h1:MB6lktGJrhw8PrUyiEoblNEGEQ+RzHPF078ddwwvV3Y= github.com/go-xmlfmt/xmlfmt v0.0.0-20191208150333-d5b6f63a941b h1:khEcpUM4yFcxg4/FHQWkvVRmgijNXRfzkIDHh23ggEo= github.com/go-xmlfmt/xmlfmt v0.0.0-20191208150333-d5b6f63a941b/go.mod h1:aUCEOzzezBEjDBbFBoSiya/gduyIiWYRP6CnSFIV8AM= github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y= diff --git a/gomodguard.go b/gomodguard.go index 865c710..1646773 100644 --- a/gomodguard.go +++ b/gomodguard.go @@ -12,31 +12,104 @@ import ( "os/exec" "strings" + "github.com/Masterminds/semver" + "golang.org/x/mod/modfile" ) const ( - blockedReasonNotInAllowedList = "import of package `%s` is blocked because the module is not in the allowed modules list." - blockedReasonInBlockedList = "import of package `%s` is blocked because the module is in the blocked modules list." - goModFilename = "go.mod" - errReadingGoModFile = "unable to read go mod file %s: %w" - errParsingGoModFile = "unable to parsing go mod file %s: %w" + goModFilename = "go.mod" + errReadingGoModFile = "unable to read go mod file %s: %w" + errParsingGoModFile = "unable to parsing go mod file %s: %w" +) + +var ( + blockReasonNotInAllowedList = "import of package `%s` is blocked because the module is not in the allowed modules list." + blockReasonInBlockedList = "import of package `%s` is blocked because the module is in the blocked modules list." ) -// Recommendations are alternative modules to use and a reason why. -type Recommendations struct { - Recommendations []string `yaml:"recommendations"` - Reason string `yaml:"reason"` +// BlockedVersion has a version constraint a reason why the the module version is blocked. +type BlockedVersion struct { + Version string `yaml:"version"` + Reason string `yaml:"reason"` + lintedModuleVersion string `yaml:"-"` +} + +// Set required values for performing checks. This must be ran before running anything else. +func (r *BlockedVersion) Set(lintedModuleVersion string) { + r.lintedModuleVersion = lintedModuleVersion +} + +// IsAllowed returns true if the blocked module is allowed. You must Set() values first. +func (r *BlockedVersion) IsAllowed() bool { + return !r.isLintedModuleVersionBlocked() +} + +// isLintedModuleVersionBlocked returns true if version constraint specified and the +// linted module version meets the constraint. +func (r *BlockedVersion) isLintedModuleVersionBlocked() bool { + if r.Version == "" { + return false + } + + constraint, err := semver.NewConstraint(r.Version) + if err != nil { + return false + } + + version, err := semver.NewVersion(strings.TrimLeft(r.lintedModuleVersion, "v")) + if err != nil { + return false + } + + return constraint.Check(version) +} + +// Message returns the reason why the module version is blocked. +func (r *BlockedVersion) Message() string { + msg := "" + + // Add version contraint to message + msg += fmt.Sprintf("version `%s` is blocked because it does not meet the version constraint `%s`.", r.lintedModuleVersion, r.Version) + + if r.Reason == "" { + return msg + } + + // Add reason to message + msg += fmt.Sprintf(" %s.", strings.TrimRight(r.Reason, ".")) + + return msg +} + +// BlockedModule has alternative modules to use and a reason why the module is blocked. +type BlockedModule struct { + Recommendations []string `yaml:"recommendations"` + Reason string `yaml:"reason"` + currentModuleName string `yaml:"-"` } -// IsRecommended returns true if the package provided is in the Recommendations list. -func (r *Recommendations) IsRecommended(pkg string) bool { +// Set required values for performing checks. This must be ran before running anything else. +func (r *BlockedModule) Set(currentModuleName string) { + r.currentModuleName = currentModuleName +} + +// IsAllowed returns true if the blocked module is allowed. You must Set() values first. +func (r *BlockedModule) IsAllowed() bool { + // If the current go.mod file being linted is a recommended module of a + // blocked module and it imports that blocked module, do not set as blocked. + // This could mean that the linted module is a wrapper for that blocked module. + return r.isCurrentModuleARecommendation() +} + +// isCurrentModuleARecommendation returns true if the current module is in the Recommendations list. +func (r *BlockedModule) isCurrentModuleARecommendation() bool { if r == nil { return false } for n := range r.Recommendations { - if strings.TrimSpace(pkg) == strings.TrimSpace(r.Recommendations[n]) { + if strings.TrimSpace(r.currentModuleName) == strings.TrimSpace(r.Recommendations[n]) { return true } } @@ -44,14 +117,11 @@ func (r *Recommendations) IsRecommended(pkg string) bool { return false } -// String returns the recommended modules and reason message. -func (r *Recommendations) String() string { +// Message returns the reason why the module is blocked and a list of recommended modules if provided. +func (r *BlockedModule) Message() string { msg := "" - if r == nil { - return msg - } - + // Add recommendations to message for i := range r.Recommendations { switch { case len(r.Recommendations) == 1: @@ -65,8 +135,15 @@ func (r *Recommendations) String() string { } } - if r.Reason != "" { - msg += fmt.Sprintf(" %s", r.Reason) + if r.Reason == "" { + return msg + } + + // Add reason to message + if msg == "" { + msg = fmt.Sprintf("%s.", strings.TrimRight(r.Reason, ".")) + } else { + msg += fmt.Sprintf(" %s.", strings.TrimRight(r.Reason, ".")) } return msg @@ -74,7 +151,7 @@ func (r *Recommendations) String() string { // HasRecommendations returns true if the blocked package has // recommended modules. -func (r *Recommendations) HasRecommendations() bool { +func (r *BlockedModule) HasRecommendations() bool { if r == nil { return false } @@ -82,21 +159,16 @@ func (r *Recommendations) HasRecommendations() bool { return len(r.Recommendations) > 0 } -// BlockedModule is a blocked module name and -// optionally a list of recommended modules -// and a reason message. -type BlockedModule map[string]Recommendations - -// BlockedModules a list of blocked modules. -type BlockedModules []BlockedModule +// BlockedVersions a list of blocked modules by a version constraint. +type BlockedVersions []map[string]BlockedVersion -// Get returns the modules that are blocked. -func (b BlockedModules) Get() []string { +// Get returns the module names that are blocked. +func (b BlockedVersions) Get() []string { modules := make([]string, len(b)) - for i := range b { - for module := range b[i] { - modules[i] = module + for n := range b { + for module := range b[n] { + modules[n] = module break } } @@ -104,46 +176,49 @@ func (b BlockedModules) Get() []string { return modules } -// RecommendedModules will return a list of recommended modules for the -// package provided. If there is no recommendation nil will be returned. -func (b BlockedModules) RecommendedModules(pkg string) *Recommendations { - for i := range b { - for blockedModule, recommendations := range b[i] { - if strings.HasPrefix(strings.ToLower(pkg), strings.ToLower(blockedModule)) && recommendations.HasRecommendations() { - return &recommendations +// GetBlockReason returns a block version if one is set for the provided linted module name. +func (b BlockedVersions) GetBlockReason(lintedModuleName, lintedModuleVersion string) *BlockedVersion { + for _, blockedModule := range b { + for blockedModuleName, blockedVersion := range blockedModule { + if strings.EqualFold(strings.TrimSpace(lintedModuleName), strings.TrimSpace(blockedModuleName)) { + blockedVersion.Set(lintedModuleVersion) + return &blockedVersion } - - break } } return nil } -// IsBlockedPackage returns true if the package name is in -// the blocked modules list. -func (b BlockedModules) IsBlockedPackage(pkg string) bool { - blockedModules := b.Get() - for i := range blockedModules { - if strings.HasPrefix(strings.ToLower(pkg), strings.ToLower(blockedModules[i])) { - return true +// BlockedModules a list of blocked modules. +type BlockedModules []map[string]BlockedModule + +// Get returns the module names that are blocked. +func (b BlockedModules) Get() []string { + modules := make([]string, len(b)) + + for n := range b { + for module := range b[n] { + modules[n] = module + break } } - return false + return modules } -// IsBlockedModule returns true if the given module name is in the -// blocked modules list. -func (b BlockedModules) IsBlockedModule(module string) bool { - blockedModules := b.Get() - for i := range blockedModules { - if strings.EqualFold(module, strings.TrimSpace(blockedModules[i])) { - return true +// GetBlockReason returns a block module if one is set for the provided linted module name. +func (b BlockedModules) GetBlockReason(currentModuleName, lintedModuleName string) *BlockedModule { + for _, blockedModule := range b { + for blockedModuleName, blockedModule := range blockedModule { + if strings.EqualFold(strings.TrimSpace(lintedModuleName), strings.TrimSpace(blockedModuleName)) { + blockedModule.Set(currentModuleName) + return &blockedModule + } } } - return false + return nil } // Allowed is a list of modules and module @@ -155,10 +230,11 @@ type Allowed struct { // IsAllowedModule returns true if the given module // name is in the allowed modules list. -func (a *Allowed) IsAllowedModule(module string) bool { +func (a *Allowed) IsAllowedModule(moduleName string) bool { allowedModules := a.Modules + for i := range allowedModules { - if strings.EqualFold(module, strings.TrimSpace(allowedModules[i])) { + if strings.EqualFold(strings.TrimSpace(moduleName), strings.TrimSpace(allowedModules[i])) { return true } } @@ -168,10 +244,11 @@ func (a *Allowed) IsAllowedModule(module string) bool { // IsAllowedModuleDomain returns true if the given modules domain is // in the allowed module domains list. -func (a *Allowed) IsAllowedModuleDomain(module string) bool { +func (a *Allowed) IsAllowedModuleDomain(moduleName string) bool { allowedDomains := a.Domains + for i := range allowedDomains { - if strings.HasPrefix(strings.ToLower(module), strings.TrimSpace(strings.ToLower(allowedDomains[i]))) { + if strings.HasPrefix(strings.TrimSpace(strings.ToLower(moduleName)), strings.TrimSpace(strings.ToLower(allowedDomains[i]))) { return true } } @@ -182,7 +259,8 @@ func (a *Allowed) IsAllowedModuleDomain(module string) bool { // Blocked is a list of modules that are // blocked and not to be used. type Blocked struct { - Modules BlockedModules `yaml:"modules"` + Modules BlockedModules `yaml:"modules"` + Versions BlockedVersions `yaml:"versions"` } // Configuration of gomodguard allow and block lists. @@ -202,16 +280,16 @@ type Result struct { // String returns the filename, line // number and reason of a Result. func (r *Result) String() string { - return fmt.Sprintf("%s:%d: %s", r.FileName, r.LineNumber, r.Reason) + return fmt.Sprintf("%s:%d:1 %s", r.FileName, r.LineNumber, r.Reason) } // Processor processes Go files. type Processor struct { - config Configuration - logger *log.Logger - modfile *modfile.File - blockedModulesFromModFile []string - result []Result + Config Configuration + Logger *log.Logger + Modfile *modfile.File + blockedModulesFromModFile map[string][]string + Result []Result } // NewProcessor will create a Processor to lint blocked packages. @@ -229,15 +307,16 @@ func NewProcessor(config Configuration, logger *log.Logger) (*Processor, error) logger.Printf("info: allowed modules, %+v", config.Allowed.Modules) logger.Printf("info: allowed module domains, %+v", config.Allowed.Domains) logger.Printf("info: blocked modules, %+v", config.Blocked.Modules.Get()) + logger.Printf("info: blocked modules with version constraints, %+v", config.Blocked.Versions.Get()) p := &Processor{ - config: config, - logger: logger, - modfile: mfile, - result: []Result{}, + Config: config, + Logger: logger, + Modfile: mfile, + Result: []Result{}, } - p.setBlockedModulesFromModFile() + p.SetBlockedModulesFromModFile() return p, nil } @@ -250,13 +329,18 @@ func (p *Processor) ProcessFiles(filenames []string) []Result { pluralModuleMsg = "" } - p.logger.Printf("info: found %d blocked module%s in the %s file, %+v", - len(p.blockedModulesFromModFile), pluralModuleMsg, goModFilename, p.blockedModulesFromModFile) + blockedModules := make([]string, 0, len(p.blockedModulesFromModFile)) + for blockedModuleName := range p.blockedModulesFromModFile { + blockedModules = append(blockedModules, blockedModuleName) + } + + p.Logger.Printf("info: found %d blocked module%s in %s: %+v", + len(p.blockedModulesFromModFile), pluralModuleMsg, goModFilename, blockedModules) for _, filename := range filenames { data, err := ioutil.ReadFile(filename) if err != nil { - p.result = append(p.result, Result{ + p.Result = append(p.Result, Result{ FileName: filename, LineNumber: 0, Reason: fmt.Sprintf("unable to read file, file cannot be linted (%s)", err.Error()), @@ -266,7 +350,7 @@ func (p *Processor) ProcessFiles(filenames []string) []Result { p.process(filename, data) } - return p.result + return p.Result } // process file imports and add lint error if blocked package is imported. @@ -275,7 +359,7 @@ func (p *Processor) process(filename string, data []byte) { file, err := parser.ParseFile(fileSet, filename, data, parser.ParseComments) if err != nil { - p.result = append(p.result, Result{ + p.Result = append(p.Result, Result{ FileName: filename, LineNumber: 0, Reason: fmt.Sprintf("invalid syntax, file cannot be linted (%s)", err.Error()), @@ -285,23 +369,16 @@ func (p *Processor) process(filename string, data []byte) { } imports := file.Imports - for i := range imports { - importedPkg := strings.TrimSpace(strings.Trim(imports[i].Path.Value, "\"")) - if p.isBlockedPackageFromModFile(importedPkg) { - reason := "" - - if p.config.Blocked.Modules.IsBlockedPackage(importedPkg) { - reason = fmt.Sprintf(blockedReasonInBlockedList, importedPkg) - } else { - reason = fmt.Sprintf(blockedReasonNotInAllowedList, importedPkg) - } + for n := range imports { + importedPkg := strings.TrimSpace(strings.Trim(imports[n].Path.Value, "\"")) - recommendedModules := p.config.Blocked.Modules.RecommendedModules(importedPkg) - if recommendedModules != nil { - reason += fmt.Sprintf(" %s", recommendedModules.String()) - } + blockReasons := p.isBlockedPackageFromModFile(importedPkg) + if blockReasons == nil { + continue + } - p.addError(fileSet, imports[i].Pos(), reason) + for _, blockReason := range blockReasons { + p.addError(fileSet, imports[n].Pos(), blockReason) } } } @@ -311,7 +388,7 @@ func (p *Processor) process(filename string, data []byte) { func (p *Processor) addError(fileset *token.FileSet, pos token.Pos, reason string) { position := fileset.Position(pos) - p.result = append(p.result, Result{ + p.Result = append(p.Result, Result{ FileName: position.Filename, LineNumber: position.Line, Position: position, @@ -319,67 +396,69 @@ func (p *Processor) addError(fileset *token.FileSet, pos token.Pos, reason strin }) } -// setBlockedModules determines which modules are blocked by reading +// SetBlockedModulesFromModFile determines which modules are blocked by reading // the go.mod file and comparing the require modules to the allowed modules. -func (p *Processor) setBlockedModulesFromModFile() { - blockedModules := make([]string, 0, len(p.modfile.Require)) - requiredModules := p.modfile.Require - lintedModule := p.modfile.Module.Mod.Path +func (p *Processor) SetBlockedModulesFromModFile() { + blockedModules := make(map[string][]string, len(p.Modfile.Require)) + currentModuleName := p.Modfile.Module.Mod.Path + lintedModules := p.Modfile.Require - for i := range requiredModules { - if requiredModules[i].Indirect { + for i := range lintedModules { + if lintedModules[i].Indirect { continue } - requiredModule := strings.TrimSpace(requiredModules[i].Mod.Path) + lintedModuleName := strings.TrimSpace(lintedModules[i].Mod.Path) + lintedModuleVersion := strings.TrimSpace(lintedModules[i].Mod.Version) - if p.config.Allowed.IsAllowedModuleDomain(requiredModule) { - continue - } + var isAllowed bool - if p.config.Allowed.IsAllowedModule(requiredModule) { - continue + switch { + case len(p.Config.Allowed.Modules) == 0 && len(p.Config.Allowed.Domains) == 0: + isAllowed = true + case p.Config.Allowed.IsAllowedModuleDomain(lintedModuleName): + isAllowed = true + case p.Config.Allowed.IsAllowedModule(lintedModuleName): + isAllowed = true + default: + isAllowed = false } - requiredModuleIsBlocked := p.config.Blocked.Modules.IsBlockedModule(requiredModule) + blockModuleReason := p.Config.Blocked.Modules.GetBlockReason(currentModuleName, lintedModuleName) + blockVersionReason := p.Config.Blocked.Versions.GetBlockReason(lintedModuleName, lintedModuleVersion) - // If module is not in allowed modules list and is not blocked it is allowed - if len(p.config.Allowed.Modules) == 0 && - len(p.config.Allowed.Domains) == 0 && - !requiredModuleIsBlocked { + if !isAllowed && blockModuleReason == nil && blockVersionReason == nil { + blockedModules[lintedModuleName] = append(blockedModules[lintedModuleName], blockReasonNotInAllowedList) continue } - // If the go.mod file being linted is a recommended module of a blocked module - // and it imports that blocked module, do not set as a blocked. This means - // that the linted module wraps that blocked module - if requiredModuleIsBlocked { - recommendedModules := p.config.Blocked.Modules.RecommendedModules(requiredModule) - - if recommendedModules.IsRecommended(lintedModule) { - continue - } + if blockModuleReason != nil && !blockModuleReason.IsAllowed() { + blockedModules[lintedModuleName] = append(blockedModules[lintedModuleName], fmt.Sprintf("%s %s", blockReasonInBlockedList, blockModuleReason.Message())) } - blockedModules = append(blockedModules, requiredModule) + if blockVersionReason != nil && !blockVersionReason.IsAllowed() { + blockedModules[lintedModuleName] = append(blockedModules[lintedModuleName], fmt.Sprintf("%s %s", blockReasonInBlockedList, blockVersionReason.Message())) + } } - if len(blockedModules) > 0 { - p.blockedModulesFromModFile = blockedModules - } + p.blockedModulesFromModFile = blockedModules } -// isBlockedPackageFromModFile returns true if the imported packages -// module is in the go.mod file and was blocked. -func (p *Processor) isBlockedPackageFromModFile(pkg string) bool { - blockedModulesFromModFile := p.blockedModulesFromModFile - for i := range blockedModulesFromModFile { - if strings.HasPrefix(strings.ToLower(pkg), strings.ToLower(blockedModulesFromModFile[i])) { - return true +// isBlockedPackageFromModFile returns the block reason if the package is blocked. +func (p *Processor) isBlockedPackageFromModFile(packageName string) []string { + for blockedModuleName, blockReasons := range p.blockedModulesFromModFile { + if strings.HasPrefix(strings.TrimSpace(packageName), strings.TrimSpace(blockedModuleName)) { + formattedReasons := make([]string, 0, len(blockReasons)) + + for _, blockReason := range blockReasons { + formattedReasons = append(formattedReasons, fmt.Sprintf(blockReason, packageName)) + } + + return formattedReasons } } - return false + return nil } func loadGoModFile() ([]byte, error) { diff --git a/gomodguard_test.go b/gomodguard_test.go index 85418a5..9745c62 100644 --- a/gomodguard_test.go +++ b/gomodguard_test.go @@ -1,251 +1,357 @@ +// nolint:scopelint package gomodguard_test import ( - "fmt" "log" "os" + "reflect" "strings" "testing" "github.com/ryancurrah/gomodguard" ) -var ( - recommendedModule = "github.com/ryancurrah/gomodguard" - invalidModule = "github.com/ryancurrah/gomodbump" - blockedModule = "github.com/someblocked/module" - allowedModule = "github.com/someallowed/module" - allowedDomain = "golang.org" - recommendations = gomodguard.Recommendations{Recommendations: []string{recommendedModule}, Reason: "test reason"} - blockedModules = gomodguard.BlockedModules{{blockedModule: recommendations}} - allowed = gomodguard.Allowed{Modules: []string{allowedModule}, Domains: []string{allowedDomain}} -) - -func TestGomodguardIsRecommended(t *testing.T) { - isRecommended := recommendations.IsRecommended(recommendedModule) - if !isRecommended { - t.Errorf("%s should have been recommended", recommendedModule) - } - - isRecommended = recommendations.IsRecommended(invalidModule) - if isRecommended { - t.Errorf("%s should NOT have been recommended", invalidModule) +func TestBlockedModuleIsAllowed(t *testing.T) { + var tests = []struct { + testName string + blockedModule gomodguard.BlockedModule + currentModuleName string + wantIsAllowed bool + }{ + { + "blocked", + gomodguard.BlockedModule{Recommendations: []string{"github.com/somerecommended/module"}}, + "github.com/ryancurrah/gomodguard", + false, + }, + { + "allowed", + gomodguard.BlockedModule{Recommendations: []string{"github.com/ryancurrah/gomodguard"}}, + "github.com/ryancurrah/gomodguard", + true, + }, + } + + for _, tt := range tests { + t.Run(tt.testName, func(t *testing.T) { + tt.blockedModule.Set(tt.currentModuleName) + isAllowed := tt.blockedModule.IsAllowed() + if isAllowed != tt.wantIsAllowed { + t.Errorf("got '%v' want '%v'", isAllowed, tt.wantIsAllowed) + } + }) } } -func TestGomodguardRecommendationsString(t *testing.T) { - recommendationsMsg := recommendations.String() - if recommendationsMsg == "" { - t.Error("recommendations string message should not be empty") - } - - if strings.Contains(recommendationsMsg, "modules") { - t.Errorf("recommendations string message should be singular: %s", recommendationsMsg) - } - - multipleRecommendations := recommendations - multipleRecommendations.Recommendations = append(multipleRecommendations.Recommendations, "github.com/some/thing", "github.com/some/otherthing") - - multipleRecommendationsMsg := multipleRecommendations.String() - if !strings.Contains(multipleRecommendationsMsg, "modules") { - t.Errorf("recommendations string message should be plural: %s", recommendationsMsg) - } - - emptyRecommendations := gomodguard.Recommendations{} - - recommendationsMsg = emptyRecommendations.String() - if recommendationsMsg != "" { - t.Error("recommendations string message should be empty") +func TestBlockedModuleMessage(t *testing.T) { + blockedWithNoRecommendation := "Some reason." + blockedWithRecommendation := "`github.com/somerecommended/module` is a recommended module. Some reason." + blockedWithRecommendations := "`github.com/somerecommended/module`, `github.com/someotherrecommended/module` and `github.com/someotherotherrecommended/module` are recommended modules. Some reason." + + var tests = []struct { + testName string + blockedModule gomodguard.BlockedModule + currentModuleName string + wantMessage string + }{ + { + "blocked with no recommendation", + gomodguard.BlockedModule{Recommendations: []string{}, Reason: "Some reason."}, + "github.com/ryancurrah/gomodguard", + blockedWithNoRecommendation, + }, + { + "blocked with recommendation", + gomodguard.BlockedModule{Recommendations: []string{"github.com/somerecommended/module"}, Reason: "Some reason."}, + "github.com/ryancurrah/gomodguard", + blockedWithRecommendation, + }, + { + "blocked with multiple recommendations", + gomodguard.BlockedModule{Recommendations: []string{"github.com/somerecommended/module", "github.com/someotherrecommended/module", "github.com/someotherotherrecommended/module"}, Reason: "Some reason."}, + "github.com/ryancurrah/gomodguard", + blockedWithRecommendations, + }, + } + + for _, tt := range tests { + t.Run(tt.testName, func(t *testing.T) { + tt.blockedModule.Set(tt.currentModuleName) + message := tt.blockedModule.Message() + if !strings.EqualFold(message, tt.wantMessage) { + t.Errorf("got '%s' want '%s'", message, tt.wantMessage) + } + }) } } -func TestGomodguardHasRecommendations(t *testing.T) { - hasRecommendations := recommendations.HasRecommendations() - if !hasRecommendations { - t.Error("should have recommendations when more than one recommended module in list") - } - - emptyRecommendations := gomodguard.Recommendations{} - - hasRecommendations = emptyRecommendations.HasRecommendations() - if hasRecommendations { - t.Error("should not have recommendations when no recommended modules in list") +func TestBlockedModuleHasRecommendations(t *testing.T) { + var tests = []struct { + testName string + blockedModule gomodguard.BlockedModule + wantIsAllowed bool + }{ + { + "does not have recommendations", + gomodguard.BlockedModule{Recommendations: []string{}}, + false, + }, + { + "does have recommendations", + gomodguard.BlockedModule{Recommendations: []string{"github.com/ryancurrah/gomodguard"}}, + true, + }, + } + + for _, tt := range tests { + t.Run(tt.testName, func(t *testing.T) { + hasRecommendations := tt.blockedModule.HasRecommendations() + if hasRecommendations != tt.wantIsAllowed { + t.Errorf("got '%v' want '%v'", hasRecommendations, tt.wantIsAllowed) + } + }) } } -func TestGomodguardBlockedModulesGet(t *testing.T) { - blockedModulesList := blockedModules.Get() - if len(blockedModulesList) == 0 { - t.Error("blocked modules list should not be empty") +func TestBlockedModulesGet(t *testing.T) { + var tests = []struct { + testName string + blockedModules gomodguard.BlockedModules + wantBlockedModules []string + }{ + { + "get all blocked module names", + gomodguard.BlockedModules{{"github.com/someblocked/module": gomodguard.BlockedModule{Recommendations: []string{"github.com/ryancurrah/gomodguard"}}}}, + []string{"github.com/someblocked/module"}, + }, + } + + for _, tt := range tests { + t.Run(tt.testName, func(t *testing.T) { + blockedModules := tt.blockedModules.Get() + if !reflect.DeepEqual(blockedModules, tt.wantBlockedModules) { + t.Errorf("got '%v' want '%v'", blockedModules, tt.wantBlockedModules) + } + }) } } -func TestGomodguardRecommendedModules(t *testing.T) { - recommendedModules := blockedModules.RecommendedModules(blockedModule) - if len(recommendedModules.Recommendations) == 0 { - t.Error("recommended modules list should not be empty") - } - - recommendedModules = blockedModules.RecommendedModules(invalidModule) - if recommendedModules != nil { - t.Error("recommended modules should be nil when no recommendations for blocked module") +func TestBlockedVersionMessage(t *testing.T) { + blockedWithVersionConstraint := "version `1.0.0` is blocked because it does not meet the version constraint `1.0.0`. Some reason." + blockedWithVersionConstraintNoReason := "version `1.0.0` is blocked because it does not meet the version constraint `<= 1.0.0`." + + var tests = []struct { + testName string + blockedVersion gomodguard.BlockedVersion + lintedModuleVersion string + wantMessage string + }{ + { + "blocked with version constraint", + gomodguard.BlockedVersion{Version: "1.0.0", Reason: "Some reason."}, + "1.0.0", + blockedWithVersionConstraint, + }, + { + "blocked with version constraint and no reason", + gomodguard.BlockedVersion{Version: "<= 1.0.0"}, + "1.0.0", + blockedWithVersionConstraintNoReason, + }, + } + + for _, tt := range tests { + t.Run(tt.testName, func(t *testing.T) { + tt.blockedVersion.Set(tt.lintedModuleVersion) + message := tt.blockedVersion.Message() + if !strings.EqualFold(message, tt.wantMessage) { + t.Errorf("got '%s' want '%s'", message, tt.wantMessage) + } + }) } } -func TestGomodguardIsBlockedPackage(t *testing.T) { - blockedPkg := fmt.Sprintf("%s/util", blockedModule) - - isBlockedPackage := blockedModules.IsBlockedPackage(blockedPkg) - if !isBlockedPackage { - t.Errorf("package %s should be blocked when the module is in the blocked list", blockedPkg) - } - - allowedPkg := "github.com/someallowed/module" - - isBlockedPackage = blockedModules.IsBlockedPackage(allowedPkg) - if isBlockedPackage { - t.Errorf("package %s should NOT be blocked when the module is NOT in the blocked list", allowedPkg) +func TestBlockedModulesGetBlockedModule(t *testing.T) { + var tests = []struct { + testName string + blockedModules gomodguard.BlockedModules + currentModuleName string + lintedModuleName string + wantIsAllowed bool + }{ + { + "blocked", + gomodguard.BlockedModules{{"github.com/someblocked/module": gomodguard.BlockedModule{Recommendations: []string{"github.com/someother/module"}}}}, + "github.com/ryancurrah/gomodguard", + "github.com/someblocked/module", + false, + }, + { + "allowed", + gomodguard.BlockedModules{{"github.com/someblocked/module": gomodguard.BlockedModule{Recommendations: []string{"github.com/ryancurrah/gomodguard"}}}}, + "github.com/ryancurrah/gomodguard", + "github.com/someblocked/module", + true, + }, + } + + for _, tt := range tests { + t.Run(tt.testName, func(t *testing.T) { + blockedModule := tt.blockedModules.GetBlockReason(tt.currentModuleName, tt.lintedModuleName) + blockedModule.Set(tt.currentModuleName) + if blockedModule.IsAllowed() != tt.wantIsAllowed { + t.Errorf("got '%+v' want '%+v'", blockedModule.IsAllowed(), tt.wantIsAllowed) + } + }) } } -func TestGomodguardIsBlockedModule(t *testing.T) { - isBlockedPackage := blockedModules.IsBlockedModule(blockedModule) - if !isBlockedPackage { - t.Errorf("module %s should be blocked when the module is in the blocked list", blockedModule) - } - - isBlockedPackage = blockedModules.IsBlockedModule(allowedModule) - if isBlockedPackage { - t.Errorf("module %s should NOT be blocked when the module is NOT in the blocked list", allowedModule) +func TestAllowedIsAllowedModule(t *testing.T) { + var tests = []struct { + testName string + allowedModules gomodguard.Allowed + lintedModuleName string + wantIsAllowedModule bool + }{ + { + "module is allowed", + gomodguard.Allowed{Modules: []string{"github.com/someallowed/module"}}, + "github.com/someallowed/module", + true, + }, + { + "module not allowed", + gomodguard.Allowed{}, + "github.com/someblocked/module", + false, + }, + } + + for _, tt := range tests { + t.Run(tt.testName, func(t *testing.T) { + isAllowedModule := tt.allowedModules.IsAllowedModule(tt.lintedModuleName) + if !reflect.DeepEqual(isAllowedModule, tt.wantIsAllowedModule) { + t.Errorf("got '%v' want '%v'", isAllowedModule, tt.wantIsAllowedModule) + } + }) } } -func TestGomodguardIsAllowedModule(t *testing.T) { - isAllowedModule := allowed.IsAllowedModule(allowedModule) - if !isAllowedModule { - t.Errorf("module %s should be allowed when the module is in the allowed modules list", allowedModule) - } - - isAllowedModule = allowed.IsAllowedModule(blockedModule) - if isAllowedModule { - t.Errorf("module %s should NOT be allowed when the module is NOT in the allowed modules list", blockedModule) +func TestAllowedIsAllowedModuleDomain(t *testing.T) { + var tests = []struct { + testName string + allowedModules gomodguard.Allowed + lintedModuleName string + wantIsAllowedModuleDomain bool + }{ + { + "module is allowed", + gomodguard.Allowed{Domains: []string{"github.com"}}, + "github.com/someallowed/module", + true, + }, + { + "module not allowed", + gomodguard.Allowed{}, + "github.com/someblocked/module", + false, + }, + } + + for _, tt := range tests { + t.Run(tt.testName, func(t *testing.T) { + isAllowedModuleDomain := tt.allowedModules.IsAllowedModuleDomain(tt.lintedModuleName) + if !reflect.DeepEqual(isAllowedModuleDomain, tt.wantIsAllowedModuleDomain) { + t.Errorf("got '%v' want '%v'", isAllowedModuleDomain, tt.wantIsAllowedModuleDomain) + } + }) } } -func TestGomodguardIsAllowedModuleDomain(t *testing.T) { - isAllowedModuleDomain := allowed.IsAllowedModuleDomain(allowedDomain) - if !isAllowedModuleDomain { - t.Errorf("module domain %s should be allowed when the module domain is in the allowed domains list", allowedDomain) - } - - isAllowedModuleDomain = allowed.IsAllowedModuleDomain("blocked.domain") - if isAllowedModuleDomain { - t.Errorf("module domain %s should NOT be allowed when the module domain is NOT in the allowed domains list", "blocked.domain") +func TestResultString(t *testing.T) { + var tests = []struct { + testName string + result gomodguard.Result + wantString string + }{ + { + "reason lint failed", + gomodguard.Result{FileName: "test.go", LineNumber: 1, Reason: "Some reason."}, + "test.go:1:1 Some reason.", + }, + } + + for _, tt := range tests { + t.Run(tt.testName, func(t *testing.T) { + result := tt.result.String() + if !strings.EqualFold(result, tt.wantString) { + t.Errorf("got '%s' want '%s'", result, tt.wantString) + } + }) } } -func TestGomodguardProcessFilesWithAllowed(t *testing.T) { - config, logger, cwd, err := setup() +func TestProcessorNewProcessor(t *testing.T) { + config, logger, _, err := setup() if err != nil { t.Error(err) } - // Test that setting skip files to true does NOT return test files - filteredFilesNoTests := gomodguard.GetFilteredFiles(cwd, true, []string{"./..."}) - - testFileFound := false - - for _, finalFile := range filteredFilesNoTests { - if strings.HasSuffix(finalFile, "_test.go") { - testFileFound = true - } - } - - if testFileFound { - t.Errorf("should NOT have returned files found that end with _test.go") - } - - // Test that setting skip files to false DOES return test files - filteredFiles := gomodguard.GetFilteredFiles(cwd, false, []string{"./..."}) - if len(filteredFiles) == 0 { - t.Errorf("should have found a file to lint") - } - - testFileFound = false - - for _, finalFile := range filteredFiles { - if strings.HasSuffix(finalFile, "_test.go") { - testFileFound = true - } - } - - if !testFileFound { - t.Errorf("should have been able to find files that end with _test.go") - } - - processor, err := gomodguard.NewProcessor(*config, logger) + _, err = gomodguard.NewProcessor(*config, logger) if err != nil { - t.Errorf("should have been able to init a new processor without an error") - } - - results := processor.ProcessFiles(filteredFiles) - if len(results) > 0 { - t.Errorf("should not have found a lint error") + t.Error(err) } } -func TestGomodguardProcessFilesAllAllowed(t *testing.T) { +func TestProcessorProcessFiles(t *testing.T) { config, logger, cwd, err := setup() if err != nil { t.Error(err) } - config.Allowed.Modules = []string{} - config.Allowed.Domains = []string{} - config.Blocked.Modules = gomodguard.BlockedModules{} - - filteredFiles := gomodguard.GetFilteredFiles(cwd, false, []string{"./..."}) - if len(filteredFiles) == 0 { - t.Errorf("should have found a file to lint") - } - processor, err := gomodguard.NewProcessor(*config, logger) - if err != nil { - t.Errorf("should have been able to init a new processor without an error") - } - - results := processor.ProcessFiles(filteredFiles) - if len(results) > 0 { - t.Errorf("should not have found a lint error") - } -} - -func TestGomodguardProcessFilesWithBlockedModules(t *testing.T) { - config, logger, cwd, err := setup() if err != nil { t.Error(err) } - config.Allowed.Modules = []string{"github.com/someallowed/module"} - config.Allowed.Domains = []string{} - config.Blocked.Modules = gomodguard.BlockedModules{ - gomodguard.BlockedModule{"golang.org/x/mod": gomodguard.Recommendations{}}, - gomodguard.BlockedModule{"gopkg.in/yaml.v2": gomodguard.Recommendations{Recommendations: []string{"github.com/something/else"}, Reason: "test reason"}}, - } - filteredFiles := gomodguard.GetFilteredFiles(cwd, false, []string{"./..."}) - if len(filteredFiles) == 0 { - t.Errorf("should have found a file to lint") - } - - processor, err := gomodguard.NewProcessor(*config, logger) - if err != nil { - t.Errorf("should have been able to init a new processor without an error") - } - results := processor.ProcessFiles(filteredFiles) - if len(results) == 0 { - t.Errorf("should have found at least one lint error") + var tests = []struct { + testName string + processor gomodguard.Processor + wantReason string + }{ + { + "process module blocked because of recommendation", + gomodguard.Processor{Config: *config, Logger: logger, Modfile: processor.Modfile, Result: []gomodguard.Result{}}, + "cmd.go:14:1 import of package `github.com/phayes/checkstyle` is blocked because the module is in the blocked modules list. `github.com/someother/module` is a recommended module. testing if module is blocked with recommendation.", + }, + { + "process module blocked because of version constraint", + gomodguard.Processor{Config: *config, Logger: logger, Modfile: processor.Modfile, Result: []gomodguard.Result{}}, + "cmd.go:13:1 import of package `github.com/mitchellh/go-homedir` is blocked because the module is in the blocked modules list. version `v1.1.0` is blocked because it does not meet the version constraint `<= 1.1.0`. testing if blocked version constraint works.", + }, + } + + for _, tt := range tests { + t.Run(tt.testName, func(t *testing.T) { + tt.processor.SetBlockedModulesFromModFile() + results := tt.processor.ProcessFiles(filteredFiles) + if len(results) == 0 { + t.Fatal("result should be greater than zero") + } + + foundWantReason := false + for _, result := range results { + if strings.EqualFold(result.String(), tt.wantReason) { + foundWantReason = true + } + } + + if !foundWantReason { + t.Errorf("got '%+v' want '%s'", results, tt.wantReason) + } + }) } }