Extract logic to dedicated valid_syntax checker

.travis.yml

@@ -16,15 +16,15 @@ script: skip
 jobs:
   include:
     - stage: test
-      name: "Build and unit-test with Go 1.12.x"
+      name: "Build and unit-test with Go 1.14.x"
       go: 1.14.x
       script: make test-unit
     - stage: test
-      name: "Build and unit-test with Go 1.13.x"
+      name: "Build and unit-test with Go 1.15.x"
       go: 1.15.x
       script: make test-unit
     - stage: test
-      name: "Hammer unit-test with Go 1.13.x"
+      name: "Hammer unit-test with Go 1.15.x"
       go: 1.15.x
       script: make test-hammer
     - stage: test

README.md

@@ -88,7 +88,7 @@ You can also download [latest version](https://github.com/mszostok/codeowners-va
 
 You can install `codeowners-validator` with `env GO111MODULE=on go get -u github.com/mszostok/codeowners-validator`.
 
-> NOTE: please use the latest Go to do this, ideally Go 1.12 or greater.
+> NOTE: please use the latest Go to do this, ideally Go 1.15 or greater.
 
 This will put `codeowners-validator` in `$(go env GOPATH)/bin`
 
@@ -118,7 +118,8 @@ The following checks are enabled by default:
 |------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
 | duppatterns | **[Duplicated Pattern Checker]** <br /><br /> Reports if CODEOWNERS file contain duplicated lines with the same file pattern.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                 |
 | files      | **[File Exist Checker]** <br /><br /> Reports if CODEOWNERS file contain lines with the file pattern that do not exist in a given repository.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                 |
-| owners     | **[Valid Owner Checker]** <br /><br /> Reports if CODEOWNERS file contain invalid owners definition. Allowed owner syntax: `@username`, `@org/team-name` or `user@example.com` <br /> _source: https://help.github.com/articles/about-code-owners/#codeowners-syntax_. <br /> <br /> **Checks:** <br /> &#x09; 1. Check if the owner's definition is valid (is either a GitHub user name, an organization team name or an email address). <br /><br /> 2. Check if a GitHub owner has a GitHub account <br /><br /> 3. Check if a GitHub owner is in a given organization <br /> <br />4. Check if an organization team exists |
+| owners     | **[Valid Owner Checker]** <br /><br /> Reports if CODEOWNERS file contain invalid owners definition. Allowed owner syntax: `@username`, `@org/team-name` or `user@example.com` <br /> _source: https://help.github.com/articles/about-code-owners/#codeowners-syntax_. <br /> <br /> **Checks:** <br /> &#x09; &nbsp;&nbsp;&nbsp;&nbsp;1. Check if the owner's definition is valid (is either a GitHub user name, an organization team name or an email address). <br /><br />&nbsp;&nbsp;&nbsp;&nbsp;2. Check if a GitHub owner has a GitHub account <br /><br />&nbsp;&nbsp;&nbsp;&nbsp;3. Check if a GitHub owner is in a given organization <br /> <br />&nbsp;&nbsp;&nbsp;&nbsp;4. Check if an organization team exists |
+| syntax     | **[Valid Syntax Checker]** <br /><br /> Reports if CODEOWNERS file contain invalid syntax definition. It is imported as: <br />&nbsp;&nbsp;&nbsp;&nbsp;"If any line in your CODEOWNERS file contains invalid syntax, the file will not be detected<br />&nbsp;&nbsp;&nbsp;&nbsp;and will not be used to request reviews. Invalid syntax includes inline comments <br />&nbsp;&nbsp;&nbsp;&nbsp;and user or team names that do not exist on GitHub." <br /> <br /> _source: https://help.github.com/articles/about-code-owners/#codeowners-syntax_. |
 
 The experimental checks are disabled by default:
 

internal/check/valid_owner.go

@@ -39,7 +39,7 @@ func NewValidOwner(cfg ValidOwnerConfig, ghClient *github.Client) (*ValidOwner,
 	}, nil
 }
 
-// Check checks if defined owners are the valid ones.
+// Check if defined owners are the valid ones.
 // Allowed owner syntax:
 // @username
 // @org/team-name

