Repository: preslavmihaylov/todocheck
Pull request #74 (2020-10-17 15:38:15)

extracted config.Validate into a separate package
This refactor would allow future changes to the Validate function to include dependencies on issuetracker and others which is not currently possible as that would lead to cyclic dependencies.


Patch:
diff --git a/config/config.go b/config/config.go
index 5389032..849a5c0 100644
--- a/config/config.go
+++ b/config/config.go
@@ -3,54 +3,21 @@ package config
 import (
 	"fmt"
 	"io/ioutil"
-	"net/url"
 	"os"
 	"regexp"
 	"strings"
 
-	"github.com/fatih/color"
 	"gopkg.in/yaml.v2"
 )
 
 // DefaultLocal contains the default filepath to the local todocheck config for the current repository
 const DefaultLocal = ".todocheck.yaml"
 
-// IssueTracker enum
-type IssueTracker string
-
-// Issue tracker types
-const (
-	IssueTrackerInvalid IssueTracker = ""
-	IssueTrackerJira                 = "JIRA"
-	IssueTrackerGithub               = "GITHUB"
-	IssueTrackerGitlab               = "GITLAB"
-	IssueTrackerPivotal              = "PIVOTAL_TRACKER"
-	IssueTrackerRedmine              = "REDMINE"
-
-	githubAPITokenWarning = "WARNING: Github has API rate limits for all requests which do not contain a token.\nPlease create a read-only access token to increase that limit.\nGo to https://developer.github.com/v3/#rate-limiting for more information."
-)
-
-var validIssueTrackers = []IssueTracker{
-	IssueTrackerJira,
-	IssueTrackerGithub,
-	IssueTrackerGitlab,
-	IssueTrackerPivotal,
-	IssueTrackerRedmine,
-}
-
 var (
 	windowsAbsolutePathPattern = regexp.MustCompile("^[A-Z]{1}:")
 	gitRemoteOriginPattern     = regexp.MustCompile(`(?Um)url\s=\s\w+(://|@)(?P<origin>(?P<host>.+)?(:|/).+)(\.git)?$`)
 )
 
-var originPatterns = map[IssueTracker]*regexp.Regexp{
-	IssueTrackerJira:    regexp.MustCompile(`^(https?://)?[a-zA-Z0-9\-]+(\.[a-zA-Z0-9]+)+(:[0-9]+)?$`),
-	IssueTrackerGithub:  regexp.MustCompile(`^(https?://)?(www\.)?github\.com/[\w-]+/[\w-]+`),
-	IssueTrackerGitlab:  regexp.MustCompile(`^(https?://)?[a-zA-Z0-9\-]+(\.[a-zA-Z0-9]+)+(:[0-9]+)?/[\w-]+/[\w-]+$`),
-	IssueTrackerPivotal: regexp.MustCompile(`^(https?://)?(www\.)?pivotaltracker\.com/n/projects/[0-9]+`),
-	IssueTrackerRedmine: regexp.MustCompile(`^(https?://)?[a-zA-Z0-9\-]+(\.[a-zA-Z0-9]+)+(:[0-9]+)?$`),
-}
-
 // Local todocheck configuration struct definition
 type Local struct {
 	Origin       string       `yaml:"origin"`
@@ -144,50 +111,6 @@ func autoDetect(basepath string) (*Local, error) {
 	}, nil
 }
 
