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>

diff --git a/src/go/doc/exports.go b/src/go/doc/exports.go
index da9ea1f027db5efa3ba522e3a00dd593fdb69b01..40cbb22797e06081ef81f599393092eaaedadf3c 100644 (file)
--- 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 17635f05615b42eaa3eb6b1d80db9611806e68fe..140f5872336dd7eea8df178209bd5d2a55401578 100644 (file)
--- 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 c2987cf140ec4dfa0a9e78e647b698f52c38f75c..70f2929f8a6f6a978289b38e1de8c11536ae3a9a 100644 (file)
--- 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 ee5054a4ed616f73169eadfe5741a06012d9b242..8098cb6e885957615aa737fbca2ded0b64a83c84 100644 (file)
--- 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 c2987cf140ec4dfa0a9e78e647b698f52c38f75c..70f2929f8a6f6a978289b38e1de8c11536ae3a9a 100644 (file)
--- 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 419a78f7d5116f0779f3f9d2381fe771ba3cbac7..5ea6186935a1383dc326f726d15685aaf2bdb388 100644 (file)
--- 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 (file)
index 0000000..189260b
--- /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 (file)
index 0000000..803df3e
--- /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 (file)
index 0000000..189260b
--- /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 (file)
index 0000000..528be42
--- /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
+)