bastion 0.55.3

client-go/publicname_test.go

@@ -1,7 +1,6 @@
 package main
 
 import (
-	"bufio"
 	"bytes"
 	"os"
 	"os/exec"
@@ -18,7 +17,10 @@ import (
 // harness images, or the host still read or write under that name. Each is
 // listed in keptOldNames with what holds it in place; removing one means
 // changing the other side first. Prose, comments and the changelog are never
-// exceptions.
+// exceptions: a kept identifier is set aside only in Go code, outside its
+// comments (goCodeAndComments). In a comment, a Markdown file, a script or
+// any other file it is a hit like the bare word, and is described instead
+// (the device-fingerprint prefix, fpSANPrefix in pair.go).
 //
 // The forbidden word is assembled from bytes so that this file does not
 // contain it outside the exception list.
@@ -75,6 +77,118 @@ func isNameByte(c byte) bool {
 	return c == '_' || c >= '0' && c <= '9' || c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z'
 }
 
+// hasOldName reports whether text carries the old name at all, kept
+// identifiers included.
+func hasOldName(text string) bool {
+	return strings.Contains(strings.ToLower(text), oldName)
+}
+
+// goCodeAndComments splits Go source into its lines twice over: code[i] is
+// line i with its comment text blanked, comments[i] the same line with
+// everything but comment text blanked. String, raw string and rune literals
+// are code, so a "//" inside one does not start a comment, and a raw string
+// spanning lines stays code on every line it covers.
+func goCodeAndComments(src string) (code, comments []string) {
+	const (
+		inCode = iota
+		inLineComment
+		inBlockComment
+		inString
+		inRawString
+		inRune
+	)
+	codeB := []byte(src)
+	commB := []byte(src)
+	state := inCode
+	for i := 0; i < len(src); i++ {
+		c := src[i]
+		isComment := false
+		switch state {
+		case inCode:
+			switch {
+			case c == '/' && i+1 < len(src) && src[i+1] == '/':
+				state, isComment = inLineComment, true
+			case c == '/' && i+1 < len(src) && src[i+1] == '*':
+				// Both opener bytes are taken here: the opener's '*' cannot
+				// also start the closer, so "/*/" leaves the comment open.
+				codeB[i], codeB[i+1] = ' ', ' '
+				i++
+				state = inBlockComment
+				continue
+			case c == '"':
+				state = inString
+			case c == '`':
+				state = inRawString
+			case c == '\'':
+				state = inRune
+			}
+		case inLineComment:
+			if c == '\n' {
+				state = inCode
+			} else {
+				isComment = true
+			}
+		case inBlockComment:
+			isComment = true
+			if c == '*' && i+1 < len(src) && src[i+1] == '/' {
+				codeB[i], codeB[i+1] = ' ', ' '
+				i++
+				state = inCode
+				continue
+			}
+		case inString, inRune:
+			end := byte('"')
+			if state == inRune {
+				end = '\''
+			}
+			switch {
+			case c == '\\' && i+1 < len(src) && src[i+1] != '\n':
+				commB[i], commB[i+1] = ' ', ' '
+				i++
+				continue
+			case c == end, c == '\n':
+				state = inCode
+			}
+		case inRawString:
+			if c == '`' {
+				state = inCode
+			}
+		}
+		if c == '\n' {
+			continue
+		}
+		if isComment {
+			codeB[i] = ' '
+		} else {
+			commB[i] = ' '
+		}
+	}
+	return strings.Split(string(codeB), "\n"), strings.Split(string(commB), "\n")
+}
+
+// oldNameHits lists, as "line: text", every line of the published file rel
+// that carries the old name. Kept identifiers are set aside only in the code
+// of a Go file; in its comments and in every other file they count.
+func oldNameHits(rel, src string) []string {
+	lines := strings.Split(src, "\n")
+	var hits []string
+	if !strings.HasSuffix(rel, ".go") {
+		for i, l := range lines {
+			if hasOldName(l) {
+				hits = append(hits, strconv.Itoa(i+1)+": "+strings.TrimSpace(l))
+			}
+		}
+		return hits
+	}
+	code, comments := goCodeAndComments(src)
+	for i, l := range lines {
+		if lineHasOldName(code[i]) || hasOldName(comments[i]) {
+			hits = append(hits, strconv.Itoa(i+1)+": "+strings.TrimSpace(l))
+		}
+	}
+	return hits
+}
+
 // publishedFiles lists the files the member repository receives: what git
 // tracks at the repository root (the publisher stages `git archive` of the
 // release commit). Outside a git checkout it walks the tree instead.
@@ -127,19 +241,14 @@ func TestPublishedSourceCarriesNoOldInternalName(t *testing.T) {
 		if err != nil {
 			continue // a tracked file deleted in the working tree
 		}
-		sc := bufio.NewScanner(bytes.NewReader(data))
-		sc.Buffer(make([]byte, 0, 1<<20), 1<<24)
-		n := 0
-		for sc.Scan() {
-			n++
-			if lineHasOldName(sc.Text()) {
-				hits = append(hits, rel+":"+strconv.Itoa(n)+": "+strings.TrimSpace(sc.Text()))
-			}
+		for _, h := range oldNameHits(rel, string(data)) {
+			hits = append(hits, rel+":"+h)
 		}
 	}
 	if len(hits) > 0 {
-		t.Errorf("the old internal name appears in %d published lines. Reword prose; an identifier "+
-			"may be added to keptOldNames only when something outside this repository still reads it:\n  %s",
+		t.Errorf("the old internal name appears in %d published lines. Reword prose and comments; an "+
+			"identifier may be added to keptOldNames only when something outside this repository still "+
+			"reads it, and is set aside only in Go code:\n  %s",
 			len(hits), strings.Join(hits, "\n  "))
 	}
 }
@@ -150,7 +259,7 @@ func TestOldNameCheckCatchesProseAndPassesKeptIdentifiers(t *testing.T) {
 	up := strings.ToUpper(oldName)
 	title := strings.ToUpper(oldName[:1]) + oldName[1:]
 	for _, line := range []string{
-		"// see " + oldName + " #649",
+		"// see " + oldName + " for the rest",
 		"// the " + title + " server",
 		up + "_NEW_VAR",
 		keptOldNames[0] + "X", // a kept name with a suffix is a different name
@@ -173,3 +282,34 @@ func TestOldNameCheckCatchesProseAndPassesKeptIdentifiers(t *testing.T) {
 		}
 	}
 }
+
+// Where a kept identifier is set aside: in Go code only. In a Go comment, the
+// changelog, any other Markdown file or a script it is a hit like the bare
+// word. A "//" inside a string, a raw string spanning lines and an escaped
+// quote do not move the line between code and comment.
+func TestKeptIdentifiersAreSetAsideOnlyInGoCode(t *testing.T) {
+	prefix := keptOldNames[len(keptOldNames)-1] // the fingerprint prefix
+	env := keptOldNames[0]
+	for _, c := range []struct {
+		rel, src string
+		hit      bool
+	}{
+		{"CHANGELOG.md", "- the fingerprint goes under `" + prefix + "<fp>`, as before\n", true},
+		{"docs/claims.md", "the agent token is read from " + env + "\n", true},
+		{"deploy/run.sh", "export " + env + "=x\n", true},
+		{"a.go", "package a\n\n// names it " + prefix + "<fp>\n", true},
+		{"a.go", "x := 1 /* " + env + " */\n", true},
+		{"a.go", "x := 1 /*/ " + env + " */\n", true},
+		{"a.go", "s := \"a\\\"b\" // " + env + "\n", true},
+		{"a.go", "const p = \"" + prefix + "\"\n", false},
+		{"a.go", "v := []string{\"" + env + "\"} // the host mints this tag\n", false},
+		{"a.go", "u := \"https://example.com/" + env + "\"\n", false},
+		{"a.go", "y := `\nname: x // " + env + "\n`\n", false},
+		{"a.go", "r := '\"' + 1 // " + env + "\n", true},
+	} {
+		got := oldNameHits(c.rel, c.src)
+		if (len(got) > 0) != c.hit {
+			t.Errorf("%s %q: hits %q, want a hit: %v", c.rel, c.src, got, c.hit)
+		}
+	}
+}