Skip to content

Commit

Permalink
Fix gotype partition strategy (alecthomas#353)
Browse files Browse the repository at this point in the history
* Extract issue type into a package

It can be re-used by regresiontests
Includes some refactoring of regressiontests/support.go

Signed-off-by: Daniel Nephin <dnephin@gmail.com>

* Fix sorting with multiple keys

Signed-off-by: Daniel Nephin <dnephin@gmail.com>

* Refactor regressiontests to support testing multiple packages

Signed-off-by: Daniel Nephin <dnephin@gmail.com>

* Drop issues package

Sort issues when running regressiontests

Signed-off-by: Daniel Nephin <dnephin@gmail.com>

* Fix partition strategy for gotype

Also test against multiple packages

Signed-off-by: Daniel Nephin <dnephin@gmail.com>
  • Loading branch information
dnephin authored and alecthomas committed Sep 23, 2017
1 parent bfcc1d6 commit 3eeb3c6
Show file tree
Hide file tree
Showing 14 changed files with 257 additions and 188 deletions.
30 changes: 12 additions & 18 deletions aggregate.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,21 @@ import (
"strings"
)

type (
issueKey struct {
path string
line, col int
message string
}

multiIssue struct {
*Issue
linterNames []string
}
)
type issueKey struct {
path string
line, col int
message string
}

func maybeAggregateIssues(issues chan *Issue) chan *Issue {
if !config.Aggregate {
return issues
}
return aggregateIssues(issues)
type multiIssue struct {
*Issue
linterNames []string
}

func aggregateIssues(issues chan *Issue) chan *Issue {
// AggregateIssueChan reads issues from a channel, aggregates issues which have
// the same file, line, vol, and message, and returns aggregated issues on
// a new channel.
func AggregateIssueChan(issues chan *Issue) chan *Issue {
out := make(chan *Issue, 1000000)
issueMap := make(map[issueKey]*multiIssue)
go func() {
Expand Down
2 changes: 1 addition & 1 deletion checkstyle.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"encoding/xml"
"fmt"

"gopkg.in/alecthomas/kingpin.v3-unstable"
kingpin "gopkg.in/alecthomas/kingpin.v3-unstable"
)

type checkstyleOutput struct {
Expand Down
7 changes: 3 additions & 4 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ type Config struct { // nolint: aligncheck
EnableGC bool
Aggregate bool
EnableAll bool

formatTemplate *template.Template
}

type StringOrLinterConfig LinterConfig
Expand Down Expand Up @@ -93,14 +95,11 @@ func (td *jsonDuration) Duration() time.Duration {
return time.Duration(*td)
}

// TODO: should be a field on Config struct
var formatTemplate = &template.Template{}

var sortKeys = []string{"none", "path", "line", "column", "severity", "message", "linter"}

// Configuration defaults.
var config = &Config{
Format: "{{.Path}}:{{.Line}}:{{if .Col}}{{.Col}}{{end}}:{{.Severity}}: {{.Message}} ({{.Linter}})",
Format: DefaultIssueFormat,

Linters: map[string]StringOrLinterConfig{},
Severity: map[string]string{
Expand Down
96 changes: 11 additions & 85 deletions execute.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,13 @@ import (
"path/filepath"
"reflect"
"regexp"
"sort"
"strconv"
"strings"
"sync"
"time"

"github.com/google/shlex"
"gopkg.in/alecthomas/kingpin.v3-unstable"
kingpin "gopkg.in/alecthomas/kingpin.v3-unstable"
)

type Vars map[string]string
Expand All @@ -41,31 +40,6 @@ func (v Vars) Replace(s string) string {
return s
}

// Severity of linter message.
type Severity string

// Linter message severity levels.
const ( // nolint: deadcode
Error Severity = "error"
Warning Severity = "warning"
)

type Issue struct {
Linter string `json:"linter"`
Severity Severity `json:"severity"`
Path string `json:"path"`
Line int `json:"line"`
Col int `json:"col"`
Message string `json:"message"`
}

func (i *Issue) String() string {
buf := new(bytes.Buffer)
err := formatTemplate.Execute(buf, i)
kingpin.FatalIfError(err, "Invalid output format")
return buf.String()
}

type linterState struct {
*Linter
id int
Expand Down Expand Up @@ -243,7 +217,9 @@ func processOutput(dbg debugFunction, state *linterState, out []byte) {
group = append(group, fragment)
}

issue := &Issue{Line: 1, Linter: state.Linter.Name}
issue, err := NewIssue(state.Linter.Name, config.formatTemplate)
kingpin.FatalIfError(err, "Invalid output format")

for i, name := range re.SubexpNames() {
if group[i] == nil {
continue
Expand Down Expand Up @@ -323,66 +299,16 @@ func resolvePath(path string) string {
return path
}

type sortedIssues struct {
issues []*Issue
order []string
}

func (s *sortedIssues) Len() int { return len(s.issues) }
func (s *sortedIssues) Swap(i, j int) { s.issues[i], s.issues[j] = s.issues[j], s.issues[i] }

// nolint: gocyclo
func (s *sortedIssues) Less(i, j int) bool {
l, r := s.issues[i], s.issues[j]
for _, key := range s.order {
switch key {
case "path":
if l.Path > r.Path {
return false
}
case "line":
if l.Line > r.Line {
return false
}
case "column":
if l.Col > r.Col {
return false
}
case "severity":
if l.Severity > r.Severity {
return false
}
case "message":
if l.Message > r.Message {
return false
}
case "linter":
if l.Linter > r.Linter {
return false
}
}
}
return true
}

func maybeSortIssues(issues chan *Issue) chan *Issue {
if reflect.DeepEqual([]string{"none"}, config.Sort) {
return issues
}
out := make(chan *Issue, 1000000)
sorted := &sortedIssues{
issues: []*Issue{},
order: config.Sort,
return SortIssueChan(issues, config.Sort)
}

func maybeAggregateIssues(issues chan *Issue) chan *Issue {
if !config.Aggregate {
return issues
}
go func() {
for issue := range issues {
sorted.issues = append(sorted.issues, issue)
}
sort.Sort(sorted)
for _, issue := range sorted.issues {
out <- issue
}
close(out)
}()
return out
return AggregateIssueChan(issues)
}
29 changes: 0 additions & 29 deletions execute_test.go

This file was deleted.

113 changes: 113 additions & 0 deletions issue.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
package main

import (
"bytes"
"fmt"
"io/ioutil"
"sort"
"strings"
"text/template"
)

// DefaultIssueFormat used to print an issue
const DefaultIssueFormat = "{{.Path}}:{{.Line}}:{{if .Col}}{{.Col}}{{end}}:{{.Severity}}: {{.Message}} ({{.Linter}})"

// Severity of linter message
type Severity string

// Linter message severity levels.
const ( // nolint: deadcode
Error Severity = "error"
Warning Severity = "warning"
)

type Issue struct {
Linter string `json:"linter"`
Severity Severity `json:"severity"`
Path string `json:"path"`
Line int `json:"line"`
Col int `json:"col"`
Message string `json:"message"`
formatTmpl *template.Template
}

// NewIssue returns a new issue. Returns an error if formatTmpl is not a valid
// template for an Issue.
func NewIssue(linter string, formatTmpl *template.Template) (*Issue, error) {
issue := &Issue{
Line: 1,
Linter: linter,
formatTmpl: formatTmpl,
}
err := formatTmpl.Execute(ioutil.Discard, issue)
return issue, err
}

func (i *Issue) String() string {
if i.formatTmpl == nil {
col := ""
if i.Col != 0 {
col = fmt.Sprintf("%d", i.Col)
}
return fmt.Sprintf("%s:%d:%s:%s: %s (%s)", strings.TrimSpace(i.Path), i.Line, col, i.Severity, strings.TrimSpace(i.Message), i.Linter)
}
buf := new(bytes.Buffer)
_ = i.formatTmpl.Execute(buf, i)
return buf.String()
}

type sortedIssues struct {
issues []*Issue
order []string
}

func (s *sortedIssues) Len() int { return len(s.issues) }
func (s *sortedIssues) Swap(i, j int) { s.issues[i], s.issues[j] = s.issues[j], s.issues[i] }

func (s *sortedIssues) Less(i, j int) bool {
l, r := s.issues[i], s.issues[j]
return CompareIssue(*l, *r, s.order)
}

// CompareIssue two Issues and return true if left should sort before right
// nolint: gocyclo
func CompareIssue(l, r Issue, order []string) bool {
for _, key := range order {
switch {
case key == "path" && l.Path != r.Path:
return l.Path < r.Path
case key == "line" && l.Line != r.Line:
return l.Line < r.Line
case key == "column" && l.Col != r.Col:
return l.Col < r.Col
case key == "severity" && l.Severity != r.Severity:
return l.Severity < r.Severity
case key == "message" && l.Message != r.Message:
return l.Message < r.Message
case key == "linter" && l.Linter != r.Linter:
return l.Linter < r.Linter
}
}
return true
}

// SortIssueChan reads issues from one channel, sorts them, and returns them to another
// channel
func SortIssueChan(issues chan *Issue, order []string) chan *Issue {
out := make(chan *Issue, 1000000)
sorted := &sortedIssues{
issues: []*Issue{},
order: order,
}
go func() {
for issue := range issues {
sorted.issues = append(sorted.issues, issue)
}
sort.Sort(sorted)
for _, issue := range sorted.issues {
out <- issue
}
close(out)
}()
return out
}
39 changes: 39 additions & 0 deletions issue_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package main

import (
"sort"
"testing"

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

func TestSortedIssues(t *testing.T) {
actual := []*Issue{
{Path: "b.go", Line: 5, Col: 1},
{Path: "a.go", Line: 3, Col: 2},
{Path: "b.go", Line: 1, Col: 3},
{Path: "a.go", Line: 1, Col: 4},
}
issues := &sortedIssues{
issues: actual,
order: []string{"path", "line", "column"},
}
sort.Sort(issues)
expected := []*Issue{
{Path: "a.go", Line: 1, Col: 4},
{Path: "a.go", Line: 3, Col: 2},
{Path: "b.go", Line: 1, Col: 3},
{Path: "b.go", Line: 5, Col: 1},
}
require.Equal(t, expected, actual)
}

func TestCompareOrderWithMessage(t *testing.T) {
order := []string{"path", "line", "column", "message"}
issueM := Issue{Path: "file.go", Message: "message"}
issueU := Issue{Path: "file.go", Message: "unknown"}

assert.True(t, CompareIssue(issueM, issueU, order))
assert.False(t, CompareIssue(issueU, issueM, order))
}
Loading

0 comments on commit 3eeb3c6

Please sign in to comment.