Enforce the application dependency boundary

2026-07-28 14:12:36 +00:00
parent 5a00ca81a2
commit fb0b21c51d
1 changed files with 241 additions and 77 deletions
--- a/internal/adapter/dependency_test.go
+++ b/internal/adapter/dependency_test.go
@@ -13,120 +13,284 @@ import (
 	"testing"
 )

-const scriptoriumModulePath = "gitea.maximumdirect.net/eric/scriptorium"
+const (
+	scriptoriumModulePath = "gitea.maximumdirect.net/eric/scriptorium"
+	promptkitInternalPath = "gitea.maximumdirect.net/eric/promptkit/internal"
+)

-var forbiddenFrameworkPackageRoots = []string{
-	scriptoriumModulePath + "/internal/domain",
-	scriptoriumModulePath + "/internal/usecase",
-	scriptoriumModulePath + "/internal/promptdef",
-	scriptoriumModulePath + "/internal/prompt",
-	scriptoriumModulePath + "/internal/profile",
-	scriptoriumModulePath + "/internal/validate",
-	scriptoriumModulePath + "/internal/llm",
-	scriptoriumModulePath + "/internal/artifact",
-}
+var (
+	removedFrameworkPackageRoots = []string{
+		scriptoriumModulePath + "/internal/artifact",
+		scriptoriumModulePath + "/internal/domain",
+		scriptoriumModulePath + "/internal/filecatalog",
+		scriptoriumModulePath + "/internal/llm",
+		scriptoriumModulePath + "/internal/profile",
+		scriptoriumModulePath + "/internal/prompt",
+		scriptoriumModulePath + "/internal/promptdef",
+		scriptoriumModulePath + "/internal/usecase",
+		scriptoriumModulePath + "/internal/validate",
+	}
+	removedFrameworkDirectories = []string{
+		"internal/artifact",
+		"internal/domain",
+		"internal/filecatalog",
+		"internal/llm",
+		"internal/profile",
+		"internal/prompt",
+		"internal/promptdef",
+		"internal/usecase",
+		"internal/validate",
+	}
+	nonSourceDirectories = map[string]struct{}{
+		".cache":           {},
+		".codebase-memory": {},
+		".git":             {},
+		"build":            {},
+		"coverage":         {},
+		"dist":             {},
+		"node_modules":     {},
+		"out":              {},
+		"testdata":         {},
+		"vendor":           {},
+	}
+)

-type forbiddenFrameworkImport struct {
+type forbiddenImport struct {
 	filePath   string
 	importPath string
 }

-func TestScriptoriumAdaptersUseOnlyPublicFrameworkBoundary(t *testing.T) {
-	_, testFile, _, ok := runtime.Caller(0)
-	if !ok {
-		t.Fatal("locate dependency guard source")
-	}
+func TestApplicationBoundary(t *testing.T) {
+	moduleRoot := moduleRootFromTestFile(t)

-	adapterDir := filepath.Dir(testFile)
-	violations, err := findForbiddenFrameworkImports([]string{
-		filepath.Join(adapterDir, "cli"),
-		filepath.Join(adapterDir, "http"),
-		filepath.Join(adapterDir, "..", "format"),
-	})
+	violations, err := findForbiddenProductionImports(moduleRoot)
 	if err != nil {
-		t.Fatalf("scan framework imports: %v", err)
+		t.Fatalf("scan production imports: %v", err)
 	}
 	for _, violation := range violations {
-		t.Errorf("%s imports forbidden framework package %s", violation.filePath, violation.importPath)
+		t.Errorf("%s imports forbidden package %s", violation.filePath, violation.importPath)
+	}
+
+	assertFrameworkImplementationAbsent(t, moduleRoot)
+}
+
+func TestForbiddenImportScannerDetectsFormerRoot(t *testing.T) {
+	root := t.TempDir()
+	sourcePath := writeGoSource(t, root, "nested/consumer/root.go", scriptoriumModulePath)
+
+	violations, err := findForbiddenProductionImports(root)
+	if err != nil {
+		t.Fatalf("scan source fixture: %v", err)
+	}
+	assertSingleViolation(t, violations, sourcePath, scriptoriumModulePath)
+}
+
+func TestForbiddenImportScannerDetectsFormerFrameworkFamily(t *testing.T) {
+	root := t.TempDir()
+	importPath := scriptoriumModulePath + "/internal/profile/builtin"
+	sourcePath := writeGoSource(t, root, "nested/consumer/profile.go", importPath)
+
+	violations, err := findForbiddenProductionImports(root)
+	if err != nil {
+		t.Fatalf("scan source fixture: %v", err)
+	}
+	assertSingleViolation(t, violations, sourcePath, importPath)
+}
+
+func TestForbiddenImportScannerDetectsPromptkitInternalPackages(t *testing.T) {
+	tests := []struct {
+		name       string
+		importPath string
+	}{
+		{name: "exact internal root", importPath: promptkitInternalPath},
+		{name: "internal descendant", importPath: promptkitInternalPath + "/domain"},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			root := t.TempDir()
+			sourcePath := writeGoSource(t, root, "nested/consumer/promptkit.go", tc.importPath)
+
+			violations, err := findForbiddenProductionImports(root)
+			if err != nil {
+				t.Fatalf("scan source fixture: %v", err)
+			}
+			assertSingleViolation(t, violations, sourcePath, tc.importPath)
+		})
 	}
 }

