Repository: reviewdog/reviewdog
Pull request #1017 (2021-08-05 12:51:25)

Fix suggestion includes fenced code blocks
When reviewdog suggests the following code

> ```
> original code
> ```

modified as

> ````
> reviewdog suggestion
> ```
> some code
> ```
> ````

the suggestion comment should be

> ````diff 
> -original code
> +reviewdog suggestion
> +```
> +some code
> +```
> ````

but actually, reviewdog suggests the following comment:

> ```diff
> -original code
> +reviewdog suggestion
> ```
> some code
> ```
> ```

This is because the markdown parser creates wrong pairs of code fences.

> ````markdown
> ```suggestion 
> reviewdog suggestion
> ```
> some code
> ```
> ```
> ````

The document of GitHub markdown says that it is needed to wrap them inside quadruple backticks in this case.
https://docs.github.com/en/github/writing-on-github/working-with-advanced-formatting/creating-and-highlighting-code-blocks#fenced-code-blocks

> To display triple backticks in a fenced code block, wrap them inside quadruple backticks.

> `````markdown
> ````suggestion 
> reviewdog suggestion
> ```
> some code
> ```
> ````
> `````

This pull request checks whether the suggestion includes code fences, and use quadruple backticks code fences if it is necessary.

Fixes https://github.com/reviewdog/reviewdog/issues/999

