From f6aef1e63e092b9327dc22db3b64c5377b041292 Mon Sep 17 00:00:00 2001 From: schlehlein Date: Tue, 13 Jun 2023 14:37:01 +0200 Subject: [PATCH 01/18] feat: add matcher --- parse.go | 95 ++++++++++++++++++++++++++++++++++++++++----------- parse_test.go | 30 +++++++++++++++- 2 files changed, 105 insertions(+), 20 deletions(-) diff --git a/parse.go b/parse.go index cef7a8e..99d4693 100644 --- a/parse.go +++ b/parse.go @@ -3,12 +3,23 @@ package codeowners import ( "bufio" "bytes" + "errors" "fmt" "io" "regexp" "strings" ) +type ErrInvalidOwnerFormat struct { + Owner string +} + +func (err ErrInvalidOwnerFormat) Error() string { + return fmt.Sprintf("invalid owner format '%s'", err.Owner) +} + +var ErrNoMatch = errors.New("no match") + var ( emailRegexp = regexp.MustCompile(`\A[A-Z0-9a-z\._%\+\-]+@[A-Za-z0-9\.\-]+\.[A-Za-z]{2,6}\z`) teamRegexp = regexp.MustCompile(`\A@([a-zA-Z0-9\-]+\/[a-zA-Z0-9_\-]+)\z`) @@ -20,8 +31,52 @@ const ( stateOwners ) -// ParseFile parses a CODEOWNERS file, returning a set of rules. -func ParseFile(f io.Reader) (Ruleset, error) { +var DefaultMatchers = []Matcher{ + MatcherFunc(EmailMatcher), + MatcherFunc(TeamMatcher), + MatcherFunc(UsernameMatcher), +} + +type Matcher interface { + Match(s string) (Owner, error) +} + +type MatcherFunc func(s string) (Owner, error) + +func (f MatcherFunc) Match(s string) (Owner, error) { + return f(s) +} + +func EmailMatcher(s string) (Owner, error) { + match := emailRegexp.FindStringSubmatch(s) + if match == nil { + return Owner{}, ErrNoMatch + } + + return Owner{Value: match[0], Type: EmailOwner}, nil +} + +func TeamMatcher(s string) (Owner, error) { + match := teamRegexp.FindStringSubmatch(s) + if match == nil { + return Owner{}, ErrNoMatch + } + + return Owner{Value: match[1], Type: TeamOwner}, nil +} + +func UsernameMatcher(s string) (Owner, error) { + match := usernameRegexp.FindStringSubmatch(s) + if match == nil { + return Owner{}, ErrNoMatch + } + + return Owner{Value: match[1], Type: UsernameOwner}, nil +} + +// ParseFile parses a CODEOWNERS file and Matcher, returning a set of rules. +// If no Matchers are passed explicitly the DefaultMatchers are used. +func ParseFile(f io.Reader, mm ...Matcher) (Ruleset, error) { rules := Ruleset{} scanner := bufio.NewScanner(f) lineNo := 0 @@ -34,7 +89,7 @@ func ParseFile(f io.Reader) (Ruleset, error) { continue } - rule, err := parseRule(line) + rule, err := parseRule(line, mm) if err != nil { return nil, fmt.Errorf("line %d: %w", lineNo, err) } @@ -45,7 +100,7 @@ func ParseFile(f io.Reader) (Ruleset, error) { } // parseRule parses a single line of a CODEOWNERS file, returning a Rule struct -func parseRule(ruleStr string) (Rule, error) { +func parseRule(ruleStr string, mm []Matcher) (Rule, error) { r := Rule{} state := statePattern @@ -95,9 +150,9 @@ func parseRule(ruleStr string) (Rule, error) { // through whitespace before or after owner declarations if buf.Len() > 0 { ownerStr := buf.String() - owner, err := newOwner(ownerStr) + owner, err := newOwner(ownerStr, mm) if err != nil { - return r, fmt.Errorf("%s at position %d", err.Error(), i+1-len(ownerStr)) + return r, fmt.Errorf("%w at position %d", err, i+1-len(ownerStr)) } r.Owners = append(r.Owners, owner) buf.Reset() @@ -131,7 +186,7 @@ func parseRule(ruleStr string) (Rule, error) { // If there's an owner left in the buffer, don't leave it behind if buf.Len() > 0 { ownerStr := buf.String() - owner, err := newOwner(ownerStr) + owner, err := newOwner(ownerStr, mm) if err != nil { return r, fmt.Errorf("%s at position %d", err.Error(), len(ruleStr)+1-len(ownerStr)) } @@ -143,23 +198,25 @@ func parseRule(ruleStr string) (Rule, error) { } // newOwner figures out which kind of owner this is and returns an Owner struct -func newOwner(s string) (Owner, error) { - match := emailRegexp.FindStringSubmatch(s) - if match != nil { - return Owner{Value: match[0], Type: EmailOwner}, nil +func newOwner(s string, mm []Matcher) (Owner, error) { + if len(mm) == 0 { + mm = DefaultMatchers } - match = teamRegexp.FindStringSubmatch(s) - if match != nil { - return Owner{Value: match[1], Type: TeamOwner}, nil - } + for _, m := range mm { + o, err := m.Match(s) + if errors.Is(err, ErrNoMatch) { + continue + } else if err != nil { + return Owner{}, err + } - match = usernameRegexp.FindStringSubmatch(s) - if match != nil { - return Owner{Value: match[1], Type: UsernameOwner}, nil + return o, nil } - return Owner{}, fmt.Errorf("invalid owner format '%s'", s) + return Owner{}, ErrInvalidOwnerFormat{ + Owner: s, + } } func isWhitespace(ch rune) bool { diff --git a/parse_test.go b/parse_test.go index fbc2c63..fab2b81 100644 --- a/parse_test.go +++ b/parse_test.go @@ -10,6 +10,7 @@ func TestParseRule(t *testing.T) { examples := []struct { name string rule string + matcher []Matcher expected Rule err string }{ @@ -142,11 +143,38 @@ func TestParseRule(t *testing.T) { rule: "file.txt missing-at-sign", err: "invalid owner format 'missing-at-sign' at position 10", }, + { + name: "email owners without email matcher", + rule: "file.txt foo@example.com", + matcher: []Matcher{ + MatcherFunc(TeamMatcher), + MatcherFunc(UsernameMatcher), + }, + err: "invalid owner format 'foo@example.com' at position 10", + }, + { + name: "team owners without team matcher", + rule: "file.txt @org/team", + matcher: []Matcher{ + MatcherFunc(EmailMatcher), + MatcherFunc(UsernameMatcher), + }, + err: "invalid owner format '@org/team' at position 10", + }, + { + name: "username owners without username matcher", + rule: "file.txt @user", + matcher: []Matcher{ + MatcherFunc(EmailMatcher), + MatcherFunc(TeamMatcher), + }, + err: "invalid owner format '@user' at position 10", + }, } for _, e := range examples { t.Run("parses "+e.name, func(t *testing.T) { - actual, err := parseRule(e.rule) + actual, err := parseRule(e.rule, e.matcher) if e.err != "" { assert.EqualError(t, err, e.err) } else { From 489d99ea6065f823c87d308a7a277db082f3d910 Mon Sep 17 00:00:00 2001 From: schlehlein Date: Tue, 13 Jun 2023 14:57:07 +0200 Subject: [PATCH 02/18] refactor: match func names --- parse.go | 14 ++++++++------ parse_test.go | 12 ++++++------ 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/parse.go b/parse.go index 99d4693..cf877ef 100644 --- a/parse.go +++ b/parse.go @@ -32,12 +32,14 @@ const ( ) var DefaultMatchers = []Matcher{ - MatcherFunc(EmailMatcher), - MatcherFunc(TeamMatcher), - MatcherFunc(UsernameMatcher), + MatcherFunc(MatchEmail), + MatcherFunc(MatchTeam), + MatcherFunc(MatchUsername), } type Matcher interface { + // Matches give string agains a pattern e.g. a regexp. + // Should return ErrNoMatch if the pattern doesn't match. Match(s string) (Owner, error) } @@ -47,7 +49,7 @@ func (f MatcherFunc) Match(s string) (Owner, error) { return f(s) } -func EmailMatcher(s string) (Owner, error) { +func MatchEmail(s string) (Owner, error) { match := emailRegexp.FindStringSubmatch(s) if match == nil { return Owner{}, ErrNoMatch @@ -56,7 +58,7 @@ func EmailMatcher(s string) (Owner, error) { return Owner{Value: match[0], Type: EmailOwner}, nil } -func TeamMatcher(s string) (Owner, error) { +func MatchTeam(s string) (Owner, error) { match := teamRegexp.FindStringSubmatch(s) if match == nil { return Owner{}, ErrNoMatch @@ -65,7 +67,7 @@ func TeamMatcher(s string) (Owner, error) { return Owner{Value: match[1], Type: TeamOwner}, nil } -func UsernameMatcher(s string) (Owner, error) { +func MatchUsername(s string) (Owner, error) { match := usernameRegexp.FindStringSubmatch(s) if match == nil { return Owner{}, ErrNoMatch diff --git a/parse_test.go b/parse_test.go index fab2b81..4f48512 100644 --- a/parse_test.go +++ b/parse_test.go @@ -147,8 +147,8 @@ func TestParseRule(t *testing.T) { name: "email owners without email matcher", rule: "file.txt foo@example.com", matcher: []Matcher{ - MatcherFunc(TeamMatcher), - MatcherFunc(UsernameMatcher), + MatcherFunc(MatchTeam), + MatcherFunc(MatchUsername), }, err: "invalid owner format 'foo@example.com' at position 10", }, @@ -156,8 +156,8 @@ func TestParseRule(t *testing.T) { name: "team owners without team matcher", rule: "file.txt @org/team", matcher: []Matcher{ - MatcherFunc(EmailMatcher), - MatcherFunc(UsernameMatcher), + MatcherFunc(MatchEmail), + MatcherFunc(MatchUsername), }, err: "invalid owner format '@org/team' at position 10", }, @@ -165,8 +165,8 @@ func TestParseRule(t *testing.T) { name: "username owners without username matcher", rule: "file.txt @user", matcher: []Matcher{ - MatcherFunc(EmailMatcher), - MatcherFunc(TeamMatcher), + MatcherFunc(MatchEmail), + MatcherFunc(MatchTeam), }, err: "invalid owner format '@user' at position 10", }, From efe844c8ee0716d4173be726e0ca0e8f196e620b Mon Sep 17 00:00:00 2001 From: Harry Marr Date: Sun, 26 Nov 2023 14:16:50 -0500 Subject: [PATCH 03/18] ParseFile accepts parse options --- example_test.go | 38 ++++++++++++++++++++++++ parse.go | 77 +++++++++++++++++++++++++++++-------------------- parse_test.go | 34 ++++++++++++---------- 3 files changed, 102 insertions(+), 47 deletions(-) diff --git a/example_test.go b/example_test.go index a30c6b7..dd3259d 100644 --- a/example_test.go +++ b/example_test.go @@ -3,6 +3,7 @@ package codeowners_test import ( "bytes" "fmt" + "regexp" "github.com/hmarr/codeowners" ) @@ -41,6 +42,43 @@ func ExampleParseFile() { // Go code } +func ExampleParseFile_customOwnerMatchers() { + validUsernames := []string{"the-a-team", "the-b-team"} + usernameRegexp := regexp.MustCompile(`\A@([a-zA-Z0-9\-]+)\z`) + + f := bytes.NewBufferString("src/**/*.go @the-a-team # Go code") + ownerMatchers := []codeowners.OwnerMatcher{ + codeowners.OwnerMatchFunc(codeowners.MatchEmailOwner), + codeowners.OwnerMatchFunc(func(s string) (codeowners.Owner, error) { + // Custom owner matcher that only matches valid usernames + match := usernameRegexp.FindStringSubmatch(s) + if match == nil { + return codeowners.Owner{}, codeowners.ErrNoMatch + } + + for _, t := range validUsernames { + if t == match[1] { + return codeowners.Owner{Value: match[1], Type: codeowners.TeamOwner}, nil + } + } + return codeowners.Owner{}, codeowners.ErrNoMatch + }), + } + ruleset, err := codeowners.ParseFile(f, codeowners.WithOwnerMatchers(ownerMatchers)) + if err != nil { + panic(err) + } + fmt.Println(len(ruleset)) + fmt.Println(ruleset[0].RawPattern()) + fmt.Println(ruleset[0].Owners[0].String()) + fmt.Println(ruleset[0].Comment) + // Output: + // 1 + // src/**/*.go + // @the-a-team + // Go code +} + func ExampleRuleset_Match() { f := bytes.NewBufferString("src/**/*.go @acme/go-developers # Go code") ruleset, _ := codeowners.ParseFile(f) diff --git a/parse.go b/parse.go index cf877ef..d5d7e01 100644 --- a/parse.go +++ b/parse.go @@ -10,6 +10,24 @@ import ( "strings" ) +type parseOption func(*parseOptions) + +type parseOptions struct { + ownerMatchers []OwnerMatcher +} + +func WithOwnerMatchers(mm []OwnerMatcher) parseOption { + return func(opts *parseOptions) { + opts.ownerMatchers = mm + } +} + +type OwnerMatcher interface { + // Matches give string agains a pattern e.g. a regexp. + // Should return ErrNoMatch if the pattern doesn't match. + Match(s string) (Owner, error) +} + type ErrInvalidOwnerFormat struct { Owner string } @@ -26,30 +44,19 @@ var ( usernameRegexp = regexp.MustCompile(`\A@([a-zA-Z0-9\-]+)\z`) ) -const ( - statePattern = iota + 1 - stateOwners -) - -var DefaultMatchers = []Matcher{ - MatcherFunc(MatchEmail), - MatcherFunc(MatchTeam), - MatcherFunc(MatchUsername), -} - -type Matcher interface { - // Matches give string agains a pattern e.g. a regexp. - // Should return ErrNoMatch if the pattern doesn't match. - Match(s string) (Owner, error) +var DefaultOwnerMatchers = []OwnerMatcher{ + OwnerMatchFunc(MatchEmailOwner), + OwnerMatchFunc(MatchTeamOwner), + OwnerMatchFunc(MatchUsernameOwner), } -type MatcherFunc func(s string) (Owner, error) +type OwnerMatchFunc func(s string) (Owner, error) -func (f MatcherFunc) Match(s string) (Owner, error) { +func (f OwnerMatchFunc) Match(s string) (Owner, error) { return f(s) } -func MatchEmail(s string) (Owner, error) { +func MatchEmailOwner(s string) (Owner, error) { match := emailRegexp.FindStringSubmatch(s) if match == nil { return Owner{}, ErrNoMatch @@ -58,7 +65,7 @@ func MatchEmail(s string) (Owner, error) { return Owner{Value: match[0], Type: EmailOwner}, nil } -func MatchTeam(s string) (Owner, error) { +func MatchTeamOwner(s string) (Owner, error) { match := teamRegexp.FindStringSubmatch(s) if match == nil { return Owner{}, ErrNoMatch @@ -67,7 +74,7 @@ func MatchTeam(s string) (Owner, error) { return Owner{Value: match[1], Type: TeamOwner}, nil } -func MatchUsername(s string) (Owner, error) { +func MatchUsernameOwner(s string) (Owner, error) { match := usernameRegexp.FindStringSubmatch(s) if match == nil { return Owner{}, ErrNoMatch @@ -76,9 +83,14 @@ func MatchUsername(s string) (Owner, error) { return Owner{Value: match[1], Type: UsernameOwner}, nil } -// ParseFile parses a CODEOWNERS file and Matcher, returning a set of rules. -// If no Matchers are passed explicitly the DefaultMatchers are used. -func ParseFile(f io.Reader, mm ...Matcher) (Ruleset, error) { +// ParseFile parses a CODEOWNERS file, returning a set of rules. +// To override the default owner matchers, pass WithOwnerMatchers() as an option. +func ParseFile(f io.Reader, options ...parseOption) (Ruleset, error) { + opts := parseOptions{ownerMatchers: DefaultOwnerMatchers} + for _, opt := range options { + opt(&opts) + } + rules := Ruleset{} scanner := bufio.NewScanner(f) lineNo := 0 @@ -91,7 +103,7 @@ func ParseFile(f io.Reader, mm ...Matcher) (Ruleset, error) { continue } - rule, err := parseRule(line, mm) + rule, err := parseRule(line, opts) if err != nil { return nil, fmt.Errorf("line %d: %w", lineNo, err) } @@ -101,8 +113,13 @@ func ParseFile(f io.Reader, mm ...Matcher) (Ruleset, error) { return rules, nil } +const ( + statePattern = iota + 1 + stateOwners +) + // parseRule parses a single line of a CODEOWNERS file, returning a Rule struct -func parseRule(ruleStr string, mm []Matcher) (Rule, error) { +func parseRule(ruleStr string, opts parseOptions) (Rule, error) { r := Rule{} state := statePattern @@ -152,7 +169,7 @@ func parseRule(ruleStr string, mm []Matcher) (Rule, error) { // through whitespace before or after owner declarations if buf.Len() > 0 { ownerStr := buf.String() - owner, err := newOwner(ownerStr, mm) + owner, err := newOwner(ownerStr, opts.ownerMatchers) if err != nil { return r, fmt.Errorf("%w at position %d", err, i+1-len(ownerStr)) } @@ -188,7 +205,7 @@ func parseRule(ruleStr string, mm []Matcher) (Rule, error) { // If there's an owner left in the buffer, don't leave it behind if buf.Len() > 0 { ownerStr := buf.String() - owner, err := newOwner(ownerStr, mm) + owner, err := newOwner(ownerStr, opts.ownerMatchers) if err != nil { return r, fmt.Errorf("%s at position %d", err.Error(), len(ruleStr)+1-len(ownerStr)) } @@ -200,11 +217,7 @@ func parseRule(ruleStr string, mm []Matcher) (Rule, error) { } // newOwner figures out which kind of owner this is and returns an Owner struct -func newOwner(s string, mm []Matcher) (Owner, error) { - if len(mm) == 0 { - mm = DefaultMatchers - } - +func newOwner(s string, mm []OwnerMatcher) (Owner, error) { for _, m := range mm { o, err := m.Match(s) if errors.Is(err, ErrNoMatch) { diff --git a/parse_test.go b/parse_test.go index 4f48512..0c9dae3 100644 --- a/parse_test.go +++ b/parse_test.go @@ -8,11 +8,11 @@ import ( func TestParseRule(t *testing.T) { examples := []struct { - name string - rule string - matcher []Matcher - expected Rule - err string + name string + rule string + ownerMatchers []OwnerMatcher + expected Rule + err string }{ // Success cases { @@ -146,27 +146,27 @@ func TestParseRule(t *testing.T) { { name: "email owners without email matcher", rule: "file.txt foo@example.com", - matcher: []Matcher{ - MatcherFunc(MatchTeam), - MatcherFunc(MatchUsername), + ownerMatchers: []OwnerMatcher{ + OwnerMatchFunc(MatchTeamOwner), + OwnerMatchFunc(MatchUsernameOwner), }, err: "invalid owner format 'foo@example.com' at position 10", }, { name: "team owners without team matcher", rule: "file.txt @org/team", - matcher: []Matcher{ - MatcherFunc(MatchEmail), - MatcherFunc(MatchUsername), + ownerMatchers: []OwnerMatcher{ + OwnerMatchFunc(MatchEmailOwner), + OwnerMatchFunc(MatchUsernameOwner), }, err: "invalid owner format '@org/team' at position 10", }, { name: "username owners without username matcher", rule: "file.txt @user", - matcher: []Matcher{ - MatcherFunc(MatchEmail), - MatcherFunc(MatchTeam), + ownerMatchers: []OwnerMatcher{ + OwnerMatchFunc(MatchEmailOwner), + OwnerMatchFunc(MatchTeamOwner), }, err: "invalid owner format '@user' at position 10", }, @@ -174,7 +174,11 @@ func TestParseRule(t *testing.T) { for _, e := range examples { t.Run("parses "+e.name, func(t *testing.T) { - actual, err := parseRule(e.rule, e.matcher) + opts := parseOptions{ownerMatchers: DefaultOwnerMatchers} + if e.ownerMatchers != nil { + opts.ownerMatchers = e.ownerMatchers + } + actual, err := parseRule(e.rule, opts) if e.err != "" { assert.EqualError(t, err, e.err) } else { From 538b68ecc58c748a18f04fadc72da570f1ff66ee Mon Sep 17 00:00:00 2001 From: Kim Mason Date: Fri, 5 Apr 2024 11:05:11 -0600 Subject: [PATCH 04/18] Update file parsing to handle lines with only whitespace. This commit updates file parsing so that lines that contain solely whitespace (as defined by by Unicode) are ignored. It's fairly common for people to accidentally create CODEOWNERS files with lines containing solely whitespace, and as it's semantically meaningless, it's better to ignore them then raise an error. --- example_test.go | 8 +++++++- parse.go | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/example_test.go b/example_test.go index a30c6b7..2b67373 100644 --- a/example_test.go +++ b/example_test.go @@ -8,7 +8,9 @@ import ( ) func Example() { - f := bytes.NewBufferString("src/**/*.c @acme/c-developers") + f := bytes.NewBufferString(`src/**/*.c @acme/c-developers +# The following line should be ignored; it contains only spaces and tabs` + + " \t\nsrc/**/*.go @acme/go-developers") ruleset, err := codeowners.ParseFile(f) if err != nil { panic(err) @@ -19,9 +21,13 @@ func Example() { match, err = ruleset.Match("src/foo.rs") fmt.Println(match) + + match, err = ruleset.Match("src/go/bar/bar.go") + fmt.Println(match.Owners) // Output: // [@acme/c-developers] // + // [@acme/go-developers] } func ExampleParseFile() { diff --git a/parse.go b/parse.go index cef7a8e..e866066 100644 --- a/parse.go +++ b/parse.go @@ -30,7 +30,7 @@ func ParseFile(f io.Reader) (Ruleset, error) { line := scanner.Text() // Ignore blank lines and comments - if len(line) == 0 || line[0] == '#' { + if len(strings.TrimSpace(line)) == 0 || line[0] == '#' { continue } From 1dd48a9c607016cb318a9dd04712d62251466fcd Mon Sep 17 00:00:00 2001 From: Stephen Gelman Date: Wed, 8 May 2024 13:11:11 -0500 Subject: [PATCH 05/18] Add `_` as allowed character in usernameRegexp While generally disallowed from public github usernames, an underscore is a key part of github EMU usernames. See https://docs.github.com/en/enterprise-cloud@latest/admin/identity-and-access-management/understanding-iam-for-enterprises/about-enterprise-managed-users for more information. --- parse.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parse.go b/parse.go index cef7a8e..9ea7722 100644 --- a/parse.go +++ b/parse.go @@ -12,7 +12,7 @@ import ( var ( emailRegexp = regexp.MustCompile(`\A[A-Z0-9a-z\._%\+\-]+@[A-Za-z0-9\.\-]+\.[A-Za-z]{2,6}\z`) teamRegexp = regexp.MustCompile(`\A@([a-zA-Z0-9\-]+\/[a-zA-Z0-9_\-]+)\z`) - usernameRegexp = regexp.MustCompile(`\A@([a-zA-Z0-9\-]+)\z`) + usernameRegexp = regexp.MustCompile(`\A@([a-zA-Z0-9\-_]+)\z`) ) const ( From d0bf217b198e09895d6769daf7317563c6b1b87b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Weselowski?= Date: Wed, 29 May 2024 10:57:13 +0200 Subject: [PATCH 06/18] adds support for sections this change adds support for sections, and their inheritance, which are used in gitlab what this does not support, yet, is approval configuration [SECTION][:approval_count:] or optional marking approval as optional --- codeowners.go | 8 +++ example_test.go | 44 +++++++++++++++ parse.go | 140 +++++++++++++++++++++++++++++++++++++++++++++++- parse_test.go | 72 ++++++++++++++++++++++++- 4 files changed, 261 insertions(+), 3 deletions(-) diff --git a/codeowners.go b/codeowners.go index 5aef80b..2910cbe 100644 --- a/codeowners.go +++ b/codeowners.go @@ -122,6 +122,12 @@ type Rule struct { pattern pattern } +type Section struct { + Name string + Owners []Owner + Comment string +} + // RawPattern returns the rule's gitignore-style path pattern. func (r Rule) RawPattern() string { return r.pattern.pattern @@ -139,6 +145,8 @@ const ( TeamOwner string = "team" // UsernameOwner is the owner type for GitHub usernames. UsernameOwner string = "username" + // GroupOwner is the owner type for Gitlab groups. + GroupOwner string = "group" ) // Owner represents an owner found in a rule. diff --git a/example_test.go b/example_test.go index 66e7d56..7af7103 100644 --- a/example_test.go +++ b/example_test.go @@ -106,3 +106,47 @@ func ExampleRuleset_Match() { // src/foo/bar.go true // src/foo.rs false } + +func ExampleRuleset_Match_section() { + f := bytes.NewBufferString(`[SECTION] @the-a-team +src +src-b @user-b +`) + ruleset, _ := codeowners.ParseFile(f) + match, _ := ruleset.Match("src") + fmt.Println("src", match != nil) + fmt.Println(ruleset[0].Owners[0].String()) + match, _ = ruleset.Match("src-b") + fmt.Println("src-b", match != nil) + fmt.Println(ruleset[1].Owners[0].String()) + // Output: + // src true + // @the-a-team + // src-b true + // @user-b +} + +func ExampleRuleset_Match_section_groups() { + f := bytes.NewBufferString(`[SECTION] @the/a/group +src +src-b @user-b +src-c @the/c/group +`) + ruleset, _ := codeowners.ParseFile(f) + match, _ := ruleset.Match("src") + fmt.Println("src", match != nil) + fmt.Println(ruleset[0].Owners[0].String()) + match, _ = ruleset.Match("src-b") + fmt.Println("src-b", match != nil) + fmt.Println(ruleset[1].Owners[0].String()) + match, _ = ruleset.Match("src-c") + fmt.Println("src-c", match != nil) + fmt.Println(ruleset[2].Owners[0].String()) + // Output: + // src true + // @the/a/group + // src-b true + // @user-b + // src-c true + // @the/c/group +} diff --git a/parse.go b/parse.go index 8a896cd..d55bb1b 100644 --- a/parse.go +++ b/parse.go @@ -42,12 +42,14 @@ var ( emailRegexp = regexp.MustCompile(`\A[A-Z0-9a-z\._%\+\-]+@[A-Za-z0-9\.\-]+\.[A-Za-z]{2,6}\z`) teamRegexp = regexp.MustCompile(`\A@([a-zA-Z0-9\-]+\/[a-zA-Z0-9_\-]+)\z`) usernameRegexp = regexp.MustCompile(`\A@([a-zA-Z0-9\-_]+)\z`) + groupRegexp = regexp.MustCompile(`\A@(([a-zA-Z0-9\-_]+)(/[a-zA-Z0-9\-_]+)*)\z`) ) var DefaultOwnerMatchers = []OwnerMatcher{ OwnerMatchFunc(MatchEmailOwner), OwnerMatchFunc(MatchTeamOwner), OwnerMatchFunc(MatchUsernameOwner), + OwnerMatchFunc(MatchGroupOwner), } type OwnerMatchFunc func(s string) (Owner, error) @@ -83,6 +85,15 @@ func MatchUsernameOwner(s string) (Owner, error) { return Owner{Value: match[1], Type: UsernameOwner}, nil } +func MatchGroupOwner(s string) (Owner, error) { + match := groupRegexp.FindStringSubmatch(s) + if match == nil { + return Owner{}, ErrNoMatch + } + + return Owner{Value: match[1], Type: GroupOwner}, nil +} + // ParseFile parses a CODEOWNERS file, returning a set of rules. // To override the default owner matchers, pass WithOwnerMatchers() as an option. func ParseFile(f io.Reader, options ...parseOption) (Ruleset, error) { @@ -91,6 +102,7 @@ func ParseFile(f io.Reader, options ...parseOption) (Ruleset, error) { opt(&opts) } + sectionOwners := []Owner{} rules := Ruleset{} scanner := bufio.NewScanner(f) lineNo := 0 @@ -103,7 +115,18 @@ func ParseFile(f io.Reader, options ...parseOption) (Ruleset, error) { continue } - rule, err := parseRule(line, opts) + if isSectionBraces(rune(line[0])) { + section, err := parseSection(line, opts) + if err != nil { + return nil, fmt.Errorf("line %d: %w", lineNo, err) + } + + sectionOwners = section.Owners + + continue + } + + rule, err := parseRule(line, opts, sectionOwners) if err != nil { return nil, fmt.Errorf("line %d: %w", lineNo, err) } @@ -116,10 +139,101 @@ func ParseFile(f io.Reader, options ...parseOption) (Ruleset, error) { const ( statePattern = iota + 1 stateOwners + stateSection ) +// parseSection parses a single line of a CODEOWNERS file, returning a Rule struct +func parseSection(ruleStr string, opts parseOptions) (Section, error) { + s := Section{} + + state := stateSection + escaped := false + buf := bytes.Buffer{} + for i, ch := range strings.TrimSpace(ruleStr) { + // Comments consume the rest of the line and stop further parsing + if ch == '#' { + s.Comment = strings.TrimSpace(ruleStr[i+1:]) + break + } + + switch state { + case stateSection: + switch { + case ch == '\\': + // Escape the next character (important for whitespace while parsing), but + // don't lose the backslash as it's part of the pattern + escaped = true + buf.WriteRune(ch) + continue + case isSectionBraces(ch): + continue + + case isSectionChar(ch) || (isWhitespace(ch) && escaped): + // Keep any valid pattern characters and escaped whitespace + buf.WriteRune(ch) + + case isWhitespace(ch) && !escaped: + s.Name = buf.String() + buf.Reset() + state = stateOwners + + default: + return s, fmt.Errorf("section: unexpected character '%c' at position %d", ch, i+1) + } + + case stateOwners: + switch { + case isWhitespace(ch): + // Whitespace means we've reached the end of the owner or we're just chomping + // through whitespace before or after owner declarations + if buf.Len() > 0 { + ownerStr := buf.String() + owner, err := newOwner(ownerStr, opts.ownerMatchers) + if err != nil { + return s, fmt.Errorf("section: %w at position %d", err, i+1-len(ownerStr)) + } + + s.Owners = append(s.Owners, owner) + buf.Reset() + } + + case isOwnersChar(ch): + // Write valid owner characters to the buffer + buf.WriteRune(ch) + + default: + return s, fmt.Errorf("section: unexpected character '%c' at position %d", ch, i+1) + } + } + } + + escaped = false + + // We've finished consuming the line, but we might still have content in the buffer + // if the line didn't end with a separator (whitespace) + switch state { + case stateSection: + s.Name = buf.String() + + case stateOwners: + // If there's an owner left in the buffer, don't leave it behind + if buf.Len() > 0 { + ownerStr := buf.String() + owner, err := newOwner(ownerStr, opts.ownerMatchers) + if err != nil { + return s, fmt.Errorf("%s at position %d", err.Error(), len(ruleStr)+1-len(ownerStr)) + } + + s.Owners = append(s.Owners, owner) + } + + } + + return s, nil +} + // parseRule parses a single line of a CODEOWNERS file, returning a Rule struct -func parseRule(ruleStr string, opts parseOptions) (Rule, error) { +func parseRule(ruleStr string, opts parseOptions, inheritedOwners []Owner) (Rule, error) { r := Rule{} state := statePattern @@ -213,6 +327,10 @@ func parseRule(ruleStr string, opts parseOptions) (Rule, error) { } } + if len(r.Owners) == 0 { + r.Owners = inheritedOwners + } + return r, nil } @@ -259,3 +377,21 @@ func isOwnersChar(ch rune) bool { } return isAlphanumeric(ch) } + +// isSectionChar matches characters that are allowed in owner definitions +func isSectionChar(ch rune) bool { + switch ch { + case '.', '@', '/', '_', '%', '+', '-': + return true + } + return isAlphanumeric(ch) +} + +// isSectionBraces matches characters that are allowed in section definitions +func isSectionBraces(ch rune) bool { + switch ch { + case '[', ']': + return true + } + return false +} diff --git a/parse_test.go b/parse_test.go index 0c9dae3..267d3c6 100644 --- a/parse_test.go +++ b/parse_test.go @@ -178,7 +178,77 @@ func TestParseRule(t *testing.T) { if e.ownerMatchers != nil { opts.ownerMatchers = e.ownerMatchers } - actual, err := parseRule(e.rule, opts) + actual, err := parseRule(e.rule, opts, nil) + if e.err != "" { + assert.EqualError(t, err, e.err) + } else { + assert.NoError(t, err) + assert.Equal(t, e.expected, actual) + } + }) + } +} + +func TestParseSection(t *testing.T) { + examples := []struct { + name string + rule string + ownerMatchers []OwnerMatcher + expected Section + err string + }{ + // Success cases + { + name: "match sections", + rule: "[Section]", + expected: Section{ + Name: "Section", + Owners: nil, + Comment: "", + }, + }, + { + name: "match sections with owner", + rule: "[Section-B-User] @the-b-user", + expected: Section{ + Name: "Section-B-User", + Owners: []Owner{{Value: "the-b-user", Type: "username"}}, + Comment: "", + }, + }, + { + name: "match sections with comment", + rule: "[Section] # some comment", + expected: Section{ + Name: "Section", + Owners: nil, + Comment: "some comment", + }, + }, + { + name: "match sections with owner and comment", + rule: "[Section] @the/a/team # some comment", + expected: Section{ + Name: "Section", + Owners: []Owner{{Value: "the/a/team", Type: "group"}}, + Comment: "some comment", + }, + ownerMatchers: []OwnerMatcher{ + OwnerMatchFunc(MatchGroupOwner), + }, + }, + + // Error cases + // TODO + } + + for _, e := range examples { + t.Run("parses Sections "+e.name, func(t *testing.T) { + opts := parseOptions{ownerMatchers: DefaultOwnerMatchers} + if e.ownerMatchers != nil { + opts.ownerMatchers = e.ownerMatchers + } + actual, err := parseSection(e.rule, opts) if e.err != "" { assert.EqualError(t, err, e.err) } else { From ad97610a7d91d6503aa599ce88cae6945823be31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Weselowski?= Date: Thu, 30 May 2024 12:03:38 +0200 Subject: [PATCH 07/18] adds checkmode flag --- cmd/codeowners/main.go | 53 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 49 insertions(+), 4 deletions(-) diff --git a/cmd/codeowners/main.go b/cmd/codeowners/main.go index 8a9e7e1..51d254d 100644 --- a/cmd/codeowners/main.go +++ b/cmd/codeowners/main.go @@ -2,6 +2,7 @@ package main import ( "bufio" + "errors" "fmt" "io" "os" @@ -12,10 +13,13 @@ import ( flag "github.com/spf13/pflag" ) +var ErrCheck = errors.New("unowned files exist") + func main() { var ( ownerFilters []string showUnowned bool + checkMode bool codeownersPath string helpFlag bool ) @@ -23,6 +27,7 @@ func main() { flag.BoolVarP(&showUnowned, "unowned", "u", false, "only show unowned files (can be combined with -o)") flag.StringVarP(&codeownersPath, "file", "f", "", "CODEOWNERS file path") flag.BoolVarP(&helpFlag, "help", "h", false, "show this help message") + flag.BoolVarP(&checkMode, "check", "c", false, "enable check mode and exist with a non-zero status code if unowned files exist") flag.Usage = func() { fmt.Fprintf(os.Stderr, "usage: codeowners ...\n") @@ -54,10 +59,16 @@ func main() { out := bufio.NewWriter(os.Stdout) defer out.Flush() + var checkError bool for _, startPath := range paths { // godirwalk only accepts directories, so we need to handle files separately if !isDir(startPath) { - if err := printFileOwners(out, ruleset, startPath, ownerFilters, showUnowned); err != nil { + if err := printFileOwners(out, ruleset, startPath, ownerFilters, showUnowned, checkMode); err != nil { + if errors.Is(err, ErrCheck) { + checkError = true + continue + } + fmt.Fprintf(os.Stderr, "error: %v", err) os.Exit(1) } @@ -71,19 +82,44 @@ func main() { // Only show code owners for files, not directories if !d.IsDir() { - return printFileOwners(out, ruleset, path, ownerFilters, showUnowned) + err := printFileOwners(out, ruleset, path, ownerFilters, showUnowned, checkMode) + if err != nil { + if errors.Is(err, ErrCheck) { + checkError = true + return nil + } + } + + return err } return nil }) if err != nil { + if errors.Is(err, ErrCheck) { + checkError = true + continue + } + fmt.Fprintf(os.Stderr, "error: %v", err) os.Exit(1) } } + + if checkError { + if showUnowned { + out.Flush() + } + + fmt.Fprintf(os.Stderr, "error: %v\n", ErrCheck.Error()) + + os.Exit(1) + } } -func printFileOwners(out io.Writer, ruleset codeowners.Ruleset, path string, ownerFilters []string, showUnowned bool) error { +func printFileOwners(out io.Writer, ruleset codeowners.Ruleset, path string, ownerFilters []string, showUnowned bool, checkMode bool) error { + hasUnowned := false + rule, err := ruleset.Match(path) if err != nil { return err @@ -91,9 +127,18 @@ func printFileOwners(out io.Writer, ruleset codeowners.Ruleset, path string, own // If we didn't get a match, the file is unowned if rule == nil || rule.Owners == nil { // Unless explicitly requested, don't show unowned files if we're filtering by owner - if len(ownerFilters) == 0 || showUnowned { + if len(ownerFilters) == 0 || showUnowned || checkMode { fmt.Fprintf(out, "%-70s (unowned)\n", path) + + if checkMode { + hasUnowned = true + } + } + + if hasUnowned { + return ErrCheck } + return nil } From 08a23bcd9c404b8c4c3bca5f38d83f85cf366bb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Weselowski?= Date: Thu, 30 May 2024 12:10:21 +0200 Subject: [PATCH 08/18] format docblock --- codeowners.go | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/codeowners.go b/codeowners.go index 2910cbe..4ef6f07 100644 --- a/codeowners.go +++ b/codeowners.go @@ -5,27 +5,28 @@ // the CODEOWNERS file format into rulesets, which may then be used to determine // the ownership of files. // -// Usage +// # Usage // // To find the owner of a given file, parse a CODEOWNERS file and call Match() // on the resulting ruleset. -// ruleset, err := codeowners.ParseFile(file) -// if err != nil { -// log.Fatal(err) -// } // -// rule, err := ruleset.Match("path/to/file") -// if err != nil { -// log.Fatal(err) -// } +// ruleset, err := codeowners.ParseFile(file) +// if err != nil { +// log.Fatal(err) +// } // -// Command line interface +// rule, err := ruleset.Match("path/to/file") +// if err != nil { +// log.Fatal(err) +// } +// +// # Command line interface // // A command line interface is also available in the cmd/codeowners package. // When run, it will walk the directory tree showing the code owners for each // file encountered. The help flag lists available options. // -// $ codeowners --help +// $ codeowners --help package codeowners import ( From 117fa4c6f07cea2d6e1fff5c271a399d8c3692c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Weselowski?= Date: Thu, 30 May 2024 12:11:25 +0200 Subject: [PATCH 09/18] bump go version to 1.22 --- go.mod | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index f1dafa2..e342c54 100644 --- a/go.mod +++ b/go.mod @@ -1,8 +1,14 @@ module github.com/hmarr/codeowners -go 1.14 +go 1.22 require ( github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.8.0 ) + +require ( + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect +) From 21f24abdd2a47d83332cf852187b3b3176c43db8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Weselowski?= Date: Thu, 30 May 2024 12:34:07 +0200 Subject: [PATCH 10/18] update README to include checkMode --- README.md | 1 + cmd/codeowners/main.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 8950522..1866d35 100644 --- a/README.md +++ b/README.md @@ -35,6 +35,7 @@ usage: codeowners ... -h, --help show this help message -o, --owner strings filter results by owner -u, --unowned only show unowned files (can be combined with -o) + -c, --check exits with a non-zero status code if unowned files exist $ ls CODEOWNERS DOCUMENTATION.md README.md example.go example_test.go diff --git a/cmd/codeowners/main.go b/cmd/codeowners/main.go index 51d254d..8661e87 100644 --- a/cmd/codeowners/main.go +++ b/cmd/codeowners/main.go @@ -27,7 +27,7 @@ func main() { flag.BoolVarP(&showUnowned, "unowned", "u", false, "only show unowned files (can be combined with -o)") flag.StringVarP(&codeownersPath, "file", "f", "", "CODEOWNERS file path") flag.BoolVarP(&helpFlag, "help", "h", false, "show this help message") - flag.BoolVarP(&checkMode, "check", "c", false, "enable check mode and exist with a non-zero status code if unowned files exist") + flag.BoolVarP(&checkMode, "check", "c", false, "exit with a non-zero status code if unowned files exist") flag.Usage = func() { fmt.Fprintf(os.Stderr, "usage: codeowners ...\n") From 79d319c80a8bf28804c4983e8a69b0bb9df67bec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Weselowski?= Date: Fri, 31 May 2024 08:29:19 +0200 Subject: [PATCH 11/18] update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 1866d35..070fbaf 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ usage: codeowners ... -h, --help show this help message -o, --owner strings filter results by owner -u, --unowned only show unowned files (can be combined with -o) - -c, --check exits with a non-zero status code if unowned files exist + -c, --check exit with a non-zero status code if unowned files exist $ ls CODEOWNERS DOCUMENTATION.md README.md example.go example_test.go From ef76197cb41e3757637f119dfa46aee0784bb8b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Weselowski?= Date: Fri, 31 May 2024 08:41:20 +0200 Subject: [PATCH 12/18] adds WithSectionSupport option, off by default --- example_test.go | 34 ++++++++++++++++++++++++++++++++-- parse.go | 11 +++++++++-- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/example_test.go b/example_test.go index 7af7103..3e47904 100644 --- a/example_test.go +++ b/example_test.go @@ -112,7 +112,7 @@ func ExampleRuleset_Match_section() { src src-b @user-b `) - ruleset, _ := codeowners.ParseFile(f) + ruleset, _ := codeowners.ParseFile(f, codeowners.WithSectionSupport()) match, _ := ruleset.Match("src") fmt.Println("src", match != nil) fmt.Println(ruleset[0].Owners[0].String()) @@ -132,7 +132,7 @@ src src-b @user-b src-c @the/c/group `) - ruleset, _ := codeowners.ParseFile(f) + ruleset, _ := codeowners.ParseFile(f, codeowners.WithSectionSupport()) match, _ := ruleset.Match("src") fmt.Println("src", match != nil) fmt.Println(ruleset[0].Owners[0].String()) @@ -150,3 +150,33 @@ src-c @the/c/group // src-c true // @the/c/group } + +func ExampleRuleset_Match_section_groups_multiple() { + f := bytes.NewBufferString(`[SECTION] @the/a/group +* @other + +[SECTION-B] @the/b/group +b-src +b-src-b @user-b +b-src-c @the/c/group + +[SECTION-C] +`) + ruleset, _ := codeowners.ParseFile(f, codeowners.WithSectionSupport()) + match, _ := ruleset.Match("b-src") + fmt.Println("b-src", match != nil) + fmt.Println(ruleset[1].Owners[0].String()) + match, _ = ruleset.Match("b-src-b") + fmt.Println("b-src-b", match != nil) + fmt.Println(ruleset[2].Owners[0].String()) + match, _ = ruleset.Match("b-src-c") + fmt.Println("b-src-c", match != nil) + fmt.Println(ruleset[3].Owners[0].String()) + // Output: + // b-src true + // @the/b/group + // b-src-b true + // @user-b + // b-src-c true + // @the/c/group +} diff --git a/parse.go b/parse.go index d55bb1b..2723687 100644 --- a/parse.go +++ b/parse.go @@ -13,7 +13,14 @@ import ( type parseOption func(*parseOptions) type parseOptions struct { - ownerMatchers []OwnerMatcher + ownerMatchers []OwnerMatcher + sectionSupport bool +} + +func WithSectionSupport() parseOption { + return func(opts *parseOptions) { + opts.sectionSupport = true + } } func WithOwnerMatchers(mm []OwnerMatcher) parseOption { @@ -115,7 +122,7 @@ func ParseFile(f io.Reader, options ...parseOption) (Ruleset, error) { continue } - if isSectionBraces(rune(line[0])) { + if opts.sectionSupport && isSectionBraces(rune(line[0])) { section, err := parseSection(line, opts) if err != nil { return nil, fmt.Errorf("line %d: %w", lineNo, err) From de90d434cb8bfa21ad643a997ae016cda176ae6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Weselowski?= Date: Fri, 31 May 2024 09:08:38 +0200 Subject: [PATCH 13/18] refactor cmd to use a struct for passing options around, and made ParserOptions public --- cmd/codeowners/main.go | 61 +++++++++++++++++++++++++----------------- codeowners.go | 8 +++--- parse.go | 8 +++--- 3 files changed, 45 insertions(+), 32 deletions(-) diff --git a/cmd/codeowners/main.go b/cmd/codeowners/main.go index 8661e87..9ef2af4 100644 --- a/cmd/codeowners/main.go +++ b/cmd/codeowners/main.go @@ -15,19 +15,27 @@ import ( var ErrCheck = errors.New("unowned files exist") +type Codeowners struct { + ownerFilters []string + showUnowned bool + checkMode bool + codeownersPath string + helpFlag bool + sections bool +} + func main() { var ( - ownerFilters []string - showUnowned bool - checkMode bool - codeownersPath string - helpFlag bool + c Codeowners + helpFlag bool ) - flag.StringSliceVarP(&ownerFilters, "owner", "o", nil, "filter results by owner") - flag.BoolVarP(&showUnowned, "unowned", "u", false, "only show unowned files (can be combined with -o)") - flag.StringVarP(&codeownersPath, "file", "f", "", "CODEOWNERS file path") + + flag.StringSliceVarP(&c.ownerFilters, "owner", "o", nil, "filter results by owner") + flag.BoolVarP(&c.showUnowned, "unowned", "u", false, "only show unowned files (can be combined with -o)") + flag.StringVarP(&c.codeownersPath, "file", "f", "", "CODEOWNERS file path") flag.BoolVarP(&helpFlag, "help", "h", false, "show this help message") - flag.BoolVarP(&checkMode, "check", "c", false, "exit with a non-zero status code if unowned files exist") + flag.BoolVarP(&c.checkMode, "check", "c", false, "exit with a non-zero status code if unowned files exist") + flag.BoolVarP(&c.sections, "sections", "", false, "support sections and inheritance") flag.Usage = func() { fmt.Fprintf(os.Stderr, "usage: codeowners ...\n") @@ -40,7 +48,7 @@ func main() { os.Exit(0) } - ruleset, err := loadCodeowners(codeownersPath) + ruleset, err := c.loadCodeowners() if err != nil { fmt.Fprintln(os.Stderr, err) os.Exit(1) @@ -52,8 +60,8 @@ func main() { } // Make the @ optional for GitHub teams and usernames - for i := range ownerFilters { - ownerFilters[i] = strings.TrimLeft(ownerFilters[i], "@") + for i := range c.ownerFilters { + c.ownerFilters[i] = strings.TrimLeft(c.ownerFilters[i], "@") } out := bufio.NewWriter(os.Stdout) @@ -63,7 +71,7 @@ func main() { for _, startPath := range paths { // godirwalk only accepts directories, so we need to handle files separately if !isDir(startPath) { - if err := printFileOwners(out, ruleset, startPath, ownerFilters, showUnowned, checkMode); err != nil { + if err := c.printFileOwners(out, ruleset, startPath); err != nil { if errors.Is(err, ErrCheck) { checkError = true continue @@ -82,7 +90,7 @@ func main() { // Only show code owners for files, not directories if !d.IsDir() { - err := printFileOwners(out, ruleset, path, ownerFilters, showUnowned, checkMode) + err := c.printFileOwners(out, ruleset, path) if err != nil { if errors.Is(err, ErrCheck) { checkError = true @@ -107,7 +115,7 @@ func main() { } if checkError { - if showUnowned { + if c.showUnowned { out.Flush() } @@ -117,7 +125,7 @@ func main() { } } -func printFileOwners(out io.Writer, ruleset codeowners.Ruleset, path string, ownerFilters []string, showUnowned bool, checkMode bool) error { +func (c Codeowners) printFileOwners(out io.Writer, ruleset codeowners.Ruleset, path string) error { hasUnowned := false rule, err := ruleset.Match(path) @@ -127,10 +135,10 @@ func printFileOwners(out io.Writer, ruleset codeowners.Ruleset, path string, own // If we didn't get a match, the file is unowned if rule == nil || rule.Owners == nil { // Unless explicitly requested, don't show unowned files if we're filtering by owner - if len(ownerFilters) == 0 || showUnowned || checkMode { + if len(c.ownerFilters) == 0 || c.showUnowned || c.checkMode { fmt.Fprintf(out, "%-70s (unowned)\n", path) - if checkMode { + if c.checkMode { hasUnowned = true } } @@ -146,8 +154,8 @@ func printFileOwners(out io.Writer, ruleset codeowners.Ruleset, path string, own ownersToShow := make([]string, 0, len(rule.Owners)) for _, o := range rule.Owners { // If there are no filters, show all owners - filterMatch := len(ownerFilters) == 0 && !showUnowned - for _, filter := range ownerFilters { + filterMatch := len(c.ownerFilters) == 0 && !c.showUnowned + for _, filter := range c.ownerFilters { if filter == o.Value { filterMatch = true } @@ -164,11 +172,16 @@ func printFileOwners(out io.Writer, ruleset codeowners.Ruleset, path string, own return nil } -func loadCodeowners(path string) (codeowners.Ruleset, error) { - if path == "" { - return codeowners.LoadFileFromStandardLocation() +func (c Codeowners) loadCodeowners() (codeowners.Ruleset, error) { + var parseOptions []codeowners.ParseOption + if c.sections { + parseOptions = append(parseOptions, codeowners.WithSectionSupport()) + } + + if c.codeownersPath == "" { + return codeowners.LoadFileFromStandardLocation(parseOptions...) } - return codeowners.LoadFile(path) + return codeowners.LoadFile(c.codeownersPath, parseOptions...) } // isDir checks if there's a directory at the path specified. diff --git a/codeowners.go b/codeowners.go index 4ef6f07..d849150 100644 --- a/codeowners.go +++ b/codeowners.go @@ -40,21 +40,21 @@ import ( // LoadFileFromStandardLocation loads and parses a CODEOWNERS file at one of the // standard locations for CODEOWNERS files (./, .github/, docs/). If run from a // git repository, all paths are relative to the repository root. -func LoadFileFromStandardLocation() (Ruleset, error) { +func LoadFileFromStandardLocation(options ...ParseOption) (Ruleset, error) { path := findFileAtStandardLocation() if path == "" { return nil, fmt.Errorf("could not find CODEOWNERS file at any of the standard locations") } - return LoadFile(path) + return LoadFile(path, options...) } // LoadFile loads and parses a CODEOWNERS file at the path specified. -func LoadFile(path string) (Ruleset, error) { +func LoadFile(path string, options ...ParseOption) (Ruleset, error) { f, err := os.Open(path) if err != nil { return nil, err } - return ParseFile(f) + return ParseFile(f, options...) } // findFileAtStandardLocation loops through the standard locations for diff --git a/parse.go b/parse.go index 2723687..f2ae41d 100644 --- a/parse.go +++ b/parse.go @@ -10,20 +10,20 @@ import ( "strings" ) -type parseOption func(*parseOptions) +type ParseOption func(*parseOptions) type parseOptions struct { ownerMatchers []OwnerMatcher sectionSupport bool } -func WithSectionSupport() parseOption { +func WithSectionSupport() ParseOption { return func(opts *parseOptions) { opts.sectionSupport = true } } -func WithOwnerMatchers(mm []OwnerMatcher) parseOption { +func WithOwnerMatchers(mm []OwnerMatcher) ParseOption { return func(opts *parseOptions) { opts.ownerMatchers = mm } @@ -103,7 +103,7 @@ func MatchGroupOwner(s string) (Owner, error) { // ParseFile parses a CODEOWNERS file, returning a set of rules. // To override the default owner matchers, pass WithOwnerMatchers() as an option. -func ParseFile(f io.Reader, options ...parseOption) (Ruleset, error) { +func ParseFile(f io.Reader, options ...ParseOption) (Ruleset, error) { opts := parseOptions{ownerMatchers: DefaultOwnerMatchers} for _, opt := range options { opt(&opts) From c5ba8392e5b545956a6999e131d6fd4deab466f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Weselowski?= Date: Fri, 31 May 2024 10:18:29 +0200 Subject: [PATCH 14/18] remove group matches, adds username characters --- parse.go | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/parse.go b/parse.go index f2ae41d..1c5dfa2 100644 --- a/parse.go +++ b/parse.go @@ -47,16 +47,14 @@ var ErrNoMatch = errors.New("no match") var ( emailRegexp = regexp.MustCompile(`\A[A-Z0-9a-z\._%\+\-]+@[A-Za-z0-9\.\-]+\.[A-Za-z]{2,6}\z`) - teamRegexp = regexp.MustCompile(`\A@([a-zA-Z0-9\-]+\/[a-zA-Z0-9_\-]+)\z`) - usernameRegexp = regexp.MustCompile(`\A@([a-zA-Z0-9\-_]+)\z`) - groupRegexp = regexp.MustCompile(`\A@(([a-zA-Z0-9\-_]+)(/[a-zA-Z0-9\-_]+)*)\z`) + teamRegexp = regexp.MustCompile(`\A@(([a-zA-Z0-9\-_]+)([\/][a-zA-Z0-9\-_]+)+)\z`) + usernameRegexp = regexp.MustCompile(`\A@(([a-zA-Z0-9\-_]+)([\._][a-zA-Z0-9\-_]+)*)\z`) ) var DefaultOwnerMatchers = []OwnerMatcher{ OwnerMatchFunc(MatchEmailOwner), OwnerMatchFunc(MatchTeamOwner), OwnerMatchFunc(MatchUsernameOwner), - OwnerMatchFunc(MatchGroupOwner), } type OwnerMatchFunc func(s string) (Owner, error) @@ -92,15 +90,6 @@ func MatchUsernameOwner(s string) (Owner, error) { return Owner{Value: match[1], Type: UsernameOwner}, nil } -func MatchGroupOwner(s string) (Owner, error) { - match := groupRegexp.FindStringSubmatch(s) - if match == nil { - return Owner{}, ErrNoMatch - } - - return Owner{Value: match[1], Type: GroupOwner}, nil -} - // ParseFile parses a CODEOWNERS file, returning a set of rules. // To override the default owner matchers, pass WithOwnerMatchers() as an option. func ParseFile(f io.Reader, options ...ParseOption) (Ruleset, error) { From 563506cc4f15e0436858269d36b602218c15e1ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Weselowski?= Date: Fri, 31 May 2024 10:19:24 +0200 Subject: [PATCH 15/18] fix support for sections - support spaces in section names - adds optional ^ character for sections --- parse.go | 63 +++++++++++++++++++++++++++++++++++++++++---------- parse_test.go | 34 +++++++++++++++++++++++++-- 2 files changed, 83 insertions(+), 14 deletions(-) diff --git a/parse.go b/parse.go index 1c5dfa2..58c72dd 100644 --- a/parse.go +++ b/parse.go @@ -111,7 +111,7 @@ func ParseFile(f io.Reader, options ...ParseOption) (Ruleset, error) { continue } - if opts.sectionSupport && isSectionBraces(rune(line[0])) { + if opts.sectionSupport && isSectionStart(rune(line[0])) { section, err := parseSection(line, opts) if err != nil { return nil, fmt.Errorf("line %d: %w", lineNo, err) @@ -136,6 +136,7 @@ const ( statePattern = iota + 1 stateOwners stateSection + stateSectionBrace ) // parseSection parses a single line of a CODEOWNERS file, returning a Rule struct @@ -161,17 +162,49 @@ func parseSection(ruleStr string, opts parseOptions) (Section, error) { escaped = true buf.WriteRune(ch) continue - case isSectionBraces(ch): + + case isSectionStart(ch): + if ch == '^' { + // optional approval for this section + continue + } + + state = stateSectionBrace continue - case isSectionChar(ch) || (isWhitespace(ch) && escaped): - // Keep any valid pattern characters and escaped whitespace + case isSectionChar(ch): buf.WriteRune(ch) - case isWhitespace(ch) && !escaped: + case isSectionEnd(ch) || isWhitespace(ch) && !escaped: + buf.Reset() + + state = stateOwners + + default: + return s, fmt.Errorf("section: unexpected character '%c' at position %d", ch, i+1) + } + + case stateSectionBrace: + // fmt.Println("stateSectionBrace") + switch { + case ch == '\\': + // Escape the next character (important for whitespace while parsing), but + // don't lose the backslash as it's part of the pattern + escaped = true + buf.WriteRune(ch) + continue + + case isSectionEnd(ch): s.Name = buf.String() + buf.Reset() + state = stateOwners + continue + + case isSectionChar(ch): + // Keep any valid pattern characters and escaped whitespace + buf.WriteRune(ch) default: return s, fmt.Errorf("section: unexpected character '%c' at position %d", ch, i+1) @@ -208,9 +241,6 @@ func parseSection(ruleStr string, opts parseOptions) (Section, error) { // We've finished consuming the line, but we might still have content in the buffer // if the line didn't end with a separator (whitespace) switch state { - case stateSection: - s.Name = buf.String() - case stateOwners: // If there's an owner left in the buffer, don't leave it behind if buf.Len() > 0 { @@ -377,16 +407,25 @@ func isOwnersChar(ch rune) bool { // isSectionChar matches characters that are allowed in owner definitions func isSectionChar(ch rune) bool { switch ch { - case '.', '@', '/', '_', '%', '+', '-': + case '.', '@', '/', '_', '%', '+', '-', ' ': return true } return isAlphanumeric(ch) } -// isSectionBraces matches characters that are allowed in section definitions -func isSectionBraces(ch rune) bool { +// isSectionEnd matches characters that are allowed in section definitions +func isSectionEnd(ch rune) bool { + switch ch { + case ']': + return true + } + return false +} + +// isSectionStart matches characters that are allowed in section definitions +func isSectionStart(ch rune) bool { switch ch { - case '[', ']': + case '[', '^': return true } return false diff --git a/parse_test.go b/parse_test.go index 267d3c6..760e90b 100644 --- a/parse_test.go +++ b/parse_test.go @@ -15,6 +15,22 @@ func TestParseRule(t *testing.T) { err string }{ // Success cases + { + name: "username with dots", + rule: "file.txt @user.name", + expected: Rule{ + pattern: mustBuildPattern(t, "file.txt"), + Owners: []Owner{{Value: "user.name", Type: "username"}}, + }, + }, + { + name: "username with underscore", + rule: "file.txt @user_name", + expected: Rule{ + pattern: mustBuildPattern(t, "file.txt"), + Owners: []Owner{{Value: "user_name", Type: "username"}}, + }, + }, { name: "username owners", rule: "file.txt @user", @@ -143,6 +159,11 @@ func TestParseRule(t *testing.T) { rule: "file.txt missing-at-sign", err: "invalid owner format 'missing-at-sign' at position 10", }, + { + name: "malformed owners trailing dot", + rule: "file.txt @trailing-dot.", + err: "invalid owner format '@trailing-dot.' at position 10", + }, { name: "email owners without email matcher", rule: "file.txt foo@example.com", @@ -207,6 +228,15 @@ func TestParseSection(t *testing.T) { Comment: "", }, }, + { + name: "match sections with spaces", + rule: "[Section Spaces]", + expected: Section{ + Name: "Section Spaces", + Owners: nil, + Comment: "", + }, + }, { name: "match sections with owner", rule: "[Section-B-User] @the-b-user", @@ -230,11 +260,11 @@ func TestParseSection(t *testing.T) { rule: "[Section] @the/a/team # some comment", expected: Section{ Name: "Section", - Owners: []Owner{{Value: "the/a/team", Type: "group"}}, + Owners: []Owner{{Value: "the/a/team", Type: "team"}}, Comment: "some comment", }, ownerMatchers: []OwnerMatcher{ - OwnerMatchFunc(MatchGroupOwner), + OwnerMatchFunc(MatchTeamOwner), }, }, From 4a8730183dde8b266d88fc576a6012da19987a36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Weselowski?= Date: Fri, 31 May 2024 10:30:24 +0200 Subject: [PATCH 16/18] adds support for parsing section approval required, count --- codeowners.go | 8 +++++--- parse.go | 26 ++++++++++++++++++++++++-- parse_test.go | 21 +++++++++++++++++++++ 3 files changed, 50 insertions(+), 5 deletions(-) diff --git a/codeowners.go b/codeowners.go index d849150..239dbdb 100644 --- a/codeowners.go +++ b/codeowners.go @@ -124,9 +124,11 @@ type Rule struct { } type Section struct { - Name string - Owners []Owner - Comment string + Name string + Owners []Owner + Comment string + ApprovalOptional bool + ApprovalCount int } // RawPattern returns the rule's gitignore-style path pattern. diff --git a/parse.go b/parse.go index 58c72dd..6d0b710 100644 --- a/parse.go +++ b/parse.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "regexp" + "strconv" "strings" ) @@ -137,6 +138,7 @@ const ( stateOwners stateSection stateSectionBrace + stateSectionApprovalCount ) // parseSection parses a single line of a CODEOWNERS file, returning a Rule struct @@ -165,7 +167,8 @@ func parseSection(ruleStr string, opts parseOptions) (Section, error) { case isSectionStart(ch): if ch == '^' { - // optional approval for this section + s.ApprovalOptional = true + continue } @@ -185,7 +188,6 @@ func parseSection(ruleStr string, opts parseOptions) (Section, error) { } case stateSectionBrace: - // fmt.Println("stateSectionBrace") switch { case ch == '\\': // Escape the next character (important for whitespace while parsing), but @@ -210,8 +212,28 @@ func parseSection(ruleStr string, opts parseOptions) (Section, error) { return s, fmt.Errorf("section: unexpected character '%c' at position %d", ch, i+1) } + case stateSectionApprovalCount: + switch { + case isSectionEnd(ch): + approvalCount := buf.String() + approvalCountInt, err := strconv.Atoi(approvalCount) + if err != nil { + return s, fmt.Errorf("section: invalid approval count %w at position %d", err, i+1) + } + s.ApprovalCount = approvalCountInt + + buf.Reset() + state = stateOwners + + default: + buf.WriteRune(ch) + } + case stateOwners: switch { + case isSectionStart(ch): + state = stateSectionApprovalCount + case isWhitespace(ch): // Whitespace means we've reached the end of the owner or we're just chomping // through whitespace before or after owner declarations diff --git a/parse_test.go b/parse_test.go index 760e90b..cdecfbc 100644 --- a/parse_test.go +++ b/parse_test.go @@ -237,6 +237,27 @@ func TestParseSection(t *testing.T) { Comment: "", }, }, + { + name: "match sections with optional approval", + rule: "^[Section]", + expected: Section{ + Name: "Section", + Owners: nil, + Comment: "", + ApprovalOptional: true, + }, + }, + { + name: "match sections with approval count", + rule: "^[Section][2]", + expected: Section{ + Name: "Section", + Owners: nil, + Comment: "", + ApprovalOptional: true, + ApprovalCount: 2, + }, + }, { name: "match sections with owner", rule: "[Section-B-User] @the-b-user", From 5bca64730da6afc54f7434c7be941da0212bc27d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Weselowski?= Date: Fri, 31 May 2024 11:50:38 +0200 Subject: [PATCH 17/18] remove deprecated ioutil use --- match_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/match_test.go b/match_test.go index bd012ef..eb3a3ca 100644 --- a/match_test.go +++ b/match_test.go @@ -2,7 +2,7 @@ package codeowners import ( "encoding/json" - "io/ioutil" + "os" "testing" "github.com/stretchr/testify/assert" @@ -17,7 +17,7 @@ type patternTest struct { } func TestMatch(t *testing.T) { - data, err := ioutil.ReadFile("testdata/patterns.json") + data, err := os.ReadFile("testdata/patterns.json") require.NoError(t, err) var tests []patternTest From d53a18a6ed8639a5a6d5db2a263b23d5a9be91a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9?= Date: Sat, 1 Jun 2024 05:51:00 +0200 Subject: [PATCH 18/18] Fixes inconsistent function descriptions --- parse.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/parse.go b/parse.go index 6d0b710..d502898 100644 --- a/parse.go +++ b/parse.go @@ -426,7 +426,7 @@ func isOwnersChar(ch rune) bool { return isAlphanumeric(ch) } -// isSectionChar matches characters that are allowed in owner definitions +// isSectionChar matches characters that are allowed for section names func isSectionChar(ch rune) bool { switch ch { case '.', '@', '/', '_', '%', '+', '-', ' ': @@ -435,7 +435,8 @@ func isSectionChar(ch rune) bool { return isAlphanumeric(ch) } -// isSectionEnd matches characters that are allowed in section definitions +// isSectionEnd matches characters ends each section block +// e.g. [Section Name][] func isSectionEnd(ch rune) bool { switch ch { case ']': @@ -444,7 +445,8 @@ func isSectionEnd(ch rune) bool { return false } -// isSectionStart matches characters that are allowed in section definitions +// isSectionStart defines characters starting the beginning of a section +// - `^` starts an optional section func isSectionStart(ch rune) bool { switch ch { case '[', '^':