go/doc: fix constant type propagation

The old code was seriously broken: It assumed that a constant
declaration without a type would always inherit the type of
the previous declaration, but in fact it only inherits the
type of the previous declaration when there's no type and no
constant value.

While fixing this bug, found that the result was not sorted
deterministically in all situations due to a poor choice of
order value (which led to spurious test failures since the
tests assume deterministic outputs). Fixed that as well.

Added new test cases and fixed some old (broken) tests.

Fixes #16153.

Change-Id: I95b480e019b0fd3538638caba02fe651c69e0513
Reviewed-on: https://go-review.googlesource.com/68730
Reviewed-by: Alan Donovan <adonovan@google.com>

10 files changed, 184 insertions, 24 deletions

diff --git a/src/go/doc/exports.go b/src/go/doc/exports.go
index da9ea1f027..40cbb22797 100644
--- a/src/go/doc/exports.go
+++ b/src/go/doc/exports.go
@@ -200,7 +200,7 @@ func (r *reader) filterSpecList(list []ast.Spec, tok token.Token) []ast.Spec {
 		var prevType ast.Expr
 		for _, spec := range list {
 			spec := spec.(*ast.ValueSpec)
-			if spec.Type == nil && prevType != nil {
+			if spec.Type == nil && len(spec.Values) == 0 && prevType != nil {
 				// provide current spec with an explicit type
 				spec.Type = copyConstType(prevType, spec.Pos())
 			}
diff --git a/src/go/doc/reader.go b/src/go/doc/reader.go
index 17635f0561..140f587233 100644
--- a/src/go/doc/reader.go
+++ b/src/go/doc/reader.go
@@ -154,6 +154,7 @@ type reader struct {
 	imports   map[string]int
 	hasDotImp bool     // if set, package contains a dot import
 	values    []*Value // consts and vars
+	order     int      // sort order of const and var declarations (when we can't use a name)
 	types     map[string]*namedType
 	funcs     methodSet
 
@@ -256,11 +257,9 @@ func (r *reader) readValue(decl *ast.GenDecl) {
 			if n, imp := baseTypeName(s.Type); !imp {
 				name = n
 			}
-		case decl.Tok == token.CONST:
-			// no type is present but we have a constant declaration;
-			// use the previous type name (w/o more type information
-			// we cannot handle the case of unnamed variables with
-			// initializer expressions except for some trivial cases)
+		case decl.Tok == token.CONST && len(s.Values) == 0:
+			// no type or value is present but we have a constant declaration;
+			// use the previous type name (possibly the empty string)
 			name = prev
 		}
 		if name != "" {
@@ -297,9 +296,15 @@ func (r *reader) readValue(decl *ast.GenDecl) {
 		Doc:   decl.Doc.Text(),
 		Names: specNames(decl.Specs),
 		Decl:  decl,
-		order: len(*values),
+		order: r.order,
 	})
 	decl.Doc = nil // doc consumed - remove from AST
+
+	// Note: It's important that the order used here is global because the cleanupTypes
+	// methods may move values associated with types back into the global list. If the
+	// order is list-specific, sorting is not deterministic because the same order value
+	// may appear multiple times (was bug, found when fixing #16153).
+	r.order++
 }
 
 // fields returns a struct's fields or an interface's methods.
diff --git a/src/go/doc/testdata/blank.0.golden b/src/go/doc/testdata/blank.0.golden
index c2987cf140..70f2929f8a 100644
--- a/src/go/doc/testdata/blank.0.golden
+++ b/src/go/doc/testdata/blank.0.golden
@@ -21,11 +21,18 @@ CONSTANTS
 		C4	int
 	)
 
-	// Constants with an imported type that needs to be propagated. 
+	// Constants with a single type that is not propagated. 
 	const (
-		Default		os.FileMode	= 0644
-		Useless				= 0312
-		WideOpen			= 0777
+		Default		= 0644
+		Useless		= 0312
+		WideOpen	= 0777
+	)
+
+	// Constants with an imported type that is propagated. 
+	const (
+		M1	os.FileMode
+		M2
+		M3
 	)
 
 	// Package constants. 
diff --git a/src/go/doc/testdata/blank.1.golden b/src/go/doc/testdata/blank.1.golden
index ee5054a4ed..8098cb6e88 100644
--- a/src/go/doc/testdata/blank.1.golden
+++ b/src/go/doc/testdata/blank.1.golden
@@ -23,14 +23,7 @@ CONSTANTS
 		C4
 	)
 
-	// Package constants. 
-	const (
-		_	int	= iota
-		I1
-		I2
-	)
-
-	// Constants with an imported type that needs to be propagated. 
+	// Constants with a single type that is not propagated. 
 	const (
 		zero		os.FileMode	= 0
 		Default				= 0644
@@ -38,6 +31,21 @@ CONSTANTS
 		WideOpen			= 0777
 	)
 
+	// Constants with an imported type that is propagated. 
+	const (
+		zero	os.FileMode	= 0
+		M1
+		M2
+		M3
+	)
+
+	// Package constants. 
+	const (
+		_	int	= iota
+		I1
+		I2
+	)
+
 	// Unexported constants counting from blank iota. See issue 9615. 
 	const (
 		_	= iota
diff --git a/src/go/doc/testdata/blank.2.golden b/src/go/doc/testdata/blank.2.golden
index c2987cf140..70f2929f8a 100644
--- a/src/go/doc/testdata/blank.2.golden
+++ b/src/go/doc/testdata/blank.2.golden
@@ -21,11 +21,18 @@ CONSTANTS
 		C4	int
 	)
 
-	// Constants with an imported type that needs to be propagated. 
+	// Constants with a single type that is not propagated. 
 	const (
-		Default		os.FileMode	= 0644
-		Useless				= 0312
-		WideOpen			= 0777
+		Default		= 0644
+		Useless		= 0312
+		WideOpen	= 0777
+	)
+
+	// Constants with an imported type that is propagated. 
+	const (
+		M1	os.FileMode
+		M2
+		M3
 	)
 
 	// Package constants. 
diff --git a/src/go/doc/testdata/blank.go b/src/go/doc/testdata/blank.go
index 419a78f7d5..5ea6186935 100644
--- a/src/go/doc/testdata/blank.go
+++ b/src/go/doc/testdata/blank.go
@@ -29,7 +29,7 @@ const (
 	C4
 )
 
-// Constants with an imported type that needs to be propagated.
+// Constants with a single type that is not propagated.
 const (
 	zero     os.FileMode = 0
 	Default              = 0644
@@ -37,6 +37,14 @@ const (
 	WideOpen             = 0777
 )
 
+// Constants with an imported type that is propagated.
+const (
+	zero os.FileMode = 0
+	M1
+	M2
+	M3
+)
+
 // Package constants.
 const (
 	_ int = iota
diff --git a/src/go/doc/testdata/issue16153.0.golden b/src/go/doc/testdata/issue16153.0.golden
new file mode 100644
index 0000000000..189260b4d0
--- /dev/null
+++ b/src/go/doc/testdata/issue16153.0.golden
@@ -0,0 +1,32 @@
+// 
+PACKAGE issue16153
+
+IMPORTPATH
+	testdata/issue16153
+
+FILENAMES
+	testdata/issue16153.go
+
+CONSTANTS
+	// 
+	const (
+		X3	int64	= iota
+		Y3		= 1
+	)
+
+	// 
+	const (
+		X4	int64	= iota
+		Y4
+	)
+
+	// original test case 
+	const (
+		Y1 = 256
+	)
+
+	// variations 
+	const (
+		Y2 uint8
+	)
+
diff --git a/src/go/doc/testdata/issue16153.1.golden b/src/go/doc/testdata/issue16153.1.golden
new file mode 100644
index 0000000000..803df3e709
--- /dev/null
+++ b/src/go/doc/testdata/issue16153.1.golden
@@ -0,0 +1,34 @@
+// 
+PACKAGE issue16153
+
+IMPORTPATH
+	testdata/issue16153
+
+FILENAMES
+	testdata/issue16153.go
+
+CONSTANTS
+	// original test case 
+	const (
+		x1	uint8	= 255
+		Y1		= 256
+	)
+
+	// variations 
+	const (
+		x2	uint8	= 255
+		Y2
+	)
+
+	// 
+	const (
+		X3	int64	= iota
+		Y3		= 1
+	)
+
+	// 
+	const (
+		X4	int64	= iota
+		Y4
+	)
+
diff --git a/src/go/doc/testdata/issue16153.2.golden b/src/go/doc/testdata/issue16153.2.golden
new file mode 100644
index 0000000000..189260b4d0
--- /dev/null
+++ b/src/go/doc/testdata/issue16153.2.golden
@@ -0,0 +1,32 @@
+// 
+PACKAGE issue16153
+
+IMPORTPATH
+	testdata/issue16153
+
+FILENAMES
+	testdata/issue16153.go
+
+CONSTANTS
+	// 
+	const (
+		X3	int64	= iota
+		Y3		= 1
+	)
+
+	// 
+	const (
+		X4	int64	= iota
+		Y4
+	)
+
+	// original test case 
+	const (
+		Y1 = 256
+	)
+
+	// variations 
+	const (
+		Y2 uint8
+	)
+
diff --git a/src/go/doc/testdata/issue16153.go b/src/go/doc/testdata/issue16153.go
new file mode 100644
index 0000000000..528be42356
--- /dev/null
+++ b/src/go/doc/testdata/issue16153.go
@@ -0,0 +1,27 @@
+// Copyright 2017 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.
+
+package issue16153
+
+// original test case
+const (
+	x1 uint8 = 255
+	Y1       = 256
+)
+
+// variations
+const (
+	x2 uint8 = 255
+	Y2
+)
+
+const (
+	X3 int64 = iota
+	Y3       = 1
+)
+
+const (
+	X4 int64 = iota
+	Y4
+)