Skip to content

Commit

Permalink
scm/github: filter issues that belongs to PR diff (#9)
Browse files Browse the repository at this point in the history
  • Loading branch information
herlon214 authored Apr 13, 2022
1 parent f580f7d commit 3bbc48f
Show file tree
Hide file tree
Showing 5 changed files with 284 additions and 24 deletions.
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ require (
github.com/migueleliasweb/go-github-mock v0.0.5
github.com/pkg/errors v0.9.1
github.com/sirupsen/logrus v1.8.1
github.com/sourcegraph/go-diff v0.6.1
github.com/spf13/cobra v1.2.1
github.com/stretchr/testify v1.7.0
golang.org/x/oauth2 v0.0.0-20210402161424-2e8d93401602
Expand Down
4 changes: 4 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -222,11 +222,15 @@ github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFR
github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts=
github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc=
github.com/shurcooL/go v0.0.0-20180423040247-9e1955d9fb6e/go.mod h1:TDJrrUr11Vxrven61rcy3hJMUqaf/CLWYhHNPmT14Lk=
github.com/shurcooL/go-goon v0.0.0-20170922171312-37c2f522c041/go.mod h1:N5mDOmsrJOB+vfqUK+7DmDyjhSLIIBnXo9lvZJj3MWQ=
github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc=
github.com/sirupsen/logrus v1.8.1 h1:dJKuHgqk1NNQlqoA6BTlM1Wf9DOH3NBjQyu0h9+AZZE=
github.com/sirupsen/logrus v1.8.1/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0=
github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d/go.mod h1:OnSkiWE9lh6wB0YB77sQom3nweQdgAjqCqsofrRNTgc=
github.com/smartystreets/goconvey v1.6.4/go.mod h1:syvi0/a8iFYH4r/RixwvyeAJjdLS9QV7WQ/tjFTllLA=
github.com/sourcegraph/go-diff v0.6.1 h1:hmA1LzxW0n1c3Q4YbrFgg4P99GSnebYa3x8gr0HZqLQ=
github.com/sourcegraph/go-diff v0.6.1/go.mod h1:iBszgVvyxdc8SFZ7gm69go2KDdt3ag071iBaWPF6cjs=
github.com/spf13/afero v1.6.0/go.mod h1:Ai8FlHk4v/PARR026UzYexafAt9roJ7LcLMAmO6Z93I=
github.com/spf13/cast v1.3.1/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE=
github.com/spf13/cobra v1.2.1 h1:+KmjbUw1hriSNMF55oPrkZcb27aECyrj8V2ytv7kWDw=
Expand Down
81 changes: 59 additions & 22 deletions pkg/scm/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/google/go-github/v41/github"
"github.com/pkg/errors"
"github.com/sourcegraph/go-diff/diff"
"golang.org/x/oauth2"

"github.com/herlon214/sonarqube-pr-issues/pkg/sonarqube"
Expand Down Expand Up @@ -56,22 +57,72 @@ func (g *Github) PublishIssuesReviewFor(ctx context.Context, issues []sonarqube.
reviewEvent = REVIEW_EVENT_COMMENT
}

// Convert PR number into int
prNumber, err := strconv.Atoi(pr.Key)
if err != nil {
return errors.Wrap(err, "failed to convert PR number to int")
}

// Parse PR path
ghPath, err := parseGithubPath(pr.URL)
if err != nil {
return errors.Wrap(err, "failed to parse github path")
}

// Fetch PR diffs
ghDiff, _, err := g.client.PullRequests.GetRaw(ctx, ghPath.Owner, ghPath.Repo, prNumber, github.RawOptions{Type: github.Diff})
if err != nil {
return errors.Wrap(err, "failed to get raw PR")
}

// Parse diffs
fileDiffs, err := diff.ParseMultiFileDiff([]byte(ghDiff))
if err != nil {
return errors.Wrap(err, "failed to parse diff")
}

diffMap := make(map[string][]*diff.Hunk)
for i := range fileDiffs {
fileName := fileDiffs[i].OrigName[2:]
diffMap[fileName] = fileDiffs[i].Hunks
}

comments := make([]*github.DraftReviewComment, 0)

// Create a comment for each issue
for _, issue := range issues {
side := "RIGHT"
message := issue.MarkdownMessage(g.sonar.Root)
filePath := issue.FilePath()
line := issue.Line
lineNumber := issue.Line

// Skip if current issue is not part of the PR diff
hunks, ok := diffMap[filePath]
if !ok {
continue
}

comment := &github.DraftReviewComment{
Path: &filePath,
Body: &message,
Side: &side,
Line: &line,
for _, hunk := range hunks {
if lineNumber < int(hunk.OrigStartLine) || lineNumber < int(hunk.NewStartLine) {
continue
}
if lineNumber > int(hunk.OrigStartLine+hunk.OrigLines) || lineNumber > int(hunk.NewStartLine+hunk.NewLines) {
continue
}

comment := &github.DraftReviewComment{
Path: &filePath,
Body: &message,
Side: &side,
Line: &lineNumber,
}
comments = append(comments, comment)
}
comments = append(comments, comment)

}

if len(comments) == 0 {
return errors.New("failed to find relevant issues")
}

body := fmt.Sprintf(`:wave: Hey, I added %d comments about your changes, please take a look :slightly_smiling_face:`, len(issues))
Expand All @@ -82,26 +133,12 @@ func (g *Github) PublishIssuesReviewFor(ctx context.Context, issues []sonarqube.
Comments: comments,
}

// Convert PR number into int
prNumber, err := strconv.Atoi(pr.Key)
if err != nil {
return errors.Wrap(err, "failed to convert PR number to int")
}

// Parse PR path
ghPath, err := parseGithubPath(pr.URL)
if err != nil {
return errors.Wrap(err, "failed to parse github path")
}

// Create the review
out, res, err := g.client.PullRequests.CreateReview(ctx, ghPath.Owner, ghPath.Repo, prNumber, reviewRequest)
_, _, err = g.client.PullRequests.CreateReview(ctx, ghPath.Owner, ghPath.Repo, prNumber, reviewRequest)
if err != nil {
return errors.Wrap(err, "failed to create review")
}

fmt.Println(out, res)

return nil
}

Expand Down
70 changes: 68 additions & 2 deletions pkg/scm/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,22 @@ package scm

import (
"context"
_ "embed"
"io"
"net/http"
"testing"

"github.com/google/go-github/v41/github"
"github.com/herlon214/sonarqube-pr-issues/pkg/sonarqube"
"github.com/migueleliasweb/go-github-mock/src/mock"
"github.com/sourcegraph/go-diff/diff"

"github.com/stretchr/testify/assert"
)

//go:embed testdata/raw_pr1.diff
var RawPrDiff string

func TestNewGithub(t *testing.T) {
ctx := context.Background()

Expand All @@ -20,10 +26,22 @@ func TestNewGithub(t *testing.T) {
assert.NotNil(t, gh)
}

func TestGithubPublishIssuesReview(t *testing.T) {
func TestParseGithubDiff(t *testing.T) {
fileDiffs, err := diff.ParseMultiFileDiff([]byte(RawPrDiff))
assert.NoError(t, err)
assert.Equal(t, 3, len(fileDiffs))
}

func TestGithubPublishIssuesReviewWrongSonarDiffLine(t *testing.T) {
ctx := context.Background()

mockedHTTPClient := mock.NewMockedHTTPClient(
mock.WithRequestMatchHandler(
mock.GetReposPullsByOwnerByRepoByPullNumber,
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.Write([]byte(RawPrDiff))
}),
),
mock.WithRequestMatchHandler(
mock.PostReposPullsReviewsByOwnerByRepoByPullNumber,
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
Expand Down Expand Up @@ -55,8 +73,56 @@ func TestGithubPublishIssuesReview(t *testing.T) {
}

err := gh.PublishIssuesReviewFor(ctx, issues, pr, true)
assert.NoError(t, err)
assert.Equal(t, "failed to find relevant issues", err.Error())
}

func TestGithubPublishIssuesReviewCorrectSonarDiffLine(t *testing.T) {
ctx := context.Background()

mockedHTTPClient := mock.NewMockedHTTPClient(
mock.WithRequestMatchHandler(
mock.GetReposPullsByOwnerByRepoByPullNumber,
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.Write([]byte(RawPrDiff))
}),
),
mock.WithRequestMatchHandler(
mock.PostReposPullsReviewsByOwnerByRepoByPullNumber,
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
body, err := io.ReadAll(r.Body)
assert.NoError(t, err)

assert.Equal(t, `{"body":":wave: Hey, I added 1 comments about your changes, please take a look :slightly_smiling_face:","event":"REQUEST_CHANGES","comments":[{"path":"pkg/scm/github.go","body":":bug::bangbang: CRITICAL: My message ([go:S1234](root/coding_rules?open=go:S1234&rule_key=go:S1234))","side":"RIGHT","line":61}]}
`, string(body))
}),
),
)

gh := &Github{
sonar: sonarqube.New("root", "key"),
client: github.NewClient(mockedHTTPClient),
}

pr := &sonarqube.PullRequest{
Key: "3",
Branch: "feat/newtest",
URL: "https://github.com/herlon214/sonarqube-pr-issues/pull/3",
}

issues := []sonarqube.Issue{
{
Project: "myproject",
Component: "myproject:pkg/scm/github.go",
Severity: "CRITICAL",
Type: "BUG",
Rule: "go:S1234",
Message: "My message",
Line: 61,
},
}

err := gh.PublishIssuesReviewFor(ctx, issues, pr, true)
assert.NoError(t, err)
}

