Skip to content

Commit

Permalink
Merge pull request #694 from reviewdog/rdf-parser-support-impr
Browse files Browse the repository at this point in the history
parser: Improve RD Format support
  • Loading branch information
haya14busa committed Jul 25, 2020
2 parents 47f7450 + f6e64bd commit 3872614
Show file tree
Hide file tree
Showing 5 changed files with 263 additions and 30 deletions.
15 changes: 10 additions & 5 deletions parser/checkstyle.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ func (p *CheckStyleParser) Parse(r io.Reader) ([]*rdf.Diagnostic, error) {
if err := xml.NewDecoder(r).Decode(cs); err != nil {
return nil, err
}
var rs []*rdf.Diagnostic
var ds []*rdf.Diagnostic
for _, file := range cs.Files {
for _, cerr := range file.Errors {
rs = append(rs, &rdf.Diagnostic{
d := &rdf.Diagnostic{
Location: &rdf.Location{
Path: file.Name,
Range: &rdf.Range{
Expand All @@ -36,13 +36,18 @@ func (p *CheckStyleParser) Parse(r io.Reader) ([]*rdf.Diagnostic, error) {
},
},
},
Message: cerr.Message,
Message: cerr.Message,
Severity: severity(cerr.Severity),
OriginalOutput: fmt.Sprintf("%v:%d:%d: %v: %v (%v)",
file.Name, cerr.Line, cerr.Column, cerr.Severity, cerr.Message, cerr.Source),
})
}
if s := cerr.Source; s != "" {
d.Code = &rdf.Code{Value: s}
}
ds = append(ds, d)
}
}
return rs, nil
return ds, nil
}

// CheckStyleResult represents checkstyle XML result.
Expand Down
186 changes: 166 additions & 20 deletions parser/checkstyle_test.go
Original file line number Diff line number Diff line change
@@ -1,34 +1,180 @@
package parser

import (
"bytes"
"encoding/json"
"fmt"
"strings"
"testing"

"google.golang.org/protobuf/encoding/protojson"
)

