relnote: check issue relnotes

Add a function that checks that every api file has corresponding
release note files.

The CheckAPIFunction will be used by a test in the main repo.

For golang/go#64169.

Change-Id: I59e28deaf1209e289e2e29d13483629a951cb2d9
Reviewed-on: https://go-review.googlesource.com/c/build/+/556715
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>

relnote/relnote.go

@@ -5,33 +5,23 @@
 // Package relnote supports working with release notes.
 //
 // Its main feature is the ability to merge Markdown fragments into a single
-// document.
+// document. (See [Merge].)
 //
 // This package has minimal imports, so that it can be vendored into the
 // main go repo.
-//
-// # Fragments
-//
-// A release note fragment is designed to be merged into a final document.
-// The merging is done by matching headings, and inserting the contents
-// of that heading (that is, the non-heading blocks following it) into
-// the merged document.
-//
-// If the text of a heading begins with '+', then it doesn't have to match
-// with an existing heading. If it doesn't match, the heading and its contents
-// are both inserted into the result.
-//
-// A fragment must begin with a non-empty matching heading.
 package relnote
 
 import (
+	"bufio"
 	"bytes"
 	"errors"
 	"fmt"
 	"io"
 	"io/fs"
 	"path"
+	"regexp"
 	"slices"
+	"strconv"
 	"strings"
 
 	md "rsc.io/markdown"
@@ -121,7 +111,7 @@ func Merge(fsys fs.FS) (*md.Document, error) {
 	doc := &md.Document{Links: map[string]*md.Link{}}
 	var prevPkg string // previous stdlib package, if any
 	for _, filename := range filenames {
-		newdoc, err := parseFile(fsys, filename)
+		newdoc, err := parseMarkdownFile(fsys, filename)
 		if err != nil {
 			return nil, err
 		}
@@ -303,7 +293,7 @@ func position(b md.Block) *md.Position {
 	}
 }
 
-func parseFile(fsys fs.FS, path string) (*md.Document, error) {
+func parseMarkdownFile(fsys fs.FS, path string) (*md.Document, error) {
 	f, err := fsys.Open(path)
 	if err != nil {
 		return nil, err
@@ -317,3 +307,109 @@ func parseFile(fsys fs.FS, path string) (*md.Document, error) {
 	doc := NewParser().Parse(in)
 	return doc, nil
 }
+
+// An APIFeature is a symbol mentioned in an API file,
+// like the ones in the main go repo in the api directory.
+type APIFeature struct {
+	Package string // package that the feature is in
+	Feature string // everything about the feature other than the package
+	Issue   int    // the issue that introduced the feature, or 0 if none
+}
+
+var apiFileLineRegexp = regexp.MustCompile(`^pkg ([^,]+), ([^#]*)(#\d+)?$`)
+
+// parseAPIFile parses a file in the api format and returns a list of the file's features.
+// A feature is represented by a single line that looks like
+//
+//	PKG WORDS #ISSUE
+func parseAPIFile(fsys fs.FS, filename string) ([]APIFeature, error) {
+	f, err := fsys.Open(filename)
+	if err != nil {
+		return nil, err
+	}
+	defer f.Close()
+	var features []APIFeature
+	scan := bufio.NewScanner(f)
+	for scan.Scan() {
+		line := strings.TrimSpace(scan.Text())
+		if line == "" {
+			continue
+		}
+		matches := apiFileLineRegexp.FindStringSubmatch(line)
+		if len(matches) == 0 {
+			return nil, fmt.Errorf("%s: malformed line %q", filename, line)
+		}
+		f := APIFeature{
+			Package: matches[1],
+			Feature: strings.TrimSpace(matches[2]),
+		}
+		if len(matches) > 3 && len(matches[3]) > 0 {
+			var err error
+			f.Issue, err = strconv.Atoi(matches[3][1:]) // skip leading '#'
+			if err != nil {
+				return nil, err
+			}
+		}
+		features = append(features, f)
+	}
+	if scan.Err() != nil {
+		return nil, scan.Err()
+	}
+	return features, nil
+}
+
+// GroupAPIFeaturesByFile returns a map of the given features keyed by
+// the doc filename that they are associated with.
+// A feature with package P and issue N should be documented in the file
+// "P/N.md".
+func GroupAPIFeaturesByFile(fs []APIFeature) (map[string][]APIFeature, error) {
+	m := map[string][]APIFeature{}
+	for _, f := range fs {
+		if f.Issue == 0 {
+			return nil, fmt.Errorf("%+v: zero issue", f)
+		}
+		filename := fmt.Sprintf("%s/%d.md", f.Package, f.Issue)
+		m[filename] = append(m[filename], f)
+	}
+	return m, nil
+}
+
+// CheckAPIFile reads the api file at filename in apiFS, and checks the corresponding
+// release-note files under docFS. It checks that the files exist and that they have
+// some minimal content (see [CheckFragment]).
+func CheckAPIFile(apiFS fs.FS, filename string, docFS fs.FS) error {
+	features, err := parseAPIFile(apiFS, filename)
+	if err != nil {
+		return err
+	}
+	byFile, err := GroupAPIFeaturesByFile(features)
+	if err != nil {
+		return err
+	}
+	var filenames []string
+	for fn := range byFile {
+		filenames = append(filenames, fn)
+	}
+	slices.Sort(filenames)
+	var errs []error
+	for _, filename := range filenames {
+		// TODO(jba): check that the file mentions each feature?
+		if err := checkFragmentFile(docFS, filename); err != nil {
+			errs = append(errs, fmt.Errorf("%s: %v", filename, err))
+		}
+	}
+	return errors.Join(errs...)
+}
+
+func checkFragmentFile(fsys fs.FS, filename string) error {
+	f, err := fsys.Open(filename)
+	if err != nil {
+		if errors.Is(err, fs.ErrNotExist) {
+			err = fs.ErrNotExist
+		}
+		return err
+	}
+	defer f.Close()
+	data, err := io.ReadAll(f)
+	return CheckFragment(string(data))
+}

relnote/relnote_test.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"io/fs"
 	"path/filepath"
+	"reflect"
 	"slices"
 	"strings"
 	"testing"
@@ -131,6 +132,9 @@ func dump(d *md.Document) {
 		}
 	}
 }
+
+// parseTestFile translates a txtar archive into an fs.FS, except for the
+// file "want", whose contents are returned separately.
 func parseTestFile(filename string) (fsys fs.FS, want string, err error) {
 	ar, err := txtar.ParseFile(filename)
 	if err != nil {
@@ -212,3 +216,50 @@ something
 		t.Errorf("\ngot:\n%s\nwant:\n%s", got, want)
 	}
 }
+
+func TestParseAPIFile(t *testing.T) {
+	fsys := fstest.MapFS{
+		"123.next": &fstest.MapFile{Data: []byte(`
+pkg p1, type T struct
+pkg p2, func F(int, bool) #123
+	`)},
+	}
+	got, err := parseAPIFile(fsys, "123.next")
+	if err != nil {
+		t.Fatal(err)
+	}
+	want := []APIFeature{
+		{"p1", "type T struct", 0},
+		{"p2", "func F(int, bool)", 123},
+	}
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("\ngot  %+v\nwant %+v", got, want)
+	}
+}
+
+func TestCheckAPIFile(t *testing.T) {
+	testFiles, err := filepath.Glob(filepath.Join("testdata", "checkAPIFile", "*.txt"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(testFiles) == 0 {
+		t.Fatal("no tests")
+	}
+	for _, f := range testFiles {
+		t.Run(strings.TrimSuffix(filepath.Base(f), ".txt"), func(t *testing.T) {
+			fsys, want, err := parseTestFile(f)
+			if err != nil {
+				t.Fatal(err)
+			}
+			var got string
+			gotErr := CheckAPIFile(fsys, "api.txt", fsys)
+			if gotErr != nil {
+				got = gotErr.Error()
+			}
+			want = strings.TrimSpace(want)
+			if got != want {
+				t.Errorf("\ngot  %s\nwant %s", got, want)
+			}
+		})
+	}
+}

relnote/testdata/checkAPIFile/fail.txt

@@ -0,0 +1,11 @@
+errors:
+- foo/123.md does not exist
+- bar/123.md does, but doesn't have the right content.
+-- api.txt --
+pkg foo, type T #123
+pkg bar, type T #123
+-- bar/123.md --
+Not a sentence
+-- want --
+bar/123.md: needs a TODO or a sentence
+foo/123.md: file does not exist

relnote/testdata/checkAPIFile/ok.txt

@@ -0,0 +1,9 @@
+-- api.txt --
+pkg foo, type T #123
+pkg bar, type T #123
+-- foo/123.md --
+TODO
+-- bar/123.md --
+A sentence.
+-- want --
+