Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion codeowners.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ max_reviews = 2
# `unskippable_reviewers` allows you to specify reviewers that cannot be skipped via the max_reviews setting
unskippable_reviewers = ["@BakerNet"]
# `ignore` allows you to specify directories that should be ignored by the codeowners check
ignore = ["test_project"]
ignore = ["test_project", "test_project_inline"]
# `high_priority_lables` allows you to specify labels that should be considered high priority
high_priority_labels = ["P0"]

Expand All @@ -16,3 +16,5 @@ high_priority_labels = ["P0"]
approval = false
# `fail_check` (default true) means the codeowners GHA check will fail if the codeowners check fails
fail_check = true
# `inline_ownership_enabled` (default false) enables parsing of inline ownership tags in source files
inline_ownership_enabled = true
61 changes: 58 additions & 3 deletions internal/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package app
import (
"fmt"
"io"
"os"
"path/filepath"
"slices"
"strings"
"time"
Expand All @@ -12,6 +14,7 @@ import (
gh "github.com/multimediallc/codeowners-plus/internal/github"
"github.com/multimediallc/codeowners-plus/pkg/codeowners"
f "github.com/multimediallc/codeowners-plus/pkg/functional"
"github.com/multimediallc/codeowners-plus/pkg/inlineowners"
)

// Config holds the application configuration
Expand Down Expand Up @@ -101,18 +104,70 @@ func (a *App) Run() (bool, string, error) {
}
a.codeowners = codeOwners

// Inline ownership integration
if a.Conf != nil && a.Conf.InlineOwnershipEnabled {
oracle := inlineowners.Oracle{}
// build oracle blocks per file
for _, df := range gitDiff.AllChanges() {
abs := filepath.Join(a.config.RepoDir, df.FileName)
data, err := os.ReadFile(abs)
if err != nil {
a.printWarn("WARNING: unable to read file %s: %v\n", df.FileName, err)
continue
}
blks, _ := inlineowners.Parse(string(data), a.config.WarningBuffer)
if len(blks) > 0 {
// convert to Block type
b2 := make([]inlineowners.Block, 0, len(blks))
for _, pb := range blks {
b2 = append(b2, inlineowners.Block{Owners: pb.Owners, Start: pb.StartLine, End: pb.EndLine})
}
oracle[df.FileName] = b2
}
}

overrides := make(map[string]codeowners.ReviewerGroups)
for _, df := range gitDiff.AllChanges() {
// aggregate owners from hunks via oracle
rgs := codeowners.ReviewerGroups{}
for _, h := range df.Hunks {
lists := oracle.OwnersForRange(df.FileName, h.Start, h.End)
if lists == nil {
continue
}
for _, lst := range lists {
rgs = append(rgs, &codeowners.ReviewerGroup{Names: lst, Approved: false})
}
}
if len(rgs) == 0 {
// fallback to file-level
if baseGroups, ok := codeOwners.FileRequired()[df.FileName]; ok {
rgs = append(rgs, baseGroups...)
}
} else {
rgs = f.RemoveDuplicates(rgs)
}
overrides[df.FileName] = rgs
}

a.codeowners = newOverlayOwners(codeOwners, overrides)
} else {
// feature disabled, keep original codeOwners
a.codeowners = codeOwners
}

// Set author
author := fmt.Sprintf("@%s", a.client.PR().User.GetLogin())
codeOwners.SetAuthor(author)
a.codeowners.SetAuthor(author)

// Warn about unowned files
for _, uFile := range codeOwners.UnownedFiles() {
for _, uFile := range a.codeowners.UnownedFiles() {
a.printWarn("WARNING: Unowned File: %s\n", uFile)
}

// Print file owners if verbose
if a.config.Verbose {
a.printFileOwners(codeOwners)
a.printFileOwners(a.codeowners)
}

// Process approvals and reviewers
Expand Down
2 changes: 2 additions & 0 deletions internal/app/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ func (m *mockCodeOwners) UnownedFiles() []string {
return m.unownedFiles
}

func (m *mockCodeOwners) AddInlineOwners(file string, owners codeowners.ReviewerGroups) {}

type mockGitHubClient struct {
pr *github.PullRequest
userReviewerMapError error
Expand Down
69 changes: 69 additions & 0 deletions internal/app/inline_adapter.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package app

import (
"slices"

"github.com/multimediallc/codeowners-plus/pkg/codeowners"
f "github.com/multimediallc/codeowners-plus/pkg/functional"
)

// overlayOwners implements codeowners.CodeOwners by overriding the required reviewer
// groups for specific files. All other behaviour is delegated to the base object.
// Only precedence for required owners is currently affected.

type overlayOwners struct {
base codeowners.CodeOwners
required map[string]codeowners.ReviewerGroups
}

func newOverlayOwners(base codeowners.CodeOwners, overrides map[string]codeowners.ReviewerGroups) codeowners.CodeOwners {
return &overlayOwners{base: base, required: overrides}
}

// SetAuthor propagates to base and ensures author removal from override groups.
func (o *overlayOwners) SetAuthor(author string) {
o.base.SetAuthor(author)
for _, groups := range o.required {
for _, g := range groups {
g.Names = f.RemoveValue(g.Names, author)
if len(g.Names) == 0 {
g.Approved = true
}
}
}
}

func (o *overlayOwners) FileRequired() map[string]codeowners.ReviewerGroups { return o.required }

func (o *overlayOwners) FileOptional() map[string]codeowners.ReviewerGroups {
return o.base.FileOptional()
}

func (o *overlayOwners) AllRequired() codeowners.ReviewerGroups {
agg := make(codeowners.ReviewerGroups, 0)
for _, grps := range o.required {
agg = append(agg, grps...)
}
return f.RemoveDuplicates(agg)
}

func (o *overlayOwners) AllOptional() codeowners.ReviewerGroups { return o.base.AllOptional() }

func (o *overlayOwners) UnownedFiles() []string { return o.base.UnownedFiles() }

func (o *overlayOwners) ApplyApprovals(approvers []string) {
o.base.ApplyApprovals(approvers)
for _, a := range approvers {
for _, groups := range o.required {
for _, g := range groups {
if slices.Contains(g.Names, a) {
g.Approved = true
}
}
}
}
}

func (o *overlayOwners) AddInlineOwners(file string, owners codeowners.ReviewerGroups) {
// Not used; overrides already applied.
}
63 changes: 63 additions & 0 deletions internal/app/inline_integration_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package app

import (
"os"
"path/filepath"
"testing"

ownersConfig "github.com/multimediallc/codeowners-plus/internal/config"
"github.com/multimediallc/codeowners-plus/pkg/codeowners"
"github.com/multimediallc/codeowners-plus/pkg/inlineowners"
)

func TestInlinePrecedence_EndToEnd(t *testing.T) {
tmp := t.TempDir()
// create .codeowners with file-level owner
if err := os.WriteFile(filepath.Join(tmp, ".codeowners"), []byte("a.go @file\n"), 0644); err != nil {
t.Fatalf("write codeowners: %v", err)
}
// create a.go with inline block lines 1-3
src := `// <CO-inline={@inline}>
line
// </CO-inline>
`
if err := os.WriteFile(filepath.Join(tmp, "a.go"), []byte(src), 0644); err != nil {
t.Fatalf("write a.go: %v", err)
}

diffFile := codeowners.DiffFile{
FileName: "a.go",
Hunks: []codeowners.HunkRange{{Start: 2, End: 2}}, // line inside block
}

// Build base codeowners
baseCO, err := codeowners.New(tmp, []codeowners.DiffFile{diffFile}, os.Stdout)
if err != nil {
t.Fatalf("codeowners.New err: %v", err)
}

// Build oracle
blocks, _ := inlineowners.Parse(src, os.Stdout)
oracle := inlineowners.Oracle{"a.go": {
{Owners: blocks[0].Owners, Start: blocks[0].StartLine, End: blocks[0].EndLine},
}}

// Build overrides like app logic
lists := oracle.OwnersForRange("a.go", 2, 2)
overrides := map[string]codeowners.ReviewerGroups{}
rgs := codeowners.ReviewerGroups{}
for _, lst := range lists {
rgs = append(rgs, &codeowners.ReviewerGroup{Names: lst})
}
overrides["a.go"] = rgs

ov := newOverlayOwners(baseCO, overrides)

conf := &ownersConfig.Config{InlineOwnershipEnabled: true}
_ = conf // unused but indicates enable flag

req := ov.AllRequired().Flatten()
if len(req) != 1 || req[0] != "@inline" {
t.Fatalf("expected only @inline, got %v", req)
}
}
138 changes: 138 additions & 0 deletions internal/app/inline_repo_integration_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
package app

import (
"io"
"os"
"path/filepath"
"reflect"
"slices"
"strings"
"testing"

"github.com/multimediallc/codeowners-plus/pkg/codeowners"
f "github.com/multimediallc/codeowners-plus/pkg/functional"
"github.com/multimediallc/codeowners-plus/pkg/inlineowners"
"github.com/sourcegraph/go-diff/diff"
)

// TestInlineRepoFixture_EndToEnd ensures that inline ownership blocks in the
// test_project_inline fixture correctly override file-level CODEOWNERS rules
// for the changed lines contained in the diff fixture.
func TestInlineRepoFixture_EndToEnd(t *testing.T) {
// Location of the fixture relative to this test file (internal/app)
repoDir := filepath.Join("..", "..", "test_project_inline")

// 1. Load and parse the diff fixture -------------------------------------------------
diffBytes, err := os.ReadFile(filepath.Join(repoDir, ".diff_changes"))
if err != nil {
t.Fatalf("read diff fixture: %v", err)
}

parsed, err := diff.ParseMultiFileDiff(diffBytes)
if err != nil {
t.Fatalf("parse diff: %v", err)
}

// Convert to []codeowners.DiffFile (simplified in-place variant of git.toDiffFiles)
diffFiles := make([]codeowners.DiffFile, 0, len(parsed))
for _, fd := range parsed {
// Tolerate malformed diffs from the test fixture
if fd.NewName == "/dev/null" || len(fd.NewName) < 3 {
continue
}
df := codeowners.DiffFile{
FileName: strings.TrimPrefix(fd.NewName[2:], "test_project_inline/"), // strip leading "b/" & repo dir
Hunks: make([]codeowners.HunkRange, 0, len(fd.Hunks)),
}
for _, h := range fd.Hunks {
hr := codeowners.HunkRange{
Start: int(h.NewStartLine),
End: int(h.NewStartLine + h.NewLines - 1),
}
df.Hunks = append(df.Hunks, hr)
}
diffFiles = append(diffFiles, df)
}

// 2. Build the base CODEOWNERS model -------------------------------------------------
baseCO, err := codeowners.New(repoDir, diffFiles, io.Discard)
if err != nil {
t.Fatalf("codeowners.New err: %v", err)
}

// 3. Build an inline ownership oracle for all changed files --------------------------
oracle := inlineowners.Oracle{}
for _, df := range diffFiles {
absPath := filepath.Join(repoDir, df.FileName)
data, err := os.ReadFile(absPath)
if err != nil {
t.Fatalf("read file %s: %v", df.FileName, err)
}
blocks, _ := inlineowners.Parse(string(data), io.Discard)
if len(blocks) == 0 {
continue
}
b2 := make([]inlineowners.Block, 0, len(blocks))
for _, b := range blocks {
b2 = append(b2, inlineowners.Block{Owners: b.Owners, Start: b.StartLine, End: b.EndLine})
}
oracle[df.FileName] = b2
}

// 4. Compute per-file override reviewer groups based on inline blocks ---------------
overrides := map[string]codeowners.ReviewerGroups{}
for _, df := range diffFiles {
rgs := codeowners.ReviewerGroups{}
for _, h := range df.Hunks {
lists := oracle.OwnersForRange(df.FileName, h.Start, h.End)
for _, lst := range lists {
rgs = append(rgs, &codeowners.ReviewerGroup{Names: lst})
}
}
if len(rgs) == 0 {
if base, ok := baseCO.FileRequired()[df.FileName]; ok {
rgs = append(rgs, base...)
}
} else {
rgs = f.RemoveDuplicates(rgs)
}
overrides[df.FileName] = rgs
}

// 5. Overlay inline overrides onto the base model ------------------------------------
owners := newOverlayOwners(baseCO, overrides)

// 6. Validate aggregate required owners ---------------------------------------------
expectedAll := []string{
"@base-test",
"@devops",
"@frontend-inline-misspellede",
"@model-owner",
"@qa-friend",
}
slices.Sort(expectedAll)
gotAll := owners.AllRequired().Flatten()
if !slices.Equal(gotAll, expectedAll) {
t.Fatalf("AllRequired mismatch. expected %v, got %v", expectedAll, gotAll)
}

// 7. Validate per-file owners --------------------------------------------------------
wantPerFile := map[string][]string{
"frontend/a.test.ts": {"@qa-friend"},
"frontend/b.ts": {"@frontend-inline-misspellede"},
"models.py": {"@model-owner", "@devops"},
"test_a.py": {"@base-test"},
}

for file, want := range wantPerFile {
gotRGs, ok := owners.FileRequired()[file]
if !ok {
t.Fatalf("expected file %s in owners map", file)
}
got := gotRGs.Flatten()
slices.Sort(want)
if !reflect.DeepEqual(got, want) {
t.Errorf("file %s: expected required %v, got %v", file, want, got)
}
}
}
Loading