From 02896b6e122d21c922096f6b361fad0dd9ffa7b2 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 10 Oct 2026 02:32:28 +0000 Subject: [PATCH] Apply module scope to file validation checks --- check.go | 2 +- docs/cli.md | 7 +- docs/config.md | 15 ++- docs/consumers/check.md | 20 +-- docs/consumers/publish.md | 13 +- docs/roadmap/implementation.md | 9 ++ internal/validation/file_size.go | 22 ++- internal/validation/file_size_test.go | 10 +- internal/validation/format_test.go | 1 + internal/validation/local_replacements.go | 2 +- .../validation/local_replacements_test.go | 32 ++++- internal/validation/markdown_check.go | 2 +- internal/validation/scope_checks_test.go | 125 ++++++++++++++++++ internal/validation/tree.go | 73 +++++----- internal/validation/validation.go | 7 +- internal/validation/workspace_test.go | 10 +- validation_scope_test.go | 37 ++++++ 17 files changed, 308 insertions(+), 79 deletions(-) create mode 100644 internal/validation/scope_checks_test.go create mode 100644 validation_scope_test.go diff --git a/check.go b/check.go index a0f3023..a46abf6 100644 --- a/check.go +++ b/check.go @@ -87,7 +87,7 @@ type FileSizeValidationSpec struct { // LimitBytes is the maximum decimal byte count. Nil selects 1,000,000; // zero is a zero-byte limit, and negative values are invalid. LimitBytes *int64 - // Exclusions are literal repository-relative file or directory paths. + // Exclusions are literal module-relative file or directory paths. // A directory exclusion includes committed descendants. Exclusions []string } diff --git a/docs/cli.md b/docs/cli.md index 4a0d28f..f005ad5 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -95,9 +95,10 @@ go.work.sum paths as `workspace-files`. Formatting failures are reported as `go-format` with affected paths and preserve parser or source-inspection causes. When `validation.module_distribution.local_replacements` is true, the command -parses committed `go.mod` declarations, including nested and vendor modules, -after workspace-file inspection. Filesystem replacements use the -`local-replacements` identity and include the declaration path, source line, +parses committed `go.mod` declarations in the selected module after +workspace-file inspection. Vendored contents remain in scope, while +independently released nested modules are excluded. Filesystem replacements use +the `local-replacements` identity and include the declaration path, source line, and target; versioned replacements are permitted. Committed module symlinks are resolved within the candidate, regardless of `GOWORK`. When `validation.file_size` is a non-null object, the command checks each diff --git a/docs/config.md b/docs/config.md index 51db83c..abb7ab4 100644 --- a/docs/config.md +++ b/docs/config.md @@ -69,12 +69,13 @@ Anchors keep Unicode letters and numbers, underscores, and hyphens; spaces become hyphens, and repeated headings receive numeric suffixes. When `validation.module_distribution.workspace_files` is true, it reports -committed blob entries whose basename is `go.work` or `go.work.sum`, including -nested paths and symlinks. It checks candidate distribution contents regardless +committed `go.work` and `go.work.sum` files in the selected module inputs, +including nested paths and symlinks. It checks the captured candidate regardless of `GOWORK`; it does not select or change the workspace used by Go commands, -and it does not alter existing vendor behavior. Omitted or false disables it. -When `validation.module_distribution.local_replacements` is true, it parses all -committed `go.mod` declarations, including vendor and nested modules, and reports +and vendored contents remain in scope. Omitted or false disables it. When +`validation.module_distribution.local_replacements` is true, it parses the +selected module's committed `go.mod` declarations. Vendored contents remain in +scope, while independently released nested modules are excluded. It reports `replace` directives whose replacement has no version. These filesystem replacements include relative and absolute paths; versioned module replacements are permitted. Committed `go.mod` symlinks are resolved within the candidate, @@ -109,8 +110,8 @@ check; `{}` enables it with a default of 1,000,000 bytes. A missing or null `limit_bytes` selects that default, zero is a valid limit, and negative limits are invalid. -`validation.file_size.exclusions` contains literal repository-relative file or -directory paths. Directory exclusions include committed descendants, using path +`validation.file_size.exclusions` contains literal paths relative to the +selected module. Directory exclusions include committed descendants, using path boundaries (`assets` does not exclude `assets-old`). Ordinary relative path normalization applies; metacharacters are literal and unmatched paths have no effect. The check measures regular and symlink blobs, including small binaries; diff --git a/docs/consumers/check.md b/docs/consumers/check.md index 0754fc9..5e5a6f7 100644 --- a/docs/consumers/check.md +++ b/docs/consumers/check.md @@ -43,22 +43,24 @@ ignores symlink blobs and worktree changes, and reports unformatted paths as after tests, selected vet, and package compilation, before `AdditionalChecks`. Set `Validation.ModuleDistribution.WorkspaceFiles` to reject committed -`go.work` and `go.work.sum` blobs, including nested and symlink entries. It -examines the captured candidate even when `GOWORK=off`, ignores uncommitted -files, and does not change Go test workspace selection or vendor behavior. The -zero value disables the check; failures use `workspace-files`. +`go.work` and `go.work.sum` files in the selected module inputs, including +nested and symlink entries. It examines the captured candidate even when +`GOWORK=off`, ignores uncommitted files, and does not change Go test workspace +selection; vendored contents remain in scope. The zero value disables the check; +failures use `workspace-files`. Set `Validation.ModuleDistribution.LocalReplacements` independently to parse -committed `go.mod` declarations throughout the candidate, including nested and -vendor modules. It reports filesystem `replace` directives with their source -path, line, and target as `local-replacements`; versioned replacements are +the selected module's committed `go.mod` declarations. Vendored contents remain +in scope, while independently released nested modules are excluded. It reports +filesystem `replace` directives with their source path, line, and target as +`local-replacements`; versioned replacements are permitted. Committed module symlinks are resolved within the candidate. Parse and inspection errors retain their causes, and the rule applies even when `GOWORK=off`. Its zero value disables the check. Set `Validation.FileSize` to check each committed blob against a byte limit. Nil disables the check; an empty object enables the default of 1,000,000 decimal bytes. `LimitBytes` is strict greater-than, accepts zero, and rejects -negative values. `Exclusions` contains literal repository-relative files or -directories; excluded directories include descendants. Symlink blobs are +negative values. `Exclusions` contains literal files or directories relative to +the selected module; excluded directories include descendants. Symlink blobs are measured by link-text bytes, and gitlinks are omitted. The check applies to the captured candidate and reports one oversized file with its measurement as `file-size`; it does not measure historical or aggregate size. diff --git a/docs/consumers/publish.md b/docs/consumers/publish.md index 477fb5f..25e7a4f 100644 --- a/docs/consumers/publish.md +++ b/docs/consumers/publish.md @@ -62,9 +62,10 @@ are ignored, and Go workspace selection and vendor behavior are unchanged. Its zero value disables the check. Findings use `workspace-files` and stop publication before product checks or tag creation. Set `Validation.ModuleDistribution.LocalReplacements` independently to parse -committed `go.mod` declarations across the candidate, including nested and -vendor modules. Filesystem `replace` directives are reported with their source -path, line, and target as `local-replacements`; versioned replacements are +selected module's committed `go.mod` declarations. Vendored contents remain in +scope, while independently released nested modules are excluded. Filesystem +`replace` directives are reported with their source path, line, and target as +`local-replacements`; versioned replacements are permitted. Committed module symlinks are resolved within the candidate, and parse or inspection errors retain their causes. This check is independent of `GOWORK`; its zero value disables it, and a failure stops publication before @@ -72,9 +73,9 @@ product checks or tag creation. Set `Validation.FileSize` to check each committed blob against a byte limit. Nil disables the check; an empty object enables the default of 1,000,000 decimal bytes. `LimitBytes` uses strict greater-than, accepts zero, and rejects -negative values. `Exclusions` contains literal repository-relative files or -directories, including committed descendants of excluded directories. Symlink -blob size is its link text; gitlinks are omitted. This check measures only the +negative values. `Exclusions` contains literal files or directories relative to +the selected module, including committed descendants of excluded directories. +Symlink blob size is its link text; gitlinks are omitted. This check measures only the captured candidate and reports one oversized file with its measurement as `file-size`; it stops publication before product checks or tag creation. diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 8250dac..becdb96 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -332,6 +332,15 @@ at different paths keep distinct relative-link findings. Confirm selection errors with content checks disabled and preserve cancellation/cleanup regressions. Run affected validation and facade tests. +**Completed:** Formatting, workspace, local-replacement, file-size, and Markdown +checks now iterate the selected committed inputs. Targeted reads still resolve +against the full repository tree. File-size exclusions are translated from the +selected module to repository-relative paths, and shared-file failures preserve +their public check, path, and cause. Updated file-size guidance and verified +in-scope and out-of-scope findings, shared inputs, cross-module anchors, reader +cleanup, facade conversion, `go test ./internal/validation .`, and +`go run ./internal/apicheck`. + ## Stage 6: Execute Go validation in the module with reusable local settings **Outcome:** One selected standard suite runs in the module, and later composite diff --git a/internal/validation/file_size.go b/internal/validation/file_size.go index 045ac57..dedd793 100644 --- a/internal/validation/file_size.go +++ b/internal/validation/file_size.go @@ -12,9 +12,8 @@ func checkFileSize(tree *CandidateTree, settings *FileSizeSettings) error { if settings.LimitBytes != nil { limit = *settings.LimitBytes } - exclusions := normalizedExclusions(settings.Exclusions) - for _, entry := range tree.Entries() { - if entry.Type != "blob" || !entry.HasSize || excludedByPath(entry.Path, exclusions) { + for _, entry := range tree.inputEntries { + if entry.Type != "blob" || !entry.HasSize || excludedByPath(entry.Path, settings.Exclusions) { continue } if entry.Size > limit { @@ -31,6 +30,23 @@ func checkFileSize(tree *CandidateTree, settings *FileSizeSettings) error { return nil } +func fileSizeSettingsForModule(settings *FileSizeSettings, modulePath string) *FileSizeSettings { + if settings == nil { + return nil + } + selected := *settings + moduleRelative := make([]string, len(settings.Exclusions)) + for index, exclusion := range settings.Exclusions { + if modulePath == "." || path.IsAbs(exclusion) || exclusion == "" { + moduleRelative[index] = exclusion + continue + } + moduleRelative[index] = modulePath + "/" + exclusion + } + selected.Exclusions = normalizedExclusions(moduleRelative) + return &selected +} + func normalizedExclusions(exclusions []string) []string { result := make([]string, 0, len(exclusions)) for _, exclusion := range exclusions { diff --git a/internal/validation/file_size_test.go b/internal/validation/file_size_test.go index 6141df9..6d7d815 100644 --- a/internal/validation/file_size_test.go +++ b/internal/validation/file_size_test.go @@ -29,9 +29,10 @@ func TestCheckFileSizeUsesStrictLimitsAndDefault(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - tree := &CandidateTree{entries: []git.TreeEntry{{ + entries := []git.TreeEntry{{ Path: "file.bin", Type: "blob", Mode: "100644", Size: test.size, HasSize: true, - }}} + }} + tree := &CandidateTree{entries: entries, inputEntries: entries} err := checkFileSize(tree, &FileSizeSettings{LimitBytes: test.limit}) if !test.wantError { if err != nil { @@ -104,10 +105,11 @@ func TestCheckFileSizeAppliesLiteralPathExclusions(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - err := checkFileSize(&CandidateTree{entries: test.entries}, &FileSizeSettings{ + settings := fileSizeSettingsForModule(&FileSizeSettings{ LimitBytes: int64Pointer(10), Exclusions: test.exclusions, - }) + }, ".") + err := checkFileSize(&CandidateTree{entries: test.entries, inputEntries: test.entries}, settings) failure, ok := err.(*Failure) if !ok || failure.Check != FileSize || len(failure.Findings) != 1 || failure.Findings[0].Path != test.wantPath { t.Fatalf("checkFileSize() error = %v, want only %q to exceed the limit", err, test.wantPath) diff --git a/internal/validation/format_test.go b/internal/validation/format_test.go index 08d203e..fe0190f 100644 --- a/internal/validation/format_test.go +++ b/internal/validation/format_test.go @@ -89,6 +89,7 @@ func TestCheckGoFormatPreservesParserAndInspectionCauses(t *testing.T) { tree := &CandidateTree{ repositoryDir: t.TempDir(), entries: []git.TreeEntry{{Path: "missing.go", ObjectID: strings.Repeat("0", 40), Mode: "100644", Type: "blob"}}, + inputEntries: []git.TreeEntry{{Path: "missing.go", ObjectID: strings.Repeat("0", 40), Mode: "100644", Type: "blob"}}, byPath: map[string]int{"missing.go": 0}, } cleanupCandidateTree(t, tree) diff --git a/internal/validation/local_replacements.go b/internal/validation/local_replacements.go index 38842fc..9207146 100644 --- a/internal/validation/local_replacements.go +++ b/internal/validation/local_replacements.go @@ -13,7 +13,7 @@ import ( func checkLocalReplacements(ctx context.Context, tree *CandidateTree) error { var findings []Finding replacementsByObjectID := make(map[string][]moduleReplacementSummary) - for _, entry := range tree.Entries() { + for _, entry := range tree.inputEntries { if err := ctx.Err(); err != nil { return &Failure{Check: LocalReplacements, Findings: findings, Cause: err} } diff --git a/internal/validation/local_replacements_test.go b/internal/validation/local_replacements_test.go index 986617e..3948e39 100644 --- a/internal/validation/local_replacements_test.go +++ b/internal/validation/local_replacements_test.go @@ -29,9 +29,7 @@ func TestCheckLocalReplacementsParsesCommittedModuleDeclarations(t *testing.T) { t.Fatalf("checkLocalReplacements error = %v, want local-replacements failure", err) } want := map[string]Finding{ - "go.mod": {Path: "go.mod", Line: 3, Target: "../local module", Directive: "replace"}, - "nested/go.mod:4": {Path: "nested/go.mod", Line: 4, Target: "../../local", Directive: "replace"}, - "nested/go.mod:5": {Path: "nested/go.mod", Line: 5, Target: absoluteTarget, Directive: "replace"}, + "go.mod": {Path: "go.mod", Line: 3, Target: "../local module", Directive: "replace"}, } if len(failure.Findings) != len(want) { t.Fatalf("findings = %#v, want %d filesystem replacements", failure.Findings, len(want)) @@ -46,6 +44,27 @@ func TestCheckLocalReplacementsParsesCommittedModuleDeclarations(t *testing.T) { t.Errorf("finding = %#v, want %q", finding, key) } } + + nestedTree, err := ReadCandidateTreeForScope(context.Background(), repository, commit, TreeScope{ModuleDir: filepath.Join(repository, "nested")}) + if err != nil { + t.Fatal(err) + } + cleanupCandidateTree(t, nestedTree) + err = checkLocalReplacements(context.Background(), nestedTree) + if !errors.As(err, &failure) || failure.Check != LocalReplacements || len(failure.Findings) != 2 { + t.Fatalf("nested replacement findings = %#v, want the selected module's two replacements", err) + } + for _, finding := range failure.Findings { + wantLine := 4 + wantTarget := "../../local" + if finding.Line == 5 { + wantLine = 5 + wantTarget = absoluteTarget + } + if finding.Path != "nested/go.mod" || finding.Line != wantLine || finding.Target != wantTarget || finding.Directive != "replace" { + t.Errorf("nested finding = %#v, want line %d target %q", finding, wantLine, wantTarget) + } + } } func TestCheckLocalReplacementsFollowsCommittedModuleSymlinks(t *testing.T) { @@ -58,7 +77,7 @@ func TestCheckLocalReplacementsFollowsCommittedModuleSymlinks(t *testing.T) { t.Fatal(err) } commit := commitCandidateTree(t, repository) - tree, err := ReadCandidateTree(context.Background(), repository, commit) + tree, err := ReadCandidateTreeForScope(context.Background(), repository, commit, TreeScope{ModuleDir: filepath.Join(repository, "nested")}) if err != nil { t.Fatal(err) } @@ -91,7 +110,10 @@ func TestCheckLocalReplacementsSharesSummariesAcrossModuleSymlinks(t *testing.T) } } commit := commitCandidateTree(t, repository) - tree, err := ReadCandidateTree(context.Background(), repository, commit) + tree, err := ReadCandidateTreeForScope(context.Background(), repository, commit, TreeScope{ + ModuleDir: repository, + SharedFiles: []string{"first/go.mod", "second/go.mod"}, + }) if err != nil { t.Fatal(err) } diff --git a/internal/validation/markdown_check.go b/internal/validation/markdown_check.go index 6f5d0d8..959caa1 100644 --- a/internal/validation/markdown_check.go +++ b/internal/validation/markdown_check.go @@ -17,7 +17,7 @@ func checkMarkdownLinks(ctx context.Context, tree *CandidateTree, enforceCheckou } cache := newMarkdownCache(tree) var findings []Finding - for _, entry := range tree.Entries() { + for _, entry := range tree.inputEntries { if err := ctx.Err(); err != nil { return &Failure{Check: MarkdownLinks, Findings: findings, Cause: err} } diff --git a/internal/validation/scope_checks_test.go b/internal/validation/scope_checks_test.go new file mode 100644 index 0000000..6576b93 --- /dev/null +++ b/internal/validation/scope_checks_test.go @@ -0,0 +1,125 @@ +package validation + +import ( + "context" + "errors" + "path/filepath" + "strings" + "testing" +) + +func TestRunFileChecksUseSelectedInputsAndFullReferenceLookup(t *testing.T) { + repository := newTreeRepository(t) + writeTreeFile(t, repository, "go.mod", []byte("module example.com/root\n\ngo 1.25.0\nreplace example.com/rootdep => ./missing\n")) + writeTreeFile(t, repository, "go.work", []byte("go 1.25.0\n")) + writeTreeFile(t, repository, "outside.go", []byte("package outside\nfunc broken( ){}\n")) + writeTreeFile(t, repository, "outside.bin", []byte(strings.Repeat("x", 160))) + writeTreeFile(t, repository, "nested/go.mod", []byte("module example.com/nested\n\ngo 1.25.0\n")) + writeTreeFile(t, repository, "nested/source.go", []byte("package nested\n\nfunc ready() {}\n")) + writeTreeFile(t, repository, "nested/ignored.bin", []byte(strings.Repeat("x", 160))) + writeTreeFile(t, repository, "nested/docs/guide.md", []byte("# Guide\n\n[details](../../sibling/docs/target.md#details)\n")) + writeTreeFile(t, repository, "nested/child/go.mod", []byte("module example.com/nested/child\n")) + writeTreeFile(t, repository, "nested/child/broken.go", []byte("package child\nfunc broken( ){}\n")) + writeTreeFile(t, repository, "nested/child/go.work", []byte("go 1.25.0\n")) + writeTreeFile(t, repository, "sibling/go.mod", []byte("module example.com/sibling\n\nreplace example.com/siblingdep => ./missing\n")) + writeTreeFile(t, repository, "sibling/go.work", []byte("go 1.25.0\n")) + writeTreeFile(t, repository, "sibling/broken.go", []byte("package sibling\nfunc broken( ){}\n")) + writeTreeFile(t, repository, "sibling/outside.bin", []byte(strings.Repeat("x", 160))) + writeTreeFile(t, repository, "sibling/docs/target.md", []byte("# Details\n\n[broken](missing.md)\n")) + writeTreeFile(t, repository, "shared/broken.go", []byte("package shared\nfunc broken( ){}\n")) + commit := commitCandidateTree(t, repository) + installGoShim(t, "", false) + moduleDir := filepath.Join(repository, "nested") + settings := Settings{ + GoFormat: true, + ModuleDistribution: ModuleDistributionSettings{ + WorkspaceFiles: true, + LocalReplacements: true, + }, + FileSize: &FileSizeSettings{LimitBytes: int64Pointer(100), Exclusions: []string{"ignored.bin"}}, + Markdown: &MarkdownSettings{EnforceCheckoutBoundary: true}, + } + request := Request{RepositoryDir: repository, ModuleDir: moduleDir, Commit: commit, Settings: settings} + if err := Run(context.Background(), request); err != nil { + t.Fatalf("selected module checks = %v, want out-of-scope violations ignored and cross-module anchor resolved", err) + } + + request.Settings.FileSize.Exclusions = nil + err := Run(context.Background(), request) + assertScopedFailure(t, err, FileSize, "nested/ignored.bin") + + writeTreeFile(t, repository, "nested/broken.go", []byte("package nested\nfunc broken( ){}\n")) + request.Commit = commitCandidateTree(t, repository) + request.Settings = Settings{GoFormat: true} + assertScopedFailure(t, Run(context.Background(), request), GoFormat, "nested/broken.go") + + writeTreeFile(t, repository, "nested/go.work", []byte("go 1.25.0\n")) + request.Commit = commitCandidateTree(t, repository) + request.Settings = Settings{ModuleDistribution: ModuleDistributionSettings{WorkspaceFiles: true}} + assertScopedFailure(t, Run(context.Background(), request), WorkspaceFiles, "nested/go.work") + + writeTreeFile(t, repository, "nested/go.mod", []byte("module example.com/nested\n\ngo 1.25.0\nreplace example.com/nesteddep => ./missing\n")) + request.Commit = commitCandidateTree(t, repository) + request.Settings = Settings{ModuleDistribution: ModuleDistributionSettings{LocalReplacements: true}} + assertScopedFailure(t, Run(context.Background(), request), LocalReplacements, "nested/go.mod") + + writeTreeFile(t, repository, "nested/docs/guide.md", []byte("# Guide\n\n[details](../../sibling/docs/target.md#details)\n\n[missing](absent.md)\n")) + request.Commit = commitCandidateTree(t, repository) + request.Settings = Settings{Markdown: &MarkdownSettings{}} + assertScopedFailure(t, Run(context.Background(), request), MarkdownLinks, "nested/docs/guide.md") + + request.Settings = Settings{GoFormat: true, SharedFiles: []string{"shared/broken.go"}} + assertScopedFailure(t, Run(context.Background(), request), GoFormat, "shared/broken.go") +} + +func assertScopedFailure(t *testing.T, err error, check Check, path string) { + t.Helper() + failure, ok := err.(*Failure) + if !ok || failure.Check != check { + t.Fatalf("validation error = %v, want %s failure", err, check) + } + for _, finding := range failure.Findings { + if finding.Path == path { + return + } + } + t.Fatalf("%s findings = %#v, want path %q", check, failure.Findings, path) +} + +func TestFileSizeExclusionsAreModuleRelative(t *testing.T) { + repository := newTreeRepository(t) + writeTreeFile(t, repository, "nested/go.mod", []byte("module example.com/nested\n")) + writeTreeFile(t, repository, "nested/dist/output.bin", []byte(strings.Repeat("x", 20))) + writeTreeFile(t, repository, "nested/dist-old/output.bin", []byte(strings.Repeat("x", 20))) + commit := commitCandidateTree(t, repository) + tree, err := ReadCandidateTreeForScope(context.Background(), repository, commit, TreeScope{ModuleDir: filepath.Join(repository, "nested")}) + if err != nil { + t.Fatal(err) + } + cleanupCandidateTree(t, tree) + settings := fileSizeSettingsForModule(&FileSizeSettings{LimitBytes: int64Pointer(10), Exclusions: []string{"./dist/"}}, tree.modulePath) + err = checkFileSize(tree, settings) + var failure *Failure + if !errors.As(err, &failure) || failure.Check != FileSize || len(failure.Findings) != 1 || failure.Findings[0].Path != "nested/dist-old/output.bin" { + t.Fatalf("module-relative file-size exclusion error = %v, want only nested/dist-old/output.bin", err) + } +} + +func TestSharedFileSelectionFailureCarriesCauseAndPresentation(t *testing.T) { + repository := newTreeRepository(t) + writeTreeFile(t, repository, "go.mod", []byte("module example.com/root\n")) + commit := commitCandidateTree(t, repository) + installGoShim(t, "", false) + err := Run(context.Background(), Request{ + RepositoryDir: repository, + Commit: commit, + Settings: Settings{SharedFiles: []string{"missing.txt"}}, + }) + var failure *Failure + if !errors.As(err, &failure) || failure.Check != SharedFiles || failure.Cause == nil || len(failure.Findings) != 1 || failure.Findings[0].Path != "missing.txt" { + t.Fatalf("shared-file selection error = %#v, want path, cause, and shared-files identity", err) + } + if !strings.Contains(err.Error(), "shared-files") || !strings.Contains(err.Error(), "missing.txt") { + t.Fatalf("shared-file error text = %q, want existing check/path presentation", err) + } +} diff --git a/internal/validation/tree.go b/internal/validation/tree.go index b871691..4bdcef4 100644 --- a/internal/validation/tree.go +++ b/internal/validation/tree.go @@ -16,6 +16,7 @@ import ( type CandidateTree struct { context context.Context repositoryDir string + modulePath string entries []git.TreeEntry inputEntries []git.TreeEntry byPath map[string]int @@ -53,9 +54,14 @@ func ReadCandidateTreeForScope(ctx context.Context, repositoryDir, commit string if err != nil { return nil, err } + modulePath, err := repositoryModulePath(repositoryDir, scope.ModuleDir) + if err != nil { + return nil, err + } tree := &CandidateTree{ context: ctx, repositoryDir: repositoryDir, + modulePath: modulePath, entries: entries, byPath: make(map[string]int, len(entries)), directories: make(map[string]struct{}), @@ -68,7 +74,7 @@ func ReadCandidateTreeForScope(ctx context.Context, repositoryDir, commit string tree.directories[directory] = struct{}{} } } - tree.inputEntries, err = selectInputEntries(repositoryDir, entries, tree.byPath, scope) + tree.inputEntries, err = selectInputEntries(entries, tree.byPath, modulePath, scope.SharedFiles) if err != nil { _ = tree.Close() return nil, err @@ -88,34 +94,7 @@ func (tree *CandidateTree) InputEntries() []git.TreeEntry { return append([]git.TreeEntry(nil), tree.inputEntries...) } -func selectInputEntries(repositoryDir string, entries []git.TreeEntry, byPath map[string]int, scope TreeScope) ([]git.TreeEntry, error) { - repositoryAbs, err := filepath.Abs(repositoryDir) - if err != nil { - return nil, fmt.Errorf("resolve candidate repository directory: %w", err) - } - moduleDir := scope.ModuleDir - if moduleDir == "" { - moduleDir = repositoryAbs - } - if !filepath.IsAbs(moduleDir) { - return nil, fmt.Errorf("selected module directory must be absolute: %q", moduleDir) - } - moduleAbs, err := filepath.Abs(moduleDir) - if err != nil { - return nil, fmt.Errorf("resolve selected module directory: %w", err) - } - relative, err := filepath.Rel(repositoryAbs, moduleAbs) - if err != nil { - return nil, fmt.Errorf("make selected module relative to repository: %w", err) - } - if relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) || filepath.IsAbs(relative) { - return nil, fmt.Errorf("selected module directory %q is outside repository %q", moduleDir, repositoryDir) - } - modulePath := filepath.ToSlash(relative) - if relative == "." { - modulePath = "." - } - +func selectInputEntries(entries []git.TreeEntry, byPath map[string]int, modulePath string, sharedFiles []string) ([]git.TreeEntry, error) { boundaries := make([]string, 0) for _, entry := range entries { moduleRelative, ok := relativeToModule(entry.Path, modulePath) @@ -145,7 +124,6 @@ func selectInputEntries(repositoryDir string, entries []git.TreeEntry, byPath ma } } - sharedFiles := scope.SharedFiles usingDefaults := sharedFiles == nil if usingDefaults { sharedFiles = []string{"COPYRIGHT", "LICENSE", "docs/oss/attribution.md"} @@ -153,14 +131,16 @@ func selectInputEntries(repositoryDir string, entries []git.TreeEntry, byPath ma for _, requested := range sharedFiles { name, err := normalizeSharedFile(requested) if err != nil { - return nil, &Failure{Check: SharedFiles, Findings: []Finding{{Path: requested, Message: err.Error()}}} + cause := fmt.Errorf("invalid shared file path %q: %w", requested, err) + return nil, &Failure{Check: SharedFiles, Findings: []Finding{{Path: requested, Message: cause.Error()}}, Cause: cause} } index, exists := byPath[name] if !exists || entries[index].Type != "blob" { if usingDefaults { continue } - return nil, &Failure{Check: SharedFiles, Findings: []Finding{{Path: name, Message: "selected shared file is not present in the committed tree"}}} + cause := fmt.Errorf("selected shared file %q is not present in the committed tree", name) + return nil, &Failure{Check: SharedFiles, Findings: []Finding{{Path: name, Message: "selected shared file is not present in the committed tree"}}, Cause: cause} } selected[name] = entries[index] } @@ -177,6 +157,35 @@ func selectInputEntries(repositoryDir string, entries []git.TreeEntry, byPath ma return inputs, nil } +func repositoryModulePath(repositoryDir, moduleDir string) (string, error) { + repositoryAbs, err := filepath.Abs(repositoryDir) + if err != nil { + return "", fmt.Errorf("resolve candidate repository directory: %w", err) + } + if moduleDir == "" { + moduleDir = repositoryAbs + } + if !filepath.IsAbs(moduleDir) { + return "", fmt.Errorf("selected module directory must be absolute: %q", moduleDir) + } + moduleAbs, err := filepath.Abs(moduleDir) + if err != nil { + return "", fmt.Errorf("resolve selected module directory: %w", err) + } + relative, err := filepath.Rel(repositoryAbs, moduleAbs) + if err != nil { + return "", fmt.Errorf("make selected module relative to repository: %w", err) + } + if relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) || filepath.IsAbs(relative) { + return "", fmt.Errorf("selected module directory %q is outside repository %q", moduleDir, repositoryDir) + } + modulePath := filepath.ToSlash(relative) + if relative == "." { + modulePath = "." + } + return modulePath, nil +} + func relativeToModule(repositoryPath, modulePath string) (string, bool) { if modulePath == "." { return repositoryPath, true diff --git a/internal/validation/validation.go b/internal/validation/validation.go index 63891f4..caaaeaf 100644 --- a/internal/validation/validation.go +++ b/internal/validation/validation.go @@ -148,6 +148,7 @@ func Run(ctx context.Context, request Request) (result error) { return &Failure{Check: check, Cause: err} } lastContentCheck := Check("") + fileSizeSettings := fileSizeSettingsForModule(request.Settings.FileSize, tree.modulePath) defer func() { if closeErr := tree.Close(); closeErr != nil && result == nil && lastContentCheck != "" { result = &Failure{ @@ -180,7 +181,7 @@ func Run(ctx context.Context, request Request) (result error) { } } if request.Settings.FileSize != nil { - if err := checkFileSize(tree, request.Settings.FileSize); err != nil { + if err := checkFileSize(tree, fileSizeSettings); err != nil { return err } } @@ -199,7 +200,7 @@ func Run(ctx context.Context, request Request) (result error) { func checkWorkspaceFiles(tree *CandidateTree) error { var findings []Finding - for _, entry := range tree.Entries() { + for _, entry := range tree.inputEntries { if entry.Type != "blob" { continue } @@ -220,7 +221,7 @@ func checkWorkspaceFiles(tree *CandidateTree) error { func checkGoFormat(ctx context.Context, tree *CandidateTree) error { var findings []Finding formattedByObjectID := make(map[string]bool) - for _, entry := range tree.Entries() { + for _, entry := range tree.inputEntries { if entry.Type != "blob" || (entry.Mode != "100644" && entry.Mode != "100755") || !strings.HasSuffix(entry.Path, ".go") { continue } diff --git a/internal/validation/workspace_test.go b/internal/validation/workspace_test.go index aa54555..4d0f917 100644 --- a/internal/validation/workspace_test.go +++ b/internal/validation/workspace_test.go @@ -9,13 +9,14 @@ import ( ) func TestCheckWorkspaceFilesReportsCommittedBlobNames(t *testing.T) { - tree := &CandidateTree{entries: []git.TreeEntry{ + entries := []git.TreeEntry{ {Path: "go.work", Type: "blob", Mode: "100644"}, {Path: "nested/go.work.sum", Type: "blob", Mode: "100644"}, {Path: "vendor/nested/link/go.work", Type: "blob", Mode: "120000"}, {Path: "nested/not-go.work.backup", Type: "blob", Mode: "100644"}, {Path: "submodule/go.work", Type: "commit", Mode: "160000"}, - }} + } + tree := &CandidateTree{entries: entries, inputEntries: entries} err := checkWorkspaceFiles(tree) failure, ok := err.(*Failure) if !ok || failure.Check != WorkspaceFiles { @@ -43,10 +44,11 @@ func TestCheckWorkspaceFilesReportsCommittedBlobNames(t *testing.T) { } func TestCheckWorkspaceFilesAllowsOtherNames(t *testing.T) { - tree := &CandidateTree{entries: []git.TreeEntry{ + entries := []git.TreeEntry{ {Path: "go.mod", Type: "blob", Mode: "100644"}, {Path: "examples/go.work.example", Type: "blob", Mode: "100644"}, - }} + } + tree := &CandidateTree{entries: entries, inputEntries: entries} if err := checkWorkspaceFiles(tree); err != nil { t.Fatalf("checkWorkspaceFiles error = %v, want no findings", err) } diff --git a/validation_scope_test.go b/validation_scope_test.go new file mode 100644 index 0000000..6d904b7 --- /dev/null +++ b/validation_scope_test.go @@ -0,0 +1,37 @@ +package releasekit + +import ( + "errors" + "strings" + "testing" + + "gitea.maximumdirect.net/eric/ReleaseKit/internal/validation" +) + +func TestSharedFileFailureKeepsPublicIdentityPathAndCause(t *testing.T) { + cause := errors.New("selected file is missing") + err := convertValidationError(&validation.Failure{ + Check: validation.SharedFiles, + Findings: []validation.Finding{{ + Path: "shared/NOTICE.md", + Message: "selected shared file is not present in the committed tree", + }}, + Cause: cause, + }) + var failure *ValidationError + if !errors.As(err, &failure) { + t.Fatalf("converted error = %T, want *ValidationError", err) + } + if failure.Check != ValidationCheck("shared-files") { + t.Fatalf("validation check = %q, want shared-files", failure.Check) + } + if len(failure.Findings) != 1 || failure.Findings[0].Path != "shared/NOTICE.md" { + t.Fatalf("validation findings = %#v, want selected shared path", failure.Findings) + } + if !errors.Is(err, cause) { + t.Fatalf("converted error %v does not preserve its cause", err) + } + if !strings.Contains(err.Error(), "shared-files") || !strings.Contains(err.Error(), "shared/NOTICE.md") { + t.Fatalf("validation presentation = %q, want check and path", err) + } +}