Repository: bwplotka/mdox
Pull request #59 (2021-07-10 14:19:20)

Make GitHub validator smarter
This PR makes the special `github` validator smarter. This renames `github` validator to `githubPullsIssues`. (As discussed in 1:1.)

Configuration will now look like,
```
- regex: '(^http[s]?:\/\/)(www\.)?(gitlab\.com\/)thanos-io\/thanos(\/pull\/|\/issues\/)'
    type: 'githubPullsIssues'
- regex: '(^http[s]?:\/\/)(www\.)?(github\.com\/)thanos-io\/thanos(\/releases\/tag\/)'
    type: 'ignore'
```
This also avoids surprises by asking users to provide the correct regex so that `mdox` does not have to create it.

In case of faulty regex, below errors are shown,

For `githubPullsIssues`,
```
error: fmt command failed: parsing githubPullIssues Regex: GitHub PR/Issue regex not valid. Correct regex: (^http[s]?:\/\/)(www\.)?(github\.com\/){ORG_NAME}\/{REPO_NAME}(\/pull\/|\/issues\/)
```


Patch:
diff --git a/pkg/mdformatter/linktransformer/config.go b/pkg/mdformatter/linktransformer/config.go
index 78991a5..949de2b 100644
--- a/pkg/mdformatter/linktransformer/config.go
+++ b/pkg/mdformatter/linktransformer/config.go
@@ -23,14 +23,14 @@ type Config struct {
 }
 
 type ValidatorConfig struct {
-	// Regex for type github is reponame matcher, like `bwplotka\/mdox`.
+	// Regex for type of validator. For `githubPullsIssues` this is: (^http[s]?:\/\/)(www\.)?(github\.com\/){ORG_NAME}\/{REPO_NAME}(\/pull\/|\/issues\/).
 	Regex string `yaml:"regex"`
-	// By default type is `ignore`. Could be `github` or `roundtrip`.
+	// By default type is `roundtrip`. Could be `githubPullsIssues` or `ignore`.
 	Type ValidatorType `yaml:"type"`
 	// GitHub repo token to avoid getting rate limited.
 	Token string `yaml:"token"`
 
-	ghValidator GitHubValidator
+	ghValidator GitHubPullsIssuesValidator
 	rtValidator RoundTripValidator
 	igValidator IgnoreValidator
 }
@@ -39,7 +39,7 @@ type RoundTripValidator struct {
 	_regex *regexp.Regexp
 }
 
