Skip to content

Commit

Permalink
Merge pull request #673 from reviewdog/rdformat-basic
Browse files Browse the repository at this point in the history
Use Reviewdog Diagnostic Format internally
  • Loading branch information
haya14busa committed Jul 21, 2020
2 parents 0988ded + 994322d commit e74b6b8
Show file tree
Hide file tree
Showing 23 changed files with 463 additions and 187 deletions.
8 changes: 5 additions & 3 deletions cmd/reviewdog/doghouse.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,12 @@ func postResultSet(ctx context.Context, resultSet *reviewdog.ResultMap,
}

func checkResultToAnnotation(c *reviewdog.CheckResult, wd, gitRelWd string) *doghouse.Annotation {
loc := c.Diagnostic.GetLocation()
// TODO(haya14busa): Pass diagnostic to annotation instead.
return &doghouse.Annotation{
Path: filepath.ToSlash(filepath.Join(gitRelWd, reviewdog.CleanPath(c.Path, wd))),
Line: c.Lnum,
Message: c.Message,
Path: filepath.ToSlash(filepath.Join(gitRelWd, reviewdog.CleanPath(loc.GetPath(), wd))),
Line: int(loc.GetRange().GetStart().GetLine()),
Message: c.Diagnostic.GetMessage(),
RawMessage: strings.Join(c.Lines, "\n"),
}
}
Expand Down
72 changes: 51 additions & 21 deletions cmd/reviewdog/doghouse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@ import (

"github.com/google/go-cmp/cmp"
"golang.org/x/oauth2"
"google.golang.org/protobuf/testing/protocmp"

"github.com/reviewdog/reviewdog"
"github.com/reviewdog/reviewdog/cienv"
"github.com/reviewdog/reviewdog/difffilter"
"github.com/reviewdog/reviewdog/doghouse"
"github.com/reviewdog/reviewdog/doghouse/client"
"github.com/reviewdog/reviewdog/project"
"github.com/reviewdog/reviewdog/proto/rdf"
)

