Skip to content

Commit 8facf74

Browse files
committed
Fix argument handling regression.
1 parent ba4e99e commit 8facf74

7 files changed

Lines changed: 207 additions & 58 deletions

File tree

cmd/common.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -214,16 +214,16 @@ func loadCommitsFromArgs(args []string, opts summaryOpts, breakingConfig *whatCh
214214
if _, _, ok := parseGitRef(args[0]); ok {
215215
return loadLeftRightCommits(args[0], args[1], opts)
216216
}
217+
f, statErr := os.Stat(args[0])
218+
if statErr == nil && f.IsDir() {
219+
return loadGitHistoryCommits(args[0], args[1], opts, breakingConfig)
220+
}
217221
if _, _, ok := parseGitRef(args[1]); ok {
218222
return loadLeftRightCommits(args[0], args[1], opts)
219223
}
220-
f, statErr := os.Stat(args[0])
221224
if statErr != nil {
222225
return nil, fmt.Errorf("cannot open file/repository: '%s'", args[0])
223226
}
224-
if f.IsDir() {
225-
return loadGitHistoryCommits(args[0], args[1], opts, breakingConfig)
226-
}
227227
return loadLeftRightCommits(args[0], args[1], opts)
228228
}
229229

cmd/left_right_sources.go

Lines changed: 82 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,15 @@ import (
2525
)
2626

2727
type comparisonSource struct {
28-
Display string
29-
RootBytes []byte
30-
DocConfig *datamodel.DocumentConfiguration
31-
Cleanup func()
28+
Display string
29+
RootBytes []byte
30+
DocConfig *datamodel.DocumentConfiguration
31+
RewriteDocumentPath documentPathRewriter
32+
Cleanup func()
3233
}
3334

35+
type documentPathRewriter func(string) string
36+
3437
func newComparisonDocConfig(opts summaryOpts) *datamodel.DocumentConfiguration {
3538
docConfig := datamodel.NewDocumentConfiguration()
3639
docConfig.IgnoreArrayCircularReferences = true
@@ -108,10 +111,11 @@ func resolveLocalFileSource(raw string, opts summaryOpts) (comparisonSource, err
108111
}
109112

110113
return comparisonSource{
111-
Display: raw,
112-
RootBytes: bits,
113-
DocConfig: docConfig,
114-
Cleanup: func() {},
114+
Display: raw,
115+
RootBytes: bits,
116+
DocConfig: docConfig,
117+
RewriteDocumentPath: nil,
118+
Cleanup: func() {},
115119
}, nil
116120
}
117121

@@ -175,13 +179,46 @@ func resolveGitRefSource(raw, revision, filePath string, opts summaryOpts) (comp
175179
docConfig.LocalFS = revisionFS
176180

177181
return comparisonSource{
178-
Display: raw,
179-
RootBytes: rootBytes,
180-
DocConfig: docConfig,
181-
Cleanup: func() {},
182+
Display: raw,
183+
RootBytes: rootBytes,
184+
DocConfig: docConfig,
185+
RewriteDocumentPath: newGitRefDocumentPathRewriter(repoRoot, basePath, virtualBasePath),
186+
Cleanup: func() {},
182187
}, nil
183188
}
184189