internal/check/valid_syntax.go

@@ -0,0 +1,77 @@
+package check
+
+import (
+	"context"
+	"fmt"
+	"regexp"
+	"strings"
+
+	"github.com/mszostok/codeowners-validator/internal/ctxutil"
+)
+
+var (
+	// A valid username/organization name has up to 39 characters (per GitHub Join page)
+	// and is matched by the following regex: /^[a-z\d](?:[a-z\d]|-(?=[a-z\d])){0,38}$/i
+	// A valid team name consists of alphanumerics, underscores and dashes
+	usernameOrTeamRegexp = regexp.MustCompile(`^@(?i:[a-z\d](?:[a-z\d-]){0,37}[a-z\d](/[a-z\d](?:[a-z\d_-]*)[a-z\d])?)$`)
+
+	// Per: https://davidcel.is/posts/stop-validating-email-addresses-with-regex/
+	// just check if there is '@' and a '.' afterwards
+	emailRegexp = regexp.MustCompile(`.+@.+\..+`)
+)
+
+// ValidSyntax provides a syntax validation for CODEOWNERS file.
+//
+// If any line in your CODEOWNERS file contains invalid syntax, the file will not be detected and will
+// not be used to request reviews. Invalid syntax includes inline comments and user or team names that do not exist on GitHub.
+type ValidSyntax struct{}
+
+// NewValidSyntax returns new ValidSyntax instance.
+func NewValidSyntax() *ValidSyntax {
+	return &ValidSyntax{}
+}
+
+// Check for syntax issues in your CODEOWNERS file.
+func (ValidSyntax) Check(ctx context.Context, in Input) (Output, error) {
+	var bldr OutputBuilder
+
+	for _, entry := range in.CodeownersEntries {
+		if ctxutil.ShouldExit(ctx) {
+			return Output{}, ctx.Err()
+		}
+
+		if entry.Pattern == "" {
+			bldr.ReportIssue("Missing pattern", WithEntry(entry))
+		}
+
+		if len(entry.Owners) == 0 {
+			bldr.ReportIssue("Missing owner, at least one owner is required", WithEntry(entry))
+		}
+
+	ownersLoop:
+		for _, item := range entry.Owners {
+			switch {
+			case strings.EqualFold(item, "#"):
+				msg := "Comment (# sign) is not allowed in line with pattern entry. The correct format is: pattern owner1 ... ownerN"
+				bldr.ReportIssue(msg, WithEntry(entry))
+				break ownersLoop // no need to check for the rest items in this line, as the whole line is already marked as invalid
+			case strings.HasPrefix(item, "@"):
+				if !usernameOrTeamRegexp.MatchString(item) {
+					msg := fmt.Sprintf("Owner '%s' does not look like a GitHub username or team name", item)
+					bldr.ReportIssue(msg, WithEntry(entry), WithSeverity(Warning))
+				}
+			default:
+				if !emailRegexp.MatchString(item) {
+					msg := fmt.Sprintf("Owner '%s' does not look like an email", item)
+					bldr.ReportIssue(msg, WithEntry(entry))
+				}
+			}
+		}
+	}
+
+	return bldr.Output(), nil
+}
+
+func (ValidSyntax) Name() string {
+	return "Valid Syntax Checker"
+}

internal/check/valid_syntax_test.go

