go.tools/cmd/stringer: add tests

Refactor a little to make testing easier.
Add golden tests and a check fo splitIntoRuns, which is the subtlest piece.
Still to come: execution tests.

Also fix a few issues in the generated code.

LGTM=gri
R=gri
CC=golang-codereviews, josharian
https://golang.org/cl/134450044

cmd/stringer/golden_test.go

@@ -0,0 +1,274 @@
+// Copyright 2014 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// This file contains simple golden tests for various examples.
+// Besides validating the results when the implementation changes,
+// it provides a way to look at the generated code without having
+// to execute the print statements in one's head.
+
+package main
+
+import (
+	"strings"
+	"testing"
+)
+
+// Golden represents a test case.
+type Golden struct {
+	name   string
+	input  string // input; the package clause is provided when running the test.
+	output string // exected output.
+}
+
+var golden = []Golden{
+	{"day", day_in, day_out},
+	{"offset", offset_in, offset_out},
+	{"gap", gap_in, gap_out},
+	{"neg", neg_in, neg_out},
+	{"uneg", uneg_in, uneg_out},
+	{"map", map_in, map_out},
+}
+
+// Each example starts with "type XXX [u]int", with a single space separating them.
+
+// Simple test: enumeration of type int starting at 0.
+const day_in = `type Day int
+const (
+	Monday Day = iota
+	Tuesday
+	Wednesday
+	Thursday
+	Friday
+	Saturday
+	Sunday
+)
+`
+
+const day_out = `
+var (
+	_Day_indexes = [...]uint8{6, 13, 22, 30, 36, 44, 50}
+	_Day_names   = "MondayTuesdayWednesdayThursdayFridaySaturdaySunday"
+)
+
+func (i Day) String() string {
+	if i < 0 || i >= Day(len(_Day_indexes)) {
+		return fmt.Sprintf("Day(%d)", i)
+	}
+	hi := _Day_indexes[i]
+	lo := uint8(0)
+	if i > 0 {
+		lo = _Day_indexes[i-1]
+	}
+	return _Day_names[lo:hi]
+}
+`
+
+// Enumeration with an offset.
+// Also includes a duplicate.
+const offset_in = `type Number int
+const (
+	_ Number = iota
+	One
+	Two
+	Three
+	AnotherOne = One  // Duplicate; note that AnotherOne doesn't appear below.
+)
+`
+
+const offset_out = `
+var (
+	_Number_indexes = [...]uint8{3, 6, 11}
+	_Number_names   = "OneTwoThree"
+)
+
+func (i Number) String() string {
+	i -= 1
+	if i < 0 || i >= Number(len(_Number_indexes)) {
+		return fmt.Sprintf("Number(%d)", i+1)
+	}
+	hi := _Number_indexes[i]
+	lo := uint8(0)
+	if i > 0 {
+		lo = _Number_indexes[i-1]
+	}
+	return _Number_names[lo:hi]
+}
+`
+
+// Gaps and an offset.
+const gap_in = `type Num int
+const (
+	Two Num = 2
+	Three Num = 3
+	Five Num = 5
+	Six Num = 6
+	Seven Num = 7
+	Eight Num = 8
+	Nine Num = 9
+	Eleven Num = 11
+)
+`
+
+const gap_out = `
+var (
+	_Num_indexes_0 = [...]uint8{3, 8}
+	_Num_names_0   = "TwoThree"
+	_Num_indexes_1 = [...]uint8{4, 7, 12, 17, 21}
+	_Num_names_1   = "FiveSixSevenEightNine"
+	_Num_indexes_2 = [...]uint8{6}
+	_Num_names_2   = "Eleven"
+)
+
+func (i Num) String() string {
+	switch {
+	case 2 <= i && i < 3:
+		lo := uint8(0)
+		if i > 2 {
+			i -= 2
+		} else {
+			lo = _Num_indexes_0[i-1]
+		}
+		return _Num_names_0[lo:_Num_indexes_0[i]]
+	case 5 <= i && i < 9:
+		lo := uint8(0)
+		if i > 5 {
+			i -= 5
+		} else {
+			lo = _Num_indexes_1[i-1]
+		}
+		return _Num_names_1[lo:_Num_indexes_1[i]]
+	case i == 11:
+		return _Num_names_2
+	default:
+		return fmt.Sprintf("Num(%d)", i)
+	}
+}
+`
+
+// Signed integers spanning zero.
+const neg_in = `type Num int
+const (
+	m_2 Num = -2 + iota
+	m_1
+	m0
+	m1
+	m2
+)
+`
+
+const neg_out = `
+var (
+	_Num_indexes = [...]uint8{3, 6, 8, 10, 12}
+	_Num_names   = "m_2m_1m0m1m2"
+)
+
+func (i Num) String() string {
+	i -= -2
+	if i < 0 || i >= Num(len(_Num_indexes)) {
+		return fmt.Sprintf("Num(%d)", i+-2)
+	}
+	hi := _Num_indexes[i]
+	lo := uint8(0)
+	if i > 0 {
+		lo = _Num_indexes[i-1]
+	}
+	return _Num_names[lo:hi]
+}
+`
+
+// Unsigned integers spanning zero.
+const uneg_in = `type UNum uint
+const (
+	m_2 UNum = ^UNum(0)-2
+	m_1
+	m0
+	m1
+	m2
+)
+`
+
+const uneg_out = `
+var (
+	_UNum_indexes = [...]uint8{3}
+	_UNum_names   = "m_2"
+)
+
+func (i UNum) String() string {
+	i -= 18446744073709551613
+	if i >= UNum(len(_UNum_indexes)) {
+		return fmt.Sprintf("UNum(%d)", i+18446744073709551613)
+	}
+	hi := _UNum_indexes[i]
+	lo := uint8(0)
+	if i > 0 {
+		lo = _UNum_indexes[i-1]
+	}
+	return _UNum_names[lo:hi]
+}
+`
+
+// Enough gaps to trigger a map implementation of the method.
+// Also includes a duplicate to test that it doesn't cause problems
+const map_in = `type Prime int
+const (
+	p2 Prime = 2
+	p3 Prime = 3
+	p5 Prime = 5
+	p7 Prime = 7
+	p77 Prime = 7 // Duplicate; note that p77 doesn't appear below.
+	p11 Prime = 11
+	p13 Prime = 13
+	p17 Prime = 17
+	p19 Prime = 19
+	p23 Prime = 23
+	p29 Prime = 29
+	p37 Prime = 31
+	p41 Prime = 41
+	p43 Prime = 43
+)
+`
+
+const map_out = `
+var _Prime_map = map[Prime]string{
+	2:  "p2",
+	3:  "p3",
+	5:  "p5",
+	7:  "p7",
+	11: "p11",
+	13: "p13",
+	17: "p17",
+	19: "p19",
+	23: "p23",
+	29: "p29",
+	31: "p37",
+	41: "p41",
+	43: "p43",
+}
+
+func (i Prime) String() string {
+	if str, ok := _Prime_map[i]; ok {
+		return str
+	}
+	return fmt.Sprintf("Prime(%d)", i)
+}
+`
+
+func TestGolden(t *testing.T) {
+	for _, test := range golden {
+		var g Generator
+		input := "package test\n" + test.input
+		file := test.name + ".go"
+		g.parsePackage(".", []string{file}, input)
+		// Extract the name and type of the constant from the first line.
+		tokens := strings.SplitN(test.input, " ", 3)
+		if len(tokens) != 3 {
+			t.Fatalf("%s: need type declaration on first line", test.name)
+		}
+		g.generate(tokens[1])
+		got := string(g.format())
+		if got != test.output {
+			t.Errorf("%s: got\n====\n%s====\nexpected\n====%s", test.name, got, test.output)
+		}
+	}
+}