190+
func newGitRefDocumentPathRewriter(repoRoot, basePath, virtualBasePath string) documentPathRewriter {
191+
baseRelPath, err := filepath.Rel(repoRoot, basePath)
192+
if err != nil {
193+
return nil
194+
}
195+
if baseRelPath == ".." || strings.HasPrefix(baseRelPath, ".."+string(filepath.Separator)) {
196+
return nil
197+
}
198+
cleanVirtualBasePath := filepath.Clean(virtualBasePath)
199+
if baseRelPath == "." {
200+
baseRelPath = ""
201+
}
202+
203+
return func(raw string) string {
204+
cleaned := filepath.Clean(filepath.FromSlash(raw))
205+
relPath, err := filepath.Rel(cleanVirtualBasePath, cleaned)
206+
if err != nil {
207+
return raw
208+
}
209+
if relPath == "." || relPath == "" {
210+
return raw
211+
}
212+
if relPath == ".." || strings.HasPrefix(relPath, ".."+string(filepath.Separator)) {
213+
return raw
214+
}
215+
if baseRelPath != "" {
216+
relPath = filepath.Join(baseRelPath, relPath)
217+
}
218+
return filepath.ToSlash(filepath.Clean(relPath))
219+
}
220+
}
221+
185222
func resolveURLSource(raw string, opts summaryOpts) (comparisonSource, error) {
186223
display := sanitizeURLLabel(raw)
187224

@@ -232,10 +269,11 @@ func resolveURLSource(raw string, opts summaryOpts) (comparisonSource, error) {
232269
}
233270

234271
return comparisonSource{
235-
Display: display,
236-
RootBytes: bits,
237-
DocConfig: docConfig,
238-
Cleanup: func() {},
272+
Display: display,
273+
RootBytes: bits,
274+
DocConfig: docConfig,
275+
RewriteDocumentPath: nil,
276+
Cleanup: func() {},
239277
}, nil
240278
}
241279

@@ -303,3 +341,31 @@ func buildLeftRightCommit(left, right comparisonSource) (*model.Commit, error) {
303341
Synthetic: true,
304342
}, nil
305343
}
344+
345+
func buildLeftRightCommitAndSources(left, right string, opts summaryOpts) (*model.Commit, []documentPathRewriter, error) {
346+
leftSource, err := resolveComparisonSource(left, opts)
347+
if err != nil {
348+
return nil, nil, err
349+
}
350+
defer leftSource.Cleanup()
351+
352+
rightSource, err := resolveComparisonSource(right, opts)
353+
if err != nil {
354+
return nil, nil, err
355+
}
356+
defer rightSource.Cleanup()
357+
358+
commit, err := buildLeftRightCommit(leftSource, rightSource)
359+
if err != nil {
360+
return nil, nil, err
361+
}
362+
363+
var rewriters []documentPathRewriter
364+
if leftSource.RewriteDocumentPath != nil {
365+
rewriters = append(rewriters, leftSource.RewriteDocumentPath)
366+
}
367+
if rightSource.RewriteDocumentPath != nil {
368+
rewriters = append(rewriters, rightSource.RewriteDocumentPath)
369+
}
370+
return commit, rewriters, nil
371+
}

cmd/loaders.go

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -200,19 +200,7 @@ func loadGitHistoryCommits(gitPath, filePath string, opts summaryOpts, breakingC
200200
}
201201

