internal/frontend: support displaying multiple of same type

Previously on the versions page, we were not handling the case when
different identifiers are added for the same type for different build
contexts, and which build context was surfaced was based on chance.

To support this case, it is now possible to show multiple of the same
type at the same version. For example,
https://pkg.go.dev/internal/poll?tab=versions at go1.10 will show:

```
type FD — windows/amd64
+ func (fd *FD) ReadMsg(p []byte, oob []byte) (int, int, int, syscall.Sockaddr, error)
+ func (fd *FD) WriteMsg(p []byte, oob []byte, sa syscall.Sockaddr) (int, int, error)

type FD — darwin/amd64, linux/amd64
+ func (fd *FD) SetBlocking() error
+ func (fd *FD) WriteOnce(p []byte) (int, error)
```

For golang/go#37102

Change-Id: I19e6ef12f1f8f9c412aab7cea2782409eecf29f9
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/317489
Trust: Julie Qiu <julie@golang.org>
Run-TryBot: Julie Qiu <julie@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>

internal/frontend/symbol.go

@@ -55,7 +55,7 @@ type Symbol struct {
 	New bool
 }
 
-func (s *Symbol) addBuilds(builds []internal.BuildContext) {
+func (s *Symbol) addBuilds(builds ...internal.BuildContext) {
 	if s.builds == nil {
 		s.builds = map[internal.BuildContext]bool{}
 	}
@@ -94,18 +94,23 @@ func symbolsForVersion(pkgURLPath string, symbolsAtVersion map[string]map[intern
 				}
 				nameToMetaToSymbol[us.Name][sm] = s
 			}
-			s.addBuilds(us.BuildContexts())
+			s.addBuilds(us.BuildContexts()...)
 		}
 	}
 
 	for cm, cus := range children {
-		// Option 1: no parent exists
-		// - make one, add to map
-		// - append to parent
-		// Option 2: parent exists and supports child bc
-		// - append to parent
-		// Option 3 parent exists and does not support child bc
-		// - append to parent
+		// For each child symbol, 1 of 3 things can occur:
+		//
+		// Option 1: If no parent exists for this child symbol, make one
+		// and add the parent to the map.
+		//
+		// Option 2: A parent exists and does not support the build context
+		// of the child. This occurs when the parent type is introduced for
+		// another build context, but was introduced at the previous version
+		// for the current child. Create a new parent for this child.
+		//
+		// Option 3: A parent exists and does support the build context of
+		// the child. Add the child to the parent.
 		cs := &Symbol{
 			Name:     cm.Name,
 			Synopsis: cm.Synopsis,
@@ -115,37 +120,43 @@ func symbolsForVersion(pkgURLPath string, symbolsAtVersion map[string]map[intern
 			New:      true,
 		}
 
-		parents, ok := nameToMetaToSymbol[cm.ParentName]
-		var found bool
-		if ok {
-			for _, ps := range parents {
-				for build := range ps.builds {
-					if cus.SupportsBuild(build) {
-						ps.Children = append(ps.Children, cs)
-						found = true
-						break
-					}
-				}
-			}
-		}
-		if found {
+		ps := findParent(cm.ParentName, cus, nameToMetaToSymbol)
+		if ps != nil {
+			// Option 3: found a relevant parent.
+			ps.Children = append(ps.Children, cs)
 			continue
 		}
 
-		// We did not find a parent, so create one.
-		ps := createParent(cus, pkgURLPath)
-		ps.Children = append(ps.Children, cs)
+		// Option 1 and 2: We did not find a relevant parent, so create
+		// one.
+		//
+		// Since this parent is not introduced at this version, create
+		// a distinct type for each group of symbols.
+		// To do so, we make up a synopsis for the SymbolMeta below, since it
+		// is only used as a key in nameToMetaToSymbol.
+		//
+		// Example case:
+		// http://pkg.go.dev/internal/poll?tab=versions for go1.10 should show:
+		//
+		// type FD -- windows/amd64
+		//		FD.ReadMsg
+		//		FD.WriteMsg
+		// type FD -- darwin/amd64, linux/amd64
+		//		FD.SetBlocking
+		//		FD.WriteOnce
+		ps = createParent(cm.ParentName, pkgURLPath, cus.BuildContexts()...)
 		pm := internal.SymbolMeta{
 			Name:       ps.Name,
 			ParentName: ps.Name,
-			Synopsis:   ps.Synopsis,
+			Synopsis:   fmt.Sprintf("type %s (%v)", ps.Name, cus.BuildContexts()),
 			Section:    ps.Section,
 			Kind:       ps.Kind,
 		}
-		ps.addBuilds(cus.BuildContexts())
-		nameToMetaToSymbol[pm.Name] = map[internal.SymbolMeta]*Symbol{
-			pm: ps,
+		ps.Children = append(ps.Children, cs)
+		if _, ok := nameToMetaToSymbol[pm.Name]; !ok {
+			nameToMetaToSymbol[pm.Name] = map[internal.SymbolMeta]*Symbol{}
 		}
+		nameToMetaToSymbol[pm.Name][pm] = ps
 	}
 
 	var symbols []*Symbol
@@ -163,6 +174,22 @@ func symbolsForVersion(pkgURLPath string, symbolsAtVersion map[string]map[intern
 	return sortSymbols(symbols)
 }
 
+func findParent(parentName string, cus *internal.UnitSymbol,
+	nameToMetaToSymbol map[string]map[internal.SymbolMeta]*Symbol) *Symbol {
+	parents, ok := nameToMetaToSymbol[parentName]
+	if !ok {
+		return nil
+	}
+	for _, ps := range parents {
+		for build := range ps.builds {
+			if cus.SupportsBuild(build) {
+				return ps
+			}
+		}
+	}
+	return nil
+}
+
 func symbolLink(pkgURLPath, name string, builds []internal.BuildContext) string {
 	if len(builds) == len(internal.BuildContexts) {
 		return fmt.Sprintf("%s#%s", pkgURLPath, name)
@@ -179,15 +206,15 @@ func symbolLink(pkgURLPath, name string, builds []internal.BuildContext) string
 // different version. The symbol created will have New set to false, since this
 // function is only used when a parent symbol is not found for the unit symbol,
 // which means it was not introduced at the same version.
-func createParent(us *internal.UnitSymbol, pkgURLPath string) *Symbol {
+func createParent(parentName, pkgURLPath string, builds ...internal.BuildContext) *Symbol {
 	s := &Symbol{
-		Name:     us.ParentName,
-		Synopsis: fmt.Sprintf("type %s", us.ParentName),
+		Name:     parentName,
+		Synopsis: fmt.Sprintf("type %s", parentName),
 		Section:  internal.SymbolSectionTypes,
 		Kind:     internal.SymbolKindType,
-		Link:     symbolLink(pkgURLPath, us.ParentName, us.BuildContexts()),
+		Link:     symbolLink(pkgURLPath, parentName, builds),
 	}
-	s.addBuilds(us.BuildContexts())
+	s.addBuilds(builds...)
 	return s
 }
 
@@ -240,10 +267,31 @@ func sortSymbols(symbols []*Symbol) [][]*Symbol {
 
 func sortSymbolsGroup(syms []*Symbol) {
 	sort.Slice(syms, func(i, j int) bool {
-		return syms[i].Synopsis < syms[j].Synopsis
+		s1 := syms[i]
+		s2 := syms[j]
+		if s1.Synopsis != s2.Synopsis {
+			return s1.Synopsis < s2.Synopsis
+		}
+		return compareStringSlices(s1.Builds, s2.Builds) < 0
 	})
 }
 
+func compareStringSlices(ss1, ss2 []string) int {
+	for i, s1 := range ss1 {
+		if i >= len(ss2) { // first slice is longer, so greater
+			return 1
+		}
+		if c := strings.Compare(s1, ss2[i]); c != 0 {
+			return c
+		}
+	}
+	if len(ss1) == len(ss2) {
+		return 0
+	}
+	// first slice is shorter
+	return -1
+}
+
 // ParseVersionsDetails returns a map of versionToNameToUnitSymbol based on
 // data from the proovided VersionDetails.
 func ParseVersionsDetails(vd VersionsDetails) (_ *internal.SymbolHistory, err error) {

internal/frontend/symbol_test.go

@@ -0,0 +1,37 @@
+// Copyright 2021 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 frontend
+
+import (
+	"testing"
+)
+
+func TestCompareStringSlices(t *testing.T) {
+	a := []string{"a"}
+	ab := []string{"a", "b"}
+	ac := []string{"a", "c"}
+	for _, test := range []struct {
+		ss1, ss2 []string
+		want     int
+	}{
+		{nil, nil, 0},
+		{nil, a, -1},
+		{ab, ab, 0},
+		{a, ab, -1},
+		{ab, ac, -1},
+	} {
+		got := compareStringSlices(test.ss1, test.ss2)
+		if got != test.want {
+			t.Fatalf("%v, %v: got %d, want %d\n", test.ss1, test.ss2, got, test.want)
+		}
+		if test.want != 0 {
+			got := compareStringSlices(test.ss2, test.ss1)
+			want := -test.want
+			if got != want {
+				t.Fatalf("%v, %v: got %d, want %d\n", test.ss2, test.ss1, got, want)
+			}
+		}
+	}
+}

internal/postgres/symbol_history.go

@@ -93,8 +93,8 @@ func GetSymbolHistoryWithPackageSymbols(ctx context.Context, ddb *database.DB,
 // children names are also tracked.
 func GetSymbolHistoryForBuildContext(ctx context.Context, ddb *database.DB, pathID int, modulePath string,
 	bc internal.BuildContext) (_ map[string]string, err error) {
-	defer derrors.WrapStack(&err, "getSymbolHistoryForBuildContext(ctx, ddb, %d, %q)", pathID, modulePath)
-	defer middleware.ElapsedStat(ctx, "getSymbolHistoryForBuildContext")()
+	defer derrors.WrapStack(&err, "GetSymbolHistoryForBuildContext(ctx, ddb, %d, %q)", pathID, modulePath)
+	defer middleware.ElapsedStat(ctx, "GetSymbolHistoryForBuildContext")()
 
 	if bc == internal.BuildContextAll {
 		bc = internal.BuildContextLinux

internal/proxy/testdata/symbols@v1.1.0.txtar

@@ -107,6 +107,14 @@ func HelloJS() string {
 	return "Hello"
 }
 
+-- multigoos/multigoos.go --
+// +build darwin linux windows
+
+package multigoos
+
+// type FD is introduced for windows, linux and darwin at this version.
+type FD struct {}
+
 -- multigoos/multigoos_windows.go --
 // +build windows
 

internal/proxy/testdata/symbols@v1.2.0.txtar

@@ -113,6 +113,13 @@ func CloseOnExec(foo string) error {
     return nil
 }
 
+type FD struct {}
+
+// FD was introduced in v1.1.0 for linux, darwin and windows.
+// MyWindowsMethod is introduced only for windows in this version.
+func (*FD) MyWindowsMethod() {
+}
+
 -- multigoos/multigoos_unix.go --
 // +build aix darwin dragonfly freebsd linux netbsd openbsd solaris
 
@@ -122,6 +129,13 @@ func CloseOnExec(num int) (int, error) {
     return num, nil
 }
 
+type FD struct {}
+
+// FD was introduced in v1.1.0 for linux, darwin and windows.
+// MyMethod is introduced only for darwin and linux in this version.
+func (*FD) MyMethod() {
+}
+
 -- multigoos/multigoos_js.go --
 // +build js,wasm
 

internal/symbol.go

@@ -140,6 +140,9 @@ func (sh *SymbolHistory) GetSymbol(name, v string, build BuildContext) (_ *UnitS
 
 // AddSymbol adds the given symbol to SymbolHistory.
 func (sh *SymbolHistory) AddSymbol(sm SymbolMeta, v string, build BuildContext) {
+	if v == "v1.10.0" && (sm.Name == "FD" || sm.ParentName == "FD") {
+		fmt.Println(build, v, sm.Name, sm.Synopsis)
+	}
 	sav, ok := sh.m[v]
 	if !ok {
 		sav = map[string]map[SymbolMeta]*UnitSymbol{}

internal/testing/integration/data_frontend_versions_test.go

@@ -119,6 +119,44 @@ var versionsPageMultiGoos = []*frontend.VersionList{
 							Builds:   []string{"js/wasm"},
 						},
 					},
+					{
+						{
+							Name:     "FD",
+							Synopsis: "type FD",
+							Section:  "Types",
+							Kind:     "Type",
+							Link:     "/example.com/symbols@v1.2.0/multigoos?GOOS=darwin#FD",
+							Children: []*frontend.Symbol{
+								{
+									Name:     "FD.MyMethod",
+									Synopsis: "func (*FD) MyMethod()",
+									Section:  "Types",
+									Kind:     "Method",
+									Link:     "/example.com/symbols@v1.2.0/multigoos?GOOS=darwin#FD.MyMethod",
+									New:      true,
+								},
+							},
+							Builds: []string{"darwin/amd64", "linux/amd64"},
+						},
+						{
+							Name:     "FD",
+							Synopsis: "type FD",
+							Section:  "Types",
+							Kind:     "Type",
+							Link:     "/example.com/symbols@v1.2.0/multigoos?GOOS=windows#FD",
+							Children: []*frontend.Symbol{
+								{
+									Name:     "FD.MyWindowsMethod",
+									Synopsis: "func (*FD) MyWindowsMethod()",
+									Section:  "Types",
+									Kind:     "Method",
+									Link:     "/example.com/symbols@v1.2.0/multigoos?GOOS=windows#FD.MyWindowsMethod",
+									New:      true,
+								},
+							},
+							Builds: []string{"windows/amd64"},
+						},
+					},
 				},
 			},
 			{
@@ -149,6 +187,17 @@ var versionsPageMultiGoos = []*frontend.VersionList{
 							Builds:   []string{"darwin/amd64", "linux/amd64"},
 						},
 					},
+					{
+						{
+							Name:     "FD",
+							Synopsis: "type FD struct",
+							Section:  "Types",
+							Kind:     "Type",
+							Link:     "/example.com/symbols@v1.1.0/multigoos?GOOS=darwin#FD",
+							Builds:   []string{"darwin/amd64", "linux/amd64", "windows/amd64"},
+							New:      true,
+						},
+					},
 				},
 			},
 		},