cmd/stringer/stringer.go

@@ -20,11 +20,11 @@
 //	type Pill int
 //
 //	const (
-//		Undefined Pill = iota
+//		Placebo Pill = iota
 //		Aspirin
 //		Ibuprofen
-//		Acetaminophen
+//		Paracetamol
-//		Paracetamol = Acetaminophen
+//		Acetaminophen = Paracetamol
 //	)
 //
 // running this command
@@ -45,8 +45,8 @@
 //	//go:generate go tool stringer -type=Pill
 //	TODO: do we install this as a tool or as a binary?
 //
-// If multiple contants have the same value, the lexically first matching name will
+// If multiple constants have the same value, the lexically first matching name will
-// be used (in the example, Paracetamol will print as "Acetaminophen").
+// be used (in the example, Acetaminophen will print as "Paracetamol").
 //
 // With no arguments, it processes the package in the current directory.
 // Otherwise, the arguments must name a single directory holding a Go package
@@ -142,14 +142,7 @@ func main() {
 	}
 
 	// Format the output.
-	src, err := format.Source(g.buf.Bytes())
+	src := g.format()
-	if err != nil {
-		// Should never happen, but can arise when developing this code.
-		// The user can compile the output to see the error.
-		log.Printf("warning: internal error: invalid Go generated: %s", err)
-		log.Printf("warning: compile the package to analyze the error")
-		src = g.buf.Bytes()
-	}
 
 	// Write to file.
 	outputName := *output
