diff --git a/internal/diffmap/mapper.go b/internal/diffmap/mapper.go new file mode 100644 index 000000000..d309eae77 --- /dev/null +++ b/internal/diffmap/mapper.go @@ -0,0 +1,281 @@ +// Package diffmap maps working-tree line numbers +// to diff hunk positions for inline code review comments. +package diffmap + +import ( + "bufio" + "bytes" + "errors" + "fmt" + "slices" + "strconv" + "strings" +) + +// Mapper maps file:line references to diff coordinates. +type Mapper struct { + // files maps file paths to their diff hunks. + files map[string]*fileDiff +} + +type fileDiff struct { + // hunks are the diff hunks for the file, + // in order of appearance. + hunks []hunk +} + +type hunk struct { + // newStart is the starting line number + // in the new version of the file. + newStart int + + // newCount is the number of lines + // in the new version of the hunk. + newCount int + + // oldStart is the starting line number + // in the old version of the file. + oldStart int + + // oldCount is the number of lines + // in the old version of the hunk. + oldCount int + + // lines are the diff lines in the hunk. + lines []diffLine +} + +type diffLine struct { + // op is the diff operation: ' ', '+', or '-'. + op byte + + // newLineNo is the line number in the new file. + // Zero for deleted lines. + newLineNo int + + // oldLineNo is the line number in the old file. + // Zero for added lines. + oldLineNo int +} + +// New creates a Mapper from unified diff output. +// The diff should be the output of git diff base...HEAD. +func New(diff []byte) (*Mapper, error) { + m := &Mapper{ + files: make(map[string]*fileDiff), + } + + scanner := bufio.NewScanner(bytes.NewReader(diff)) + var currentFile string + var currentHunk *hunk + + for scanner.Scan() { + line := scanner.Text() + + // Detect file header: "diff --git a/path b/path" + if strings.HasPrefix(line, "diff --git ") { + currentFile = "" + currentHunk = nil + continue + } + + // Detect new file path: "+++ b/path" + if path, ok := strings.CutPrefix(line, "+++ b/"); ok { + currentFile = path + if _, ok := m.files[currentFile]; !ok { + m.files[currentFile] = &fileDiff{} + } + continue + } + + // Detect rename/copy target: + // "rename to path" or "copy to path" + if path, ok := strings.CutPrefix(line, "rename to "); ok { + currentFile = path + if _, ok := m.files[currentFile]; !ok { + m.files[currentFile] = &fileDiff{} + } + continue + } + if path, ok := strings.CutPrefix(line, "copy to "); ok { + currentFile = path + if _, ok := m.files[currentFile]; !ok { + m.files[currentFile] = &fileDiff{} + } + continue + } + + // Skip if no file context yet. + if currentFile == "" { + continue + } + + // Detect hunk header: "@@ -old,count +new,count @@" + if strings.HasPrefix(line, "@@ ") { + h, err := parseHunkHeader(line) + if err != nil { + return nil, fmt.Errorf( + "parse hunk header %q: %w", + line, err, + ) + } + fd := m.files[currentFile] + fd.hunks = append(fd.hunks, h) + currentHunk = &fd.hunks[len(fd.hunks)-1] + continue + } + + // Process diff lines within a hunk. + if currentHunk == nil || len(line) == 0 { + continue + } + + op := line[0] + switch op { + case ' ': + // Context line: present in both old and new. + dl := diffLine{ + op: ' ', + oldLineNo: currentHunk.oldStart + countOp(currentHunk.lines, ' ', '-'), + newLineNo: currentHunk.newStart + countOp(currentHunk.lines, ' ', '+'), + } + currentHunk.lines = append(currentHunk.lines, dl) + case '+': + // Added line: only in new. + dl := diffLine{ + op: '+', + newLineNo: currentHunk.newStart + countOp(currentHunk.lines, ' ', '+'), + } + currentHunk.lines = append(currentHunk.lines, dl) + case '-': + // Deleted line: only in old. + dl := diffLine{ + op: '-', + oldLineNo: currentHunk.oldStart + countOp(currentHunk.lines, ' ', '-'), + } + currentHunk.lines = append(currentHunk.lines, dl) + case '\\': + // "\ No newline at end of file" — skip. + default: + // Unknown line type — skip. + } + } + + if err := scanner.Err(); err != nil { + return nil, fmt.Errorf("scan diff: %w", err) + } + + return m, nil +} + +// Map converts a working-tree file:line reference +// to diff coordinates suitable for forge inline comments. +// +// Returns the file path, the diff line number, +// and the side ("LEFT" or "RIGHT"). +// +// Returns an error if the file or line +// is not part of the diff. +func (m *Mapper) Map(file string, line int) ( + path string, diffLine int, side string, err error, +) { + fd, ok := m.files[file] + if !ok { + return "", 0, "", + fmt.Errorf("file %q not in diff", file) + } + + for _, h := range fd.hunks { + for _, dl := range h.lines { + if dl.newLineNo == line && + (dl.op == '+' || dl.op == ' ') { + return file, dl.newLineNo, "RIGHT", nil + } + } + } + + return "", 0, "", fmt.Errorf( + "line %d of %q not in diff", line, file, + ) +} + +// Files returns all file paths present in the diff. +func (m *Mapper) Files() []string { + var files []string + for f := range m.files { + files = append(files, f) + } + return files +} + +// parseHunkHeader parses "@@ -old,count +new,count @@". +func parseHunkHeader(line string) (hunk, error) { + // Strip "@@ " prefix and " @@..." suffix. + line = strings.TrimPrefix(line, "@@ ") + idx := strings.Index(line, " @@") + if idx < 0 { + return hunk{}, + errors.New("missing @@ terminator") + } + line = line[:idx] + + parts := strings.SplitN(line, " ", 2) + if len(parts) != 2 { + return hunk{}, + errors.New("expected old and new ranges") + } + + oldStart, oldCount, err := parseRange( + strings.TrimPrefix(parts[0], "-"), + ) + if err != nil { + return hunk{}, + fmt.Errorf("parse old range: %w", err) + } + + newStart, newCount, err := parseRange( + strings.TrimPrefix(parts[1], "+"), + ) + if err != nil { + return hunk{}, + fmt.Errorf("parse new range: %w", err) + } + + return hunk{ + oldStart: oldStart, + oldCount: oldCount, + newStart: newStart, + newCount: newCount, + }, nil +} + +// parseRange parses "start,count" or "start" (count=1). +func parseRange(s string) (start, count int, err error) { + parts := strings.SplitN(s, ",", 2) + start, err = strconv.Atoi(parts[0]) + if err != nil { + return 0, 0, + fmt.Errorf("parse start: %w", err) + } + if len(parts) == 1 { + return start, 1, nil + } + count, err = strconv.Atoi(parts[1]) + if err != nil { + return 0, 0, + fmt.Errorf("parse count: %w", err) + } + return start, count, nil +} + +// countOp counts lines in the hunk +// that match any of the given operations. +func countOp(lines []diffLine, ops ...byte) int { + n := 0 + for _, l := range lines { + if slices.Contains(ops, l.op) { + n++ + } + } + return n +} diff --git a/internal/diffmap/mapper_test.go b/internal/diffmap/mapper_test.go new file mode 100644 index 000000000..6abbb5fe1 --- /dev/null +++ b/internal/diffmap/mapper_test.go @@ -0,0 +1,198 @@ +package diffmap + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestMapper_Map(t *testing.T) { + tests := []struct { + name string + diff string + file string + line int + wantPath string + wantLine int + wantSide string + wantErr string + }{ + { + name: "AddedLine", + diff: `diff --git a/main.go b/main.go +--- a/main.go ++++ b/main.go +@@ -1,3 +1,4 @@ + package main + ++func hello() {} + func main() {} +`, + file: "main.go", + line: 3, + wantPath: "main.go", + wantLine: 3, + wantSide: "RIGHT", + }, + { + name: "ContextLine", + diff: `diff --git a/main.go b/main.go +--- a/main.go ++++ b/main.go +@@ -1,3 +1,4 @@ + package main + ++func hello() {} + func main() {} +`, + file: "main.go", + line: 1, + wantPath: "main.go", + wantLine: 1, + wantSide: "RIGHT", + }, + { + name: "MultipleHunks", + diff: `diff --git a/main.go b/main.go +--- a/main.go ++++ b/main.go +@@ -1,3 +1,4 @@ + package main + ++import "fmt" + func main() {} +@@ -10,3 +11,4 @@ + func foo() {} + ++func bar() {} + func baz() {} +`, + file: "main.go", + line: 13, + wantPath: "main.go", + wantLine: 13, + wantSide: "RIGHT", + }, + { + name: "FileNotInDiff", + diff: `diff --git a/main.go b/main.go +--- a/main.go ++++ b/main.go +@@ -1,3 +1,4 @@ + package main + ++func hello() {} + func main() {} +`, + file: "other.go", + line: 1, + wantErr: `file "other.go" not in diff`, + }, + { + name: "LineNotInDiff", + diff: `diff --git a/main.go b/main.go +--- a/main.go ++++ b/main.go +@@ -1,3 +1,4 @@ + package main + ++func hello() {} + func main() {} +`, + file: "main.go", + line: 100, + wantErr: `line 100 of "main.go" not in diff`, + }, + { + name: "MultipleFiles", + diff: `diff --git a/a.go b/a.go +--- a/a.go ++++ b/a.go +@@ -1,2 +1,3 @@ + package a ++func A() {} + +diff --git a/b.go b/b.go +--- a/b.go ++++ b/b.go +@@ -1,2 +1,3 @@ + package b ++func B() {} + +`, + file: "b.go", + line: 2, + wantPath: "b.go", + wantLine: 2, + wantSide: "RIGHT", + }, + { + name: "NewFile", + diff: `diff --git a/new.go b/new.go +new file mode 100644 +--- /dev/null ++++ b/new.go +@@ -0,0 +1,3 @@ ++package new ++ ++func New() {} +`, + file: "new.go", + line: 1, + wantPath: "new.go", + wantLine: 1, + wantSide: "RIGHT", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m, err := New([]byte(tt.diff)) + require.NoError(t, err) + + path, line, side, err := m.Map(tt.file, tt.line) + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + + require.NoError(t, err) + assert.Equal(t, tt.wantPath, path) + assert.Equal(t, tt.wantLine, line) + assert.Equal(t, tt.wantSide, side) + }) + } +} + +func TestMapper_Files(t *testing.T) { + diff := `diff --git a/a.go b/a.go +--- a/a.go ++++ b/a.go +@@ -1,2 +1,3 @@ + package a ++func A() {} + +diff --git a/b.go b/b.go +--- a/b.go ++++ b/b.go +@@ -1,2 +1,3 @@ + package b ++func B() {} + +` + m, err := New([]byte(diff)) + require.NoError(t, err) + + files := m.Files() + assert.Len(t, files, 2) + assert.Contains(t, files, "a.go") + assert.Contains(t, files, "b.go") +} + +func TestNew_EmptyDiff(t *testing.T) { + m, err := New([]byte("")) + require.NoError(t, err) + assert.Empty(t, m.Files()) +}