func setupEnvs(testEnvs map[string]string) (cleanup func()) {
Expand Down Expand Up @@ -121,10 +123,16 @@ func TestCheckResultSet_Project(t *testing.T) {
var wantCheckResult reviewdog.ResultMap
wantCheckResult.Store("name1", &reviewdog.Result{CheckResults: []*reviewdog.CheckResult{
{
Lnum: 1,
Col: 14,
Message: "msg",
Path: "reviewdog.go",
Diagnostic: &rdf.Diagnostic{
Location: &rdf.Location{
Range: &rdf.Range{Start: &rdf.Position{
Line: 1,
Column: 14,
}},
Path: "reviewdog.go",
},
Message: "msg",
},
},
}})

Expand All @@ -148,7 +156,7 @@ func TestCheckResultSet_Project(t *testing.T) {
}
got.Range(func(k string, r *reviewdog.Result) {
w, _ := wantCheckResult.Load(k)
if diff := cmp.Diff(r, w); diff != "" {
if diff := cmp.Diff(r, w, protocmp.Transform()); diff != "" {
t.Errorf("result has diff:\n%s", diff)
}
})
Expand All @@ -166,11 +174,17 @@ func TestCheckResultSet_NonProject(t *testing.T) {
var want reviewdog.ResultMap
want.Store("golint", &reviewdog.Result{CheckResults: []*reviewdog.CheckResult{
{
Lnum: 14,
Col: 14,
Message: "test message",
Path: "reviewdog.go",
Lines: []string{input},
Diagnostic: &rdf.Diagnostic{
Location: &rdf.Location{
Range: &rdf.Range{Start: &rdf.Position{
Line: 14,
Column: 14,
}},
Path: "reviewdog.go",
},
Message: "test message",
},
Lines: []string{input},
},
}})

Expand All @@ -179,7 +193,7 @@ func TestCheckResultSet_NonProject(t *testing.T) {
}
got.Range(func(k string, r *reviewdog.Result) {
w, _ := want.Load(k)
if diff := cmp.Diff(r, w); diff != "" {
if diff := cmp.Diff(r, w, protocmp.Transform()); diff != "" {
t.Errorf("result has diff:\n%s", diff)
}
})
Expand Down Expand Up @@ -252,21 +266,37 @@ func TestPostResultSet_withReportURL(t *testing.T) {
var resultSet reviewdog.ResultMap
resultSet.Store("name1", &reviewdog.Result{CheckResults: []*reviewdog.CheckResult{
{
Lnum: 14,
Message: "name1: test 1",
Path: "reviewdog.go", // test relative path
Lines: []string{"L1", "L2"},
Diagnostic: &rdf.Diagnostic{
Location: &rdf.Location{
Range: &rdf.Range{Start: &rdf.Position{
Line: 14,
}},
Path: "reviewdog.go", // test relative path
},
Message: "name1: test 1",
},
Lines: []string{"L1", "L2"},
},
{
Message: "name1: test 2",
Path: absPath(t, "reviewdog.go"), // test abs path
Diagnostic: &rdf.Diagnostic{
Location: &rdf.Location{
Path: absPath(t, "reviewdog.go"), // test abs path
},
Message: "name1: test 2",
},
},
}})
resultSet.Store("name2", &reviewdog.Result{CheckResults: []*reviewdog.CheckResult{
{
Lnum: 14,
Message: "name2: test 1",
Path: "doghouse.go",
Diagnostic: &rdf.Diagnostic{
Location: &rdf.Location{
Range: &rdf.Range{Start: &rdf.Position{
Line: 14,
}},
Path: "doghouse.go",
},
Message: "name2: test 1",
},
},
}})

Expand Down Expand Up @@ -314,7 +344,7 @@ func TestPostResultSet_withoutReportURL(t *testing.T) {
if err != nil {
t.Fatalf("should have result for name1: %v", err)
}
if diff := cmp.Diff(results.FilteredCheck, wantResults); diff != "" {
if diff := cmp.Diff(results.FilteredCheck, wantResults, protocmp.Transform()); diff != "" {
t.Errorf("results has diff:\n%s", diff)
}
}
Expand Down
12 changes: 7 additions & 5 deletions comment_iowriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,13 @@ func NewUnifiedCommentWriter(w io.Writer) *UnifiedCommentWriter {
}

func (mc *UnifiedCommentWriter) Post(_ context.Context, c *Comment) error {
s := c.Result.Path
if c.Result.Lnum > 0 {
s += fmt.Sprintf(":%d", c.Result.Lnum)
if c.Result.Col > 0 {
s += fmt.Sprintf(":%d", c.Result.Col)
loc := c.Result.Diagnostic.GetLocation()
s := loc.GetPath()
start := loc.GetRange().GetStart()
if start.GetLine() > 0 {
s += fmt.Sprintf(":%d", start.GetLine())
if start.GetColumn() > 0 {
s += fmt.Sprintf(":%d", start.GetColumn())
}
}
s += fmt.Sprintf(": [%s] %s", c.ToolName, c.Body)
Expand Down
30 changes: 22 additions & 8 deletions comment_iowriter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"context"
"strings"
"testing"

"github.com/reviewdog/reviewdog/proto/rdf"
)

func TestUnifiedCommentWriter_Post(t *testing.T) {
Expand All @@ -15,7 +17,7 @@ func TestUnifiedCommentWriter_Post(t *testing.T) {
{
in: &Comment{
Result: &FilteredCheck{CheckResult: &CheckResult{
Path: "/path/to/file",
Diagnostic: &rdf.Diagnostic{Location: &rdf.Location{Path: "/path/to/file"}},
}},
ToolName: "tool name",
Body: "message",
Expand All @@ -25,8 +27,12 @@ func TestUnifiedCommentWriter_Post(t *testing.T) {
{
in: &Comment{
Result: &FilteredCheck{CheckResult: &CheckResult{
Path: "/path/to/file",
Col: 14,
Diagnostic: &rdf.Diagnostic{Location: &rdf.Location{
Path: "/path/to/file",
Range: &rdf.Range{Start: &rdf.Position{
Column: 14,
}},
}},
}},
ToolName: "tool name",
Body: "message",
Expand All @@ -36,8 +42,12 @@ func TestUnifiedCommentWriter_Post(t *testing.T) {
{
in: &Comment{
Result: &FilteredCheck{CheckResult: &CheckResult{
Path: "/path/to/file",
Lnum: 14,
Diagnostic: &rdf.Diagnostic{Location: &rdf.Location{
Path: "/path/to/file",
Range: &rdf.Range{Start: &rdf.Position{
Line: 14,
}},
}},
}},
ToolName: "tool name",
Body: "message",
Expand All @@ -47,9 +57,13 @@ func TestUnifiedCommentWriter_Post(t *testing.T) {
{
in: &Comment{
Result: &FilteredCheck{CheckResult: &CheckResult{
Path: "/path/to/file",
Lnum: 14,
Col: 7,
Diagnostic: &rdf.Diagnostic{Location: &rdf.Location{
Path: "/path/to/file",
Range: &rdf.Range{Start: &rdf.Position{
Line: 14,
Column: 7,
}},
}},
}},
ToolName: "tool name",
Body: "line1\nline2",
Expand Down
32 changes: 23 additions & 9 deletions doghouse/server/doghouse.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/reviewdog/reviewdog/diff"
"github.com/reviewdog/reviewdog/difffilter"
"github.com/reviewdog/reviewdog/doghouse"
"github.com/reviewdog/reviewdog/proto/rdf"
"github.com/reviewdog/reviewdog/service/github/githubutils"
)

Expand Down Expand Up @@ -218,15 +219,20 @@ func (ch *Checker) summaryFindings(name string, checks []*reviewdog.FilteredChec
}

func (ch *Checker) toCheckRunAnnotation(c *reviewdog.FilteredCheck) *github.CheckRunAnnotation {
loc := c.Diagnostic.GetLocation()
endLine := loc.GetRange().GetEnd().GetLine()
if endLine == 0 {
endLine = loc.GetRange().GetStart().GetLine()
}
a := &github.CheckRunAnnotation{
Path: github.String(c.Path),
StartLine: github.Int(c.Lnum),
EndLine: github.Int(c.Lnum),
Path: github.String(loc.GetPath()),
StartLine: github.Int(int(loc.GetRange().GetStart().GetLine())),
EndLine: github.Int(int(endLine)),
AnnotationLevel: github.String(ch.annotationLevel()),
Message: github.String(c.Message),
Message: github.String(c.Diagnostic.GetMessage()),
}
if ch.req.Name != "" {
a.Title = github.String(fmt.Sprintf("[%s] %s#L%d", ch.req.Name, c.Path, c.Lnum))
a.Title = github.String(fmt.Sprintf("[%s] %s#L%d", ch.req.Name, loc.GetPath(), loc.GetRange().GetStart().GetLine()))
}
if s := strings.Join(c.Lines, "\n"); s != "" {
a.RawDetails = github.String(s)
Expand Down Expand Up @@ -258,10 +264,18 @@ func annotationsToCheckResults(as []*doghouse.Annotation) []*reviewdog.CheckResu
cs := make([]*reviewdog.CheckResult, 0, len(as))
for _, a := range as {
cs = append(cs, &reviewdog.CheckResult{
Path: a.Path,
Lnum: a.Line,
Message: a.Message,
Lines: strings.Split(a.RawMessage, "\n"),
Diagnostic: &rdf.Diagnostic{
Location: &rdf.Location{
Path: a.Path,
Range: &rdf.Range{
Start: &rdf.Position{
Line: int32(a.Line),
},
},
},
Message: a.Message,
},
Lines: strings.Split(a.RawMessage, "\n"),
})
}
return cs
Expand Down
2 changes: 2 additions & 0 deletions doghouse/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,6 @@ type Annotation struct {
// Original error message of this annotation.
// Optional.
RawMessage string `json:"raw_message,omitempty"`

// TODO(haya14busa): Support RDFormat.
}
8 changes: 5 additions & 3 deletions filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,17 @@ func FilterCheck(results []*CheckResult, diff []*diff.FileDiff, strip int,
df := difffilter.New(diff, strip, cwd, mode)
for _, result := range results {
check := &FilteredCheck{CheckResult: result}
shouldReport, difffile, diffline := df.ShouldReport(result.Path, result.Lnum)
loc := result.Diagnostic.GetLocation()
lnum := int(loc.GetRange().GetStart().GetLine())
shouldReport, difffile, diffline := df.ShouldReport(loc.GetPath(), lnum)
check.ShouldReport = shouldReport
if diffline != nil {
check.LnumDiff = diffline.LnumDiff
}
result.Path = CleanPath(result.Path, cwd)
loc.Path = CleanPath(loc.GetPath(), cwd)
if difffile != nil {
check.InDiffFile = true
check.OldPath, check.OldLine = getOldPosition(difffile, strip, result.Path, result.Lnum)
check.OldPath, check.OldLine = getOldPosition(difffile, strip, loc.GetPath(), lnum)
}
checks = append(checks, check)
}
Expand Down

0 comments on commit e74b6b8

Please sign in to comment.