Point a downloaded binary at the file that is beside it

cmd/lab/main.go

@@ -71,12 +71,29 @@ const (
 // Nothing outside this package holds these strings to the tree. The paths leg
 // of the invariants scan reads this repository's own documents, which is the
 // files at the root and everything under docs/, and a path named inside the
-// runner is outside that subject by the leg's own reckoning. So the guard is
-// the test beside this declaration and there is no second one.
-var documentsAnOperatorIsOwed = []string{
-	"NOTICE.md",
-	"LICENSE",
-	"docs/privacy.md",
+// runner is outside that subject by the leg's own reckoning. So the guards are
+// the tests beside this declaration and there are none anywhere else.
+//
+// A document has TWO SPELLINGS AND THE PARAGRAPH OWES BOTH, which is what
+// issue #235 found. inTree is where a reader of a checkout looks. beside is the
+// name the same file carries as a release asset, and it is the base of inTree
+// because the release copies the three files in flat, beside the binaries,
+// rather than inside a directory or an archive.
+//
+// Two of the three are the same word twice and the third is not. A paragraph
+// naming only inTree sends somebody holding a download into a docs/ that is not
+// in front of them; one naming only beside sends a reader of a checkout to a
+// file at the root that is not there either. So the paragraph names both, and
+// the tests below hold each spelling to the layout it is for.
+type document struct {
+	inTree string
+	beside string
+}
+
+var documentsAnOperatorIsOwed = []document{
+	{inTree: "NOTICE.md", beside: "NOTICE.md"},
+	{inTree: "LICENSE", beside: "LICENSE"},
+	{inTree: "docs/privacy.md", beside: "privacy.md"},
 }
 
 // documentsParagraph is the pointer the operator is owed, and it is one string
@@ -85,8 +102,9 @@ var documentsAnOperatorIsOwed = []string{
 // one of the three files moves - which is the same failure the paragraph itself
 // is written against, one level up.
 const documentsParagraph = `NOTICE.md says what this program is for, LICENSE carries the terms it is under,
-and docs/privacy.md says what stays on the host. Reading them is on you; this
-text only says where they are.
+and privacy.md says what stays on the host. Those are the names beside a
+downloaded binary; in a checkout the third one is docs/privacy.md. Reading them
+is on you; this text only says where they are.
 `
 
 const usage = `lab reads this repository and reports what it examined.

cmd/lab/main_test.go

@@ -4,8 +4,10 @@ import (
 	"bytes"
 	"errors"
 	"os"
+	"path"
 	"path/filepath"
 	"runtime/debug"
+	"sort"
 	"strings"
 	"testing"
 	"time"
@@ -236,16 +238,26 @@ func TestTheDocumentedCodesAreTheNumbersTheRecordFixes(t *testing.T) {
 }
 
 // TestHelpNamesTheDocumentsAnOperatorIsOwed holds the last paragraph of the
-// usage text to the three files it points at. An operator who reads that
-// paragraph and then looks for one of the files is the reader this asserts for,
-// and both halves of the walk they make are here: that the text names the file,
-// and that the file is in the tree to be found.
+// usage text to the three files it points at, in both of the layouts those
+// files are read in. An operator who reads that paragraph and then looks for
+// one of the files is the reader this asserts for, and the walk they make is
+// made from a checkout or from a download rather than from one of the two.
 //
-// The second half is the one that earns its place. A pointer in the runner is
-// outside the subject of the invariants paths leg, which reads the files at the
-// root and everything under docs/ and holds those to the paths they name. So a
-// document deleted or moved under this paragraph reddens nothing anywhere else,
-// and a binary would go on telling operators to read a file that is not there.
+// So it asserts three things. That the text names the checkout spelling, that
+// it names the download spelling, and that the checkout spelling is in this
+// tree to be found.
+//
+// The tree half is the one that earns its place against the rest of the suite.
+// A pointer in the runner is outside the subject of the invariants paths leg,
+// which reads the files at the root and everything under docs/ and holds those
+// to the paths they name. So a document deleted or moved under this paragraph
+// reddens nothing anywhere else, and a binary would go on telling operators to
+// read a file that is not there.
+//
+// The download half is the one issue #235 opened, and namesOnItsOwn is what
+// makes it an assertion rather than a formality: docs/privacy.md contains
+// privacy.md, so a containment test is satisfied by a paragraph naming only the
+// checkout spelling and would have been green through the whole defect.
 //
 // What it does not judge is whether any of the three says what it should. That
 // is a reading of prose and no test here makes it.
@@ -256,11 +268,13 @@ func TestHelpNamesTheDocumentsAnOperatorIsOwed(t *testing.T) {
 	}
 
 	for _, document := range documentsAnOperatorIsOwed {
-		if !strings.Contains(out.String(), document) {
-			t.Errorf("the help output does not name %s:\n%s", document, out.String())
+		for _, spelling := range []string{document.inTree, document.beside} {
+			if !namesOnItsOwn(out.String(), spelling) {
+				t.Errorf("the help output does not name %s as a path in its own right:\n%s", spelling, out.String())
+			}
 		}
-		if _, err := os.Stat(filepath.Join("..", "..", filepath.FromSlash(document))); err != nil {
-			t.Errorf("the help output names %s and it is not in this tree: %v", document, err)
+		if _, err := os.Stat(filepath.Join("..", "..", filepath.FromSlash(document.inTree))); err != nil {
+			t.Errorf("the help output names %s and it is not in this tree: %v", document.inTree, err)
 		}
 	}
 }
@@ -270,24 +284,167 @@ func TestHelpNamesTheDocumentsAnOperatorIsOwed(t *testing.T) {
 // reaches for one or the other and not reliably for both, so a paragraph that
 // only one of them prints reaches only half of them.
 //
-// It asserts the same two things that test does. That the output names each
-// document, and that each document it names is in this tree to be found - a
-// binary pointing at a file somebody moved is worse than one pointing nowhere,
-// because the reader follows it.
+// It asserts the same three things that test does. That the output names each
+// document in the spelling a checkout has for it, that it names each one in the
+// spelling a download has for it, and that the checkout spelling is in this tree
+// to be found - a binary pointing at a file somebody moved is worse than one
+// pointing nowhere, because the reader follows it.
 func TestVersionNamesTheDocumentsAnOperatorIsOwed(t *testing.T) {
 	var out, errOut bytes.Buffer
 	if got := run([]string{"version"}, &out, &errOut, ordinary(realWalk)); got != exitClean {
 		t.Fatalf("exit code %d, want %d", got, exitClean)
 	}
 
 	for _, document := range documentsAnOperatorIsOwed {
-		if !strings.Contains(out.String(), document) {
-			t.Errorf("the version output does not name %s:\n%s", document, out.String())
+		for _, spelling := range []string{document.inTree, document.beside} {
+			if !namesOnItsOwn(out.String(), spelling) {
+				t.Errorf("the version output does not name %s as a path in its own right:\n%s", spelling, out.String())
+			}
+		}
+		if _, err := os.Stat(filepath.Join("..", "..", filepath.FromSlash(document.inTree))); err != nil {
+			t.Errorf("the version output names %s and it is not in this tree: %v", document.inTree, err)
+		}
+	}
+}
+
+// TestTheDownloadSpellingIsTheNameTheReleasePublishes holds the two spellings
+// to each other. The release copies the three files in flat beside the
+// binaries, so the name an operator sees is the base of the path the tree
+// carries, and a declaration writing anything else is a paragraph that points
+// at nothing on either route.
+//
+// It is separate from the two output tests because it judges the declaration
+// rather than the text: a second document added here with a mistyped download
+// spelling is caught before anybody asks what the runner printed.
+func TestTheDownloadSpellingIsTheNameTheReleasePublishes(t *testing.T) {
+	for _, document := range documentsAnOperatorIsOwed {
+		if want := path.Base(document.inTree); document.beside != want {
+			t.Errorf("%s is published beside a binary as %s, and this declaration calls it %s", document.inTree, want, document.beside)
+		}
+	}
+}
+
+// TestTheReleaseCopiesTheDocumentsInFlat is the other end of that relation, and
+// without it the base above is an assumption about a file this package never
+// reads. The release workflow is what puts the three documents beside the
+// binaries, and it is what decides the name they arrive under: copied into the
+// assets directory itself, each one lands under its base name, which is the
+// spelling the paragraph prints.
+//
+// WHAT IT READS IS THE ONE COPY THAT NAMES THE DOCUMENTS, and what it compares
+// is the set of paths that copy carries against the set this package declares.
+// A document dropped from the release, one added to it, a copy that preserved
+// the directory, and a flag that changed the destination all move that set or
+// take the line out of reach, and each of them makes the printed paragraph
+// wrong in a way nothing else here would notice.
+//
+// WHAT IT CANNOT SEE. It reads the workflow file rather than a release, so it
+// says what the next run of that workflow would publish and nothing about what
+// any published release contains. And it finds the copy by shape, so an
+// assemble step rewritten into a loop or split across two commands fails here
+// with the documents still travelling correctly - which is a red gate asking
+// for this test to be rewritten against the new shape, not a defect in the
+// release.
+func TestTheReleaseCopiesTheDocumentsInFlat(t *testing.T) {
+	raw, err := os.ReadFile(filepath.Join("..", "..", ".github", "workflows", "release.yml"))
+	if err != nil {
+		t.Fatalf("reading the release workflow: %v", err)
+	}
+
+	var want []string
+	for _, document := range documentsAnOperatorIsOwed {
+		want = append(want, document.inTree)
+	}
+
+	got, ok := copiedIntoTheAssets(string(raw), want[0])
+	if !ok {
+		t.Fatalf("no line of the release workflow copies %s into %q, so nothing here publishes the documents beside the binaries", want[0], "${assets}/")
+	}
+
+	sort.Strings(got)
+	sorted := append([]string(nil), want...)
+	sort.Strings(sorted)
+	if strings.Join(got, " ") != strings.Join(sorted, " ") {
+		t.Errorf("the release copies %v into the assets directory and this package declares %v", got, sorted)
+	}
+}
+
+// copiedIntoTheAssets returns the operands of the one cp in a workflow that
+// names the given path and writes into the assets directory itself. The
+// destination is matched as the assets directory and nothing under it, because
+// a copy into a subdirectory of it publishes an asset under a name that is not
+// the base of its path, which is the case the caller exists to refuse.
+func copiedIntoTheAssets(workflow, naming string) ([]string, bool) {
+	for _, line := range strings.Split(workflow, "\n") {
+		fields := strings.Fields(strings.TrimSpace(line))
+		if len(fields) < 3 || fields[0] != "cp" {
+			continue
+		}
+		if fields[len(fields)-1] != `"${assets}/"` {
+			continue
+		}
+		operands := fields[1 : len(fields)-1]
+		for _, operand := range operands {
+			if operand == naming {
+				return operands, true
+			}
+		}
+	}
+	return nil, false
+}
+
+// namesOnItsOwn says whether text names a path in its own right rather than as
+// the tail of a longer one.
+//
+// THIS IS THE WHOLE DIFFERENCE BETWEEN THE TWO SPELLINGS BEING ASSERTED AND
+// NEITHER OF THEM BEING ASSERTED. docs/privacy.md ends with privacy.md, so
+// strings.Contains cannot tell a paragraph naming both from one naming only the
+// checkout spelling, and the paragraph issue #235 was raised against would
+// satisfy it for every document in the list.
+func namesOnItsOwn(text, name string) bool {
+	for _, token := range pathTokens(text) {
+		if token == name {
+			return true
 		}
-		if _, err := os.Stat(filepath.Join("..", "..", filepath.FromSlash(document))); err != nil {
-			t.Errorf("the version output names %s and it is not in this tree: %v", document, err)
+	}
+	return false
+}
+
+// pathTokens splits text into the maximal runs of bytes a path can be written
+// with, and trims a trailing full stop from each, because a path at the end of
+// a sentence is followed by one and the sentence is not part of the path.
+//
+// Ordinary words come out of it as tokens too, which costs nothing: the caller
+// compares against a declared path, and no document in the list is spelled like
+// a word of the paragraph around it.
+func pathTokens(text string) []string {
+	var tokens []string
+	start := -1
+	for i := 0; i <= len(text); i++ {
+		if i < len(text) && partOfAPath(text[i]) {
+			if start < 0 {
+				start = i
+			}
+			continue
+		}
+		if start >= 0 {
+			tokens = append(tokens, strings.TrimRight(text[start:i], "."))
+			start = -1
 		}
 	}
+	return tokens
+}
+
+// partOfAPath says whether a byte can appear inside a path as this repository
+// writes them. The set is deliberately wider than the three documents need, so
+// that a neighbouring path in the same sentence is one token rather than
+// several and cannot be mistaken for a document by accident.
+func partOfAPath(b byte) bool {
+	switch {
+	case b >= 'a' && b <= 'z', b >= 'A' && b <= 'Z', b >= '0' && b <= '9':
+		return true
+	}
+	return b == '/' || b == '\\' || b == '.' || b == '-' || b == '_'
 }
 
 // TestVersionReportsWhatTheToolchainStamped asserts that the version the verb