202202
func loadLeftRightCommits(left, right string, opts summaryOpts) ([]*model.Commit, error) {
203-
leftSource, err := resolveComparisonSource(left, opts)
204-
if err != nil {
205-
return nil, err
206-
}
207-
defer leftSource.Cleanup()
208-
209-
rightSource, err := resolveComparisonSource(right, opts)
210-
if err != nil {
211-
return nil, err
212-
}
213-
defer rightSource.Cleanup()
214-
215-
commit, err := buildLeftRightCommit(leftSource, rightSource)
203+
commit, _, err := buildLeftRightCommitAndSources(left, right, opts)
216204
if err != nil {
217205
return nil, err
218206
}

cmd/loaders_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,3 +320,14 @@ func TestLoadCommitsFromArgs_LocalColonPathOutsideGitRepoStaysFileComparison(t *
320320
require.Len(t, commits, 1)
321321
assert.Equal(t, "Original: "+leftPath+", Modified: "+rightPath, commits[0].Message)
322322
}
323+
324+
func TestLoadCommitsFromArgs_RepoHistoryColonPathUsesHistoryDispatch(t *testing.T) {
325+
repoDir := createGitSpecRepoForFile(t, "v1:beta.yaml")
326+
chdirForTest(t, t.TempDir())
327+
328+
commits, err := loadCommitsFromArgs([]string{repoDir, "v1:beta.yaml"}, summaryOpts{limitTime: -1}, nil)
329+
require.NoError(t, err)
330+
require.NotEmpty(t, commits)
331+
assert.Equal(t, repoDir, commits[0].RepoDirectory)
332+
assert.Equal(t, "v1:beta.yaml", commits[0].FilePath)
333+
}

cmd/report.go

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -53,22 +53,20 @@ func changerateAndFlatten(commits []*model.Commit, breakingConfig *whatChangedMo
5353
}
5454

5555
func runLeftRightReport(left, right string, opts summaryOpts, breakingConfig *whatChangedModel.BreakingRulesConfig) (*model.FlatReport, error) {
56-
commits, err := loadLeftRightCommits(left, right, opts)
56+
commit, documentPathRewriters, err := buildLeftRightCommitAndSources(left, right, opts)
5757
if err != nil {
5858
return nil, err
5959
}
60-
if len(commits) == 0 {
61-
return nil, nil
62-
}
63-
result, err := changerateCommit(commits[0], breakingConfig)
60+
result, err := changerateCommit(commit, breakingConfig)
6461
if err != nil {
6562
return nil, err
6663
}
6764
if result == nil {
6865
return nil, nil
6966
}
7067
defer result.Release()
71-
flat := FlattenReportWithParameterNames(createReport(commits[0]), result.Changerator.ParameterNames)
68+
flat := FlattenReportWithParameterNames(createReport(commit), result.Changerator.ParameterNames)
69+
rewriteFlatReportDocumentPaths(flat, documentPathRewriters)
7270
flat.Commit = nil
7371
flat.OriginalPath = sourceLabelForReport(left)
7472
flat.ModifiedPath = sourceLabelForReport(right)
@@ -188,9 +186,6 @@ func GetReportCommand() *cobra.Command {
188186
if _, _, ok := parseGitRef(args[0]); ok {
189187
return printReportJSONOrNoChanges(args, opts, breakingConfig, reproducible)
190188
}
191-
if _, _, ok := parseGitRef(args[1]); ok {
192-
return printReportJSONOrNoChanges(args, opts, breakingConfig, reproducible)
193-
}
194189
f, statErr := os.Stat(args[0])
195190
if statErr == nil && f.IsDir() {
196191
flat, reportErr := runGitHistoryReport(args[0], args[1], opts, breakingConfig)
@@ -206,13 +201,16 @@ func GetReportCommand() *cobra.Command {
206201
}
207202
return printReportJSON(flat)
208203
}
204+
if _, _, ok := parseGitRef(args[1]); ok {
205+
return printReportJSONOrNoChanges(args, opts, breakingConfig, reproducible)
206+
}
209207
}
210208

211209
return printReportJSONOrNoChanges(args, opts, breakingConfig, reproducible)
212210
},
213211
}
214212
addTerminalThemeFlags(cmd)
215-
cmd.Flags().Bool("reproducible", false, "Omit generated timestamps so report JSON is stable across runs")
213+
cmd.Flags().Bool("reproducible", false, "Omit generated timestamps from report JSON")
216214
return cmd
217215
}
218216

