cmd/api: remove unused functionality

We no longer use the optional parameter to compareAPI.
We now always set allowAdd to false.
(Except in tests, making them less useful than they could be.)
Flags and parsing their value are no more.

Remove all the unused functionality and update test cases so they're
closer to what the API checker does when it runs for real. Order the
features, required, exception variables and fields more consistently.

For #43956.

Change-Id: Iaa4656a89a3fca3129742165a448d385e55e4a98
Reviewed-on: https://go-review.googlesource.com/c/go/+/489436
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

src/cmd/api/api.go

@@ -10,7 +10,6 @@ import (
 	"bufio"
 	"bytes"
 	"encoding/json"
-	"flag"
 	"fmt"
 	"go/ast"
 	"go/build"
@@ -46,8 +45,7 @@ func goCmd() string {
 	return "go"
 }
 
-// contexts are the default contexts which are scanned, unless
-// overridden by the -contexts flag.
+// contexts are the default contexts which are scanned.
 var contexts = []*build.Context{
 	{GOOS: "linux", GOARCH: "386", CgoEnabled: true},
 	{GOOS: "linux", GOARCH: "386"},
@@ -96,25 +94,6 @@ func contextName(c *build.Context) string {
 	return s
 }
 
-func parseContext(c string) *build.Context {
-	parts := strings.Split(c, "-")
-	if len(parts) < 2 {
-		log.Fatalf("bad context: %q", c)
-	}
-	bc := &build.Context{
-		GOOS:   parts[0],
-		GOARCH: parts[1],
-	}
-	if len(parts) == 3 {
-		if parts[2] == "cgo" {
-			bc.CgoEnabled = true
-		} else {
-			log.Fatalf("bad context: %q", c)
-		}
-	}
-	return bc
-}
-
 var internalPkg = regexp.MustCompile(`(^|/)internal($|/)`)
 
 var exitCode = 0
@@ -152,12 +131,7 @@ func Check(t *testing.T) {
 
 	var featureCtx = make(map[string]map[string]bool) // feature -> context name -> true
 	for _, w := range walkers {
-		pkgNames := w.stdPackages
-		if flag.NArg() > 0 {
-			pkgNames = flag.Args()
-		}
-
-		for _, name := range pkgNames {
+		for _, name := range w.stdPackages {
 			pkg, err := w.import_(name)
 			if _, nogo := err.(*build.NoGoError); nogo {
 				continue
@@ -193,7 +167,7 @@ func Check(t *testing.T) {
 	bw := bufio.NewWriter(os.Stdout)
 	defer bw.Flush()
 
-	var required, optional []string
+	var required []string
 	for _, file := range checkFiles {
 		required = append(required, fileFeatures(file, needApproval(file))...)
 	}
@@ -205,7 +179,7 @@ func Check(t *testing.T) {
 	if exitCode == 1 {
 		t.Errorf("API database problems found")
 	}
-	if !compareAPI(bw, features, required, optional, exception, false) {
+	if !compareAPI(bw, features, required, exception) {
 		t.Errorf("API differences found")
 	}
 }
@@ -251,12 +225,11 @@ func portRemoved(feature string) bool {
 		strings.Contains(feature, "(darwin-386-cgo)")
 }
 
-func compareAPI(w io.Writer, features, required, optional, exception []string, allowAdd bool) (ok bool) {
+func compareAPI(w io.Writer, features, required, exception []string) (ok bool) {
 	ok = true
 
-	optionalSet := set(optional)
-	exceptionSet := set(exception)
 	featureSet := set(features)
+	exceptionSet := set(exception)
 
 	sort.Strings(features)
 	sort.Strings(required)
@@ -267,7 +240,7 @@ func compareAPI(w io.Writer, features, required, optional, exception []string, a
 		return s
 	}
 
-	for len(required) > 0 || len(features) > 0 {
+	for len(features) > 0 || len(required) > 0 {
 		switch {
 		case len(features) == 0 || (len(required) > 0 && required[0] < features[0]):
 			feature := take(&required)
@@ -288,33 +261,15 @@ func compareAPI(w io.Writer, features, required, optional, exception []string, a
 			}
 		case len(required) == 0 || (len(features) > 0 && required[0] > features[0]):
 			newFeature := take(&features)
-			if optionalSet[newFeature] {
-				// Known added feature to the upcoming release.
-				// Delete it from the map so we can detect any upcoming features
-				// which were never seen.  (so we can clean up the nextFile)
-				delete(optionalSet, newFeature)
-			} else {
-				fmt.Fprintf(w, "+%s\n", newFeature)
-				if !allowAdd {
-					ok = false // we're in lock-down mode for next release
-				}
-			}
+			fmt.Fprintf(w, "+%s\n", newFeature)
+			ok = false // feature not in api/next/*
 		default:
 			take(&required)
 			take(&features)
 		}
 	}
 
-	// In next file, but not in API.
-	var missing []string
-	for feature := range optionalSet {
-		missing = append(missing, feature)
-	}
-	sort.Strings(missing)
-	for _, feature := range missing {
-		fmt.Fprintf(w, "±%s\n", feature)
-	}
-	return
+	return ok
 }
 
 // aliasReplacer applies type aliases to earlier API files,

src/cmd/api/api_test.go

@@ -115,16 +115,23 @@ func TestGolden(t *testing.T) {
 
 func TestCompareAPI(t *testing.T) {
 	tests := []struct {
-		name                                    string
-		features, required, optional, exception []string
-		ok                                      bool   // want
-		out                                     string // want
+		name                          string
+		features, required, exception []string
+		ok                            bool   // want
+		out                           string // want
 	}{
+		{
+			name:     "equal",
+			features: []string{"A", "B", "C"},
+			required: []string{"A", "B", "C"},
+			ok:       true,
+			out:      "",
+		},
 		{
 			name:     "feature added",
 			features: []string{"A", "B", "C", "D", "E", "F"},
 			required: []string{"B", "D"},
-			ok:       true,
+			ok:       false,
 			out:      "+A\n+C\n+E\n+F\n",
 		},
 		{
@@ -134,42 +141,52 @@ func TestCompareAPI(t *testing.T) {
 			ok:       false,
 			out:      "-B\n",
 		},
-		{
-			name:     "feature added then removed",
-			features: []string{"A", "C"},
-			optional: []string{"B"},
-			required: []string{"A", "C"},
-			ok:       true,
-			out:      "±B\n",
-		},
 		{
 			name:      "exception removal",
-			required:  []string{"A", "B", "C"},
 			features:  []string{"A", "C"},
+			required:  []string{"A", "B", "C"},
 			exception: []string{"B"},
 			ok:        true,
 			out:       "",
 		},
+
+		// Test that a feature required on a subset of ports is implicitly satisfied
+		// by the same feature being implemented on all ports. That is, it shouldn't
+		// say "pkg syscall (darwin-amd64), type RawSockaddrInet6 struct" is missing.
+		// See https://go.dev/issue/4303.
 		{
-			// https://golang.org/issue/4303
-			name: "contexts reconverging",
-			required: []string{
-				"A",
-				"pkg syscall (darwin-amd64), type RawSockaddrInet6 struct",
-			},
+			name: "contexts reconverging after api/next/* update",
 			features: []string{
 				"A",
 				"pkg syscall, type RawSockaddrInet6 struct",
 			},
+			required: []string{
+				"A",
+				"pkg syscall (darwin-amd64), type RawSockaddrInet6 struct", // api/go1.n.txt
+				"pkg syscall, type RawSockaddrInet6 struct",                // api/next/n.txt
+			},
 			ok:  true,
+			out: "",
+		},
+		{
+			name: "contexts reconverging before api/next/* update",
+			features: []string{
+				"A",
+				"pkg syscall, type RawSockaddrInet6 struct",
+			},
+			required: []string{
+				"A",
+				"pkg syscall (darwin-amd64), type RawSockaddrInet6 struct",
+			},
+			ok:  false,
 			out: "+pkg syscall, type RawSockaddrInet6 struct\n",
 		},
 	}
 	for _, tt := range tests {
 		buf := new(strings.Builder)
-		gotok := compareAPI(buf, tt.features, tt.required, tt.optional, tt.exception, true)
-		if gotok != tt.ok {
-			t.Errorf("%s: ok = %v; want %v", tt.name, gotok, tt.ok)
+		gotOK := compareAPI(buf, tt.features, tt.required, tt.exception)
+		if gotOK != tt.ok {
+			t.Errorf("%s: ok = %v; want %v", tt.name, gotOK, tt.ok)
 		}
 		if got := buf.String(); got != tt.out {
 			t.Errorf("%s: output differs\nGOT:\n%s\nWANT:\n%s", tt.name, got, tt.out)