@@ -157,7 +150,7 @@ func main() {
 		baseName := fmt.Sprintf("%s_string.go", types[0])
 		outputName = filepath.Join(dir, strings.ToLower(baseName))
 	}
-	err = ioutil.WriteFile(outputName, src, 0644)
+	err := ioutil.WriteFile(outputName, src, 0644)
 	if err != nil {
 		log.Fatalf("writing output: %s", err)
 	}
@@ -214,12 +207,12 @@ func (g *Generator) parsePackageDir(directory string) {
 	// names = append(names, pkg.TestGoFiles...) // These are also in the "foo" package.
 	names = append(names, pkg.SFiles...)
 	names = prefixDirectory(directory, names)
-	g.parsePackage(directory, names)
+	g.parsePackage(directory, names, "")
 }
 
 // parsePackageFiles parses the package occupying the named files.
 func (g *Generator) parsePackageFiles(names []string) {
-	g.parsePackage(".", names)
+	g.parsePackage(".", names, "")
 }
 
 // prefixDirectory places the directory name on the beginning of each name in the list.
@@ -235,7 +228,9 @@ func prefixDirectory(directory string, names []string) []string {
 }
 
 // doPackage analyzes the single package constructed from the named files.
-func (g *Generator) parsePackage(directory string, names []string) {
+// If text is non-nil, it is a string to be used instead of the content of the file,
+// to be used for testing. doPackage exits if there is an error.
+func (g *Generator) parsePackage(directory string, names []string, text interface{}) {
 	var files []*File
 	var astFiles []*ast.File
 	g.pkg = new(Package)
@@ -244,7 +239,7 @@ func (g *Generator) parsePackage(directory string, names []string) {
 		if !strings.HasSuffix(name, ".go") {
 			continue
 		}
-		parsedFile, err := parser.ParseFile(fs, name, nil, 0)
+		parsedFile, err := parser.ParseFile(fs, name, text, 0)
 		if err != nil {
 			log.Fatalf("parsing package: %s: %s", name, err)
 		}
@@ -323,6 +318,19 @@ func (g *Generator) generate(typeName string) {
 func splitIntoRuns(values []Value) [][]Value {
 	// We use stable sort so the lexically first name is chosen for equal elements.
 	sort.Stable(byValue(values))
+	// Remove duplicates. Stable sort has put the one we want to print first,
+	// so use that one. The String method won't care about which named constant
+	// was the argument, so the first name for the given value is the only one to keep.
+	// We need to do this because identical values would cause the switch or map
+	// to fail to compile.
+	j := 1
+	for i := 1; i < len(values); i++ {
+		if values[i].value != values[i-1].value {
+			values[j] = values[i]
+			j++
+		}
+	}
+	values = values[:j]
 	runs := make([][]Value, 0, 10)
 	for len(values) > 0 {
 		// One contiguous sequence per outer loop.
@@ -336,6 +344,19 @@ func splitIntoRuns(values []Value) [][]Value {
 	return runs
 }
 
+// format returns the gofmt-ed contents of the Generator's buffer.
+func (g *Generator) format() []byte {
+	src, err := format.Source(g.buf.Bytes())
+	if err != nil {
+		// Should never happen, but can arise when developing this code.
+		// The user can compile the output to see the error.
+		log.Printf("warning: internal error: invalid Go generated: %s", err)
+		log.Printf("warning: compile the package to analyze the error")
+		return g.buf.Bytes()
+	}
+	return src
+}
+
 // Value represents a declared constant.
 type Value struct {
 	name string // The name of the constant.
@@ -470,7 +491,7 @@ func (g *Generator) declareIndexAndNameVars(run []Value, typeName string, suffix
 		indexes[i] = b.Len()
 	}
 	names := b.String()
-	g.Printf("\t_%s_indexes%s = []uint%d{", typeName, suffix, usize(len(names)))
+	g.Printf("\t_%s_indexes%s = [...]uint%d{", typeName, suffix, usize(len(names)))
 	for i, v := range indexes {
 		if i > 0 {
 			g.Printf(", ")
@@ -553,7 +574,7 @@ func (g *Generator) buildMultipleRuns(runs [][]Value, typeName string) {
 			g.Printf("\t\treturn _%s_names_%d\n", typeName, i)
 			continue
 		}
-		g.Printf("\tcase %s < i && i < %s:\n", &values[0], &values[len(values)-1])
+		g.Printf("\tcase %s <= i && i < %s:\n", &values[0], &values[len(values)-1])
 		g.Printf("\t\tlo := uint%d(0)\n", usize(len(values)))
 		g.Printf("\t\tif i > %s {\n", &values[0])
 		g.Printf("\t\t\ti -= %s\n", &values[0])
@@ -583,10 +604,9 @@ func (g *Generator) buildMap(runs [][]Value, typeName string) {
 
 // Argument to format is the type name.
 const stringMap = `func (i %[1]s) String() string {
-	str, ok := _%[1]s_map[i]
+	if str, ok := _%[1]s_map[i]; ok {
-	if !ok {
+		return str
-		return fmt.Sprintf("%[1]s(%%d)", i)
 	}
-	return str
+	return fmt.Sprintf("%[1]s(%%d)", i)
 }
 `

cmd/stringer/util_test.go

@@ -0,0 +1,77 @@
+// Copyright 2014 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// This file contains tests for some of the internal functions.
+
+package main
+
+import (
+	"fmt"
+	"testing"
+)
+
+// Helpers to save typing in the test cases.
+type u []uint64
+type uu [][]uint64
+
+type SplitTest struct {
+	input  u
+	output uu
+	signed bool
+}
+
+var (
+	m2  = uint64(2)
+	m1  = uint64(1)
+	m0  = uint64(0)
+	m_1 = ^uint64(0)     // -1 when signed.
+	m_2 = ^uint64(0) - 1 // -2 when signed.
+)
+
+var splitTests = []SplitTest{
+	// No need for a test for the empty case; that's picked off before splitIntoRuns.
+	// Single value.
+	{u{1}, uu{u{1}}, false},
+	// Out of order.
+	{u{3, 2, 1}, uu{u{1, 2, 3}}, true},
+	// Out of order.
+	{u{3, 2, 1}, uu{u{1, 2, 3}}, false},
+	// A gap at the beginning.
+	{u{1, 33, 32, 31}, uu{u{1}, u{31, 32, 33}}, true},
+	// A gap in the middle, in mixed order.
+	{u{33, 7, 32, 31, 9, 8}, uu{u{7, 8, 9}, u{31, 32, 33}}, true},
+	// Gaps throughout
+	{u{33, 44, 1, 32, 45, 31}, uu{u{1}, u{31, 32, 33}, u{44, 45}}, true},
+	// Unsigned values spanning 0.
+	{u{m1, m0, m_1, m2, m_2}, uu{u{m0, m1, m2}, u{m_2, m_1}}, false},
+	// Signed values spanning 0
+	{u{m1, m0, m_1, m2, m_2}, uu{u{m_2, m_1, m0, m1, m2}}, true},
+}
+
+func TestSplitIntoRuns(t *testing.T) {
+Outer:
+	for n, test := range splitTests {
+		values := make([]Value, len(test.input))
+		for i, v := range test.input {
+			values[i] = Value{"", v, test.signed, fmt.Sprint(v)}
+		}
+		runs := splitIntoRuns(values)
+		if len(runs) != len(test.output) {
+			t.Errorf("#%d: %v: got %d runs; expected %d", n, test.input, len(runs), len(test.output))
+			continue
+		}
+		for i, run := range runs {
+			if len(run) != len(test.output[i]) {
+				t.Errorf("#%d: got %v; expected %v", n, runs, test.output)
+				continue Outer
+			}
+			for j, v := range run {
+				if v.value != test.output[i][j] {
+					t.Errorf("#%d: got %v; expected %v", n, runs, test.output)
+					continue Outer
+				}
+			}
+		}
+	}
+}