-// Validate validates the values of given configuration
-func (l *Local) Validate() []error {
-	var errors []error
-
-	if err := l.validateIssueTracker(); err != nil {
-		errors = append(errors, err)
-	}
-
-	if err := l.validateAuthOfflineURL(); err != nil {
-		errors = append(errors, err)
-	}
-
-	if l.IssueTracker != "" {
-		pattern, ok := originPatterns[l.IssueTracker]
-		if !ok || !pattern.MatchString(l.Origin) {
-			errors = append(errors, fmt.Errorf("%s is not a valid origin for issue tracker %s", l.Origin, l.IssueTracker))
-		}
-	}
-
-	if l.Auth.Token == "" && l.IssueTracker == IssueTrackerGithub {
-		fmt.Fprintln(color.Output, color.YellowString(githubAPITokenWarning))
-	}
-
-	return errors
-}
-
-func (l *Local) validateIssueTracker() error {
-	for _, issueTracker := range validIssueTrackers {
-		if l.IssueTracker == issueTracker {
-			return nil
-		}
-	}
-	return fmt.Errorf("invalid issue tracker: %q is not supported. the valid issue trackers are: %v",
-		l.IssueTracker, validIssueTrackers)
-}
-
-func (l *Local) validateAuthOfflineURL() error {
-	if _, err := url.ParseRequestURI(l.Auth.OfflineURL); l.Auth.Type == AuthTypeOffline && err != nil {
-		return fmt.Errorf("invalid offline URL: %q", l.Auth.OfflineURL)
-	}
-
-	return nil
-}
-
 func exists(filepath string) bool {
 	info, err := os.Stat(filepath)
 	if os.IsNotExist(err) {
diff --git a/config/issuetracker.go b/config/issuetracker.go
new file mode 100644
index 0000000..495d230
--- /dev/null
+++ b/config/issuetracker.go
@@ -0,0 +1,53 @@
+package config
+
+import "regexp"
+
+// IssueTracker enum
+type IssueTracker string
+
+// Issue tracker types
+const (
+	IssueTrackerInvalid IssueTracker = ""
+	IssueTrackerJira                 = "JIRA"
+	IssueTrackerGithub               = "GITHUB"
+	IssueTrackerGitlab               = "GITLAB"
+	IssueTrackerPivotal              = "PIVOTAL_TRACKER"
+	IssueTrackerRedmine              = "REDMINE"
+)
+
+var validIssueTrackers = []IssueTracker{
+	IssueTrackerJira,
+	IssueTrackerGithub,
+	IssueTrackerGitlab,
+	IssueTrackerPivotal,
+	IssueTrackerRedmine,
+}
+
+var originPatterns = map[IssueTracker]*regexp.Regexp{
+	IssueTrackerJira:    regexp.MustCompile(`^(https?://)?[a-zA-Z0-9\-]+(\.[a-zA-Z0-9]+)+(:[0-9]+)?$`),
+	IssueTrackerGithub:  regexp.MustCompile(`^(https?://)?(www\.)?github\.com/[\w-]+/[\w-]+`),
+	IssueTrackerGitlab:  regexp.MustCompile(`^(https?://)?[a-zA-Z0-9\-]+(\.[a-zA-Z0-9]+)+(:[0-9]+)?/[\w-]+/[\w-]+$`),
+	IssueTrackerPivotal: regexp.MustCompile(`^(https?://)?(www\.)?pivotaltracker\.com/n/projects/[0-9]+`),
+	IssueTrackerRedmine: regexp.MustCompile(`^(https?://)?[a-zA-Z0-9\-]+(\.[a-zA-Z0-9]+)+(:[0-9]+)?$`),
+}
+
+// IsValid checks if the given issue tracker is among the valid enum values
+func (it IssueTracker) IsValid() bool {
+	for _, other := range validIssueTrackers {
+		if it == other {
+			return true
+		}
+	}
+
+	return false
+}
+
+// IsValidOrigin checks if the given origin is among the valid patterns for the given issue tracker
+func (it IssueTracker) IsValidOrigin(origin string) bool {
+	pattern, ok := originPatterns[it]
+	if !ok || !pattern.MatchString(origin) {
+		return false
+	}
+
+	return true
+}
diff --git a/main.go b/main.go
index 171f6ce..492deb7 100644
--- a/main.go
+++ b/main.go
@@ -15,6 +15,7 @@ import (
 	"github.com/preslavmihaylov/todocheck/fetcher"
 	"github.com/preslavmihaylov/todocheck/issuetracker"
 	"github.com/preslavmihaylov/todocheck/traverser/todoerrs"
+	"github.com/preslavmihaylov/todocheck/validation"
 )
 
 // set dynamically on build time. See Makefile for more info
@@ -41,7 +42,7 @@ func main() {
 		log.Fatalf("couldn't open configuration file: %s\n", err)
 	}
 
-	if errors := localCfg.Validate(); len(errors) > 0 {
+	if errors := validation.Validate(localCfg); len(errors) > 0 {
 		for _, err := range errors {
 			log.Println(err)
 		}
diff --git a/validation/validation.go b/validation/validation.go
new file mode 100644
index 0000000..320590b
--- /dev/null
+++ b/validation/validation.go
@@ -0,0 +1,59 @@
+package validation
+
+import (
+	"fmt"
+	"net/url"
+
+	"github.com/fatih/color"
+	"github.com/preslavmihaylov/todocheck/config"
+)
+
+// Validate validates the values of given configuration
+func Validate(cfg *config.Local) []error {
+	var errors []error
+
+	if err := validateIssueTracker(cfg); err != nil {
+		errors = append(errors, err)
+	}
+
+	if err := validateAuthOfflineURL(cfg); err != nil {
+		errors = append(errors, err)
+	}
+
+	if err := validateIssueTrackerOrigin(cfg); err != nil {
+		errors = append(errors, err)
+	}
+
+	if cfg.Auth.Token == "" && cfg.IssueTracker == config.IssueTrackerGithub {
+		fmt.Fprintln(color.Output, color.YellowString(
+			"WARNING: Github has API rate limits for all requests which do not contain a token.\n"+
+				"         Please create a read-only access token to increase that limit.\n"+
+				"         Go to https://developer.github.com/v3/#rate-limiting for more information."))
+	}
+
+	return errors
+}
+
+func validateIssueTracker(cfg *config.Local) error {
+	if !cfg.IssueTracker.IsValid() {
+		return fmt.Errorf("invalid issue tracker: %q is not supported", cfg.IssueTracker)
+	}
+
+	return nil
+}
+
+func validateAuthOfflineURL(cfg *config.Local) error {
+	if _, err := url.ParseRequestURI(cfg.Auth.OfflineURL); cfg.Auth.Type == config.AuthTypeOffline && err != nil {
+		return fmt.Errorf("invalid offline URL: %q", cfg.Auth.OfflineURL)
+	}
+
+	return nil
+}
+
+func validateIssueTrackerOrigin(cfg *config.Local) error {
+	if cfg.IssueTracker != "" && !cfg.IssueTracker.IsValidOrigin(cfg.Origin) {
+		return fmt.Errorf("%s is not a valid origin for issue tracker %s", cfg.Origin, cfg.IssueTracker)
+	}
+
+	return nil
+}


Test patch:
diff --git a/testing/todocheck_test.go b/testing/todocheck_test.go
index e5e1580..c7e6da8 100644
--- a/testing/todocheck_test.go
+++ b/testing/todocheck_test.go
@@ -7,6 +7,7 @@ import (
 	"github.com/preslavmihaylov/todocheck/config"
 	"github.com/preslavmihaylov/todocheck/testing/scenariobuilder"
 	"github.com/preslavmihaylov/todocheck/testing/scenariobuilder/issuetracker"
+	"github.com/preslavmihaylov/todocheck/validation"
 )
 
 func TestSingleLineMalformedTodos(t *testing.T) {
@@ -353,7 +354,7 @@ func TestInvalidOrigins(t *testing.T) {
 			t.Errorf("%s", err)
 			continue
 		}
-		errors := cfg.Validate()
+		errors := validation.Validate(cfg)
 		if 0 == len(errors) {
 			t.Errorf("%s should be invalid", path)
 		}
@@ -383,7 +384,7 @@ func TestValidOrigins(t *testing.T) {
 			t.Errorf("%s", err)
 			continue
 		}
-		errors := cfg.Validate()
+		errors := validation.Validate(cfg)
 		if len(errors) > 0 {
 			t.Errorf("%s should be valid but has errors: %v", path, errors)
 		}