-func TestForbiddenFrameworkImportScannerDetectsNestedPackageFamilies(t *testing.T) {
+func TestForbiddenImportScannerAllowsRetainedApplicationPackages(t *testing.T) {
 	root := t.TempDir()
-	nestedDir := filepath.Join(root, "nested", "adapter")
-	if err := os.MkdirAll(nestedDir, 0o755); err != nil {
-		t.Fatalf("create nested source directory: %v", err)
+	sourcePath := filepath.Join(root, "nested/consumer/application.go")
+	if err := os.MkdirAll(filepath.Dir(sourcePath), 0o755); err != nil {
+		t.Fatalf("create source fixture directory: %v", err)
 	}
-
-	sourcePath := filepath.Join(nestedDir, "imports.go")
-	source := `package nested
+	source := `package consumer

 import (
-	_ "gitea.maximumdirect.net/eric/scriptorium"
-	_ "gitea.maximumdirect.net/eric/scriptorium/internal/profile/builtin"
+	_ "gitea.maximumdirect.net/eric/promptkit"
+	_ "gitea.maximumdirect.net/eric/scriptorium/internal/adapter/http"
+	_ "gitea.maximumdirect.net/eric/scriptorium/internal/config"
+	_ "gitea.maximumdirect.net/eric/scriptorium/internal/defaults"
+	_ "gitea.maximumdirect.net/eric/scriptorium/internal/format"
 )
 `
 	if err := os.WriteFile(sourcePath, []byte(source), 0o644); err != nil {
-		t.Fatalf("write nested source fixture: %v", err)
+		t.Fatalf("write source fixture: %v", err)
 	}

-	violations, err := findForbiddenFrameworkImports([]string{root})
+	violations, err := findForbiddenProductionImports(root)
 	if err != nil {
-		t.Fatalf("scan nested source fixture: %v", err)
+		t.Fatalf("scan source fixture: %v", err)
 	}
-	if len(violations) != 1 {
-		t.Fatalf("expected one forbidden import, got %#v", violations)
-	}
-	if violations[0].filePath != sourcePath {
-		t.Fatalf("unexpected importing file: %q", violations[0].filePath)
-	}
-	wantImport := scriptoriumModulePath + "/internal/profile/builtin"
-	if violations[0].importPath != wantImport {
-		t.Fatalf("unexpected forbidden import: %q", violations[0].importPath)
+	if len(violations) != 0 {
+		t.Fatalf("expected retained application imports to be allowed, got %#v", violations)
 	}
 }

-func findForbiddenFrameworkImports(roots []string) ([]forbiddenFrameworkImport, error) {
-	var violations []forbiddenFrameworkImport
-	for _, root := range roots {
-		err := filepath.WalkDir(root, func(path string, entry fs.DirEntry, err error) error {
-			if err != nil {
-				return err
-			}
-			if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".go") || strings.HasSuffix(entry.Name(), "_test.go") {
-				return nil
-			}
-
-			file, err := parser.ParseFile(token.NewFileSet(), path, nil, parser.ImportsOnly)
-			if err != nil {
-				return fmt.Errorf("parse imports in %s: %w", path, err)
-			}
-			for _, imported := range file.Imports {
-				importPath, err := strconv.Unquote(imported.Path.Value)
-				if err != nil {
-					return fmt.Errorf("parse import path in %s: %w", path, err)
-				}
-				if isForbiddenFrameworkImport(importPath) {
-					violations = append(violations, forbiddenFrameworkImport{
-						filePath:   path,
-						importPath: importPath,
-					})
-				}
+func findForbiddenProductionImports(root string) ([]forbiddenImport, error) {
+	var violations []forbiddenImport
+	err := filepath.WalkDir(root, func(path string, entry fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+		if entry.IsDir() {
+			if path != root && shouldSkipSourceDirectory(entry.Name()) {
+				return filepath.SkipDir
 			}
 			return nil
-		})
-		if err != nil {
-			return nil, fmt.Errorf("walk source root %s: %w", root, err)
 		}
+		if !strings.HasSuffix(entry.Name(), ".go") || strings.HasSuffix(entry.Name(), "_test.go") {
+			return nil
+		}
+
+		file, err := parser.ParseFile(token.NewFileSet(), path, nil, parser.ImportsOnly)
+		if err != nil {
+			return fmt.Errorf("parse imports in %s: %w", path, err)
+		}
+		for _, imported := range file.Imports {
+			importPath, err := strconv.Unquote(imported.Path.Value)
+			if err != nil {
+				return fmt.Errorf("parse import path in %s: %w", path, err)
+			}
+			if isForbiddenProductionImport(importPath) {
+				violations = append(violations, forbiddenImport{
+					filePath:   path,
+					importPath: importPath,
+				})
+			}
+		}
+		return nil
+	})
+	if err != nil {
+		return nil, fmt.Errorf("walk repository root %s: %w", root, err)
 	}
 	return violations, nil
 }

-func isForbiddenFrameworkImport(importPath string) bool {
-	for _, root := range forbiddenFrameworkPackageRoots {
+func shouldSkipSourceDirectory(name string) bool {
+	_, skip := nonSourceDirectories[name]
+	return skip
+}
+
+func isForbiddenProductionImport(importPath string) bool {
+	if importPath == scriptoriumModulePath {
+		return true
+	}
+	if importPath == promptkitInternalPath || strings.HasPrefix(importPath, promptkitInternalPath+"/") {
+		return true
+	}
+	for _, root := range removedFrameworkPackageRoots {
 		if importPath == root || strings.HasPrefix(importPath, root+"/") {
 			return true
 		}
 	}
 	return false
 }
+
+func moduleRootFromTestFile(t *testing.T) string {
+	t.Helper()
+
+	_, testFile, _, ok := runtime.Caller(0)
+	if !ok {
+		t.Fatal("locate dependency guard source")
+	}
+	root, err := findModuleRoot(filepath.Dir(testFile))
+	if err != nil {
+		t.Fatal(err)
+	}
+	return root
+}
+
+func findModuleRoot(start string) (string, error) {
+	dir, err := filepath.Abs(start)
+	if err != nil {
+		return "", fmt.Errorf("resolve module search path: %w", err)
+	}
+	for {
+		goMod := filepath.Join(dir, "go.mod")
+		if info, err := os.Stat(goMod); err == nil && !info.IsDir() {
+			return dir, nil
+		} else if err != nil && !os.IsNotExist(err) {
+			return "", fmt.Errorf("inspect %s: %w", goMod, err)
+		}
+
+		parent := filepath.Dir(dir)
+		if parent == dir {
+			return "", fmt.Errorf("locate go.mod from %s", start)
+		}
+		dir = parent
+	}
+}
+
+func assertFrameworkImplementationAbsent(t *testing.T, moduleRoot string) {
+	t.Helper()
+
+	entries, err := os.ReadDir(moduleRoot)
+	if err != nil {
+		t.Fatalf("read module root: %v", err)
+	}
+	for _, entry := range entries {
+		if !entry.IsDir() && strings.HasSuffix(entry.Name(), ".go") && !strings.HasSuffix(entry.Name(), "_test.go") {
+			t.Errorf("module root contains production Go file %s", entry.Name())
+		}
+	}
+
+	for _, relativePath := range removedFrameworkDirectories {
+		path := filepath.Join(moduleRoot, filepath.FromSlash(relativePath))
+		if _, err := os.Stat(path); err == nil {
+			t.Errorf("removed framework directory still exists: %s", relativePath)
+		} else if !os.IsNotExist(err) {
+			t.Errorf("inspect removed framework directory %s: %v", relativePath, err)
+		}
+	}
+}
+
+func writeGoSource(t *testing.T, root, relativePath, importPath string) string {
+	t.Helper()
+
+	sourcePath := filepath.Join(root, filepath.FromSlash(relativePath))
+	if err := os.MkdirAll(filepath.Dir(sourcePath), 0o755); err != nil {
+		t.Fatalf("create source fixture directory: %v", err)
+	}
+	source := fmt.Sprintf("package consumer\n\nimport _ %q\n", importPath)
+	if err := os.WriteFile(sourcePath, []byte(source), 0o644); err != nil {
+		t.Fatalf("write source fixture: %v", err)
+	}
+	return sourcePath
+}
+
+func assertSingleViolation(t *testing.T, violations []forbiddenImport, sourcePath, importPath string) {
+	t.Helper()
+
+	if len(violations) != 1 {
+		t.Fatalf("expected one forbidden import, got %#v", violations)
+	}
+	if violations[0].filePath != sourcePath {
+		t.Fatalf("unexpected importing file: %q", violations[0].filePath)
+	}
+	if violations[0].importPath != importPath {
+		t.Fatalf("unexpected forbidden import: %q", violations[0].importPath)
+	}
+}