-type GitHubValidator struct {
+type GitHubPullsIssuesValidator struct {
 	_regex  *regexp.Regexp
 	_maxNum int
 }
@@ -51,9 +51,9 @@ type IgnoreValidator struct {
 type ValidatorType string
 
 const (
-	roundtripValidator ValidatorType = "roundtrip"
-	githubValidator    ValidatorType = "github"
-	ignoreValidator    ValidatorType = "ignore"
+	roundtripValidator         ValidatorType = "roundtrip"
+	githubPullsIssuesValidator ValidatorType = "githubPullsIssues"
+	ignoreValidator            ValidatorType = "ignore"
 )
 
 const (
@@ -81,10 +81,11 @@ func ParseConfig(c []byte) (Config, error) {
 		switch cfg.Validators[i].Type {
 		case roundtripValidator:
 			cfg.Validators[i].rtValidator._regex = regexp.MustCompile(cfg.Validators[i].Regex)
-		case githubValidator:
+		case githubPullsIssuesValidator:
+			// Get maxNum from provided regex or fail.
 			regex, maxNum, err := getGitHubRegex(cfg.Validators[i].Regex, cfg.Validators[i].Token)
 			if err != nil {
-				return Config{}, errors.Wrapf(err, "parsing GitHub Regex %v", err)
+				return Config{}, errors.Wrapf(err, "parsing githubPullsIssues Regex")
 			}
 			cfg.Validators[i].ghValidator._regex = regex
 			cfg.Validators[i].ghValidator._maxNum = maxNum
@@ -98,12 +99,12 @@ func ParseConfig(c []byte) (Config, error) {
 }
 
 // getGitHubRegex returns GitHub pulls/issues regex from repo name.
-func getGitHubRegex(repoRe string, repoToken string) (*regexp.Regexp, int, error) {
-	// Get reponame from regex.
-	getRepo := regexp.MustCompile(`(?P<org>[A-Za-z0-9_.-]+)\\\/(?P<repo>[A-Za-z0-9_.-]+)`)
-	match := getRepo.FindStringSubmatch(repoRe)
+func getGitHubRegex(pullsIssuesRe string, repoToken string) (*regexp.Regexp, int, error) {
+	// Get reponame from Pulls & Issues regex. This also checks whether user provided regex is valid (inception again!).
+	getRepo := regexp.MustCompile(`\(\^http\[s\]\?:\\\/\\\/\)\(www\\\.\)\?\(github\\\.com\\\/\)(?P<org>[A-Za-z0-9_.-]+)\\\/(?P<repo>[A-Za-z0-9_.-]+)\(\\\/pull\\\/\|\\\/issues\\\/\)`)
+	match := getRepo.FindStringSubmatch(pullsIssuesRe)
 	if len(match) != 3 {
-		return nil, math.MaxInt64, errors.New("repo name regex not valid")
+		return nil, math.MaxInt64, errors.New(`GitHub PR/Issue regex not valid. Correct regex: (^http[s]?:\/\/)(www\.)?(github\.com\/){ORG_NAME}\/{REPO_NAME}(\/pull\/|\/issues\/)`)
 	}
 	reponame := match[1] + "/" + match[2]
 
@@ -163,5 +164,5 @@ func getGitHubRegex(repoRe string, repoToken string) (*regexp.Regexp, int, error
 		max = issueNum[0].Number
 	}
 
-	return regexp.MustCompile(`(^http[s]?:\/\/)(www\.)?(github\.com\/)(` + repoRe + `)(\/pull\/|\/issues\/)`), max, nil
+	return regexp.MustCompile(pullsIssuesRe), max, nil
 }
diff --git a/pkg/mdformatter/linktransformer/validator.go b/pkg/mdformatter/linktransformer/validator.go
index 1000813..17c94d0 100644
--- a/pkg/mdformatter/linktransformer/validator.go
+++ b/pkg/mdformatter/linktransformer/validator.go
@@ -15,7 +15,7 @@ type Validator interface {
 }
 
 // GitHubValidator.IsValid skips visiting all github issue/PR links.
-func (v GitHubValidator) IsValid(k futureKey, r *validator) (bool, error) {
+func (v GitHubPullsIssuesValidator) IsValid(k futureKey, r *validator) (bool, error) {
 	// Find rightmost index of match i.e, where regex match ends.
 	// This will be where issue/PR number starts. Split incase of section link and convert to int.
 	rightmostIndex := v._regex.FindStringIndex(k.dest)
@@ -70,7 +70,7 @@ func (v Config) GetValidatorForURL(URL string) Validator {
 				continue
 			}
 			return val.rtValidator
-		case githubValidator:
+		case githubPullsIssuesValidator:
 			if !val.ghValidator._regex.MatchString(URL) {
 				continue
 			}
diff --git a/pkg/mdformatter/transformer.go b/pkg/mdformatter/transformer.go
index 09037fa..e804a22 100644
--- a/pkg/mdformatter/transformer.go
+++ b/pkg/mdformatter/transformer.go
@@ -196,7 +196,7 @@ func getLinkLines(source []byte, link []byte, lenfm int) string {
 		lenfm += 2
 	}
 	// Using regex, as two links may have same host but diff params. Same in case of local links.
-	linkRe := regexp.MustCompile(`(^|[^/\-~&=#?@%a-zA-Z0-9])` + string(link) + `($|[^/\-~&=#?@%a-zA-Z0-9])`)
+	linkRe := regexp.MustCompile(`(^|[^/\-~&=#?@%a-zA-Z0-9])` + regexp.QuoteMeta(string(link)) + `($|[^/\-~&=#?@%a-zA-Z0-9])`)
 	for i, line := range sourceLines {
 		if linkRe.Match(line) {
 			// Easier to just return int slice, but then cannot use it in futureKey.


Test patch:
diff --git a/pkg/mdformatter/linktransformer/link_test.go b/pkg/mdformatter/linktransformer/link_test.go
index 7407c8c..fe47e0e 100644
--- a/pkg/mdformatter/linktransformer/link_test.go
+++ b/pkg/mdformatter/linktransformer/link_test.go
@@ -256,7 +256,7 @@ func TestValidator_TransformDestination(t *testing.T) {
 
 	t.Run("check 404 link", func(t *testing.T) {
 		testFile := filepath.Join(tmpDir, "repo", "docs", "test", "invalid-link.md")
-		testutil.Ok(t, ioutil.WriteFile(testFile, []byte("https://bwplotka.dev/does-not-exists\n"), os.ModePerm))
+		testutil.Ok(t, ioutil.WriteFile(testFile, []byte("https://bwplotka.dev/does-not-exists https://docs.gfoogle.com/drawings/d/e/2PACX-1vTBFK_cGMbxFpYcv/pub?w=960&h=720\n"), os.ModePerm))
 		filePath := "/repo/docs/test/invalid-link.md"
 		wdir, err := os.Getwd()
 		testutil.Ok(t, err)
@@ -271,7 +271,9 @@ func TestValidator_TransformDestination(t *testing.T) {
 			MustNewValidator(logger, []byte(""), anchorDir),
 		))
 		testutil.NotOk(t, err)
-		testutil.Equals(t, fmt.Sprintf("%v%v: %v%v:1: \"https://bwplotka.dev/does-not-exists\" not accessible; status code 404: Not Found", tmpDir, filePath, relDirPath, filePath), err.Error())
+		testutil.Equals(t, fmt.Sprintf("%v: 2 errors: "+
+			"%v:1: \"https://docs.gfoogle.com/drawings/d/e/2PACX-1vTBFK_cGMbxFpYcv/pub?w=960&h=720\" not accessible; status code 0: Get \"https://docs.gfoogle.com/drawings/d/e/2PACX-1vTBFK_cGMbxFpYcv/pub?w=960&h=720\": dial tcp: lookup docs.gfoogle.com: no such host; "+
+			"%v:1: \"https://bwplotka.dev/does-not-exists\" not accessible; status code 404: Not Found", tmpDir+filePath, relDirPath+filePath, relDirPath+filePath), err.Error())
 	})
 
 	t.Run("check valid & 404 link with validate config", func(t *testing.T) {
@@ -313,7 +315,7 @@ func TestValidator_TransformDestination(t *testing.T) {
 		testutil.Equals(t, 0, len(diff), diff.String())
 
 		_, err = mdformatter.IsFormatted(context.TODO(), logger, []string{testFile}, mdformatter.WithLinkTransformer(
-			MustNewValidator(logger, []byte("version: 1\n\nvalidators:\n  - regex: 'bwplotka\\/mdox'\n    token: '"+repoToken+"'\n    type: 'github'\n"), anchorDir),
+			MustNewValidator(logger, []byte("version: 1\n\nvalidators:\n  - regex: '(^http[s]?:\\/\\/)(www\\.)?(github\\.com\\/)bwplotka\\/mdox(\\/pull\\/|\\/issues\\/)'\n    token: '"+repoToken+"'\n    type: 'githubPullsIssues'\n"), anchorDir),
 		))
 		testutil.Ok(t, err)
 	})