ingestion/internal/extract/callsites_test.go

@@ -1,43 +1,119 @@
 package extract
 
 import (
+	"go/ast"
+	"go/parser"
+	"go/token"
 	"os"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"testing"
 )
 
-// PR #156 review: putting extract.Text(path) back at the ingest call sites
-// stayed green, because only TextIn itself was tested. Reading a caller-chosen
-// path by name reopens it after the containment check (hyperguild#153 item 3),
-// so outside this package only TextIn may be used.
-func TestNothingOutsideExtractCallsTextByPath(t *testing.T) {
-	root := filepath.Join("..", "..")
-	var offenders []string
-	err := filepath.WalkDir(root, func(p string, d os.DirEntry, err error) error {
+const extractPkg = "github.com/mathiasbq/hyperguild/ingestion/internal/extract"
+
+// parseNonTest parses every non-test Go file under the module.
+func parseNonTest(t *testing.T) map[string]*ast.File {
+	t.Helper()
+	fset := token.NewFileSet()
+	out := map[string]*ast.File{}
+	err := filepath.WalkDir(filepath.Join("..", ".."), func(p string, d os.DirEntry, err error) error {
 		if err != nil {
 			return err
 		}
 		if d.IsDir() && (d.Name() == "vendor" || d.Name() == ".git") {
 			return filepath.SkipDir
 		}
-		if d.IsDir() || !strings.HasSuffix(p, ".go") || strings.HasSuffix(p, "_test.go") ||
-			strings.Contains(filepath.ToSlash(p), "internal/extract/") {
+		if d.IsDir() || !strings.HasSuffix(p, ".go") || strings.HasSuffix(p, "_test.go") {
 			return nil
 		}
-		b, err := os.ReadFile(p)
-		if err != nil {
-			return err
-		}
-		if strings.Contains(string(b), "extract.Text(") {
-			offenders = append(offenders, p)
+		f, perr := parser.ParseFile(fset, p, nil, 0)
+		if perr != nil {
+			return perr
 		}
+		out[filepath.ToSlash(p)] = f
 		return nil
 	})
 	if err != nil {
 		t.Fatal(err)
 	}
-	if len(offenders) > 0 {
-		t.Fatalf("extract.Text( called outside internal/extract (use TextIn through an os.Root): %v", offenders)
+	return out
+}
+
+// PR #156 review: reading a caller-chosen path by name reopens it after the
+// containment check (hyperguild#153 item 3). Outside this package, nothing
+// may reference extract.Text — called OR taken as a value — under whatever
+// name the package is imported (round 2: an alias and a func value slipped
+// past a plain text search).
+func TestNoPackageReferencesExtractTextOutsideExtract(t *testing.T) {
+	for path, f := range parseNonTest(t) {
+		if strings.Contains(path, "internal/extract/") {
+			continue
+		}
+		names := map[string]bool{}
+		for _, imp := range f.Imports {
+			if p, _ := strconv.Unquote(imp.Path.Value); p == extractPkg {
+				name := "extract"
+				if imp.Name != nil {
+					name = imp.Name.Name
+				}
+				names[name] = true
+			}
+		}
+		if len(names) == 0 {
+			continue
+		}
+		ast.Inspect(f, func(n ast.Node) bool {
+			if sel, ok := n.(*ast.SelectorExpr); ok {
+				if id, ok := sel.X.(*ast.Ident); ok && names[id.Name] && sel.Sel.Name == "Text" {
+					t.Errorf("%s references %s.Text; use extract.TextIn through an os.Root", path, id.Name)
+				}
+			}
+			return true
+		})
+	}
+}
+
+// PR #156 review round 2: the FIFO fix (open non-blocking, check the handle)
+// lives in OpenRegular, and reverting any caller to a plain Root.Open stayed
+// green. In the raw/ readers, the only Open/OpenFile on anything is inside
+// OpenRegular itself.
+// osOpenAllowed names the functions in guarded files that may call os.Open*
+// on a path that is not a raw/ read.
+var osOpenAllowed = map[string]bool{"internal/watcher/watcher.go:appendWatcherLog": true}
+
+func TestRawReadersOpenOnlyThroughOpenRegular(t *testing.T) {
+	guarded := []string{"internal/api/rawroot.go", "internal/extract/extract.go", "internal/watcher/watcher.go"}
+	files := parseNonTest(t)
+	for _, g := range guarded {
+		var f *ast.File
+		for p, af := range files {
+			if strings.HasSuffix(p, g) {
+				f = af
+			}
+		}
+		if f == nil {
+			t.Fatalf("guarded file %s not found", g)
+		}
+		for _, decl := range f.Decls {
+			fn, ok := decl.(*ast.FuncDecl)
+			if !ok || fn.Name.Name == "OpenRegular" {
+				continue
+			}
+			ast.Inspect(fn, func(n ast.Node) bool {
+				call, ok := n.(*ast.CallExpr)
+				if !ok {
+					return true
+				}
+				if sel, ok := call.Fun.(*ast.SelectorExpr); ok && (sel.Sel.Name == "Open" || sel.Sel.Name == "OpenFile") {
+					if id, ok := sel.X.(*ast.Ident); ok && id.Name == "os" && osOpenAllowed[g+":"+fn.Name.Name] {
+						return true // a named non-raw write (the ingest log append)
+					}
+					t.Errorf("%s: %s calls .%s; raw/ reads must go through OpenRegular", g, fn.Name.Name, sel.Sel.Name)
+				}
+				return true
+			})
+		}
 	}
 }