cmd/api: require proposal # for new API features

Having the proposal numbers recorded in the API files
should help significantly when it comes time to audit
the new API additions at the end of each release cycle.

Change-Id: Id18e8cbdf892228a10ac17e4e21c7e17de5d4ff7
Reviewed-on: https://go-review.googlesource.com/c/go/+/392414
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>

2 files changed, 83 insertions, 41 deletions

diff --git a/src/cmd/api/goapi.go b/src/cmd/api/goapi.go
index 5ae059e4ce..2a0e109575 100644
--- a/src/cmd/api/goapi.go
+++ b/src/cmd/api/goapi.go
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// Binary api computes the exported API of a set of Go packages.
+// Api computes the exported API of a set of Go packages.
 package main
 
 import (
@@ -24,6 +24,7 @@ import (
 	"regexp"
 	"runtime"
 	"sort"
+	"strconv"
 	"strings"
 	"sync"
 )
@@ -42,12 +43,13 @@ func goCmd() string {
 
 // Flags
 var (
-	checkFile  = flag.String("c", "", "optional comma-separated filename(s) to check API against")
-	allowNew   = flag.Bool("allow_new", true, "allow API additions")
-	exceptFile = flag.String("except", "", "optional filename of packages that are allowed to change without triggering a failure in the tool")
-	nextFile   = flag.String("next", "", "optional filename of tentative upcoming API features for the next release. This file can be lazily maintained. It only affects the delta warnings from the -c file printed on success.")
-	verbose    = flag.Bool("v", false, "verbose debugging")
-	forceCtx   = flag.String("contexts", "", "optional comma-separated list of <goos>-<goarch>[-cgo] to override default contexts.")
+	checkFiles      = flag.String("c", "", "optional comma-separated filename(s) to check API against")
+	requireApproval = flag.String("approval", "", "require approvals in comma-separated list of `files`")
+	allowNew        = flag.Bool("allow_new", true, "allow API additions")
+	exceptFile      = flag.String("except", "", "optional filename of packages that are allowed to change without triggering a failure in the tool")
+	nextFiles       = flag.String("next", "", "comma-separated list of `files` for upcoming API features for the next release. These files can be lazily maintained. They only affects the delta warnings from the -c file printed on success.")
+	verbose         = flag.Bool("v", false, "verbose debugging")
+	forceCtx        = flag.String("contexts", "", "optional comma-separated list of <goos>-<goarch>[-cgo] to override default contexts.")
 )
 
 // contexts are the default contexts which are scanned, unless
@@ -126,9 +128,9 @@ func main() {
 	flag.Parse()
 
 	if !strings.Contains(runtime.Version(), "weekly") && !strings.Contains(runtime.Version(), "devel") {
-		if *nextFile != "" {
-			fmt.Printf("Go version is %q, ignoring -next %s\n", runtime.Version(), *nextFile)
-			*nextFile = ""
+		if *nextFiles != "" {
+			fmt.Printf("Go version is %q, ignoring -next %s\n", runtime.Version(), *nextFiles)
+			*nextFiles = ""
 		}
 	}
 
@@ -201,7 +203,7 @@ func main() {
 	bw := bufio.NewWriter(os.Stdout)
 	defer bw.Flush()
 
-	if *checkFile == "" {
+	if *checkFiles == "" {
 		sort.Strings(features)
 		for _, f := range features {
 			fmt.Fprintln(bw, f)
@@ -210,10 +212,15 @@ func main() {
 	}
 
 	var required []string
-	for _, file := range strings.Split(*checkFile, ",") {
+	for _, file := range strings.Split(*checkFiles, ",") {
 		required = append(required, fileFeatures(file)...)
 	}
-	optional := fileFeatures(*nextFile)
+	var optional []string
+	if *nextFiles != "" {
+		for _, file := range strings.Split(*nextFiles, ",") {
+			optional = append(optional, fileFeatures(file)...)
+		}
+	}
 	exception := fileFeatures(*exceptFile)
 	fail = !compareAPI(bw, features, required, optional, exception, *allowNew)
 }
@@ -340,6 +347,13 @@ func fileFeatures(filename string) []string {
 	if filename == "" {
 		return nil
 	}
+	needApproval := false
+	for _, name := range strings.Split(*requireApproval, ",") {
+		if filename == name {
+			needApproval = true
+			break
+		}
+	}
 	bs, err := os.ReadFile(filename)
 	if err != nil {
 		log.Fatalf("Error reading file %s: %v", filename, err)
@@ -348,11 +362,23 @@ func fileFeatures(filename string) []string {
 	s = aliasReplacer.Replace(s)
 	lines := strings.Split(s, "\n")
 	var nonblank []string
-	for _, line := range lines {
+	for i, line := range lines {
 		line = strings.TrimSpace(line)
-		if line != "" && !strings.HasPrefix(line, "#") {
-			nonblank = append(nonblank, line)
+		if line == "" || strings.HasPrefix(line, "#") {
+			continue
+		}
+		if needApproval {
+			feature, approval, ok := strings.Cut(line, "#")
+			if !ok {
+				log.Fatalf("%s:%d: missing proposal approval\n", filename, i+1)
+			}
+			_, err := strconv.Atoi(approval)
+			if err != nil {
+				log.Fatalf("%s:%d: malformed proposal approval #%s\n", filename, i+1, approval)
+			}
+			line = strings.TrimSpace(feature)
 		}
+		nonblank = append(nonblank, line)
 	}
 	return nonblank
 }
diff --git a/src/cmd/api/run.go b/src/cmd/api/run.go
index 1b94a1b883..130166e7b9 100644
--- a/src/cmd/api/run.go
+++ b/src/cmd/api/run.go
@@ -18,6 +18,7 @@ import (
 	"os"
 	"path/filepath"
 	"runtime"
+	"strconv"
 	"strings"
 )
 
@@ -41,51 +42,66 @@ func main() {
 	if goroot == "" {
 		log.Fatal("No $GOROOT set.")
 	}
-
-	apiDir := filepath.Join(goroot, "api")
-	out, err := exec.Command(goCmd(), "tool", "api",
-		"-c", findAPIDirFiles(apiDir),
-		allowNew(apiDir),
-		"-next", filepath.Join(apiDir, "next.txt"),
-		"-except", filepath.Join(apiDir, "except.txt")).CombinedOutput()
-	if err != nil {
-		log.Fatalf("Error running API checker: %v\n%s", err, out)
+	if err := os.Chdir(filepath.Join(goroot, "api")); err != nil {
+		log.Fatal(err)
 	}
-	fmt.Print(string(out))
-}
 
-// findAPIDirFiles returns a comma-separated list of Go API files
-// (go1.txt, go1.1.txt, etc.) located in apiDir.
-func findAPIDirFiles(apiDir string) string {
-	dir, err := os.Open(apiDir)
+	files, err := filepath.Glob("go1*.txt")
 	if err != nil {
 		log.Fatal(err)
 	}
-	defer dir.Close()
-	fs, err := dir.Readdirnames(-1)
+	next, err := filepath.Glob(filepath.Join("next", "*.txt"))
 	if err != nil {
 		log.Fatal(err)
 	}
-	var apiFiles []string
-	for _, fn := range fs {
-		if strings.HasPrefix(fn, "go1") {
-			apiFiles = append(apiFiles, filepath.Join(apiDir, fn))
+	cmd := exec.Command(goCmd(), "tool", "api",
+		"-c", strings.Join(files, ","),
+		"-approval", strings.Join(append(approvalNeeded(files), next...), ","),
+		allowNew(),
+		"-next", strings.Join(next, ","),
+		"-except", "except.txt",
+	)
+	fmt.Println(cmd.Args)
+	out, err := cmd.CombinedOutput()
+	if err != nil {
+		log.Fatalf("Error running API checker: %v\n%s", err, out)
+	}
+	fmt.Print(string(out))
+}
+
+func approvalNeeded(files []string) []string {
+	var out []string
+	for _, f := range files {
+		name := filepath.Base(f)
+		if name == "go1.txt" {
+			continue
+		}
+		minor := strings.TrimSuffix(strings.TrimPrefix(name, "go1."), ".txt")
+		n, err := strconv.Atoi(minor)
+		if err != nil {
+			log.Fatalf("unexpected api file: %v", f)
+		}
+		if n >= 19 { // approvals started being tracked in Go 1.19
+			out = append(out, f)
 		}
 	}
-	return strings.Join(apiFiles, ",")
+	return out
 }
 
 // allowNew returns the -allow_new flag to use for the 'go tool api' invocation.
-func allowNew(apiDir string) string {
+func allowNew() string {
+	// Experiment for Go 1.19: always require api file updates.
+	return "-allow_new=false"
+
 	// Verify that the api/go1.n.txt for previous Go version exists.
 	// It definitely should, otherwise it's a signal that the logic below may be outdated.
-	if _, err := os.Stat(filepath.Join(apiDir, fmt.Sprintf("go1.%d.txt", goversion.Version-1))); err != nil {
+	if _, err := os.Stat(fmt.Sprintf("go1.%d.txt", goversion.Version-1)); err != nil {
 		log.Fatalln("Problem with api file for previous release:", err)
 	}
 
 	// See whether the api/go1.n.txt for this Go version has been created.
 	// (As of April 2021, it gets created during the release of the first Beta.)
-	_, err := os.Stat(filepath.Join(apiDir, fmt.Sprintf("go1.%d.txt", goversion.Version)))
+	_, err := os.Stat(fmt.Sprintf("go1.%d.txt", goversion.Version))
 	if errors.Is(err, fs.ErrNotExist) {
 		// It doesn't exist, so we're in development or before Beta 1.
 		// At this stage, unmentioned API additions are deemed okay.