From d0abd468efa50c2c27221bcfbf8acd7b24a3771a Mon Sep 17 00:00:00 2001 From: Herlon Aguiar Date: Thu, 14 Apr 2022 00:38:38 +0200 Subject: [PATCH 1/2] scm/github: filter issues that belongs to PR diff --- go.mod | 1 + go.sum | 4 +++ pkg/scm/github.go | 81 ++++++++++++++++++++++++++++++++++------------- 3 files changed, 64 insertions(+), 22 deletions(-) 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 } From cefe2c79fb50dfc05f83fba6be50b604d4772470 Mon Sep 17 00:00:00 2001 From: Herlon Aguiar Date: Thu, 14 Apr 2022 01:26:50 +0200 Subject: [PATCH 2/2] fixed tests --- go.mod | 2 +- pkg/scm/github.go | 4 +- pkg/scm/github_test.go | 70 +++++++++++++++- pkg/scm/testdata/raw_pr1.diff | 152 ++++++++++++++++++++++++++++++++++ 4 files changed, 223 insertions(+), 5 deletions(-) create mode 100644 pkg/scm/testdata/raw_pr1.diff diff --git a/go.mod b/go.mod index 9521466..37771c5 100644 --- a/go.mod +++ b/go.mod @@ -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 @@ -21,7 +22,6 @@ 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/pkg/scm/github.go b/pkg/scm/github.go index 8475900..a542f9b 100644 --- a/pkg/scm/github.go +++ b/pkg/scm/github.go @@ -70,7 +70,7 @@ func (g *Github) PublishIssuesReviewFor(ctx context.Context, issues []sonarqube. } // Fetch PR diffs - ghDiff, _, err := g.client.PullRequests.GetRaw(ctx, ghPath.Owner, ghPath.Repo, prNumber, github.RawOptions{github.Diff}) + 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") } @@ -122,7 +122,7 @@ func (g *Github) PublishIssuesReviewFor(ctx context.Context, issues []sonarqube. } if len(comments) == 0 { - return errors.Wrap(err, "failed to find relevant issues") + 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)) diff --git a/pkg/scm/github_test.go b/pkg/scm/github_test.go index 821c864..74f7c2f 100644 --- a/pkg/scm/github_test.go +++ b/pkg/scm/github_test.go @@ -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() @@ -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) { @@ -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) { diff --git a/pkg/scm/testdata/raw_pr1.diff b/pkg/scm/testdata/raw_pr1.diff new file mode 100644 index 0000000..cb287b1 --- /dev/null +++ b/pkg/scm/testdata/raw_pr1.diff @@ -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 + } +