@@ -0,0 +1,125 @@
+package check_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/mszostok/codeowners-validator/internal/check"
+	"github.com/mszostok/codeowners-validator/internal/ptr"
+	"github.com/mszostok/codeowners-validator/pkg/codeowners"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestValidSyntaxChecker(t *testing.T) {
+	tests := map[string]struct {
+		codeowners string
+		issue      check.Issue
+	}{
+		"No owners": {
+			codeowners: `*`,
+			issue: check.Issue{
+				Severity: check.Error,
+				LineNo:   ptr.Uint64Ptr(1),
+				Message:  "Missing owner, at least one owner is required",
+			},
+		},
+		"Bad username": {
+			codeowners: `pkg/github.com/** @-`,
+			issue: check.Issue{
+				Severity: check.Warning,
+				LineNo:   ptr.Uint64Ptr(1),
+				Message:  "Owner '@-' does not look like a GitHub username or team name",
+			},
+		},
+		"Bad org": {
+			codeowners: `* @bad+org`,
+			issue: check.Issue{
+				Severity: check.Warning,
+				LineNo:   ptr.Uint64Ptr(1),
+				Message:  "Owner '@bad+org' does not look like a GitHub username or team name",
+			},
+		},
+		"Bad team name on first place": {
+			codeowners: `* @org/+not+a+good+name`,
+			issue: check.Issue{
+				Severity: check.Warning,
+				LineNo:   ptr.Uint64Ptr(1),
+				Message:  "Owner '@org/+not+a+good+name' does not look like a GitHub username or team name",
+			},
+		},
+		"Bad team name on second place": {
+			codeowners: `* @org/hakuna-matata @org/-a-team`,
+			issue: check.Issue{
+				Severity: check.Warning,
+				LineNo:   ptr.Uint64Ptr(1),
+				Message:  "Owner '@org/-a-team' does not look like a GitHub username or team name",
+			},
+		},
+		"Doesn't look like username, team name, nor email": {
+			codeowners: `* something_weird`,
+			issue: check.Issue{
+				Severity: check.Error,
+				LineNo:   ptr.Uint64Ptr(1),
+				Message:  "Owner 'something_weird' does not look like an email",
+			},
+		},
+		"Comment in pattern line": {
+			codeowners: `* @org/hakuna-matata # this should be an error`,
+			issue: check.Issue{
+				Severity: check.Error,
+				LineNo:   ptr.Uint64Ptr(1),
+				Message:  "Comment (# sign) is not allowed in line with pattern entry. The correct format is: pattern owner1 ... ownerN",
+			},
+		},
+	}
+	for tn, tc := range tests {
+		t.Run(tn, func(t *testing.T) {
+			// when
+			out, err := check.NewValidSyntax().
+				Check(context.Background(), loadInput(tc.codeowners))
+
+			// then
+			require.NoError(t, err)
+
+			require.Len(t, out.Issues, 1)
+			assert.EqualValues(t, tc.issue, out.Issues[0])
+		})
+	}
+}
+
+func TestValidSyntaxZeroValueEntry(t *testing.T) {
+	// given
+	zeroValueInput := check.Input{
+		CodeownersEntries: []codeowners.Entry{
+			{
+				LineNo:  0,
+				Pattern: "",
+				Owners:  nil,
+			},
+		},
+	}
+	expIssues := []check.Issue{
+		{
+			LineNo:   ptr.Uint64Ptr(0),
+			Severity: check.Error,
+			Message:  "Missing pattern",
+		},
+		{
+			LineNo:   ptr.Uint64Ptr(0),
+			Severity: check.Error,
+			Message:  "Missing owner, at least one owner is required",
+		},
+	}
+
+	// when
+	out, err := check.NewValidSyntax().
+		Check(context.Background(), zeroValueInput)
+
+	// then
+	require.NoError(t, err)
+
+	require.Len(t, out.Issues, 2)
+	assert.EqualValues(t, expIssues, out.Issues)
+}

internal/load/load.go