@@ -253,9 +251,25 @@ func makeFlatReportReproducible(report *model.FlatReport) {
253251
return
254252
}
255253
report.DateGenerated = ""
254+
}
255+
256+
func rewriteFlatReportDocumentPaths(report *model.FlatReport, rewriters []documentPathRewriter) {
257+
if report == nil || len(rewriters) == 0 {
258+
return
259+
}
256260
for _, change := range report.Changes {
257-
if change != nil {
258-
change.RawPath = ""
261+
if change == nil || change.Context == nil || change.Context.DocumentLocation == "" {
262+
continue
263+
}
264+
for _, rewrite := range rewriters {
265+
if rewrite == nil {
266+
continue
267+
}
268+
rewritten := rewrite(change.Context.DocumentLocation)
269+
if rewritten != change.Context.DocumentLocation {
270+
change.Context.DocumentLocation = rewritten
271+
break
272+
}
259273
}
260274
}
261275
}

cmd/report_test.go

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -184,10 +184,10 @@ func TestReportCommand_ReproducibleOutputIsStableAcrossRuns(t *testing.T) {
184184
first := runOnce()
185185
second := runOnce()
186186

187-
assert.Equal(t, first, second)
187+
assert.Equal(t, stripRawPaths(t, first), stripRawPaths(t, second))
188188
assert.NotContains(t, first, `"dateGenerated"`)
189189
assert.NotContains(t, first, `"commitDetails"`)
190-
assert.NotContains(t, first, `"rawPath"`)
190+
assert.Contains(t, first, `"rawPath"`)
191191
}
192192

193193
func TestRunLeftRightReport_GitRefExplodedSpecIncludesSiblingChanges(t *testing.T) {
@@ -207,8 +207,11 @@ func TestRunLeftRightReport_GitRefExplodedSpecIncludesSiblingChanges(t *testing.
207207

208208
encoded, err := json.Marshal(report)
209209
require.NoError(t, err)
210-
assert.Contains(t, string(encoded), `"property":"required"`)
211-
assert.Contains(t, string(encoded), `"path":"$.paths['/pets'].get.responses['200'].content['application/json'].schema"`)
210+
content := string(encoded)
211+
assert.Contains(t, content, `"property":"required"`)
212+
assert.Contains(t, content, `"path":"$.paths['/pets'].get.responses['200'].content['application/json'].schema"`)
213+
assert.Contains(t, content, `"document":"apis/components/pet.yaml"`)
214+
assert.NotContains(t, content, ".openapi-changes-gitref")
212215
}
213216

214217
func TestRunLeftRightReport_LocalExplodedSpecIncludesSiblingChanges(t *testing.T) {
@@ -282,3 +285,44 @@ func TestReportCommand_GitRefUsesLeftRightMode(t *testing.T) {
282285
assert.Contains(t, output, `"summary"`)
283286
assert.NotContains(t, output, `"reports"`)
284287
}
288+
289+
func TestReportCommand_RepoHistoryColonPathUsesHistoryMode(t *testing.T) {
290+
repoDir := createGitSpecRepoForFile(t, "v1:beta.yaml")
291+
chdirForTest(t, t.TempDir())
292+
293+
cmd := testRootCmd(GetReportCommand(), repoDir, "v1:beta.yaml")
294+
output := captureStdout(t, func() {
295+
require.NoError(t, cmd.Execute())
296+
})
297+
298+
assert.Contains(t, output, `"gitRepoPath": "`+repoDir+`"`)
299+
assert.Contains(t, output, `"gitFilePath": "v1:beta.yaml"`)
300+
assert.Contains(t, output, `"reports"`)
301+
assert.NotContains(t, output, `"originalPath"`)
302+
}
303+
304+
func stripRawPaths(t *testing.T, raw string) string {
305+
t.Helper()
306+
307+
var payload any
308+
require.NoError(t, json.Unmarshal([]byte(raw), &payload))
309+
removeJSONKey(payload, "rawPath")
310+
311+
bits, err := json.MarshalIndent(payload, "", " ")
312+
require.NoError(t, err)
313+
return string(bits)
314+
}
315+
316+
func removeJSONKey(value any, key string) {
317+
switch typed := value.(type) {
318+
case map[string]any:
319+
delete(typed, key)
320+
for _, child := range typed {
321+
removeJSONKey(child, key)
322+
}
323+
case []any:
324+
for _, child := range typed {
325+
removeJSONKey(child, key)
326+
}
327+
}
328+
}

0 commit comments

Comments
 (0)