- [x] Updated Unreleased section in [CHANGELOG](https://github.com/reviewdog/reviewdog/blob/master/CHANGELOG.md) or it's not notable changes.




Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0a712e46..a16ad00e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### :bug: Fixes
 - [#1014](https://github.com/reviewdog/reviewdog/pull/1014) Fix incorrect detection of the `GITHUB_TOKEN` permissions. fixes [#1010](https://github.com/reviewdog/reviewdog/issues/1010)
+- [#1017](https://github.com/reviewdog/reviewdog/pull/1017) Fix suggestions that include fenced code blocks. fixes [#999](https://github.com/reviewdog/reviewdog/issues/999)
 
 ### :rotating_light: Breaking changes
 - ...
diff --git a/service/commentutil/code_fence.go b/service/commentutil/code_fence.go
new file mode 100644
index 00000000..f49efc38
--- /dev/null
+++ b/service/commentutil/code_fence.go
@@ -0,0 +1,84 @@
+package commentutil
+
+import "io"
+
+// GetCodeFenceLength returns the length of a code fence needed to wrap code.
+// A test suggestion that uses four backticks w/o code fence block.
+// Fixes: https://github.com/reviewdog/reviewdog/issues/999
+//
+// Code fenced blocks are supported by GitHub Flavor Markdown.
+// A code fence is typically three backticks.
+//
+//     ```
+//     code
+//     ```
+//
+// However, we sometimes need more backticks.
+// https://docs.github.com/en/github/writing-on-github/working-with-advanced-formatting/creating-and-highlighting-code-blocks#fenced-code-blocks
+//
+// > To display triple backticks in a fenced code block, wrap them inside quadruple backticks.
+// >
+// >     ````
+// >     ```
+// >     Look! You can see my backticks.
+// >     ```
+// >     ````
+func GetCodeFenceLength(code string) int {
+	backticks := countBackticks(code) + 1
+	if backticks < 3 {
+		// At least three backticks are required.
+		// https://github.github.com/gfm/#fenced-code-blocks
+		// > A code fence is a sequence of at least three consecutive backtick characters (`) or tildes (~). (Tildes and backticks cannot be mixed.)
+		backticks = 3
+	}
+	return backticks
+}
+
+// WriteCodeFence writes a code fence to w.
+func WriteCodeFence(w io.Writer, length int) error {
+	if w, ok := w.(io.ByteWriter); ok {
+		// use WriteByte instead of Write to avoid memory allocation.
+		for i := 0; i < length; i++ {
+			if err := w.WriteByte('`'); err != nil {
+				return err
+			}
+		}
+		return nil
+	}
+
+	buf := make([]byte, length)
+	for i := range buf {
+		buf[i] = '`'
+	}
+	_, err := w.Write(buf)
+	return err
+}
+
+// find code fences in s, and returns the maximum length of them.
+func countBackticks(s string) int {
+	inBackticks := true
+
+	var count int
+	var maxCount int
+	for _, r := range s {
+		if inBackticks {
+			if r == '`' {
+				count++
+			} else {
+				inBackticks = false
+				if count > maxCount {
+					maxCount = count
+				}
+				count = 0
+			}
+		}
+		if r == '\n' {
+			inBackticks = true
+			count = 0
+		}
+	}
+	if count > maxCount {
+		maxCount = count
+	}
+	return maxCount
+}
diff --git a/service/github/github.go b/service/github/github.go
index 692838a7..592703a2 100644
--- a/service/github/github.go
+++ b/service/github/github.go
@@ -309,13 +309,19 @@ func buildSingleSuggestion(c *reviewdog.Comment, s *rdf.Suggestion) (string, err
 	if start.GetColumn() > 0 || end.GetColumn() > 0 {
 		return buildNonLineBasedSuggestion(c, s)
 	}
+
+	txt := s.GetText()
+	backticks := commentutil.GetCodeFenceLength(txt)
+
 	var sb strings.Builder
-	sb.WriteString("```suggestion\n")
-	if txt := s.GetText(); txt != "" {
+	sb.Grow(backticks + len("suggestion\n") + len(txt) + len("\n") + backticks)
+	commentutil.WriteCodeFence(&sb, backticks)
+	sb.WriteString("suggestion\n")
+	if txt != "" {
 		sb.WriteString(txt)
 		sb.WriteString("\n")
 	}
-	sb.WriteString("```")
+	commentutil.WriteCodeFence(&sb, backticks)
 	return sb.String(), nil
 }
 
@@ -334,12 +340,17 @@ func buildNonLineBasedSuggestion(c *reviewdog.Comment, s *rdf.Suggestion) (strin
 	if err != nil {
 		return "", err
 	}
+
+	txt := startLineContent[:max(start.GetColumn()-1, 0)] + s.GetText() + endLineContent[max(end.GetColumn()-1, 0):]
+	backticks := commentutil.GetCodeFenceLength(txt)
+
 	var sb strings.Builder
-	sb.WriteString("```suggestion\n")
-	sb.WriteString(startLineContent[:max(start.GetColumn()-1, 0)])
-	sb.WriteString(s.GetText())
-	sb.WriteString(endLineContent[max(end.GetColumn()-1, 0):])
-	sb.WriteString("\n```")
+	sb.Grow(backticks + len("suggestion\n") + len(txt) + len("\n") + backticks)
+	commentutil.WriteCodeFence(&sb, backticks)
+	sb.WriteString("suggestion\n")
+	sb.WriteString(txt)
+	sb.WriteString("\n")
+	commentutil.WriteCodeFence(&sb, backticks)
 	return sb.String(), nil
 }
 


Test patch:
diff --git a/service/commentutil/code_fence_test.go b/service/commentutil/code_fence_test.go
new file mode 100644
index 00000000..68734452
--- /dev/null
+++ b/service/commentutil/code_fence_test.go
@@ -0,0 +1,86 @@
+package commentutil
+
+import (
+	"io"
+	"strings"
+	"testing"
+)
+
+func TestGetCodeFenceLength(t *testing.T) {
+	tests := []struct {
+		in   string
+		want int
+	}{
+		{
+			in:   "",
+			want: 3,
+		},
+		{
+			in:   "`inline code`",
+			want: 3,
+		},
+		{
+			in:   "``foo`bar``",
+			want: 3,
+		},
+		{
+			in:   "func main() {\nprintln(\"Hello World\")\n}\n",
+			want: 3,
+		},
+		{
+			in:   "```\nLook! You can see my backticks.\n```\n",
+			want: 4,
+		},
+		{
+			in:   "```go\nfunc main() {\nprintln(\"Hello World\")\n}\n```",
+			want: 4,
+		},
+		{
+			in:   "```go\nfunc main() {\nprintln(\"Hello World\")\n}\n`````",
+			want: 6,
+		},
+		{
+			in:   "`````\n````\n```",
+			want: 6,
+		},
+	}
+
+	for _, tt := range tests {
+		want := tt.want
+		if got := GetCodeFenceLength(tt.in); got != want {
+			t.Errorf("got unexpected length.\ngot:\n%d\nwant:\n%d", got, want)
+		}
+	}
+}
+
+// A version of strings.Builder without WriteByte
+type Builder struct {
+	strings.Builder
+	io.ByteWriter // conflicts with and hides strings.Builder's WriteByte.
+}
+
+func TestWriteCodeFence(t *testing.T) {
+	var buf Builder
+	err := WriteCodeFence(&buf, 10)
+	if err != nil {
+		t.Fatal(err)
+	}
+	got := buf.String()
+	want := "``````````"
+	if got != want {
+		t.Errorf("got %q, want %q", got, want)
+	}
+}
+
+func TestWriteCodeFenceWriteByte(t *testing.T) {
+	var buf strings.Builder
+	err := WriteCodeFence(&buf, 10)
+	if err != nil {
+		t.Fatal(err)
+	}
+	got := buf.String()
+	want := "``````````"
+	if got != want {
+		t.Errorf("got %q, want %q", got, want)
+	}
+}
diff --git a/service/github/github_test.go b/service/github/github_test.go
index a26498b6..47234c4b 100644
--- a/service/github/github_test.go
+++ b/service/github/github_test.go
@@ -434,6 +434,48 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 					"```",
 				}, "\n") + "\n"),
 			},