@@ -17,6 +17,10 @@ import (
 func Checks(ctx context.Context, enabledChecks []string, experimentalChecks []string) ([]check.Checker, error) {
 	var checks []check.Checker
 
+	if isEnabled(enabledChecks, "syntax") {
+		checks = append(checks, check.NewValidSyntax())
+	}
+
 	if isEnabled(enabledChecks, "duppatterns") {
 		checks = append(checks, check.NewDuplicatedPattern())
 	}

pkg/codeowners/owners.go

@@ -6,7 +6,6 @@ import (
 	"io"
 	"os"
 	"path"
-	"regexp"
 	"strings"
 
 	"github.com/spf13/afero"
@@ -33,7 +32,7 @@ func NewFromPath(path string) ([]Entry, error) {
 		return nil, err
 	}
 
-	return parseCodeowners(r)
+	return ParseCodeowners(r), nil
 }
 
 // openCodeownersFile finds a CODEOWNERS file and returns content.
@@ -66,63 +65,28 @@ func openCodeownersFile(dir string) (io.Reader, error) {
 	return nil, fmt.Errorf("No CODEOWNERS found in the root, docs/, or .github/ directory of the repository %s", dir)
 }
 
-func parseCodeowners(r io.Reader) ([]Entry, error) {
+func ParseCodeowners(r io.Reader) []Entry {
 	var e []Entry
-
-	usernameOrTeamRegexp := regexp.MustCompile(`^@(?i:[a-z\d](?:[a-z\d-]){0,37}[a-z\d](/[a-z\d](?:[a-z\d_-]*)[a-z\d])?)$`)
-	emailRegexp := regexp.MustCompile(`.+@.+\..+`)
-
 	s := bufio.NewScanner(r)
 	no := uint64(0)
 	for s.Scan() {
 		no++
+		fields := strings.Fields(s.Text())
 
-		line := s.Text()
-		if strings.HasPrefix(line, "#") { // comment
+		if len(fields) == 0 { // empty
 			continue
 		}
 
-		if len(line) == 0 { // empty
+		if strings.HasPrefix(fields[0], "#") { // comment
 			continue
 		}
 
-		fields := strings.Fields(s.Text())
-
-		if len(fields) < 2 {
-			return e, fmt.Errorf("line %d does not have 2 or more fields", no)
-		}
-
-		// This does syntax validation only
-		//
-		// Syntax check: all fields are valid team/username identifiers or emails
-		// Allowed owner syntax:
-		// @username
-		// @org/team-name
-		// user@example.com
-		// source: https://help.github.com/articles/about-code-owners/#codeowners-syntax
-		for _, entry := range fields[1:] {
-			if strings.HasPrefix(entry, "@") {
-				// A valid username/organization name has up to 39 characters (per GitHub Join page)
-				// and is matched by the following regex: /^[a-z\d](?:[a-z\d]|-(?=[a-z\d])){0,38}$/i
-				// A valid team name consists of alphanumerics, underscores and dashes
-				if !usernameOrTeamRegexp.MatchString(entry) {
-					return e, fmt.Errorf("entry '%s' on line %d does not look like a GitHub username or team name", entry, no)
-				}
-			} else {
-				// Per: https://davidcel.is/posts/stop-validating-email-addresses-with-regex/
-				// just check if there is '@' and a '.' afterwards
-				if !emailRegexp.MatchString(entry) {
-					return e, fmt.Errorf("entry '%s' on line %d does not look like an email", entry, no)
-				}
-			}
-		}
-
 		e = append(e, Entry{
 			Pattern: fields[0],
 			Owners:  fields[1:],
 			LineNo:  no,
 		})
 	}
 
-	return e, nil
+	return e
 }

pkg/codeowners/owners_test.go

@@ -20,51 +20,6 @@ pkg/github.com/**	@myk
 
 `
 
-func TestParseCodeownersFailure(t *testing.T) {
-	// given
-	givenCodeownerPath := "workspace/go/repo-name"
-	badInputs := []string{
-		`# one token
-*
-`,
-		`# bad username
-* @myk
-pkg/github.com/** @-
-`,
-		`# bad org
-* @bad+org
-`,
-		`# comment mid line
-* @org/hakuna-matata # this should be an error
-`,
-		`# bad team name second place
-* @org/hakuna-matata @org/-a-team
-`,
-		`# bad team first
-* @org/+not+a+good+name
-`,
-		`# doesn't look like username, team name, nor email
-* something_weird
-`,
-	}
-
-	for _, input := range badInputs {
-		tFS := afero.NewMemMapFs()
-		revert := codeowners.SetFS(tFS)
-		defer revert()
-
-		f, _ := tFS.Create(path.Join(givenCodeownerPath, "CODEOWNERS"))
-		_, err := f.WriteString(input)
-		require.NoError(t, err)
-
-		// when
-		_, err = codeowners.NewFromPath(givenCodeownerPath)
-
-		// then
-		require.Error(t, err)
-	}
-}
-
 func TestParseCodeownersSuccess(t *testing.T) {
 	// given
 	givenCodeownerPath := "workspace/go/repo-name"