Repository: reviewdog/reviewdog
Pull request #1012 (2021-08-03 13:43:55)

Use Gitlab suggestions in merge request comments
Follow #678, this pull request allow use Gitlab suggestions in merge request discussion

Most of code in this PR come from #842 but the author @caseycs is inactive for a long time, I create a new one.

Please take a look @haya14busa




Patch:
diff --git a/service/gitlab/gitlab_mr_discussion.go b/service/gitlab/gitlab_mr_discussion.go
index 4ce8d09e..de7a11fb 100644
--- a/service/gitlab/gitlab_mr_discussion.go
+++ b/service/gitlab/gitlab_mr_discussion.go
@@ -4,16 +4,23 @@ import (
 	"context"
 	"fmt"
 	"path/filepath"
+	"strings"
 	"sync"
 
 	"github.com/xanzy/go-gitlab"
 	"golang.org/x/sync/errgroup"
 
 	"github.com/reviewdog/reviewdog"
+	"github.com/reviewdog/reviewdog/proto/rdf"
 	"github.com/reviewdog/reviewdog/service/commentutil"
 	"github.com/reviewdog/reviewdog/service/serviceutil"
 )
 
+const (
+	invalidSuggestionPre  = "<details><summary>reviewdog suggestion error</summary>"
+	invalidSuggestionPost = "</details>"
+)
+
 // MergeRequestDiscussionCommenter is a comment and diff service for GitLab MergeRequest.
 //
 // API:
@@ -104,6 +111,11 @@ func (g *MergeRequestDiscussionCommenter) postCommentsForEach(ctx context.Contex
 		loc := c.Result.Diagnostic.GetLocation()
 		lnum := int(loc.GetRange().GetStart().GetLine())
 		body := commentutil.MarkdownComment(c)
+
+		if suggestion := buildSuggestions(c); suggestion != "" {
+			body = body + "\n\n" + suggestion
+		}
+
 		if !c.Result.InDiffFile || lnum == 0 || postedcs.IsPosted(c, lnum, body) {
 			continue
 		}
@@ -152,3 +164,41 @@ func listAllMergeRequestDiscussion(cli *gitlab.Client, projectID string, mergeRe
 	}
 	return append(discussions, restDiscussions...), nil
 }
+
+// creates diff in markdown for suggested changes
+// Ref gitlab suggestion: https://docs.gitlab.com/ee/user/project/merge_requests/reviews/suggestions.html
+func buildSuggestions(c *reviewdog.Comment) string {
+	var sb strings.Builder
+	for _, s := range c.Result.Diagnostic.GetSuggestions() {
+		if s.Range == nil || s.Range.Start == nil || s.Range.End == nil {
+			continue
+		}
+
+		txt, err := buildSingleSuggestion(c, s)
+		if err != nil {
+			sb.WriteString(invalidSuggestionPre + err.Error() + invalidSuggestionPost + "\n")
+			continue
+		}
+		sb.WriteString(txt)
+		sb.WriteString("\n")
+	}
+
+	return sb.String()
+}
+
+func buildSingleSuggestion(c *reviewdog.Comment, s *rdf.Suggestion) (string, error) {
+	var sb strings.Builder
+
+	// user 4 backticks for safety: https://docs.gitlab.com/ee/user/project/merge_requests/reviews/suggestions.html#code-block-nested-in-suggestions
+	sb.WriteString("````suggestion:-0+")
+	sb.WriteString(fmt.Sprintf("%d", s.Range.End.Line-s.Range.Start.Line))
+	sb.WriteString("\n")
+
+	if txt := s.GetText(); txt != "" {
+		sb.WriteString(txt)
+		sb.WriteString("\n")
+	}
+
+	sb.WriteString("````")
+	return sb.String(), nil
+}


Test patch:
diff --git a/service/gitlab/gitlab_mr_discussion_test.go b/service/gitlab/gitlab_mr_discussion_test.go
index 92905b02..3b7fa028 100644
--- a/service/gitlab/gitlab_mr_discussion_test.go
+++ b/service/gitlab/gitlab_mr_discussion_test.go
@@ -6,16 +6,16 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"strings"
 	"sync/atomic"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
-	"github.com/xanzy/go-gitlab"
-
 	"github.com/reviewdog/reviewdog"
 	"github.com/reviewdog/reviewdog/filter"
 	"github.com/reviewdog/reviewdog/proto/rdf"
 	"github.com/reviewdog/reviewdog/service/commentutil"
+	"github.com/xanzy/go-gitlab"
 )
 
 func TestGitLabMergeRequestDiscussionCommenter_Post_Flush_review_api(t *testing.T) {
@@ -121,6 +121,34 @@ func TestGitLabMergeRequestDiscussionCommenter_Post_Flush_review_api(t *testing.
 		},
 	}
 
