Skip to content

Commit

Permalink
Refactor Parameters (#39)
Browse files Browse the repository at this point in the history
  • Loading branch information
@fmenezes
fmenezes authored Aug 21, 2020
1 parent be05457 commit d6b6251
Show file tree
Hide file tree
Showing 7 changed files with 161 additions and 39 deletions.
50 changes: 39 additions & 11 deletions checker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,20 @@ func RegisterChecker(name string, checker Checker) error {
return nil
}

// ValidatorOptions provide input arguments for checkers to use
type ValidatorOptions struct {
Directory string
CodeownersFileLocation string
}

// Checker provides tools for validating CODEOWNER file contents
type Checker interface {
CheckLine(file string, lineNo int, line string) []CheckResult
NewValidator(options ValidatorOptions) Validator
}

// Validator provides tools for validating CODEOWNER file contents
type Validator interface {
ValidateLine(lineNo int, line string) []CheckResult
}

// SeverityLevel exposes all possible levels of severity check results
Expand Down Expand Up @@ -86,15 +97,23 @@ type CheckResult struct {
CheckName string
}

// CheckOptions provides parameters for running a list of checks
type CheckOptions struct {
Directory string
Checkers []string
GithubTokenType string
GithubToken string
}

// Check evaluates the file contents against the checkers and return the results back.
func Check(directory string, checkers ...string) ([]CheckResult, error) {
func Check(options CheckOptions) ([]CheckResult, error) {

fileLocation, result := findCodeownersFile(directory)
fileLocation, result := findCodeownersFile(options.Directory)
if result != nil {
return []CheckResult{*result}, nil
}

file, err := os.Open(fileLocation)
file, err := os.Open(filepath.Join(options.Directory, fileLocation))
if err != nil {
return nil, err
}
Expand All @@ -103,15 +122,24 @@ func Check(directory string, checkers ...string) ([]CheckResult, error) {
results := []CheckResult{}
scanner := bufio.NewScanner(file)
lineNo := 0

validators := make(map[string]Validator)
for _, checker := range options.Checkers {
c, ok := availableCheckers[checker]
if !ok {
return nil, fmt.Errorf("'%s' not found", checker)
}
validators[checker] = c.NewValidator(ValidatorOptions{
Directory: options.Directory,
CodeownersFileLocation: fileLocation,
})
}

for scanner.Scan() {
line := scanner.Text()
lineNo++
for _, checker := range checkers {
c, ok := availableCheckers[checker]
if !ok {
return nil, fmt.Errorf("'%s' not found", checker)
}
lineResults := c.CheckLine(fileLocation, lineNo, line)
for _, c := range validators {
lineResults := c.ValidateLine(lineNo, line)
if lineResults != nil {
results = append(results, lineResults...)
}
Expand Down Expand Up @@ -139,7 +167,7 @@ func findCodeownersFile(dir string) (string, *CheckResult) {
if fileExists(currentFile) {
filesFound = append(filesFound, fileLocation)
if len(codeownersLocation) == 0 {
codeownersLocation = currentFile
codeownersLocation = fileLocation
}
}
}
Expand Down
48 changes: 38 additions & 10 deletions checker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,30 @@ import (
"testing"

"github.com/fmenezes/codeowners"
_ "github.com/fmenezes/codeowners/checkers"
)

const dummyCheckerName string = "dummy"

type dummyChecker struct {
}

func (c dummyChecker) CheckLine(file string, lineNo int, line string) []codeowners.CheckResult {
type dummyCheckerValidator struct {
codeownersFileLocation string
directory string
}

func (c dummyChecker) NewValidator(options codeowners.ValidatorOptions) codeowners.Validator {
return dummyCheckerValidator{
codeownersFileLocation: options.CodeownersFileLocation,
directory: options.Directory,
}
}

func (c dummyCheckerValidator) ValidateLine(lineNo int, line string) []codeowners.CheckResult {
return []codeowners.CheckResult{
{
Position: codeowners.Position{
FilePath: file,
FilePath: c.codeownersFileLocation,
StartLine: lineNo,
EndLine: lineNo,
},
Expand Down Expand Up @@ -136,7 +147,7 @@ func TestSimpleCheck(t *testing.T) {
want := []codeowners.CheckResult{
{
Position: codeowners.Position{
FilePath: "test/data/pass/CODEOWNERS",
FilePath: "CODEOWNERS",
StartLine: 1,
StartColumn: 0,
EndLine: 1,
Expand All @@ -149,7 +160,10 @@ func TestSimpleCheck(t *testing.T) {
}

codeowners.RegisterChecker(dummyCheckerName, dummyChecker{})
got, err := codeowners.Check(input, dummyCheckerName)
got, err := codeowners.Check(codeowners.CheckOptions{
Directory: input,
Checkers: []string{dummyCheckerName},
})
if err != nil {
t.Errorf("Input %s, Error %v", input, err)
}
Expand All @@ -160,7 +174,9 @@ func TestSimpleCheck(t *testing.T) {

func TestNoProblemsFound(t *testing.T) {
input := "./test/data/pass"
got, err := codeowners.Check(input)
got, err := codeowners.Check(codeowners.CheckOptions{
Directory: input,
})
if err != nil {
t.Errorf("Input %s, Error %v", input, err)
}
Expand All @@ -171,7 +187,10 @@ func TestNoProblemsFound(t *testing.T) {

func TestCheckerNotFound(t *testing.T) {
input := "./test/data/pass"
_, err := codeowners.Check(input, "NonExistentChecker")
_, err := codeowners.Check(codeowners.CheckOptions{
Directory: input,
Checkers: []string{"NonExistentChecker"},
})
if err == nil {
t.Error("Should have errored")
}
Expand All @@ -187,7 +206,10 @@ func TestNoCodeownersCheck(t *testing.T) {
},
}

got, err := codeowners.Check(input, dummyCheckerName)
got, err := codeowners.Check(codeowners.CheckOptions{
Directory: input,
Checkers: []string{dummyCheckerName},
})
if err != nil {
t.Errorf("Input %s, Error %v", input, err)
}
Expand All @@ -206,7 +228,10 @@ func TestMultipleCodeownersCheck(t *testing.T) {
},
}

got, err := codeowners.Check(input, dummyCheckerName)
got, err := codeowners.Check(codeowners.CheckOptions{
Directory: input,
Checkers: []string{dummyCheckerName},
})
if err != nil {
t.Errorf("Input %s, Error %v", input, err)
}
Expand All @@ -216,7 +241,10 @@ func TestMultipleCodeownersCheck(t *testing.T) {
}

func ExampleCheck() {
checks, err := codeowners.Check(".", codeowners.AvailableCheckers()...)
checks, err := codeowners.Check(codeowners.CheckOptions{
Directory: ".",
Checkers: codeowners.AvailableCheckers(),
})
if err != nil {
panic(err)
}
Expand Down
20 changes: 16 additions & 4 deletions checkers/invalidowner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,15 @@ func init() {
}

// InvalidOwner represents checker to decide validate owners in each of CODEOWNERS lines
type InvalidOwner struct{}
type InvalidOwner struct {
}

// NewValidator returns validating capabilities for this checker
func (c InvalidOwner) NewValidator(options codeowners.ValidatorOptions) codeowners.Validator {
return invalidOwnerValidator{
options: options,
}
}

func ownerValid(owner string) bool {
var emailRegex = regexp.MustCompile("^[a-zA-Z0-9.!#$%&'*+\\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$")
Expand All @@ -42,8 +50,12 @@ func ownerValid(owner string) bool {
return true
}

// CheckLine runs this InvalidOwner's check against each line
func (c InvalidOwner) CheckLine(file string, lineNo int, line string) []codeowners.CheckResult {
type invalidOwnerValidator struct {
options codeowners.ValidatorOptions
}

// ValidateLine runs this InvalidOwner's check against each line
func (v invalidOwnerValidator) ValidateLine(lineNo int, line string) []codeowners.CheckResult {
var results []codeowners.CheckResult

_, owners := codeowners.ParseLine(line)
Expand All @@ -54,7 +66,7 @@ func (c InvalidOwner) CheckLine(file string, lineNo int, line string) []codeowne
}
result := codeowners.CheckResult{
Position: codeowners.Position{
FilePath: file,
FilePath: v.options.CodeownersFileLocation,
StartLine: lineNo,
EndLine: lineNo,
StartColumn: strings.Index(line, owner) + 1,
Expand Down
54 changes: 45 additions & 9 deletions checkers/invalidowner_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@ func TestInvalidOwnerCheckInvalidLongUsername(t *testing.T) {
}

checker := checkers.InvalidOwner{}
got := checker.CheckLine("CODEOWNERS", input.lineNo, input.line)
validator := checker.NewValidator(codeowners.ValidatorOptions{
Directory: ".",
CodeownersFileLocation: "CODEOWNERS",
})
got := validator.ValidateLine(input.lineNo, input.line)
if !reflect.DeepEqual(got, want) {
t.Errorf("Input: %v, Want: %v, Got: %v", input, want, got)
}
Expand Down Expand Up @@ -62,7 +66,11 @@ func TestInvalidOwnerCheckInvalidNoAt(t *testing.T) {
}

checker := checkers.InvalidOwner{}
got := checker.CheckLine("CODEOWNERS", input.lineNo, input.line)
validator := checker.NewValidator(codeowners.ValidatorOptions{
Directory: ".",
CodeownersFileLocation: "CODEOWNERS",
})
got := validator.ValidateLine(input.lineNo, input.line)
if !reflect.DeepEqual(got, want) {
t.Errorf("Input: %v, Want: %v, Got: %v", input, want, got)
}
Expand Down Expand Up @@ -92,7 +100,11 @@ func TestInvalidOwnerCheckInvalidHyphens(t *testing.T) {
}

checker := checkers.InvalidOwner{}
got := checker.CheckLine("CODEOWNERS", input.lineNo, input.line)
validator := checker.NewValidator(codeowners.ValidatorOptions{
Directory: ".",
CodeownersFileLocation: "CODEOWNERS",
})
got := validator.ValidateLine(input.lineNo, input.line)
if !reflect.DeepEqual(got, want) {
t.Errorf("Input: %v, Want: %v, Got: %v", input, want, got)
}
Expand Down Expand Up @@ -122,7 +134,11 @@ func TestInvalidOwnerCheckInvalidFormat(t *testing.T) {
}

checker := checkers.InvalidOwner{}
got := checker.CheckLine("CODEOWNERS", input.lineNo, input.line)
validator := checker.NewValidator(codeowners.ValidatorOptions{
Directory: ".",
CodeownersFileLocation: "CODEOWNERS",
})
got := validator.ValidateLine(input.lineNo, input.line)
if !reflect.DeepEqual(got, want) {
t.Errorf("Input: %v, Want: %v, Got: %v", input, want, got)
}
Expand Down Expand Up @@ -151,7 +167,11 @@ func TestInvalidOwnerCheckInvalidTrailingHyphen(t *testing.T) {
}

checker := checkers.InvalidOwner{}
got := checker.CheckLine("CODEOWNERS", input.lineNo, input.line)
validator := checker.NewValidator(codeowners.ValidatorOptions{
Directory: ".",
CodeownersFileLocation: "CODEOWNERS",
})
got := validator.ValidateLine(input.lineNo, input.line)
if !reflect.DeepEqual(got, want) {
t.Errorf("Input: %v, Want: %v, Got: %v", input, want, got)
}
Expand Down Expand Up @@ -193,7 +213,11 @@ func TestInvalidOwnerCheckMultipleInvalid(t *testing.T) {
}

checker := checkers.InvalidOwner{}
got := checker.CheckLine("CODEOWNERS", input.lineNo, input.line)
validator := checker.NewValidator(codeowners.ValidatorOptions{
Directory: ".",
CodeownersFileLocation: "CODEOWNERS",
})
got := validator.ValidateLine(input.lineNo, input.line)
if !reflect.DeepEqual(got, want) {
t.Errorf("Input: %v, Want: %v, Got: %v", input, want, got)
}
Expand All @@ -209,7 +233,11 @@ func TestInvalidOwnerCheckPassUser(t *testing.T) {
}

checker := checkers.InvalidOwner{}
got := checker.CheckLine("CODEOWNERS", input.lineNo, input.line)
validator := checker.NewValidator(codeowners.ValidatorOptions{
Directory: ".",
CodeownersFileLocation: "CODEOWNERS",
})
got := validator.ValidateLine(input.lineNo, input.line)
if got != nil {
t.Errorf("Input: %v, Want: %v, Got: %v", input, nil, got)
}
Expand All @@ -225,7 +253,11 @@ func TestInvalidOwnerCheckPassEmail(t *testing.T) {
}

checker := checkers.InvalidOwner{}
got := checker.CheckLine("CODEOWNERS", input.lineNo, input.line)
validator := checker.NewValidator(codeowners.ValidatorOptions{
Directory: ".",
CodeownersFileLocation: "CODEOWNERS",
})
got := validator.ValidateLine(input.lineNo, input.line)
if got != nil {
t.Errorf("Input: %v, Want: %v, Got: %v", input, nil, got)
}
Expand All @@ -241,7 +273,11 @@ func TestInvalidOwnerCheckPassUserOrg(t *testing.T) {
}

checker := checkers.InvalidOwner{}
got := checker.CheckLine("CODEOWNERS", input.lineNo, input.line)
validator := checker.NewValidator(codeowners.ValidatorOptions{
Directory: ".",
CodeownersFileLocation: "CODEOWNERS",
})
got := validator.ValidateLine(input.lineNo, input.line)
if got != nil {
t.Errorf("Input: %v, Want: %v, Got: %v", input, nil, got)
}
Expand Down
17 changes: 14 additions & 3 deletions checkers/noowner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,19 @@ func init() {
// NoOwner represents checker to decide validate presence of owners in each of CODEOWNERS lines
type NoOwner struct{}

// CheckLine runs this NoOwner's check against each line
func (c NoOwner) CheckLine(file string, lineNo int, line string) []codeowners.CheckResult {
// NewValidator returns validating capabilities for this checker
func (c NoOwner) NewValidator(options codeowners.ValidatorOptions) codeowners.Validator {
return noOwnerValidator{
options: options,
}
}

type noOwnerValidator struct {
options codeowners.ValidatorOptions
}

// ValidateLine runs this NoOwner's check against each line
func (v noOwnerValidator) ValidateLine(lineNo int, line string) []codeowners.CheckResult {
var results []codeowners.CheckResult

_, owners := codeowners.ParseLine(line)
Expand All @@ -21,7 +32,7 @@ func (c NoOwner) CheckLine(file string, lineNo int, line string) []codeowners.Ch
results = []codeowners.CheckResult{
{
Position: codeowners.Position{
FilePath: file,
FilePath: v.options.CodeownersFileLocation,
StartLine: lineNo,
EndLine: lineNo,
StartColumn: 0,
Expand Down
Loading

0 comments on commit d6b6251

Please sign in to comment.