From c21a6007f0efce9580d173b6ccf9218be691bc87 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 1/2] adds support for sections this change adds support for sections, and their inheritance, which are used in gitlab - adds WithSectionSupport option, off by default - support spaces in section names - adds optional ^ character for sections adds support for parsing section approval required, count Fixes inconsistent function descriptions --- codeowners.go | 8 ++ example_test.go | 74 +++++++++++++++++ parse.go | 205 ++++++++++++++++++++++++++++++++++++++++++++++-- parse_test.go | 123 ++++++++++++++++++++++++++++- 4 files changed, 404 insertions(+), 6 deletions(-) diff --git a/codeowners.go b/codeowners.go index 5aef80b..3a7bcab 100644 --- a/codeowners.go +++ b/codeowners.go @@ -122,6 +122,14 @@ type Rule struct { pattern pattern } +type Section struct { + Name string + Owners []Owner + Comment string + ApprovalOptional bool + ApprovalCount int +} + // RawPattern returns the rule's gitignore-style path pattern. func (r Rule) RawPattern() string { return r.pattern.pattern diff --git a/example_test.go b/example_test.go index dd3259d..383d443 100644 --- a/example_test.go +++ b/example_test.go @@ -100,3 +100,77 @@ 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, codeowners.WithSectionSupport()) + 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, codeowners.WithSectionSupport()) + 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 +} + +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 4a930a4..b25c7f2 100644 --- a/parse.go +++ b/parse.go @@ -7,13 +7,21 @@ import ( "fmt" "io" "regexp" + "strconv" "strings" ) 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 { @@ -40,8 +48,8 @@ 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`) + 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`) ) // DefaultOwnerMatchers is the default set of owner matchers, which includes the @@ -103,6 +111,7 @@ func ParseFile(f io.Reader, options ...parseOption) (Ruleset, error) { opt(&opts) } + sectionOwners := []Owner{} rules := Ruleset{} scanner := bufio.NewScanner(f) lineNo := 0 @@ -115,7 +124,18 @@ func ParseFile(f io.Reader, options ...parseOption) (Ruleset, error) { continue } - rule, err := parseRule(line, opts) + if opts.sectionSupport && isSectionStart(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) } @@ -128,10 +148,152 @@ func ParseFile(f io.Reader, options ...parseOption) (Ruleset, error) { const ( statePattern = iota + 1 stateOwners + stateSection + stateSectionBrace + stateSectionApprovalCount ) +// 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 isSectionStart(ch): + if ch == '^' { + s.ApprovalOptional = true + + continue + } + + state = stateSectionBrace + continue + + case isSectionChar(ch): + buf.WriteRune(ch) + + 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: + 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) + } + + 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 + 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 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 @@ -225,6 +387,10 @@ func parseRule(ruleStr string, opts parseOptions) (Rule, error) { } } + if len(r.Owners) == 0 { + r.Owners = inheritedOwners + } + return r, nil } @@ -271,3 +437,32 @@ func isOwnersChar(ch rune) bool { } return isAlphanumeric(ch) } + +// isSectionChar matches characters that are allowed for section names +func isSectionChar(ch rune) bool { + switch ch { + case '.', '@', '/', '_', '%', '+', '-', ' ': + return true + } + return isAlphanumeric(ch) +} + +// isSectionEnd matches characters ends each section block +// e.g. [Section Name][] +func isSectionEnd(ch rune) bool { + switch ch { + case ']': + return true + } + return false +} + +// isSectionStart defines characters starting the beginning of a section +// - `^` starts an optional section +func isSectionStart(ch rune) bool { + switch ch { + case '[', '^': + return true + } + return false +} diff --git a/parse_test.go b/parse_test.go index f45a17e..cb5dbe4 100644 --- a/parse_test.go +++ b/parse_test.go @@ -95,6 +95,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", @@ -255,6 +271,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", @@ -290,7 +311,107 @@ 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 spaces", + rule: "[Section Spaces]", + expected: Section{ + Name: "Section Spaces", + Owners: nil, + 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", + 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: "team"}}, + Comment: "some comment", + }, + ownerMatchers: []OwnerMatcher{ + OwnerMatchFunc(MatchTeamOwner), + }, + }, + + // 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 e1898943581fa89274a758bb28cca1cda01ae75e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Weselowski?= Date: Mon, 1 Jul 2024 10:46:02 +0200 Subject: [PATCH 2/2] refactor codeowners to simplify options passing before adding extra options via func parameters was getting overly long. now we can pass ParseOptions... --- cmd/codeowners/main.go | 53 ++++++++++++++++++++++++++---------------- codeowners.go | 8 +++---- parse.go | 8 +++---- 3 files changed, 41 insertions(+), 28 deletions(-) diff --git a/cmd/codeowners/main.go b/cmd/codeowners/main.go index 8a9e7e1..49e1cc0 100644 --- a/cmd/codeowners/main.go +++ b/cmd/codeowners/main.go @@ -12,17 +12,25 @@ import ( flag "github.com/spf13/pflag" ) +type Codeowners struct { + ownerFilters []string + showUnowned bool + codeownersPath string + helpFlag bool + sections bool +} + func main() { var ( - ownerFilters []string - showUnowned 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(&c.sections, "sections", "", false, "support sections and inheritance") flag.Usage = func() { fmt.Fprintf(os.Stderr, "usage: codeowners ...\n") @@ -35,7 +43,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) @@ -47,8 +55,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) @@ -57,7 +65,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); err != nil { + if err := c.printFileOwners(out, ruleset, startPath); err != nil { fmt.Fprintf(os.Stderr, "error: %v", err) os.Exit(1) } @@ -71,7 +79,7 @@ func main() { // Only show code owners for files, not directories if !d.IsDir() { - return printFileOwners(out, ruleset, path, ownerFilters, showUnowned) + return c.printFileOwners(out, ruleset, path) } return nil }) @@ -83,7 +91,7 @@ func main() { } } -func printFileOwners(out io.Writer, ruleset codeowners.Ruleset, path string, ownerFilters []string, showUnowned bool) error { +func (c Codeowners) printFileOwners(out io.Writer, ruleset codeowners.Ruleset, path string) error { rule, err := ruleset.Match(path) if err != nil { return err @@ -91,7 +99,7 @@ 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(c.ownerFilters) == 0 || c.showUnowned { fmt.Fprintf(out, "%-70s (unowned)\n", path) } return nil @@ -101,8 +109,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 } @@ -119,11 +127,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 3a7bcab..9abee52 100644 --- a/codeowners.go +++ b/codeowners.go @@ -39,21 +39,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 b25c7f2..519afc8 100644 --- a/parse.go +++ b/parse.go @@ -11,20 +11,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 } @@ -105,7 +105,7 @@ func MatchUsernameOwner(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)