diff --git a/integrationtest/command/testdata/gitlab_output/stdout/stdout.txt b/integrationtest/command/testdata/gitlab_output/stdout/stdout.txt index d675074..1fba7d1 100644 --- a/integrationtest/command/testdata/gitlab_output/stdout/stdout.txt +++ b/integrationtest/command/testdata/gitlab_output/stdout/stdout.txt @@ -5,7 +5,11 @@ "fingerprint": "e9b14e45ca01a9a72fda9b8356a9ddbbaf7fe8c47116790a51cd699ae1679353", "severity": "major", "location": { - "path": "needs_format.yaml" + "path": "needs_format.yaml", + "lines": { + "begin": 2, + "end": 4 + } } } ] diff --git a/internal/gitlab/codequality.go b/internal/gitlab/codequality.go index 6643322..28492fd 100644 --- a/internal/gitlab/codequality.go +++ b/internal/gitlab/codequality.go @@ -18,6 +18,7 @@ package gitlab import ( "crypto/sha256" "fmt" + "strings" "github.com/google/yamlfmt" ) @@ -35,7 +36,14 @@ type CodeQuality struct { // Location is the location of a Code Quality finding. type Location struct { - Path string `json:"path,omitempty"` + Path string `json:"path,omitempty"` + Lines *Lines `json:"lines,omitempty"` +} + +// Lines follows the GitLab Code Quality schema. +type Lines struct { + Begin int `json:"begin"` + End *int `json:"end,omitempty"` } // NewCodeQuality creates a new CodeQuality object from a yamlfmt.FileDiff. @@ -46,6 +54,8 @@ func NewCodeQuality(diff yamlfmt.FileDiff) (CodeQuality, bool) { return CodeQuality{}, false } + begin, end := detectChangedLines(&diff) + return CodeQuality{ Description: "Not formatted correctly, run yamlfmt to resolve.", Name: "yamlfmt", @@ -53,10 +63,51 @@ func NewCodeQuality(diff yamlfmt.FileDiff) (CodeQuality, bool) { Severity: Major, Location: Location{ Path: diff.Path, + Lines: &Lines{ + Begin: begin, + End: &end, + }, }, }, true } +// detectChangedLines finds the first and last lines that differ between original and formatted content. +func detectChangedLines(diff *yamlfmt.FileDiff) (begin, end int) { + original := strings.Split(string(diff.Diff.Original), "\n") + formatted := strings.Split(string(diff.Diff.Formatted), "\n") + + maxLines := max(len(original), len(formatted)) + + begin = -1 + end = -1 + + for i := 0; i < maxLines; i++ { + origLine := "" + fmtLine := "" + + if i < len(original) { + origLine = original[i] + } + if i < len(formatted) { + fmtLine = formatted[i] + } + + if origLine != fmtLine { + if begin == -1 { + begin = i + 1 + } + end = i + 1 + } + } + + if begin == -1 { + begin = 1 + end = 1 + } + + return begin, end +} + // fingerprint returns a 256-bit SHA256 hash of the original unformatted file. // This is used to uniquely identify a code quality finding. func fingerprint(diff yamlfmt.FileDiff) string { diff --git a/internal/gitlab/codequality_test.go b/internal/gitlab/codequality_test.go index 52c8a1c..6a02d96 100644 --- a/internal/gitlab/codequality_test.go +++ b/internal/gitlab/codequality_test.go @@ -16,10 +16,13 @@ package gitlab_test import ( "encoding/json" + "os" + "path/filepath" "testing" "github.com/google/go-cmp/cmp" "github.com/google/yamlfmt" + "github.com/google/yamlfmt/internal/assert" "github.com/google/yamlfmt/internal/gitlab" ) @@ -66,30 +69,256 @@ func TestCodeQuality(t *testing.T) { t.Parallel() got, gotOK := gitlab.NewCodeQuality(tc.diff) - if gotOK != tc.wantOK { - t.Fatalf("NewCodeQuality() = (%#v, %v), want (*, %v)", got, gotOK, tc.wantOK) - } + assert.Equal(t, tc.wantOK, gotOK) if !gotOK { return } - if tc.wantFingerprint != "" && tc.wantFingerprint != got.Fingerprint { - t.Fatalf("NewCodeQuality().Fingerprint = %q, want %q", got.Fingerprint, tc.wantFingerprint) + if tc.wantFingerprint != "" { + assert.Equal(t, tc.wantFingerprint, got.Fingerprint) } data, err := json.Marshal(got) - if err != nil { - t.Fatal(err) - } + assert.NilErr(t, err) var gotUnmarshal gitlab.CodeQuality - if err := json.Unmarshal(data, &gotUnmarshal); err != nil { - t.Fatal(err) + err = json.Unmarshal(data, &gotUnmarshal) + assert.NilErr(t, err) + + if d := cmp.Diff(got, gotUnmarshal); d != "" { + assert.EqualMsg(t, "", d, "json.Marshal() and json.Unmarshal() mismatch (-got +want):\n%s") } + }) + } +} + +func TestCodeQuality_DetectChangedLine(t *testing.T) { + t.Parallel() + + testdataDir := "./testdata/changed_line" + originalPath := filepath.Join(testdataDir, "original.yaml") + formattedPath := filepath.Join(testdataDir, "formatted.yaml") + + original, err := os.ReadFile(originalPath) + assert.NilErr(t, err) + + formatted, err := os.ReadFile(formattedPath) + assert.NilErr(t, err) + + diff := yamlfmt.FileDiff{ + Path: "testdata/original.yaml", + Diff: &yamlfmt.FormatDiff{ + Original: original, + Formatted: formatted, + }, + } + + cq, ok := gitlab.NewCodeQuality(diff) + assert.Assert(t, ok, "NewCodeQuality() returned false, expected true") + + assert.Assert(t, cq.Location.Lines != nil, "Location.Lines is nil") + + wantBeginLine := 6 + gotBeginLine := cq.Location.Lines.Begin + assert.Equal(t, wantBeginLine, gotBeginLine) + + assert.Assert(t, cq.Location.Lines.End != nil, "Location.Lines.End is nil") + + wantEndLine := 8 + gotEndLine := *cq.Location.Lines.End + assert.Equal(t, wantEndLine, gotEndLine) + + assert.Equal(t, diff.Path, cq.Location.Path) + + assert.Assert(t, cq.Description != "", "Description is empty") + assert.Assert(t, cq.Name != "", "Name is empty") + assert.Assert(t, cq.Fingerprint != "", "Fingerprint is empty") + assert.Assert(t, cq.Severity != "", "Severity is empty") +} + +func TestCodeQuality_DetectChangedLines_FromTestdata(t *testing.T) { + t.Parallel() + + type tc struct { + name string + dir string + wantOK bool + wantBegin int + wantEnd int + } + + cases := []tc{ + { + name: "no lines changed", + dir: "no_lines_changed", + wantOK: false, + }, + { + name: "all lines changed", + dir: "all_lines_changed", + wantOK: true, + wantBegin: 1, + wantEnd: 2, + }, + { + name: "single line changed", + dir: "single_line_changed", + wantOK: true, + wantBegin: 2, + wantEnd: 2, + }, + { + name: "only the last line changed", + dir: "last_line_changed", + wantOK: true, + wantBegin: 3, + wantEnd: 3, + }, + { + name: "only change is appending a last line", + dir: "append_last_line", + wantOK: true, + wantBegin: 3, + wantEnd: 3, + }, + } + + for _, c := range cases { + c := c + + t.Run(c.name, func(t *testing.T) { + t.Parallel() + + testdataDir := filepath.Join("./testdata/changed_line", c.dir) + originalPath := filepath.Join(testdataDir, "original.yaml") + formattedPath := filepath.Join(testdataDir, "formatted.yaml") - if diff := cmp.Diff(got, gotUnmarshal); diff != "" { - t.Errorf("json.Marshal() and json.Unmarshal() mismatch (-got +want):\n%s", diff) + original, err := os.ReadFile(originalPath) + assert.NilErr(t, err) + + formatted, err := os.ReadFile(formattedPath) + assert.NilErr(t, err) + + diff := yamlfmt.FileDiff{ + Path: filepath.ToSlash(filepath.Join("testdata/changed_line", c.dir, "original.yaml")), + Diff: &yamlfmt.FormatDiff{ + Original: original, + Formatted: formatted, + }, + } + + cq, ok := gitlab.NewCodeQuality(diff) + assert.Equal(t, c.wantOK, ok) + if !ok { + return + } + + assert.Assert(t, cq.Location.Lines != nil, "Location.Lines is nil") + assert.Equal(t, c.wantBegin, cq.Location.Lines.Begin) + + assert.Assert(t, cq.Location.Lines.End != nil, "Location.Lines.End is nil") + assert.Equal(t, c.wantEnd, *cq.Location.Lines.End) + + assert.Equal(t, diff.Path, cq.Location.Path) + assert.Assert(t, cq.Description != "", "Description is empty") + assert.Assert(t, cq.Name != "", "Name is empty") + assert.Assert(t, cq.Fingerprint != "", "Fingerprint is empty") + assert.Assert(t, cq.Severity != "", "Severity is empty") + }) + } +} + +func TestCodeQuality_DetectChangedLines_MultipleCases(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + original string + formatted string + wantBegin int + wantEnd int + }{ + { + name: "single line change", + original: "a: b", + formatted: "a: b", + wantBegin: 1, + wantEnd: 1, + }, + { + name: "multiple consecutive lines", + original: `line1 +line2: value +line3: value +line4`, + formatted: `line1 +line2: value +line3: value +line4`, + wantBegin: 2, + wantEnd: 3, + }, + { + name: "non-consecutive changes", + original: `line1 +line2: value +line3 +line4: value +line5`, + formatted: `line1 +line2: value +line3 +line4: value +line5`, + wantBegin: 2, + wantEnd: 4, + }, + { + name: "change at beginning", + original: `key: value +line2 +line3`, + formatted: `key: value +line2 +line3`, + wantBegin: 1, + wantEnd: 1, + }, + { + name: "change at end", + original: `line1 +line2 +key: value`, + formatted: `line1 +line2 +key: value`, + wantBegin: 3, + wantEnd: 3, + }, + } + + for _, tc := range cases { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + diff := yamlfmt.FileDiff{ + Path: "test.yaml", + Diff: &yamlfmt.FormatDiff{ + Original: []byte(tc.original), + Formatted: []byte(tc.formatted), + }, } + + cq, ok := gitlab.NewCodeQuality(diff) + assert.Assert(t, ok, "NewCodeQuality() returned false, expected true") + + assert.Assert(t, cq.Location.Lines != nil, "Location.Lines is nil") + assert.Equal(t, tc.wantBegin, cq.Location.Lines.Begin) + + assert.Assert(t, cq.Location.Lines.End != nil, "Location.Lines.End is nil") + assert.Equal(t, tc.wantEnd, *cq.Location.Lines.End) }) } } diff --git a/internal/gitlab/testdata/changed_line/all_lines_changed/formatted.yaml b/internal/gitlab/testdata/changed_line/all_lines_changed/formatted.yaml new file mode 100644 index 0000000..2d0039a --- /dev/null +++ b/internal/gitlab/testdata/changed_line/all_lines_changed/formatted.yaml @@ -0,0 +1,2 @@ +x: y +z: w diff --git a/internal/gitlab/testdata/changed_line/all_lines_changed/original.yaml b/internal/gitlab/testdata/changed_line/all_lines_changed/original.yaml new file mode 100644 index 0000000..5851568 --- /dev/null +++ b/internal/gitlab/testdata/changed_line/all_lines_changed/original.yaml @@ -0,0 +1,2 @@ +a: b +c: d diff --git a/internal/gitlab/testdata/changed_line/append_last_line/formatted.yaml b/internal/gitlab/testdata/changed_line/append_last_line/formatted.yaml new file mode 100644 index 0000000..efe5502 --- /dev/null +++ b/internal/gitlab/testdata/changed_line/append_last_line/formatted.yaml @@ -0,0 +1,3 @@ +line1: ok +line2: ok +line3: added diff --git a/internal/gitlab/testdata/changed_line/append_last_line/original.yaml b/internal/gitlab/testdata/changed_line/append_last_line/original.yaml new file mode 100644 index 0000000..cf9b973 --- /dev/null +++ b/internal/gitlab/testdata/changed_line/append_last_line/original.yaml @@ -0,0 +1,2 @@ +line1: ok +line2: ok diff --git a/internal/gitlab/testdata/changed_line/formatted.yaml b/internal/gitlab/testdata/changed_line/formatted.yaml new file mode 100644 index 0000000..413d266 --- /dev/null +++ b/internal/gitlab/testdata/changed_line/formatted.yaml @@ -0,0 +1,18 @@ +# Example configuration file +version: 1.0 + +services: + web: + image: nginx:latest + ports: + - 80:80 + - 443:443 + environment: + - ENV=production + - DEBUG=false + + database: + image: postgres:14 + environment: + - POSTGRES_DB=myapp + - POSTGRES_USER=admin \ No newline at end of file diff --git a/internal/gitlab/testdata/changed_line/last_line_changed/formatted.yaml b/internal/gitlab/testdata/changed_line/last_line_changed/formatted.yaml new file mode 100644 index 0000000..89e9d23 --- /dev/null +++ b/internal/gitlab/testdata/changed_line/last_line_changed/formatted.yaml @@ -0,0 +1,3 @@ +line1: ok +line2: ok +line3: spaced diff --git a/internal/gitlab/testdata/changed_line/last_line_changed/original.yaml b/internal/gitlab/testdata/changed_line/last_line_changed/original.yaml new file mode 100644 index 0000000..15ed2cc --- /dev/null +++ b/internal/gitlab/testdata/changed_line/last_line_changed/original.yaml @@ -0,0 +1,3 @@ +line1: ok +line2: ok +line3: spaced diff --git a/internal/gitlab/testdata/changed_line/no_lines_changed/formatted.yaml b/internal/gitlab/testdata/changed_line/no_lines_changed/formatted.yaml new file mode 100644 index 0000000..5851568 --- /dev/null +++ b/internal/gitlab/testdata/changed_line/no_lines_changed/formatted.yaml @@ -0,0 +1,2 @@ +a: b +c: d diff --git a/internal/gitlab/testdata/changed_line/no_lines_changed/original.yaml b/internal/gitlab/testdata/changed_line/no_lines_changed/original.yaml new file mode 100644 index 0000000..5851568 --- /dev/null +++ b/internal/gitlab/testdata/changed_line/no_lines_changed/original.yaml @@ -0,0 +1,2 @@ +a: b +c: d diff --git a/internal/gitlab/testdata/changed_line/original.yaml b/internal/gitlab/testdata/changed_line/original.yaml new file mode 100644 index 0000000..8c237a5 --- /dev/null +++ b/internal/gitlab/testdata/changed_line/original.yaml @@ -0,0 +1,18 @@ +# Example configuration file +version: 1.0 + +services: + web: + image: nginx:latest + ports: + - 80:80 + - 443:443 + environment: + - ENV=production + - DEBUG=false + + database: + image: postgres:14 + environment: + - POSTGRES_DB=myapp + - POSTGRES_USER=admin \ No newline at end of file diff --git a/internal/gitlab/testdata/changed_line/single_line_changed/formatted.yaml b/internal/gitlab/testdata/changed_line/single_line_changed/formatted.yaml new file mode 100644 index 0000000..6420006 --- /dev/null +++ b/internal/gitlab/testdata/changed_line/single_line_changed/formatted.yaml @@ -0,0 +1,3 @@ +line1: ok +line2: spaced +line3: ok diff --git a/internal/gitlab/testdata/changed_line/single_line_changed/original.yaml b/internal/gitlab/testdata/changed_line/single_line_changed/original.yaml new file mode 100644 index 0000000..ed56226 --- /dev/null +++ b/internal/gitlab/testdata/changed_line/single_line_changed/original.yaml @@ -0,0 +1,3 @@ +line1: ok +line2: spaced +line3: ok