func TestParsePullRequestUrl(t *testing.T) {
Expand Down
152 changes: 152 additions & 0 deletions pkg/scm/testdata/raw_pr1.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
diff --git a/go.mod b/go.mod
index 5d90f4b..9521466 100644
--- a/go.mod
+++ b/go.mod
@@ -21,6 +21,7 @@ require (
github.com/gorilla/mux v1.8.0 // indirect
github.com/inconshreveable/mousetrap v1.0.0 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
+ github.com/sourcegraph/go-diff v0.6.1 // indirect
github.com/spf13/pflag v1.0.5 // indirect
golang.org/x/crypto v0.0.0-20210817164053-32db794688a5 // indirect
golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4 // indirect
diff --git a/go.sum b/go.sum
index c7ec3b3..f13e00b 100644
--- a/go.sum
+++ b/go.sum
@@ -222,11 +222,15 @@ github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFR
github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts=
github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc=
+github.com/shurcooL/go v0.0.0-20180423040247-9e1955d9fb6e/go.mod h1:TDJrrUr11Vxrven61rcy3hJMUqaf/CLWYhHNPmT14Lk=
+github.com/shurcooL/go-goon v0.0.0-20170922171312-37c2f522c041/go.mod h1:N5mDOmsrJOB+vfqUK+7DmDyjhSLIIBnXo9lvZJj3MWQ=
github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc=
github.com/sirupsen/logrus v1.8.1 h1:dJKuHgqk1NNQlqoA6BTlM1Wf9DOH3NBjQyu0h9+AZZE=
github.com/sirupsen/logrus v1.8.1/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0=
github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d/go.mod h1:OnSkiWE9lh6wB0YB77sQom3nweQdgAjqCqsofrRNTgc=
github.com/smartystreets/goconvey v1.6.4/go.mod h1:syvi0/a8iFYH4r/RixwvyeAJjdLS9QV7WQ/tjFTllLA=
+github.com/sourcegraph/go-diff v0.6.1 h1:hmA1LzxW0n1c3Q4YbrFgg4P99GSnebYa3x8gr0HZqLQ=
+github.com/sourcegraph/go-diff v0.6.1/go.mod h1:iBszgVvyxdc8SFZ7gm69go2KDdt3ag071iBaWPF6cjs=
github.com/spf13/afero v1.6.0/go.mod h1:Ai8FlHk4v/PARR026UzYexafAt9roJ7LcLMAmO6Z93I=
github.com/spf13/cast v1.3.1/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE=
github.com/spf13/cobra v1.2.1 h1:+KmjbUw1hriSNMF55oPrkZcb27aECyrj8V2ytv7kWDw=
diff --git a/pkg/scm/github.go b/pkg/scm/github.go
index 0150f40..8475900 100644
--- a/pkg/scm/github.go
+++ b/pkg/scm/github.go
@@ -9,6 +9,7 @@ import (

"github.com/google/go-github/v41/github"
"github.com/pkg/errors"
+ "github.com/sourcegraph/go-diff/diff"
"golang.org/x/oauth2"

"github.com/herlon214/sonarqube-pr-issues/pkg/sonarqube"
@@ -56,6 +57,36 @@ func (g *Github) PublishIssuesReviewFor(ctx context.Context, issues []sonarqube.
reviewEvent = REVIEW_EVENT_COMMENT
}

+ // Convert PR number into int
+ prNumber, err := strconv.Atoi(pr.Key)
+ if err != nil {
+ return errors.Wrap(err, "failed to convert PR number to int")
+ }
+
+ // Parse PR path
+ ghPath, err := parseGithubPath(pr.URL)
+ if err != nil {
+ return errors.Wrap(err, "failed to parse github path")
+ }
+
+ // Fetch PR diffs
+ ghDiff, _, err := g.client.PullRequests.GetRaw(ctx, ghPath.Owner, ghPath.Repo, prNumber, github.RawOptions{github.Diff})
+ if err != nil {
+ return errors.Wrap(err, "failed to get raw PR")
+ }
+
+ // Parse diffs
+ fileDiffs, err := diff.ParseMultiFileDiff([]byte(ghDiff))
+ if err != nil {
+ return errors.Wrap(err, "failed to parse diff")
+ }
+
+ diffMap := make(map[string][]*diff.Hunk)
+ for i := range fileDiffs {
+ fileName := fileDiffs[i].OrigName[2:]
+ diffMap[fileName] = fileDiffs[i].Hunks
+ }
+
comments := make([]*github.DraftReviewComment, 0)

// Create a comment for each issue
@@ -63,15 +94,35 @@ func (g *Github) PublishIssuesReviewFor(ctx context.Context, issues []sonarqube.
side := "RIGHT"
message := issue.MarkdownMessage(g.sonar.Root)
filePath := issue.FilePath()
- line := issue.Line
+ lineNumber := issue.Line
+
+ // Skip if current issue is not part of the PR diff
+ hunks, ok := diffMap[filePath]
+ if !ok {
+ continue
+ }

- comment := &github.DraftReviewComment{
- Path: &filePath,
- Body: &message,
- Side: &side,
- Line: &line,
+ for _, hunk := range hunks {
+ if lineNumber < int(hunk.OrigStartLine) || lineNumber < int(hunk.NewStartLine) {
+ continue
+ }
+ if lineNumber > int(hunk.OrigStartLine+hunk.OrigLines) || lineNumber > int(hunk.NewStartLine+hunk.NewLines) {
+ continue
+ }
+
+ comment := &github.DraftReviewComment{
+ Path: &filePath,
+ Body: &message,
+ Side: &side,
+ Line: &lineNumber,
+ }
+ comments = append(comments, comment)
}
- comments = append(comments, comment)
+
+ }
+
+ if len(comments) == 0 {
+ return errors.Wrap(err, "failed to find relevant issues")
}

body := fmt.Sprintf(`:wave: Hey, I added %d comments about your changes, please take a look :slightly_smiling_face:`, len(issues))
@@ -82,26 +133,12 @@ func (g *Github) PublishIssuesReviewFor(ctx context.Context, issues []sonarqube.
Comments: comments,
}

- // Convert PR number into int
- prNumber, err := strconv.Atoi(pr.Key)
- if err != nil {
- return errors.Wrap(err, "failed to convert PR number to int")
- }
-
- // Parse PR path
- ghPath, err := parseGithubPath(pr.URL)
- if err != nil {
- return errors.Wrap(err, "failed to parse github path")
- }
-
// Create the review
- out, res, err := g.client.PullRequests.CreateReview(ctx, ghPath.Owner, ghPath.Repo, prNumber, reviewRequest)
+ _, _, err = g.client.PullRequests.CreateReview(ctx, ghPath.Owner, ghPath.Repo, prNumber, reviewRequest)
if err != nil {
return errors.Wrap(err, "failed to create review")
}

- fmt.Println(out, res)
-
return nil
}

0 comments on commit 3bbc48f

Please sign in to comment.