+			{
+				Path:      github.String("reviewdog.go"),
+				Side:      github.String("RIGHT"),
+				StartSide: github.String("RIGHT"),
+				StartLine: github.Int(15),
+				Line:      github.Int(16),
+				Body: github.String(commentutil.BodyPrefix + strings.Join([]string{
+					"multiline suggestion comment including a code fence block",
+					"````suggestion",
+					"```",
+					"some code",
+					"```",
+					"````",
+				}, "\n") + "\n"),
+			},
+			{
+				Path: github.String("reviewdog.go"),
+				Side: github.String("RIGHT"),
+				Line: github.Int(15),
+				Body: github.String(commentutil.BodyPrefix + strings.Join([]string{
+					"singleline suggestion comment including a code fence block",
+					"````suggestion",
+					"```",
+					"some code",
+					"```",
+					"````",
+				}, "\n") + "\n"),
+			},
+			{
+				Path:      github.String("reviewdog.go"),
+				Side:      github.String("RIGHT"),
+				StartSide: github.String("RIGHT"),
+				StartLine: github.Int(15),
+				Line:      github.Int(16),
+				Body: github.String(commentutil.BodyPrefix + strings.Join([]string{
+					"multiline suggestion comment including an empty code fence block",
+					"``````suggestion",
+					"```",
+					"`````",
+					"``````",
+				}, "\n") + "\n"),
+			},
 		}
 		if diff := pretty.Compare(want, req.Comments); diff != "" {
 			t.Errorf("req.Comments diff: (-got +want)\n%s", diff)
@@ -945,6 +987,96 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 				InDiffContext: true,
 			},
 		},
+		{
+			Result: &filter.FilteredDiagnostic{
+				Diagnostic: &rdf.Diagnostic{
+					Location: &rdf.Location{
+						Path: "reviewdog.go",
+						Range: &rdf.Range{
+							Start: &rdf.Position{
+								Line: 15,
+							},
+							End: &rdf.Position{
+								Line: 16,
+							},
+						},
+					},
+					Suggestions: []*rdf.Suggestion{
+						{
+							Range: &rdf.Range{
+								Start: &rdf.Position{
+									Line: 15,
+								},
+								End: &rdf.Position{
+									Line: 16,
+								},
+							},
+							Text: "```\nsome code\n```",
+						},
+					},
+					Message: "multiline suggestion comment including a code fence block",
+				},
+				InDiffContext: true,
+			},
+		},
+		{
+			Result: &filter.FilteredDiagnostic{
+				Diagnostic: &rdf.Diagnostic{
+					Location: &rdf.Location{
+						Path: "reviewdog.go",
+						Range: &rdf.Range{
+							Start: &rdf.Position{
+								Line: 15,
+							},
+						},
+					},
+					Suggestions: []*rdf.Suggestion{
+						{
+							Range: &rdf.Range{
+								Start: &rdf.Position{
+									Line: 15,
+								},
+							},
+							Text: "```\nsome code\n```",
+						},
+					},
+					Message: "singleline suggestion comment including a code fence block",
+				},
+				InDiffContext: true,
+			},
+		},
+		{
+			Result: &filter.FilteredDiagnostic{
+				Diagnostic: &rdf.Diagnostic{
+					Location: &rdf.Location{
+						Path: "reviewdog.go",
+						Range: &rdf.Range{
+							Start: &rdf.Position{
+								Line: 15,
+							},
+							End: &rdf.Position{
+								Line: 16,
+							},
+						},
+					},
+					Suggestions: []*rdf.Suggestion{
+						{
+							Range: &rdf.Range{
+								Start: &rdf.Position{
+									Line: 15,
+								},
+								End: &rdf.Position{
+									Line: 16,
+								},
+							},
+							Text: "```\n`````",
+						},
+					},
+					Message: "multiline suggestion comment including an empty code fence block",
+				},
+				InDiffContext: true,
+			},
+		},
 	}
 	for _, c := range comments {
 		if err := g.Post(context.Background(), c); err != nil {