Skip to content

Commit

Permalink
github-pr-review: Support range suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
@haya14busa
haya14busa committed Jul 25, 2020
1 parent 6cfc23e commit 21c0e0f
Show file tree
Hide file tree
Showing 2 changed files with 188 additions and 5 deletions.
30 changes: 28 additions & 2 deletions service/github/github.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,8 +280,7 @@ func buildSingleSuggestion(c *reviewdog.Comment, s *rdf.Suggestion) (string, err
drange.GetStart().GetLine(), drange.GetEnd().GetLine(), start.GetLine(), end.GetLine())
}
if start.GetColumn() > 0 || end.GetColumn() > 0 {
// TODO(haya14busa): Support non-line based suggestion.
return "", errors.New("non line based suggestions (contains column) are not supported yet")
return buildNonLineBasedSuggestion(c, s)
}
var sb strings.Builder
sb.WriteString("```suggestion\n")
Expand All @@ -292,3 +291,30 @@ func buildSingleSuggestion(c *reviewdog.Comment, s *rdf.Suggestion) (string, err
sb.WriteString("```")
return sb.String(), nil
}

func buildNonLineBasedSuggestion(c *reviewdog.Comment, s *rdf.Suggestion) (string, error) {
sourceLines := c.Result.SourceLines
slen := len(sourceLines)
if slen == 0 {
return "", errors.New("source lines are not available")
}
start := s.GetRange().GetStart()
end := s.GetRange().GetEnd()
if slen != int(end.GetLine()-start.GetLine()+1) {
return "", errors.New("invalid source lines: not all source lines for this suggestion are available")
}
var sb strings.Builder
sb.WriteString("```suggestion\n")
sb.WriteString(sourceLines[0][:max(start.GetColumn()-1, 0)])
sb.WriteString(s.GetText())
sb.WriteString(sourceLines[slen-1][max(end.GetColumn()-1, 0):])
sb.WriteString("\n```")
return sb.String(), nil
}

func max(x, y int32) int32 {
if x < y {
return y
}
return x
}
163 changes: 160 additions & 3 deletions service/github/github_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -323,10 +323,58 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
StartLine: github.Int(15),
Line: github.Int(16),
Body: github.String(commentutil.BodyPrefix + "\n" + strings.Join([]string{
"non-line based suggestion comment",
"Invalid suggestion: non line based suggestions (contains column) are not supported yet",
"non-line based suggestion comment (no source lines)",
"Invalid suggestion: source lines are not available",
}, "\n")),
},
{
Path: github.String("reviewdog.go"),
Side: github.String("RIGHT"),
Line: github.Int(15),
Body: github.String(commentutil.BodyPrefix + "\n" + strings.Join([]string{
"range suggestion (single line)",
"```suggestion",
"haya14busa",
"```",
}, "\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 + "\n" + strings.Join([]string{
"range suggestion (multi-line)",
"```suggestion",
"haya14busa (multi-line)",
"```",
}, "\n") + "\n"),
},
{
Path: github.String("reviewdog.go"),
Side: github.String("RIGHT"),
StartSide: github.String("RIGHT"),
StartLine: github.Int(15),
Line: github.Int(17),
Body: github.String(commentutil.BodyPrefix + "\n" + strings.Join([]string{
"range suggestion (line-break, remove)",
"```suggestion",
"line 15 (content at line 15)",
"```",
}, "\n") + "\n"),
},
{
Path: github.String("reviewdog.go"),
Side: github.String("RIGHT"),
Line: github.Int(15),
Body: github.String(commentutil.BodyPrefix + "\n" + strings.Join([]string{
"range suggestion (insert)",
"```suggestion",
"haya14busa",
"```",
}, "\n") + "\n"),
},
}
if diff := pretty.Compare(want, req.Comments); diff != "" {
t.Errorf("req.Comments diff: (-got +want)\n%s", diff)
Expand Down Expand Up @@ -579,7 +627,116 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
Text: "replacement",
},
},
Message: "non-line based suggestion comment",
Message: "non-line based suggestion comment (no source lines)",
},
InDiffContext: true,
},
},
{
Result: &filter.FilteredDiagnostic{
SourceLines: []string{"haya15busa"},
Diagnostic: &rdf.Diagnostic{
Location: &rdf.Location{
Path: "reviewdog.go",
Range: &rdf.Range{
Start: &rdf.Position{Line: 15, Column: 5},
End: &rdf.Position{Line: 15, Column: 7},
},
},
Suggestions: []*rdf.Suggestion{
{
Range: &rdf.Range{
Start: &rdf.Position{Line: 15, Column: 5},
End: &rdf.Position{Line: 15, Column: 7},
},
Text: "14",
},
},
Message: "range suggestion (single line)",
},
InDiffContext: true,
},
},
{
Result: &filter.FilteredDiagnostic{
SourceLines: []string{
"haya???",
"???busa (multi-line)",
},
Diagnostic: &rdf.Diagnostic{
Location: &rdf.Location{
Path: "reviewdog.go",
Range: &rdf.Range{
Start: &rdf.Position{Line: 15, Column: 5},
End: &rdf.Position{Line: 16, Column: 4},
},
},
Suggestions: []*rdf.Suggestion{
{
Range: &rdf.Range{
Start: &rdf.Position{Line: 15, Column: 5},
End: &rdf.Position{Line: 16, Column: 4},
},
Text: "14",
},
},
Message: "range suggestion (multi-line)",
},
InDiffContext: true,
},
},
{
Result: &filter.FilteredDiagnostic{
SourceLines: []string{
"line 15 xxx",
"line 16",
"(content at line 15)",
},
Diagnostic: &rdf.Diagnostic{
Location: &rdf.Location{
Path: "reviewdog.go",
Range: &rdf.Range{
Start: &rdf.Position{Line: 15, Column: 9},
End: &rdf.Position{Line: 17, Column: 1},
},
},
Suggestions: []*rdf.Suggestion{
{
Range: &rdf.Range{
Start: &rdf.Position{Line: 15, Column: 9},
End: &rdf.Position{Line: 17, Column: 1},
},
Text: "",
},
},
Message: "range suggestion (line-break, remove)",
},
InDiffContext: true,
},
},
{
Result: &filter.FilteredDiagnostic{
SourceLines: []string{
"hayabusa",
},
Diagnostic: &rdf.Diagnostic{
Location: &rdf.Location{
Path: "reviewdog.go",
Range: &rdf.Range{
Start: &rdf.Position{Line: 15, Column: 5},
End: &rdf.Position{Line: 15, Column: 5},
},
},
Suggestions: []*rdf.Suggestion{
{
Range: &rdf.Range{
Start: &rdf.Position{Line: 15, Column: 5},
End: &rdf.Position{Line: 15, Column: 5},
},
Text: "14",
},
},
Message: "range suggestion (insert)",
},
InDiffContext: true,
},
Expand Down

0 comments on commit 21c0e0f

Please sign in to comment.