+	newCommentWithSuggestion := &reviewdog.Comment{
+		Result: &filter.FilteredDiagnostic{
+			Diagnostic: &rdf.Diagnostic{
+				Location: &rdf.Location{
+					Path: "file3.go",
+					Range: &rdf.Range{Start: &rdf.Position{
+						Line: 14,
+					}},
+				},
+				Message: "new comment with suggestion",
+				Suggestions: []*rdf.Suggestion{
+					{
+						Text: "line1-fixed\nline2-fixed",
+						Range: &rdf.Range{
+							Start: &rdf.Position{
+								Line: 14,
+							},
+							End: &rdf.Position{
+								Line: 15,
+							},
+						},
+					},
+				},
+			},
+			InDiffFile: true,
+		},
+	}
+
 	comments := []*reviewdog.Comment{
 		alreadyCommented1,
 		alreadyCommented2,
@@ -129,9 +157,10 @@ func TestGitLabMergeRequestDiscussionCommenter_Post_Flush_review_api(t *testing.
 		newComment3,
 		commentOutsideDiff,
 		commentWithoutLnum,
+		newCommentWithSuggestion,
 	}
 	var postCalled int32
-	const wantPostCalled = 3
+	const wantPostCalled = 4
 
 	mux := http.NewServeMux()
 	mux.HandleFunc("/api/v4/projects/o/r/merge_requests/14/discussions", func(w http.ResponseWriter, r *http.Request) {
@@ -219,6 +248,18 @@ func TestGitLabMergeRequestDiscussionCommenter_Post_Flush_review_api(t *testing.
 				if diff := cmp.Diff(got, want); diff != "" {
 					t.Error(diff)
 				}
+			case "file3.go":
+				suggestions := buildSuggestions(newCommentWithSuggestion)
+				bodyExpected := commentutil.MarkdownComment(newCommentWithSuggestion) + "\n\n" + suggestions
+
+				want := &gitlab.CreateMergeRequestDiscussionOptions{
+					Body: gitlab.String(bodyExpected),
+					Position: &gitlab.NotePosition{
+						BaseSHA: "xxx", StartSHA: "xxx", HeadSHA: "sha", PositionType: "text", NewPath: "file3.go", NewLine: 14},
+				}
+				if diff := cmp.Diff(got, want); diff != "" {
+					t.Error(diff)
+				}
 			default:
 				t.Errorf("got unexpected discussion: %#v", got)
 			}
@@ -267,3 +308,143 @@ func TestGitLabMergeRequestDiscussionCommenter_Post_Flush_review_api(t *testing.
 		t.Errorf("%d discussions posted, but want %d", postCalled, wantPostCalled)
 	}
 }
+
+func TestBuildSuggestions(t *testing.T) {
+	tests := []struct {
+		in   *reviewdog.Comment
+		want string
+	}{
+		{
+			in: &reviewdog.Comment{
+				ToolName: "tool-name",
+				Result: &filter.FilteredDiagnostic{
+					Diagnostic: &rdf.Diagnostic{
+						Message: "no suggestion",
+					},
+				},
+			},
+			want: "",
+		},
+		{
+			in: buildTestComment(
+				"one suggestion",
+				[]*rdf.Suggestion{
+					buildTestsSuggestion("line1-fixed\nline2-fixed", 10, 10),
+				},
+			),
+			want: strings.Join([]string{
+				"````suggestion:-0+0",
+				"line1-fixed",
+				"line2-fixed",
+				"````",
+				"",
+			}, "\n"),
+		},
+		{
+			in: buildTestComment(
+				"two suggestions",
+				[]*rdf.Suggestion{
+					buildTestsSuggestion("line1-fixed\nline2-fixed", 10, 11),
+					buildTestsSuggestion("line3-fixed\nline4-fixed", 20, 21),
+				},
+			),
+			want: strings.Join([]string{
+				"````suggestion:-0+1",
+				"line1-fixed",
+				"line2-fixed",
+				"````",
+				"````suggestion:-0+1",
+				"line3-fixed",
+				"line4-fixed",
+				"````",
+				"",
+			}, "\n"),
+		},
+	}
+	for _, tt := range tests {
+		suggestion := buildSuggestions(tt.in)
+		if suggestion != tt.want {
+			t.Errorf("got unexpected suggestion.\ngot:\n%s\nwant:\n%s", suggestion, tt.want)
+		}
+	}
+}
+
+func TestBuildSuggestionsInvalid(t *testing.T) {
+	tests := []struct {
+		in   *reviewdog.Comment
+		want string
+	}{
+		{
+			in: buildTestComment(
+				"two suggestions, one without range",
+				[]*rdf.Suggestion{
+					{
+						Text: "line3-fixed\nline4-fixed",
+					},
+					buildTestsSuggestion("line1-fixed\nline2-fixed", 10, 11),
+				},
+			),
+			want: strings.Join([]string{
+				"````suggestion:-0+1",
+				"line1-fixed",
+				"line2-fixed",
+				"````",
+				"",
+			}, "\n"),
+		},
+		{
+			in: buildTestComment(
+				"two suggestions, one without range end",
+				[]*rdf.Suggestion{
+					{
+						Text: "line3-fixed\nline4-fixed",
+						Range: &rdf.Range{
+							Start: &rdf.Position{
+								Line: 20,
+							},
+						},
+					},
+					buildTestsSuggestion("line1-fixed\nline2-fixed", 10, 11),
+				}),
+			want: strings.Join([]string{
+				"````suggestion:-0+1",
+				"line1-fixed",
+				"line2-fixed",
+				"````",
+				"",
+			}, "\n"),
+		},
+	}
+	for _, tt := range tests {
+		suggestion := buildSuggestions(tt.in)
+		if suggestion != tt.want {
+			t.Errorf("got unexpected suggestion.\ngot:\n%s\nwant:\n%s", suggestion, tt.want)
+		}
+	}
+}
+
+func buildTestsSuggestion(text string, start int32, end int32) *rdf.Suggestion {
+	return &rdf.Suggestion{
+		Text: text,
+		Range: &rdf.Range{
+			Start: &rdf.Position{
+				Line: start,
+			},
+			End: &rdf.Position{
+				Line: end,
+			},
+		},
+	}
+}
+
+func buildTestComment(message string, suggestions []*rdf.Suggestion) *reviewdog.Comment {
+	return &reviewdog.Comment{
+		ToolName: "tool-name",
+		Result: &filter.FilteredDiagnostic{
+			Diagnostic: &rdf.Diagnostic{
+				Message:     message,
+				Suggestions: suggestions,
+			},
+		},
+	}
+}