Improve coverage and fix lint errors (#6)

yolken-segment · web-flow · commit f675a17f17da · 2020-01-08T17:13:17.000-08:00
* Add more badges into README

* Fix lint issues

* Add coverage badge

* Add more comments

* Improve test coverage more

* More test coverage improvements
diff --git a/README.md b/README.md
@@ -1,4 +1,8 @@
 [![Circle CI](https://circleci.com/gh/segmentio/golines.svg?style=svg&circle-token=b1d01d8b035ef0aa71ccd183580586a80cd85271)](https://circleci.com/gh/segmentio/golines)
+[![Go Report Card](https://goreportcard.com/badge/github.com/segmentio/golines)](https://goreportcard.com/report/github.com/segmentio/golines)
+[![GoDoc](https://godoc.org/github.com/segmentio/golines?status.svg)](https://godoc.org/github.com/segmentio/golines)
+[![Coverage](https://img.shields.io/badge/Go%20Coverage-78%25-brightgreen.svg?longCache=true&style=flat)](https://github.com/jpoles1/gopherbadger)
+
 # golines
 Golines is a golang formatter that shortens long lines, in addition to all
 of the formatting fixes done by [`gofmt`](https://golang.org/cmd/gofmt/).
diff --git a/_fixtures/end_to_end.go b/_fixtures/end_to_end.go
@@ -73,7 +73,7 @@ func shortFunc(a int, b int) error {
 	}
 
 	if a > 5 {
-		panic(fmt.Sprintf(">>>>>>>>>>>>>>>>>>> %s %s %s", "really long argument", "another really long argument", "a third really long arguement"))
+		panic(fmt.Sprintf(">>>>>>>>>>>>>>>>>>> %s %s %s %s", "really long argument", "another really long argument", "a third really long arguement", abc[1:2]))
 	}
 
 	return nil
diff --git a/_fixtures/end_to_end__exp.go b/_fixtures/end_to_end__exp.go
@@ -143,10 +143,11 @@ func shortFunc(a int, b int) error {
 	if a > 5 {
 		panic(
 			fmt.Sprintf(
-				">>>>>>>>>>>>>>>>>>> %s %s %s",
+				">>>>>>>>>>>>>>>>>>> %s %s %s %s",
 				"really long argument",
 				"another really long argument",
 				"a third really long arguement",
+				abc[1:2],
 			),
 		)
 	}
diff --git a/_fixtures/interface.go b/_fixtures/interface.go
@@ -1,8 +1,21 @@
 package fixtures
 
+import "fmt"
+
 type MyEmptyInterface interface{}
 
 type MyInterface2 interface {
 	aReallyLongFunctionName(argument1 string, argument2 string, argument3 string, argument4 string, argument5 string, argument6 string) (string, error)
 	shortFunc(x int, y int) error
 }
+
+func interfaceFuncs() {
+	var m MyEmptyInterface
+	_, ok := m.(MyInterface2)
+	fmt.Println(ok)
+
+	switch m.(type) {
+	case MyEmptyInterface:
+		fmt.Println("Got MyEmptyInterface")
+	}
+}
diff --git a/_fixtures/interface__exp.go b/_fixtures/interface__exp.go
@@ -1,5 +1,7 @@
 package fixtures
 
+import "fmt"
+
 type MyEmptyInterface interface{}
 
 type MyInterface2 interface {
@@ -13,3 +15,14 @@ type MyInterface2 interface {
 	) (string, error)
 	shortFunc(x int, y int) error
 }
+
+func interfaceFuncs() {
+	var m MyEmptyInterface
+	_, ok := m.(MyInterface2)
+	fmt.Println(ok)
+
+	switch m.(type) {
+	case MyEmptyInterface:
+		fmt.Println("Got MyEmptyInterface")
+	}
+}
diff --git a/annotation.go b/annotation.go
@@ -10,21 +10,24 @@ import (
 
 const annotationPrefix = "// __golines:shorten:"
 
+// CreateAnnotation generates the text of a comment that will annotate long lines.
 func CreateAnnotation(length int) string {
 	return fmt.Sprintf(
-		"%s:%d",
+		"%s%d",
 		annotationPrefix,
 		length,
 	)
 }
 
+// IsAnnotation determines whether the given line is an annotation created with CreateAnnotation.
 func IsAnnotation(line string) bool {
 	return strings.HasPrefix(
 		strings.Trim(line, " \t"),
 		annotationPrefix,
 	)
 }
 
+// HasAnnotation determines whether the given AST node has a line length annotation on it.
 func HasAnnotation(node dst.Node) bool {
 	startDecorations := node.Decorations().Start.All()
 	return len(startDecorations) > 0 &&
@@ -61,6 +64,8 @@ func HasAnnotationRecursive(node dst.Node) bool {
 	return false
 }
 
+// ParseAnnotation returns the line length encoded in a golines annotation. If none is found,
+// it returns -1.
 func ParseAnnotation(line string) int {
 	if IsAnnotation(line) {
 		components := strings.SplitN(line, ":", 3)
diff --git a/annotation_test.go b/annotation_test.go
@@ -0,0 +1,44 @@
+package main
+
+import (
+	"testing"
+
+	"github.com/dave/dst"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestAnnotationStrings(t *testing.T) {
+	assert.Equal(t, "// __golines:shorten:5", CreateAnnotation(5))
+	assert.Equal(t, 5, ParseAnnotation("// __golines:shorten:5"))
+	assert.Equal(t, -1, ParseAnnotation("// __golines:shorten:not_a_number"))
+	assert.Equal(t, -1, ParseAnnotation("// not an annotation"))
+	assert.True(t, IsAnnotation("// __golines:shorten:5"))
+	assert.False(t, IsAnnotation("// not an annotation"))
+}
+
+func TestHasAnnotation(t *testing.T) {
+	node1 := &dst.Ident{
+		Name: "x",
+		Decs: dst.IdentDecorations{
+			NodeDecs: dst.NodeDecs{
+				Start: []string{
+					"// not an annotation",
+					CreateAnnotation(55),
+				},
+			},
+		},
+	}
+	assert.True(t, HasAnnotation(node1))
+
+	node2 := &dst.Ident{
+		Name: "x",
+		Decs: dst.IdentDecorations{
+			NodeDecs: dst.NodeDecs{
+				Start: []string{
+					"// not an annotation",
+				},
+			},
+		},
+	}
+	assert.False(t, HasAnnotation(node2))
+}
diff --git a/diff_test.go b/diff_test.go
@@ -0,0 +1,17 @@
+package main
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestPrettyDiff(t *testing.T) {
+	// For now, this just tests that the script runs without error
+	err := PrettyDiff(
+		"test_path.txt",
+		[]byte("line 1\nline 2"),
+		[]byte("line 1\nline 2 modified"),
+	)
+	assert.Nil(t, err)
+}
diff --git a/generate/graph.go b/generate/graph.go
@@ -4,7 +4,7 @@ import (
 	"sort"
 
 	"github.com/dave/dst/gendst/data"
-	. "github.com/dave/jennifer/jen"
+	"github.com/dave/jennifer/jen"
 )
 
 const (
@@ -15,69 +15,69 @@ const (
 // graph node. The latter can then be used to generate a dot file for debugging
 // purposes.
 func genNodeToGraphNode() error {
-	f := NewFile("main")
+	f := jen.NewFile("main")
 	f.HeaderComment("Generated by generate/main.go. DO NOT EDIT.")
 
 	// Sort the keys in the dst map so that output is in stable order
 	dataKeys := []string{}
 
-	for key, _ := range data.Info {
+	for key := range data.Info {
 		dataKeys = append(dataKeys, key)
 	}
 
 	sort.Strings(dataKeys)
 
 	f.Func().Id("NodeToGraphNode").Params(
-		Id("node").Qual(dstPath, "Node"),
+		jen.Id("node").Qual(dstPath, "Node"),
 	).Op("*").Id("GraphNode").BlockFunc(
-		func(g *Group) {
+		func(g *jen.Group) {
 			g.Id("gNode").Op(":=").Op("&").Id("GraphNode").Values(
-				Id("Node").Op(":").Id("node"),
-				Id("Edges").Op(":").Index().Op("*").Id("GraphEdge").Values(),
+				jen.Id("Node").Op(":").Id("node"),
+				jen.Id("Edges").Op(":").Index().Op("*").Id("GraphEdge").Values(),
 			)
 
 			g.Var().Id("child").Op("*").Id("GraphNode")
 
-			g.Switch(Id("n").Op(":=").Id("node").Assert(Id("type"))).BlockFunc(
-				func(g *Group) {
+			g.Switch(jen.Id("n").Op(":=").Id("node").Assert(jen.Id("type"))).BlockFunc(
+				func(g *jen.Group) {
 					for _, key := range dataKeys {
 						parts := data.Info[key]
 
-						g.Case(Op("*").Qual(dstPath, key)).BlockFunc(
-							func(g *Group) {
+						g.Case(jen.Op("*").Qual(dstPath, key)).BlockFunc(
+							func(g *jen.Group) {
 								g.Id("gNode").Dot("Type").Op("=").Lit(key)
 
 								for _, part := range parts {
 									switch p := part.(type) {
 									case data.Node:
 										g.If(p.Field.Get("n").Op("!=").Nil()).Block(
-											Id("child").Op("=").Id("NodeToGraphNode").
+											jen.Id("child").Op("=").Id("NodeToGraphNode").
 												Call(
 													p.Field.Get("n"),
 												),
-											Id("gNode").Dot("Edges").Op("=").Append(
-												Id("gNode").Dot("Edges"),
-												Op("&").Id("GraphEdge").Values(
-													Id("Dest").Op(":").Id("child"),
-													Id("Relationship").Op(":").Lit(p.Name),
+											jen.Id("gNode").Dot("Edges").Op("=").Append(
+												jen.Id("gNode").Dot("Edges"),
+												jen.Op("&").Id("GraphEdge").Values(
+													jen.Id("Dest").Op(":").Id("child"),
+													jen.Id("Relationship").Op(":").Lit(p.Name),
 												),
 											),
 										)
 									case data.List:
 										g.If(p.Field.Get("n").Op("!=").Nil()).Block(
-											For(
-												List(Id("_"), Id("obj")).
+											jen.For(
+												jen.List(jen.Id("_"), jen.Id("obj")).
 													Op(":=").Range().Add(p.Field.Get("n")),
 											).Block(
-												Id("child").Op("=").Id("NodeToGraphNode").
+												jen.Id("child").Op("=").Id("NodeToGraphNode").
 													Call(
-														Id("obj"),
+														jen.Id("obj"),
 													),
-												Id("gNode").Dot("Edges").Op("=").Append(
-													Id("gNode").Dot("Edges"),
-													Op("&").Id("GraphEdge").Values(
-														Id("Dest").Op(":").Id("child"),
-														Id("Relationship").Op(":").Lit(p.Name),
+												jen.Id("gNode").Dot("Edges").Op("=").Append(
+													jen.Id("gNode").Dot("Edges"),
+													jen.Op("&").Id("GraphEdge").Values(
+														jen.Id("Dest").Op(":").Id("child"),
+														jen.Id("Relationship").Op(":").Lit(p.Name),
 													),
 												),
 											),
@@ -92,8 +92,8 @@ func genNodeToGraphNode() error {
 						)
 					}
 					g.Default().Block(
-						Qual("log", "Println").Call(
-							Lit("Unrecognized type"),
+						jen.Qual("log", "Println").Call(
+							jen.Lit("Unrecognized type"),
 						),
 					)
 				},
diff --git a/graph_generated.go b/graph_generated.go
@@ -3,9 +3,8 @@
 package main
 
 import (
-	"log"
-
 	dst "github.com/dave/dst"
+	"log"
 )
 
 func NodeToGraphNode(node dst.Node) *GraphNode {
diff --git a/graph_test.go b/graph_test.go
@@ -2,7 +2,6 @@ package main
 
 import (
 	"bytes"
-	"io/ioutil"
 	"strings"
 	"testing"
 
@@ -49,18 +48,3 @@ func TestCreateDot(t *testing.T) {
 
 	assert.Equal(t, strings.TrimSpace(expDot), string(out.Bytes()))
 }
-
-func TestCreateDotLarge(t *testing.T) {
-	codeBytes, err := ioutil.ReadFile("_fixtures/end_to_end.go")
-	assert.Nil(t, err)
-
-	node, err := decorator.Parse(codeBytes)
-	assert.Nil(t, err)
-
-	out := &bytes.Buffer{}
-	err = CreateDot(node, out)
-	outStr := strings.TrimSpace(string(out.Bytes()))
-
-	assert.Equal(t, true, strings.HasPrefix(outStr, "digraph {"))
-	assert.Equal(t, true, strings.HasSuffix(outStr, "}"))
-}
diff --git a/shortener.go b/shortener.go
@@ -57,6 +57,7 @@ type Shortener struct {
 	baseFormatterArgs []string
 }
 
+// NewShortener creates a new shortener instance from the provided config.
 func NewShortener(config ShortenerConfig) *Shortener {
 	var formatterComponents []string
 
diff --git a/shortener_test.go b/shortener_test.go
@@ -36,6 +36,12 @@ func TestShortener(t *testing.T) {
 		)
 	}
 
+	dotDir, err := ioutil.TempDir("", "dot")
+	if err != nil {
+		t.Fatalf("Error creating output directory for dot files: %+v", err)
+	}
+	defer os.RemoveAll(dotDir)
+
 	shortener := NewShortener(
 		ShortenerConfig{
 			MaxLen:           100,
@@ -45,6 +51,7 @@ func TestShortener(t *testing.T) {
 			ReformatTags:     true,
 			IgnoreGenerated:  true,
 			BaseFormatterCmd: "gofmt",
+			DotFile:          filepath.Join(dotDir, "out.dot"),
 		},
 	)
 

Original file line number	Diff line number	Diff line change
`@@ -73,7 +73,7 @@ func shortFunc(a int, b int) error {`
`73`	`73`	`}`
`74`	`74`
`75`	`75`	`if a > 5 {`
`76`		`- panic(fmt.Sprintf(">>>>>>>>>>>>>>>>>>> %s %s %s", "really long argument", "another really long argument", "a third really long arguement"))`
	`76`	`+ panic(fmt.Sprintf(">>>>>>>>>>>>>>>>>>> %s %s %s %s", "really long argument", "another really long argument", "a third really long arguement", abc[1:2]))`
`77`	`77`	`}`
`78`	`78`
`79`	`79`	`return nil`
Original file line number	Diff line number	Diff line change
`@@ -143,10 +143,11 @@ func shortFunc(a int, b int) error {`
`143`	`143`	`if a > 5 {`
`144`	`144`	`panic(`
`145`	`145`	`fmt.Sprintf(`
`146`		`- ">>>>>>>>>>>>>>>>>>> %s %s %s",`
	`146`	`+ ">>>>>>>>>>>>>>>>>>> %s %s %s %s",`
`147`	`147`	`"really long argument",`
`148`	`148`	`"another really long argument",`
`149`	`149`	`"a third really long arguement",`
	`150`	`+ abc[1:2],`
`150`	`151`	`),`
`151`	`152`	`)`
`152`	`153`	`}`
Original file line number	Diff line number	Diff line change
`@@ -10,21 +10,24 @@ import (`
`10`	`10`
`11`	`11`	`const annotationPrefix = "// __golines:shorten:"`
`12`	`12`
	`13`	`+// CreateAnnotation generates the text of a comment that will annotate long lines.`
`13`	`14`	`func CreateAnnotation(length int) string {`
`14`	`15`	`return fmt.Sprintf(`
`15`		`- "%s:%d",`
	`16`	`+ "%s%d",`
`16`	`17`	`annotationPrefix,`
`17`	`18`	`length,`
`18`	`19`	`)`
`19`	`20`	`}`
`20`	`21`
	`22`	`+// IsAnnotation determines whether the given line is an annotation created with CreateAnnotation.`
`21`	`23`	`func IsAnnotation(line string) bool {`
`22`	`24`	`return strings.HasPrefix(`
`23`	`25`	`strings.Trim(line, " \t"),`
`24`	`26`	`annotationPrefix,`
`25`	`27`	`)`
`26`	`28`	`}`
`27`	`29`
	`30`	`+// HasAnnotation determines whether the given AST node has a line length annotation on it.`
`28`	`31`	`func HasAnnotation(node dst.Node) bool {`
`29`	`32`	`startDecorations := node.Decorations().Start.All()`
`30`	`33`	`return len(startDecorations) > 0 &&`
`@@ -61,6 +64,8 @@ func HasAnnotationRecursive(node dst.Node) bool {`
`61`	`64`	`return false`
`62`	`65`	`}`
`63`	`66`
	`67`	`+// ParseAnnotation returns the line length encoded in a golines annotation. If none is found,`
	`68`	`+// it returns -1.`
`64`	`69`	`func ParseAnnotation(line string) int {`
`65`	`70`	`if IsAnnotation(line) {`
`66`	`71`	`components := strings.SplitN(line, ":", 3)`
Original file line number	Diff line number	Diff line change
`@@ -3,9 +3,8 @@`
`3`	`3`	`package main`
`4`	`4`
`5`	`5`	`import (`
`6`		`- "log"`
`7`		`-`
`8`	`6`	`dst "github.com/dave/dst"`
	`7`	`+ "log"`
`9`	`8`	`)`
`10`	`9`
`11`	`10`	`func NodeToGraphNode(node dst.Node) *GraphNode {`