From d6b625116cabc0c16e0d78f6c36f5589ff31e849 Mon Sep 17 00:00:00 2001 From: Filipe Constantinov Menezes Date: Fri, 21 Aug 2020 10:12:23 +0100 Subject: [PATCH] Refactor Parameters (#39) --- checker.go | 50 +++++++++++++++++++++++++------- checker_test.go | 48 ++++++++++++++++++++++++------- checkers/invalidowner.go | 20 ++++++++++--- checkers/invalidowner_test.go | 54 +++++++++++++++++++++++++++++------ checkers/noowner.go | 17 +++++++++-- checkers/noowner_test.go | 6 +++- cmd/codeownerslint/linter.go | 5 +++- 7 files changed, 161 insertions(+), 39 deletions(-) diff --git a/checker.go b/checker.go index 104e6ee..a186496 100644 --- a/checker.go +++ b/checker.go @@ -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 @@ -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 } @@ -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...) } @@ -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 } } } diff --git a/checker_test.go b/checker_test.go index 8bdf166..fce0241 100644 --- a/checker_test.go +++ b/checker_test.go @@ -6,7 +6,6 @@ import ( "testing" "github.com/fmenezes/codeowners" - _ "github.com/fmenezes/codeowners/checkers" ) const dummyCheckerName string = "dummy" @@ -14,11 +13,23 @@ 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, }, @@ -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, @@ -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) } @@ -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) } @@ -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") } @@ -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) } @@ -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) } @@ -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) } diff --git a/checkers/invalidowner.go b/checkers/invalidowner.go index 7300a39..b1f5efc 100644 --- a/checkers/invalidowner.go +++ b/checkers/invalidowner.go @@ -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])?)*$") @@ -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) @@ -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, diff --git a/checkers/invalidowner_test.go b/checkers/invalidowner_test.go index 5e57e16..315a322 100644 --- a/checkers/invalidowner_test.go +++ b/checkers/invalidowner_test.go @@ -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) } @@ -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) } @@ -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) } @@ -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) } @@ -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) } @@ -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) } @@ -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) } @@ -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) } @@ -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) } diff --git a/checkers/noowner.go b/checkers/noowner.go index ddd7045..183cd06 100644 --- a/checkers/noowner.go +++ b/checkers/noowner.go @@ -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) @@ -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, diff --git a/checkers/noowner_test.go b/checkers/noowner_test.go index b481c03..d8bfa9d 100644 --- a/checkers/noowner_test.go +++ b/checkers/noowner_test.go @@ -32,7 +32,11 @@ func TestNoOwnerCheck(t *testing.T) { } checker := checkers.NoOwner{} - 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) } diff --git a/cmd/codeownerslint/linter.go b/cmd/codeownerslint/linter.go index 7908328..e468e0c 100644 --- a/cmd/codeownerslint/linter.go +++ b/cmd/codeownerslint/linter.go @@ -44,7 +44,10 @@ func run(wr io.Writer, opt options) exitCode { checkers := codeowners.AvailableCheckers() - checks, _ := codeowners.Check(dir, checkers...) + checks, _ := codeowners.Check(codeowners.CheckOptions{ + Directory: dir, + Checkers: checkers, + }) code := successCode for _, check := range checks {