From 97fb795a577cb06259c6b1be5e0f16874464dc5a Mon Sep 17 00:00:00 2001
From: Miguel Lemos <miguelslemos@gmail.com>
Date: Sat, 31 Oct 2020 07:11:14 -0300
Subject: [PATCH] Ignore ghost user (#53)

---
 internal/check/valid_owner.go            | 38 ++++-------------
 internal/check/valid_owner_check.go      | 26 ++++++++++++
 internal/check/valid_owner_check_test.go | 52 ++++++++++++++++++++++++
 3 files changed, 87 insertions(+), 29 deletions(-)
 create mode 100644 internal/check/valid_owner_check.go
 create mode 100644 internal/check/valid_owner_check_test.go

diff --git a/internal/check/valid_owner.go b/internal/check/valid_owner.go
index 85b6fde..b8509f6 100644
--- a/internal/check/valid_owner.go
+++ b/internal/check/valid_owner.go
@@ -3,7 +3,6 @@ package check
 import (
 	"context"
 	"net/http"
-	"net/mail"
 	"strings"
 
 	"github.com/mszostok/codeowners-validator/internal/ctxutil"
@@ -82,11 +81,11 @@ func (v *ValidOwner) Check(ctx context.Context, in Input) (Output, error) {
 
 func (v *ValidOwner) selectValidateFn(name string) func(context.Context, string) *validateError {
 	switch {
-	case isGithubUser(name):
+	case IsGithubUser(name):
 		return v.validateGithubUser
-	case isGithubTeam(name):
+	case IsGithubTeam(name):
 		return v.validateTeam
-	case isEmailAddress(name):
+	case IsEmailAddress(name):
 		// TODO(mszostok): try to check if e-mail really exists
 		return func(context.Context, string) *validateError { return nil }
 	default:
@@ -162,6 +161,12 @@ func (v *ValidOwner) validateTeam(ctx context.Context, name string) *validateErr
 }
 
 func (v *ValidOwner) validateGithubUser(ctx context.Context, name string) *validateError {
+	// Ignore @ghost user
+	// https://github.community/t5/How-to-use-Git-and-GitHub/CODEOWNERS-file-with-a-NOT-file-type-condition/m-p/31013/highlight/true#M8523
+	if IsGithubGhostUser(name) {
+		return nil
+	}
+
 	if v.orgMembers == nil { //TODO(mszostok): lazy init, make it more robust.
 		if err := v.initOrgListMembers(ctx); err != nil {
 			return newValidateError("Cannot initialize organization member list: %v", err).AsPermanent()
@@ -222,31 +227,6 @@ func (v *ValidOwner) initOrgListMembers(ctx context.Context) error {
 	return nil
 }
 
-func isEmailAddress(s string) bool {
-	_, err := mail.ParseAddress(s)
-
-	return err == nil
-}
-
-func isGithubTeam(s string) bool {
-	hasPrefix := strings.HasPrefix(s, "@")
-	containsSlash := strings.Contains(s, "/")
-	split := strings.SplitN(s, "/", 3) // 3 is enough to confirm that is invalid + will not overflow the buffer
-
-	if hasPrefix && containsSlash && len(split) == 2 {
-		return true
-	}
-
-	return false
-}
-
-func isGithubUser(s string) bool {
-	if strings.HasPrefix(s, "@") && !strings.Contains(s, "/") {
-		return true
-	}
-	return false
-}
-
 // Name returns human readable name of the validator
 func (ValidOwner) Name() string {
 	return "Valid Owner Checker"
diff --git a/internal/check/valid_owner_check.go b/internal/check/valid_owner_check.go
new file mode 100644
index 0000000..d3b38fa
--- /dev/null
+++ b/internal/check/valid_owner_check.go
@@ -0,0 +1,26 @@
+package check
+
+import (
+	"net/mail"
+	"strings"
+)
+
+func IsEmailAddress(s string) bool {
+	_, err := mail.ParseAddress(s)
+	return err == nil
+}
+
+func IsGithubTeam(s string) bool {
+	hasPrefix := strings.HasPrefix(s, "@")
+	containsSlash := strings.Contains(s, "/")
+	split := strings.SplitN(s, "/", 3) // 3 is enough to confirm that is invalid + will not overflow the buffer
+	return hasPrefix && containsSlash && len(split) == 2 && len(split[1]) > 0
+}
+
+func IsGithubUser(s string) bool {
+	return !strings.Contains(s, "/") && strings.HasPrefix(s, "@")
+}
+
+func IsGithubGhostUser(s string) bool {
+	return s == "@ghost"
+}
diff --git a/internal/check/valid_owner_check_test.go b/internal/check/valid_owner_check_test.go
new file mode 100644
index 0000000..670ed1b
--- /dev/null
+++ b/internal/check/valid_owner_check_test.go
@@ -0,0 +1,52 @@
+package check_test
+
+import (
+	"testing"
+
+	"github.com/mszostok/codeowners-validator/internal/check"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestValidOwnerChecker(t *testing.T) {
+	tests := map[string]struct {
+		user    string
+		isValid bool
+	}{
+		"Invalid Email": {
+			user:    `asda.comm`,
+			isValid: false,
+		},
+		"Valid Email": {
+			user:    `gmail@gmail.com`,
+			isValid: true,
+		},
+		"Invalid Team": {
+			user:    `@org/`,
+			isValid: false,
+		},
+		"Valid Team": {
+			user:    `@org/user`,
+			isValid: true,
+		},
+		"Invalid User": {
+			user:    `user`,
+			isValid: false,
+		},
+		"Valid User": {
+			user:    `@user`,
+			isValid: true,
+		},
+	}
+	for tn, tc := range tests {
+		t.Run(tn, func(t *testing.T) {
+			// when
+			result := isValidUser(tc.user)
+			assert.Equal(t, tc.isValid, result)
+		})
+	}
+}
+
+func isValidUser(user string) bool {
+	return check.IsEmailAddress(user) || check.IsGithubUser(user) || check.IsGithubTeam(user)
+}