Repository: googleapis/api-linter
Pull request #288 (2019-11-03 02:04:33)

Rewrote summary table.
Used the [`tablewriter`](https://github.com/olekukonko/tablewriter) package.

Run `api-linter --output_format summary test.proto` and produce:

```text
+------------------------------------+------------------+----------------+
|                RULE                | TOTAL VIOLATIONS | VIOLATED FILES |
+------------------------------------+------------------+----------------+
| core::0131::request-name-field     |                1 |              1 |
| core::0192::comments::only-leading |                1 |              1 |
| core::0131::request-unknown-fields |                1 |              1 |
| core::0192::comments::present      |                6 |              1 |
+------------------------------------+------------------+----------------+
Linted 1 proto files
```

The printed table is sorted by `TOTAL VIOLATIONS`, assume that people are interested in knowing which rule their protos violate most.


Patch:
diff --git a/cmd/api-linter/cli.go b/cmd/api-linter/cli.go
index b3dfacf..9b31e7e 100644
--- a/cmd/api-linter/cli.go
+++ b/cmd/api-linter/cli.go
@@ -54,7 +54,7 @@ func newCli(args []string) *cli {
 	// Register flag variables.
 	fs := flag.NewFlagSet("api-linter", flag.ExitOnError)
 	fs.StringVar(&cfgFlag, "config", "", "The linter config file.")
-	fs.StringVar(&fmtFlag, "output_format", "", "The format of the linting results.\nSupported formats include YAML, JSON and summary text.\nYAML is the default.")
+	fs.StringVar(&fmtFlag, "output_format", "", "The format of the linting results.\nSupported formats include \"yaml\", \"json\" and \"summary\" table.\nYAML is the default.")
 	fs.StringVar(&outFlag, "output_path", "", "The output file path.\nIf not given, the linting results will be printed out to STDOUT.")
 	fs.Var(&protoImportFlag, "proto_path", "The folder for searching proto imports.\nMay be specified multiple times; directories will be searched in order.\nThe current working directory is always used.")
 	fs.StringVar(&protoDescFlag, "proto_descriptor_set", "", "A delimited (':') list of files each containing a FileDescriptorSet for searching proto imports.")
@@ -159,7 +159,7 @@ func (c *cli) lint(rules lint.RuleRegistry, configs lint.Configs) error {
 		marshal = json.Marshal
 	case "summary":
 		marshal = func(i interface{}) ([]byte, error) {
-			return emitSummary(i.([]lint.Response))
+			return printSummaryTable(i.([]lint.Response))
 		}
 	}
 
diff --git a/cmd/api-linter/summary.go b/cmd/api-linter/summary.go
index 78cba66..1be9db6 100644
--- a/cmd/api-linter/summary.go
+++ b/cmd/api-linter/summary.go
@@ -16,86 +16,60 @@ package main
 
 import (
 	"bytes"
-	"strconv"
-	"text/template"
+	"fmt"
+	"sort"
 
 	"github.com/googleapis/api-linter/lint"
+	"github.com/olekukonko/tablewriter"
 )
 
-// emitSummary will return a []byte that shows the percentage of files failing for each rule
-func emitSummary(responses []lint.Response) ([]byte, error) {
-	var buffer bytes.Buffer
-	summary := createSummary(responses)
-	defaultSummaryColWidth := 25
-	colOneFormat := "%-" + strconv.Itoa(max(summary.LongestRuleLen+5, defaultSummaryColWidth)) + "s"
-	colTwoFormat := "%" + strconv.Itoa(defaultSummaryColWidth) + "s"
+// printSummaryTable returns a summary table of violation counts.
+func printSummaryTable(responses []lint.Response) ([]byte, error) {
+	s := createSummary(responses)
 
-	lintSummaryTemplate, err := template.New("lintSummary").Funcs(
-		template.FuncMap{"calcPercentage": func(filePaths map[string]bool, numSourceFiles int) float64 {
-			return float64(len(filePaths)) / float64(numSourceFiles) * 100
-		}}).Parse(`
-----------SUMMARY TABLE---------
-{{ printf "Linted %d proto files." .Summary.LongestRuleLen | printf .ColOneFormat }}
-{{ printf .ColOneFormat "Rule"}} {{ printf .ColTwoFormat "Violations (Percent)" -}}
-{{$colOneFormat := .ColOneFormat -}}
-{{$colTwoFormat := .ColTwoFormat -}}
-{{$numSourceFiles := .Summary.NumSourceFiles}}
-{{range $ruleID, $filePaths := .Summary.Violations -}}
-{{printf 	$colOneFormat $ruleID}}{{printf "%d (%.2f%%)" (len $filePaths) (calcPercentage $filePaths $numSourceFiles) | printf $colTwoFormat}}
-{{end}}
-`)
-	if err != nil {
-		panic(err)
+	data := []summary{}
+	for ruleID, fileViolations := range s {
+		totalViolations := 0
+		for _, count := range fileViolations {
+			totalViolations += count
+		}
+		data = append(data, summary{ruleID, totalViolations, len(fileViolations)})
 	}
-	templateData := LintSummaryTemplateData{colOneFormat, colTwoFormat, summary}
-	err = lintSummaryTemplate.Execute(&buffer, templateData)
-	if err != nil {
-		panic(err)
+	sort.SliceStable(data, func(i, j int) bool { return data[i].violations < data[j].violations })
+
+	var buf bytes.Buffer
+	table := tablewriter.NewWriter(&buf)
+	table.SetHeader([]string{"Rule", "Total Violations", "Violated Files"})
+	table.SetCaption(true, fmt.Sprintf("Linted %d proto files", len(responses)))
+	for _, d := range data {
+		table.Append([]string{
+			d.ruleID,
+			fmt.Sprintf("%d", d.violations),
+			fmt.Sprintf("%d", d.files),
+		})
 	}
-	return buffer.Bytes(), nil
+	table.Render()
+
+	return buf.Bytes(), nil
 }
 
-func createSummary(responses []lint.Response) (summary LintSummary) {
-	summary.NumSourceFiles = len(responses)
-	summary.Violations = make(map[string]map[string]bool)
-	for _, response := range responses {
-		pathToAdd := string(response.FilePath)
-		problems := response.Problems
-		for _, currentProb := range problems {
-			ruleName := string(currentProb.RuleID)
-			if existingPaths, ok := summary.Violations[ruleName]; ok {
-				if _, isExist := existingPaths[pathToAdd]; !isExist {
-					existingPaths[pathToAdd] = true
-				}
-			} else {
-				summary.LongestRuleLen = max(summary.LongestRuleLen, len(ruleName))
-				summary.Violations[ruleName] = map[string]bool{pathToAdd: true}
+func createSummary(responses []lint.Response) map[string]map[string]int {
+	summary := make(map[string]map[string]int)
+	for _, r := range responses {
+		filePath := string(r.FilePath)
+		for _, p := range r.Problems {
+			ruleID := string(p.RuleID)
+			if summary[ruleID] == nil {
+				summary[ruleID] = make(map[string]int)
 			}
+			summary[ruleID][filePath]++
 		}
 	}
-	return
-}
-
-// LintSummary summarizes a lint run, including which files have violations.
-type LintSummary struct {
-	// key = rule_id, value = set of unique files that violated rule
-	Violations map[string]map[string]bool
-	// length of the rule_id of the longest rule added
-	LongestRuleLen int
-	// count of files from the original source.
-	NumSourceFiles int
-}
-
-// LintSummaryTemplateData provides formatting data for the file-level report.
-type LintSummaryTemplateData struct {
-	ColOneFormat string
-	ColTwoFormat string
-	Summary      LintSummary
+	return summary
 }
 
-func max(x, y int) int {
-	if x < y {
-		return y
-	}
-	return x
+type summary struct {
+	ruleID     string
+	violations int
+	files      int
 }
diff --git a/go.mod b/go.mod
index cfb83f4..e29b039 100644
--- a/go.mod
+++ b/go.mod
@@ -10,8 +10,9 @@ require (
 	github.com/google/go-cmp v0.3.1
 	github.com/jhump/protoreflect v1.5.1-0.20191024142022-1d0024204e58
 	github.com/lithammer/dedent v1.1.0
+	github.com/mattn/go-runewidth v0.0.5 // indirect
+	github.com/olekukonko/tablewriter v0.0.1
 	github.com/stoewer/go-strcase v1.0.2
-	github.com/urfave/cli v1.22.1
 	google.golang.org/genproto v0.0.0-20191028173616-919d9bdd9fe6
 	gopkg.in/yaml.v2 v2.2.4
 )
diff --git a/go.sum b/go.sum
index 4d0cce0..966fb43 100644
--- a/go.sum
+++ b/go.sum
@@ -5,8 +5,6 @@ github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03
 github.com/bmatcuk/doublestar v1.1.5 h1:2bNwBOmhyFEFcoB3tGvTD5xanq+4kyOZlB8wFYbMjkk=
 github.com/bmatcuk/doublestar v1.1.5/go.mod h1:wiQtGV+rzVYxB7WIlirSN++5HPtPlXEo9MEoZQC/PmE=
 github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
-github.com/cpuguy83/go-md2man/v2 v2.0.0-20190314233015-f79a8a8ca69d h1:U+s90UTSYgptZMwQh2aRr3LuazLJIa+Pg3Kc1ylSYVY=
-github.com/cpuguy83/go-md2man/v2 v2.0.0-20190314233015-f79a8a8ca69d/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU=
 github.com/creachadair/staticfile v0.1.2/go.mod h1:a3qySzCIXEprDGxk6tSxSI+dBBdLzqeBOMhZ+o2d3pM=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
@@ -24,22 +22,18 @@ github.com/google/go-cmp v0.3.1 h1:Xye71clBPdm5HgqGwUkwhbynsUJZhDbS20FvLhQ2izg=
 github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
 github.com/jhump/protoreflect v1.5.1-0.20191024142022-1d0024204e58 h1:RHSRszt3DIDwIwV1fPXtZI74OL36BkDIzsp2IOdp+Xc=
 github.com/jhump/protoreflect v1.5.1-0.20191024142022-1d0024204e58/go.mod h1:eaTn3RZAmMBcV0fifFvlm6VHNz3wSkYyXYWUh7ymB74=
-github.com/kjk/inflect v0.0.0-20190213232030-bfc9dcd6393b h1:pSKZGMcdvyNmiHxmVni0zd28IL+xf+kKAixrsMdS3Mg=
-github.com/kjk/inflect v0.0.0-20190213232030-bfc9dcd6393b/go.mod h1:dI+lsL14MLXYhREARMljP6+dati5FyM0eg9QEMUdUUU=
 github.com/lithammer/dedent v1.1.0 h1:VNzHMVCBNG1j0fh3OrsFRkVUwStdDArbgBWoPAffktY=
 github.com/lithammer/dedent v1.1.0/go.mod h1:jrXYCQtgg0nJiN+StA2KgR7w6CiQNv9Fd/Z9BP0jIOc=
+github.com/mattn/go-runewidth v0.0.5 h1:jrGtp51JOKTWgvLFzfG6OtZOJcK2sEnzc/U+zw7TtbA=
+github.com/mattn/go-runewidth v0.0.5/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI=
+github.com/olekukonko/tablewriter v0.0.1 h1:b3iUnf1v+ppJiOfNX4yxxqfWKMQPZR5yoh8urCTFX88=
+github.com/olekukonko/tablewriter v0.0.1/go.mod h1:vsDQFd/mU46D+Z4whnwzcISnGGzXWMclvtLoiIKAKIo=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
-github.com/russross/blackfriday/v2 v2.0.1 h1:lPqVAte+HuHNfhJ/0LC98ESWRz8afy9tM/0RK8m9o+Q=
-github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
-github.com/shurcooL/sanitized_anchor_name v1.0.0 h1:PdmoCO6wvbs+7yrJyMORt4/BmY5IYyJwS/kOiWx8mHo=
-github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc=
 github.com/stoewer/go-strcase v1.0.2 h1:l3iQ2FPu8+36ars/7syO1dQAkjwMCb1IE3J+Th0ohfE=
 github.com/stoewer/go-strcase v1.0.2/go.mod h1:eLfe5bL3qbL7ep/KafHzthxejrOF5J3xmt03uL5tzek=
 github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1w=
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
-github.com/urfave/cli v1.22.1 h1:+mkCCcOFKPnCmVYVcURKps1Xe+3zP90gSYGNfRkjoIY=
-github.com/urfave/cli v1.22.1/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0=
 golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
 golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
 golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU=
@@ -62,8 +56,6 @@ google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7
 google.golang.org/genproto v0.0.0-20170818010345-ee236bd376b0 h1:ZvI3lsq5AIkr7axxmT3tfwFlJVRFLqe6Fp0W03+MJ38=
 google.golang.org/genproto v0.0.0-20170818010345-ee236bd376b0/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc=
 google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc=
-google.golang.org/genproto v0.0.0-20191009194640-548a555dbc03 h1:4HYDjxeNXAOTv3o1N2tjo8UUSlhQgAD52FVkwxnWgM8=
-google.golang.org/genproto v0.0.0-20191009194640-548a555dbc03/go.mod h1:n3cpQtvxv34hfy77yVDNjmbRyujviMdxYliBSkLhpCc=
 google.golang.org/genproto v0.0.0-20191028173616-919d9bdd9fe6 h1:UXl+Zk3jqqcbEVV7ace5lrt4YdA4tXiz3f/KbmD29Vo=
 google.golang.org/genproto v0.0.0-20191028173616-919d9bdd9fe6/go.mod h1:n3cpQtvxv34hfy77yVDNjmbRyujviMdxYliBSkLhpCc=
 google.golang.org/grpc v1.8.0 h1:HN69LlNA/SpyBIRxTfuU0QOntYfdeEeBWlVhRHRCOyw=
@@ -72,8 +64,6 @@ google.golang.org/grpc v1.19.0 h1:cfg4PD8YEdSFnm7qLV4++93WcmhH2nIUhMjhdCvl3j8=
 google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
-gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
-gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.2.4 h1:/eiJrUcujPVeJ3xlSWaiNi3uSVmDGBK1pDHUHAnao1I=
 gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=


Test patch:
diff --git a/cmd/api-linter/summary_test.go b/cmd/api-linter/summary_test.go
index 36019db..8303c8e 100644
--- a/cmd/api-linter/summary_test.go
+++ b/cmd/api-linter/summary_test.go
@@ -15,9 +15,9 @@
 package main
 
 import (
-	"reflect"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/googleapis/api-linter/lint"
 )
 
@@ -25,11 +25,11 @@ func TestCreateSummary(t *testing.T) {
 	tests := []struct {
 		name        string
 		data        []lint.Response
-		wantSummary *LintSummary
+		wantSummary map[string]map[string]int
 	}{{
 		name:        "Empty input",
 		data:        []lint.Response{},
-		wantSummary: &LintSummary{Violations: make(map[string]map[string]bool)},
+		wantSummary: make(map[string]map[string]int),
 	}, {
 		name: "Example with a couple of responses",
 		data: []lint.Response{{
@@ -59,33 +59,24 @@ func TestCreateSummary(t *testing.T) {
 					{RuleID: "core::0132::response_message::name"},
 				},
 			}},
-		wantSummary: &LintSummary{
-			Violations: map[string]map[string]bool{
-				"core::0131::request_message::name": map[string]bool{"example2.proto": true},
-				"core::0132::response_message::name": map[string]bool{
-					"example2.proto": true,
-					"example4.proto": true,
-				},
-				"core::naming_formats::field_names": map[string]bool{
-					"example.proto":  true,
-					"example3.proto": true,
-					"example4.proto": true,
-				},
+		wantSummary: map[string]map[string]int{
+			"core::0131::request_message::name": map[string]int{"example2.proto": 1},
+			"core::0132::response_message::name": map[string]int{
+				"example2.proto": 1,
+				"example4.proto": 1,
+			},
+			"core::naming_formats::field_names": map[string]int{
+				"example.proto":  2,
+				"example3.proto": 1,
+				"example4.proto": 1,
 			},
-			LongestRuleLen: len("core::0132::response_message::name"),
-			NumSourceFiles: 4,
 		},
 	}}
 	for _, test := range tests {
-		gotSummary := createSummary(test.data)
-		if !reflect.DeepEqual(gotSummary.Violations, test.wantSummary.Violations) {
-			t.Errorf("Incorrect violation data:\nGot: %#v\n Want: %#v", gotSummary.Violations, test.wantSummary.Violations)
-		}
-		if gotSummary.LongestRuleLen != test.wantSummary.LongestRuleLen {
-			t.Errorf("Incorrect longest rule length:\nGot: %d want: %d", gotSummary.LongestRuleLen, test.wantSummary.LongestRuleLen)
-		}
-		if gotSummary.NumSourceFiles != test.wantSummary.NumSourceFiles {
-			t.Errorf("Incorrect numSourceFiles:\nGot: %d want: %d", gotSummary.NumSourceFiles, test.wantSummary.NumSourceFiles)
+		want := test.wantSummary
+		got := createSummary(test.data)
+		if diff := cmp.Diff(want, got); diff != "" {
+			t.Errorf("createSummary() mismatch (-want +got):\n%s", diff)
 		}
 	}
 }