func TestCheckStyleParser(t *testing.T) {
func ExampleCheckStyleParser() {
const sample = `<?xml version="1.0" encoding="utf-8"?><checkstyle version="4.3"><file name="/path/to/file"><error line="1" column="10" severity="error" message="&apos;addOne&apos; is defined but never used. (no-unused-vars)" source="eslint.rules.no-unused-vars" /><error line="2" column="9" severity="error" message="Use the isNaN function to compare with NaN. (use-isnan)" source="eslint.rules.use-isnan" /><error line="3" column="16" severity="error" message="Unexpected space before unary operator &apos;++&apos;. (space-unary-ops)" source="eslint.rules.space-unary-ops" /><error line="3" column="20" severity="warning" message="Missing semicolon. (semi)" source="eslint.rules.semi" /><error line="4" column="12" severity="warning" message="Unnecessary &apos;else&apos; after &apos;return&apos;. (no-else-return)" source="eslint.rules.no-else-return" /><error line="5" column="7" severity="warning" message="Expected indentation of 8 spaces but found 6. (indent)" source="eslint.rules.indent" /><error line="5" column="7" severity="error" message="Expected a return value. (consistent-return)" source="eslint.rules.consistent-return" /><error line="5" column="13" severity="warning" message="Missing semicolon. (semi)" source="eslint.rules.semi" /><error line="7" column="2" severity="error" message="Unnecessary semicolon. (no-extra-semi)" source="eslint.rules.no-extra-semi" /></file></checkstyle>`

wants := []string{

"/path/to/file:1:10: error: 'addOne' is defined but never used. (no-unused-vars) (eslint.rules.no-unused-vars)",
"/path/to/file:2:9: error: Use the isNaN function to compare with NaN. (use-isnan) (eslint.rules.use-isnan)",
"/path/to/file:3:16: error: Unexpected space before unary operator '++'. (space-unary-ops) (eslint.rules.space-unary-ops)",
"/path/to/file:3:20: warning: Missing semicolon. (semi) (eslint.rules.semi)",
"/path/to/file:4:12: warning: Unnecessary 'else' after 'return'. (no-else-return) (eslint.rules.no-else-return)",
"/path/to/file:5:7: warning: Expected indentation of 8 spaces but found 6. (indent) (eslint.rules.indent)",
"/path/to/file:5:7: error: Expected a return value. (consistent-return) (eslint.rules.consistent-return)",
"/path/to/file:5:13: warning: Missing semicolon. (semi) (eslint.rules.semi)",
"/path/to/file:7:2: error: Unnecessary semicolon. (no-extra-semi) (eslint.rules.no-extra-semi)",
}

p := NewCheckStyleParser()
diagnostics, err := p.Parse(strings.NewReader(sample))
if err != nil {
t.Error(err)
panic(err)
}
for i, d := range diagnostics {
if got, want := d.GetOriginalOutput(), wants[i]; got != want {
t.Errorf("%d: got %v, want %v", i, got, want)
}
for _, d := range diagnostics {
rdjson, _ := protojson.MarshalOptions{Indent: " "}.Marshal(d)
var out bytes.Buffer
json.Indent(&out, rdjson, "", " ")
fmt.Println(out.String())
}
// Output:
// {
// "message": "'addOne' is defined but never used. (no-unused-vars)",
// "location": {
// "path": "/path/to/file",
// "range": {
// "start": {
// "line": 1,
// "column": 10
// }
// }
// },
// "severity": "ERROR",
// "code": {
// "value": "eslint.rules.no-unused-vars"
// },
// "originalOutput": "/path/to/file:1:10: error: 'addOne' is defined but never used. (no-unused-vars) (eslint.rules.no-unused-vars)"
// }
// {
// "message": "Use the isNaN function to compare with NaN. (use-isnan)",
// "location": {
// "path": "/path/to/file",
// "range": {
// "start": {
// "line": 2,
// "column": 9
// }
// }
// },
// "severity": "ERROR",
// "code": {
// "value": "eslint.rules.use-isnan"
// },
// "originalOutput": "/path/to/file:2:9: error: Use the isNaN function to compare with NaN. (use-isnan) (eslint.rules.use-isnan)"
// }
// {
// "message": "Unexpected space before unary operator '++'. (space-unary-ops)",
// "location": {
// "path": "/path/to/file",
// "range": {
// "start": {
// "line": 3,
// "column": 16
// }
// }
// },
// "severity": "ERROR",
// "code": {
// "value": "eslint.rules.space-unary-ops"
// },
// "originalOutput": "/path/to/file:3:16: error: Unexpected space before unary operator '++'. (space-unary-ops) (eslint.rules.space-unary-ops)"
// }
// {
// "message": "Missing semicolon. (semi)",
// "location": {
// "path": "/path/to/file",
// "range": {
// "start": {
// "line": 3,
// "column": 20
// }
// }
// },
// "severity": "WARNING",
// "code": {
// "value": "eslint.rules.semi"
// },
// "originalOutput": "/path/to/file:3:20: warning: Missing semicolon. (semi) (eslint.rules.semi)"
// }
// {
// "message": "Unnecessary 'else' after 'return'. (no-else-return)",
// "location": {
// "path": "/path/to/file",
// "range": {
// "start": {
// "line": 4,
// "column": 12
// }
// }
// },
// "severity": "WARNING",
// "code": {
// "value": "eslint.rules.no-else-return"
// },
// "originalOutput": "/path/to/file:4:12: warning: Unnecessary 'else' after 'return'. (no-else-return) (eslint.rules.no-else-return)"
// }
// {
// "message": "Expected indentation of 8 spaces but found 6. (indent)",
// "location": {
// "path": "/path/to/file",
// "range": {
// "start": {
// "line": 5,
// "column": 7
// }
// }
// },
// "severity": "WARNING",
// "code": {
// "value": "eslint.rules.indent"
// },
// "originalOutput": "/path/to/file:5:7: warning: Expected indentation of 8 spaces but found 6. (indent) (eslint.rules.indent)"
// }
// {
// "message": "Expected a return value. (consistent-return)",
// "location": {
// "path": "/path/to/file",
// "range": {
// "start": {
// "line": 5,
// "column": 7
// }
// }
// },
// "severity": "ERROR",
// "code": {
// "value": "eslint.rules.consistent-return"
// },
// "originalOutput": "/path/to/file:5:7: error: Expected a return value. (consistent-return) (eslint.rules.consistent-return)"
// }
// {
// "message": "Missing semicolon. (semi)",
// "location": {
// "path": "/path/to/file",
// "range": {
// "start": {
// "line": 5,
// "column": 13
// }
// }
// },
// "severity": "WARNING",
// "code": {
// "value": "eslint.rules.semi"
// },
// "originalOutput": "/path/to/file:5:13: warning: Missing semicolon. (semi) (eslint.rules.semi)"
// }
// {
// "message": "Unnecessary semicolon. (no-extra-semi)",
// "location": {
// "path": "/path/to/file",
// "range": {
// "start": {
// "line": 7,
// "column": 2
// }
// }
// },
// "severity": "ERROR",
// "code": {
// "value": "eslint.rules.no-extra-semi"
// },
// "originalOutput": "/path/to/file:7:2: error: Unnecessary semicolon. (no-extra-semi) (eslint.rules.no-extra-semi)"
// }
}
14 changes: 10 additions & 4 deletions parser/errorformat.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package parser

import (
"fmt"
"io"
"strings"

Expand Down Expand Up @@ -32,11 +33,11 @@ func NewErrorformatParserString(efms []string) (*ErrorformatParser, error) {

func (p *ErrorformatParser) Parse(r io.Reader) ([]*rdf.Diagnostic, error) {
s := p.efm.NewScanner(r)
var rs []*rdf.Diagnostic
var ds []*rdf.Diagnostic
for s.Scan() {
e := s.Entry()
if e.Valid {
rs = append(rs, &rdf.Diagnostic{
d := &rdf.Diagnostic{
Location: &rdf.Location{
Path: e.Filename,
Range: &rdf.Range{
Expand All @@ -47,9 +48,14 @@ func (p *ErrorformatParser) Parse(r io.Reader) ([]*rdf.Diagnostic, error) {
},
},
Message: e.Text,
Severity: severity(string(e.Type)),
OriginalOutput: strings.Join(e.Lines, "\n"),
})
}
if e.Nr != 0 {
d.Code = &rdf.Code{Value: fmt.Sprintf("%d", e.Nr)}
}
ds = append(ds, d)
}
}
return rs, nil
return ds, nil
}
64 changes: 63 additions & 1 deletion parser/errorformat_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
package parser

import "testing"
import (
"bytes"
"encoding/json"
"fmt"
"strings"
"testing"

"google.golang.org/protobuf/encoding/protojson"
)

func TestNewErrorformatParserString(t *testing.T) {
in := []string{`%f:%l:%c:%m`, `%-G%.%#`}
Expand All @@ -14,3 +22,57 @@ func TestNewErrorformatParserString(t *testing.T) {
t.Errorf("NewErrorformatParserString: len: got %v, want %v", len(got.efm.Efms), len(in))
}
}
func ExampleErrorformatParser() {
const sample = `/path/to/file1.txt:1:14: [E][RULE:14] message 1
/path/to/file2.txt:2:14: [N][RULE:7] message 2`

p, err := NewErrorformatParserString([]string{`%f:%l:%c: [%t][RULE:%n] %m`})
if err != nil {
panic(err)
}
diagnostics, err := p.Parse(strings.NewReader(sample))
if err != nil {
panic(err)
}
for _, d := range diagnostics {
rdjson, _ := protojson.MarshalOptions{Indent: " "}.Marshal(d)
var out bytes.Buffer
json.Indent(&out, rdjson, "", " ")
fmt.Println(out.String())
}
// Output:
// {
// "message": "message 1",
// "location": {
// "path": "/path/to/file1.txt",
// "range": {
// "start": {
// "line": 1,
// "column": 14
// }
// }
// },
// "severity": "ERROR",
// "code": {
// "value": "14"
// },
// "originalOutput": "/path/to/file1.txt:1:14: [E][RULE:14] message 1"
// }
// {
// "message": "message 2",
// "location": {
// "path": "/path/to/file2.txt",
// "range": {
// "start": {
// "line": 2,
// "column": 14
// }
// }
// },
// "severity": "INFO",
// "code": {
// "value": "7"
// },
// "originalOutput": "/path/to/file2.txt:2:14: [N][RULE:7] message 2"
// }
}
14 changes: 14 additions & 0 deletions parser/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,17 @@ func New(opt *Option) (Parser, error) {
}
return NewErrorformatParserString(opt.Errorformat)
}

func severity(s string) rdf.Severity {
switch s {
case "error", "ERROR", "Error", "e", "E":
return rdf.Severity_ERROR
case "warning", "WARNING", "Warning", "w", "W":
return rdf.Severity_WARNING
case "info", "INFO", "Info", "i", "I",
"note", "NOTE", "Note", "n", "N": // Treat note as info.
return rdf.Severity_INFO
default:
return rdf.Severity_UNKNOWN_SEVERITY
}
}

0 comments on commit 3872614

Please sign in to comment.