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/68730Reviewed-by: Alan Donovan <adonovan@google.com>

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())
 			}

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.

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. 

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

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. 

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

src/go/doc/testdata/issue16153.0.golden

+// 
+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
+	)
+

src/go/doc/testdata/issue16153.1.golden

+// 
+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
+	)
+

src/go/doc/testdata/issue16153.2.golden

+// 
+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
+	)
+

src/go/doc/testdata/issue16153.go

+// 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
+)