diff --git a/cmd/review-bot/main.go b/cmd/review-bot/main.go index f31cae4..ac1a6d1 100644 --- a/cmd/review-bot/main.go +++ b/cmd/review-bot/main.go @@ -241,8 +241,24 @@ func main() { sentinel := fmt.Sprintf("", *reviewerName) + // Map findings to inline comments for lines present in the diff + diffRanges := gitea.ParseDiffNewLines(diff) + var inlineComments []gitea.ReviewComment + for _, f := range result.Findings { + if f.File != "" && f.Line > 0 && diffRanges.Contains(f.File, f.Line) { + inlineComments = append(inlineComments, gitea.ReviewComment{ + Path: f.File, + NewPosition: int64(f.Line), + Body: fmt.Sprintf("**[%s]** %s", f.Severity, f.Finding), + }) + } + } + if len(inlineComments) > 0 { + log.Printf("Attaching %d inline comments", len(inlineComments)) + } + log.Printf("Posting review (event=%s)...", event) - posted, err := giteaClient.PostReview(ctx, owner, repoName, prNumber, event, reviewBody) + posted, err := giteaClient.PostReview(ctx, owner, repoName, prNumber, event, reviewBody, inlineComments) if err != nil { log.Fatalf("Failed to post review: %v", err) } @@ -272,7 +288,7 @@ func main() { if err := giteaClient.DeleteReview(ctx, owner, repoName, prNumber, posted.ID); err != nil { log.Printf("Warning: could not delete review for escalation: %v", err) } else { - _, err := giteaClient.PostReview(ctx, owner, repoName, prNumber, "REQUEST_CHANGES", reviewBody) + _, err := giteaClient.PostReview(ctx, owner, repoName, prNumber, "REQUEST_CHANGES", reviewBody, inlineComments) if err != nil { log.Printf("Warning: could not re-post as REQUEST_CHANGES: %v", err) } else { diff --git a/gitea/client.go b/gitea/client.go index 1b4a472..4185f18 100644 --- a/gitea/client.go +++ b/gitea/client.go @@ -57,6 +57,13 @@ type ChangedFile struct { Status string `json:"status"` } +// ReviewComment represents an inline comment to attach to a review. +type ReviewComment struct { + Path string `json:"path"` + NewPosition int64 `json:"new_position"` + Body string `json:"body"` +} + // GetPullRequest fetches PR metadata. func (c *Client) GetPullRequest(ctx context.Context, owner, repo string, number int) (*PullRequest, error) { reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d", c.baseURL, owner, repo, number) @@ -131,15 +138,18 @@ func (c *Client) GetFileContentRef(ctx context.Context, owner, repo, filepath, r // PostReview submits a review to a PR and returns the created review. // event should be "APPROVED" or "REQUEST_CHANGES". -func (c *Client) PostReview(ctx context.Context, owner, repo string, number int, event, body string) (*Review, error) { +// comments are optional inline comments attached to specific lines. +func (c *Client) PostReview(ctx context.Context, owner, repo string, number int, event, body string, comments []ReviewComment) (*Review, error) { reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews", c.baseURL, owner, repo, number) payload := struct { - Body string `json:"body"` - Event string `json:"event"` + Body string `json:"body"` + Event string `json:"event"` + Comments []ReviewComment `json:"comments,omitempty"` }{ - Body: body, - Event: event, + Body: body, + Event: event, + Comments: comments, } data, err := json.Marshal(payload) diff --git a/gitea/client_test.go b/gitea/client_test.go index eabdd9e..415e83b 100644 --- a/gitea/client_test.go +++ b/gitea/client_test.go @@ -128,7 +128,7 @@ func TestPostReview(t *testing.T) { defer server.Close() client := NewClient(server.URL, "test-token") - review, err := client.PostReview(context.Background(), "owner", "repo", 3, "APPROVED", "LGTM") + review, err := client.PostReview(context.Background(), "owner", "repo", 3, "APPROVED", "LGTM", nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -175,7 +175,7 @@ func TestPostReview_Non200(t *testing.T) { defer server.Close() client := NewClient(server.URL, "test-token") - _, err := client.PostReview(context.Background(), "owner", "repo", 1, "APPROVED", "test") + _, err := client.PostReview(context.Background(), "owner", "repo", 1, "APPROVED", "test", nil) if err == nil { t.Fatal("expected error for 403, got nil") } diff --git a/gitea/diff.go b/gitea/diff.go new file mode 100644 index 0000000..e45ef47 --- /dev/null +++ b/gitea/diff.go @@ -0,0 +1,76 @@ +package gitea + +import ( + "strconv" + "strings" +) + +// DiffLineRanges maps filenames to the set of new-file line numbers present in the diff. +type DiffLineRanges struct { + files map[string]map[int]bool +} + +// Contains reports whether the given file+line is within the diff hunks. +func (d *DiffLineRanges) Contains(file string, line int) bool { + if d == nil || d.files == nil { + return false + } + lines, ok := d.files[file] + if !ok { + return false + } + return lines[line] +} + +// ParseDiffNewLines parses a unified diff and extracts the new-file line numbers +// that appear in each hunk (both added and context lines). +func ParseDiffNewLines(diff string) *DiffLineRanges { + result := &DiffLineRanges{files: make(map[string]map[int]bool)} + + var currentFile string + var newLine int + + for _, line := range strings.Split(diff, "\n") { + // Track current file from +++ header + if strings.HasPrefix(line, "+++ b/") { + currentFile = strings.TrimPrefix(line, "+++ b/") + if result.files[currentFile] == nil { + result.files[currentFile] = make(map[int]bool) + } + continue + } + if strings.HasPrefix(line, "+++ /dev/null") { + currentFile = "" + continue + } + + // Parse hunk header: @@ -old,count +new,count @@ + if strings.HasPrefix(line, "@@") && currentFile != "" { + // Extract the +N part + parts := strings.Split(line, "+") + if len(parts) >= 2 { + numStr := strings.Split(parts[1], ",")[0] + n, err := strconv.Atoi(numStr) + if err == nil { + newLine = n + } + } + continue + } + + if currentFile == "" { + continue + } + + // Count lines in hunk + if strings.HasPrefix(line, "+") || strings.HasPrefix(line, " ") { + result.files[currentFile][newLine] = true + newLine++ + } else if strings.HasPrefix(line, "-") { + // Removed lines don't advance new line counter + continue + } + } + + return result +} diff --git a/gitea/diff_test.go b/gitea/diff_test.go new file mode 100644 index 0000000..a017aff --- /dev/null +++ b/gitea/diff_test.go @@ -0,0 +1,75 @@ +package gitea + +import ( + "testing" +) + +func TestParseDiffLineRanges(t *testing.T) { + diff := `diff --git a/main.go b/main.go +index abc1234..def5678 100644 +--- a/main.go ++++ b/main.go +@@ -10,6 +10,8 @@ func main() { + fmt.Println("hello") ++ fmt.Println("new line 11") ++ fmt.Println("new line 12") + fmt.Println("existing") + } +@@ -30,4 +32,5 @@ func other() { + return nil ++ // added at line 33 + } +diff --git a/util.go b/util.go +new file mode 100644 +--- /dev/null ++++ b/util.go +@@ -0,0 +1,5 @@ ++package main ++ ++func helper() string { ++ return "hi" ++} +` + + ranges := ParseDiffNewLines(diff) + + // main.go should have lines 10-17 (first hunk) and 32-36 (second hunk) + if !ranges.Contains("main.go", 11) { + t.Error("expected main.go:11 to be in diff") + } + if !ranges.Contains("main.go", 12) { + t.Error("expected main.go:12 to be in diff") + } + if !ranges.Contains("main.go", 10) { + t.Error("expected main.go:10 to be in diff (context line)") + } + if !ranges.Contains("main.go", 33) { + t.Error("expected main.go:33 to be in diff") + } + if ranges.Contains("main.go", 25) { + t.Error("main.go:25 should NOT be in diff") + } + + // util.go is entirely new, lines 1-5 + if !ranges.Contains("util.go", 1) { + t.Error("expected util.go:1 to be in diff") + } + if !ranges.Contains("util.go", 5) { + t.Error("expected util.go:5 to be in diff") + } + if ranges.Contains("util.go", 6) { + t.Error("util.go:6 should NOT be in diff") + } + + // Unknown file + if ranges.Contains("unknown.go", 1) { + t.Error("unknown.go should not be in diff") + } +} + +func TestParseDiffNewLines_Empty(t *testing.T) { + ranges := ParseDiffNewLines("") + if ranges.Contains("any.go", 1) { + t.Error("empty diff should contain nothing") + } +} diff --git a/gitea/post_review_comments_test.go b/gitea/post_review_comments_test.go new file mode 100644 index 0000000..e58d05d --- /dev/null +++ b/gitea/post_review_comments_test.go @@ -0,0 +1,88 @@ +package gitea + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" +) + +func TestPostReview_WithComments(t *testing.T) { + var gotPayload struct { + Body string `json:"body"` + Event string `json:"event"` + Comments []struct { + Path string `json:"path"` + NewPosition int64 `json:"new_position"` + Body string `json:"body"` + } `json:"comments"` + } + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + json.NewDecoder(r.Body).Decode(&gotPayload) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(200) + json.NewEncoder(w).Encode(map[string]any{ + "id": 99, + "body": gotPayload.Body, + "user": map[string]any{"login": "bot"}, + }) + })) + defer server.Close() + + client := NewClient(server.URL, "test-token") + comments := []ReviewComment{ + {Path: "main.go", NewPosition: 42, Body: "[MAJOR] Something bad"}, + {Path: "util.go", NewPosition: 10, Body: "[MINOR] Style issue"}, + } + + _, err := client.PostReview(context.Background(), "owner", "repo", 1, "REQUEST_CHANGES", "summary", comments) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(gotPayload.Comments) != 2 { + t.Fatalf("expected 2 comments, got %d", len(gotPayload.Comments)) + } + if gotPayload.Comments[0].Path != "main.go" { + t.Errorf("expected path main.go, got %s", gotPayload.Comments[0].Path) + } + if gotPayload.Comments[0].NewPosition != 42 { + t.Errorf("expected new_position 42, got %d", gotPayload.Comments[0].NewPosition) + } + if gotPayload.Comments[1].Body != "[MINOR] Style issue" { + t.Errorf("unexpected body: %s", gotPayload.Comments[1].Body) + } +} + +func TestPostReview_NilComments(t *testing.T) { + var gotPayload map[string]any + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + json.NewDecoder(r.Body).Decode(&gotPayload) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(200) + json.NewEncoder(w).Encode(map[string]any{ + "id": 100, + "body": "test", + "user": map[string]any{"login": "bot"}, + }) + })) + defer server.Close() + + client := NewClient(server.URL, "test-token") + _, err := client.PostReview(context.Background(), "owner", "repo", 1, "APPROVED", "all good", nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // With nil comments, the field should be omitted (omitempty) + comments, ok := gotPayload["comments"] + if ok && comments != nil { + arr, isArr := comments.([]any) + if isArr && len(arr) > 0 { + t.Error("expected no comments in payload when nil passed") + } + } +}