From 722b5ddf5936edf4b0e77cc4078a9c5a47a3378c Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 22 Sep 2026 04:48:58 +0000 Subject: [PATCH] Add public facade boundary checks --- docs/policy/architecture.md | 6 +- docs/roadmap/implementation.md | 11 ++ public_boundary_test.go | 346 +++++++++++++++++++++++++++++++++ 3 files changed, 361 insertions(+), 2 deletions(-) create mode 100644 public_boundary_test.go diff --git a/docs/policy/architecture.md b/docs/policy/architecture.md index 874d648..8cbe859 100644 --- a/docs/policy/architecture.md +++ b/docs/policy/architecture.md @@ -129,8 +129,10 @@ consumer contract requires a public package. Each package must have one cohesive responsibility and a clear dependency direction. Internal packages must not expose their types merely to simplify wiring, and the public facade must not leak internal representations through -exported signatures. New public packages require a durable consumer need that -cannot be served cleanly by the root facade. +exported signatures. Production packages under `internal/` must not import the +module root; the root facade may depend on internal implementation packages. +New public packages require a durable consumer need that cannot be served +cleanly by the root facade. The [internal component overview](../internal/overview.md) must be updated as packages are implemented or their responsibilities change. diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 1c5e358..9a4b32f 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -273,6 +273,17 @@ Acceptance: the completed refactor passes the checks; representative violations fail for the intended reason, and ordinary private implementation changes remain possible. No ConfigKit checkout or other sibling is needed. +### Completion + +- [x] Added a compiled-export-data checker for the root facade's reachable + public type surface, including aliases, promoted methods, and generic + constraints. +- [x] Added package-metadata validation that production internal packages do + not import the root facade. +- [x] Added compact in-memory fixtures for permitted private state and sealed + options, and for direct, nested, field, alias, and promoted-method leaks. +- [x] Recorded the dependency-direction rule in the architecture policy. + ## Stage 6: Complete Compatibility And Documentation Review Purpose: verify the whole refactor, not just successful compilation of each move. diff --git a/public_boundary_test.go b/public_boundary_test.go new file mode 100644 index 0000000..a148920 --- /dev/null +++ b/public_boundary_test.go @@ -0,0 +1,346 @@ +package promptkit_test + +import ( + "encoding/json" + "fmt" + "go/ast" + "go/importer" + "go/parser" + "go/token" + "go/types" + "io" + "os" + "os/exec" + "runtime" + "sort" + "strings" + "testing" +) + +func TestPublicFacadeDoesNotExposeInternalTypes(t *testing.T) { + metadata := listedPackages(t, "-export", ".") + if len(metadata) != 1 { + t.Fatalf("root package metadata count = %d, want one", len(metadata)) + } + + pkg := importCompiledPackage(t, metadata[0]) + violations := publicSurfaceViolations(pkg, metadata[0].ImportPath) + if len(violations) > 0 { + t.Fatalf("public facade exposes internal implementation:\n%s", strings.Join(violations, "\n")) + } +} + +func TestInternalProductionPackagesDoNotImportFacade(t *testing.T) { + root := listedPackages(t, ".") + if len(root) != 1 { + t.Fatalf("root package metadata count = %d, want one", len(root)) + } + + var violations []string + for _, pkg := range listedPackages(t, "./internal/...") { + for _, imported := range pkg.Imports { + if imported == root[0].ImportPath { + violations = append(violations, pkg.ImportPath) + } + } + } + sort.Strings(violations) + if len(violations) > 0 { + t.Fatalf("internal production packages import the public facade:\n%s", strings.Join(violations, "\n")) + } +} + +func TestPublicBoundaryCheckerFixtures(t *testing.T) { + tests := []struct { + name string + source string + wantReject bool + }{ + { + name: "private internal state and sealed option are allowed", + source: `package facade +import "example.test/facade/internal/hidden" +type privateState struct { value hidden.Value } +type Public struct { state privateState } +type Option interface { apply(*privateState) } +`, + }, + { + name: "direct internal signature is rejected", + source: `package facade +import "example.test/facade/internal/hidden" +func Direct(value hidden.Value) {} +`, + wantReject: true, + }, + { + name: "nested internal signature is rejected", + source: `package facade +import "example.test/facade/internal/hidden" +func Nested(values []map[string]hidden.Value) {} +`, + wantReject: true, + }, + { + name: "internal public field is rejected", + source: `package facade +import "example.test/facade/internal/hidden" +type Public struct { Value hidden.Value } +`, + wantReject: true, + }, + { + name: "internal alias is rejected", + source: `package facade +import "example.test/facade/internal/hidden" +type Public = hidden.Value +`, + wantReject: true, + }, + { + name: "promoted internal method is rejected", + source: `package facade +import "example.test/facade/internal/hidden" +type wrapper struct { hidden.Value } +type Public struct { wrapper } +`, + wantReject: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + pkg := fixturePackage(t, test.source) + violations := publicSurfaceViolations(pkg, "example.test/facade") + if got := len(violations) > 0; got != test.wantReject { + t.Fatalf("rejected = %t, want %t; violations = %v", got, test.wantReject, violations) + } + }) + } +} + +type packageMetadata struct { + ImportPath string + Export string + Imports []string +} + +func listedPackages(t *testing.T, args ...string) []packageMetadata { + t.Helper() + command := exec.Command("go", append([]string{"list", "-json"}, args...)...) + output, err := command.Output() + if err != nil { + t.Fatalf("go list %s: %v", strings.Join(args, " "), err) + } + + decoder := json.NewDecoder(strings.NewReader(string(output))) + var packages []packageMetadata + for decoder.More() { + var metadata packageMetadata + if err := decoder.Decode(&metadata); err != nil { + t.Fatalf("decode go list %s: %v", strings.Join(args, " "), err) + } + packages = append(packages, metadata) + } + return packages +} + +func importCompiledPackage(t *testing.T, root packageMetadata) *types.Package { + t.Helper() + exports := map[string]string{root.ImportPath: root.Export} + lookup := func(importPath string) (io.ReadCloser, error) { + export, ok := exports[importPath] + if !ok { + metadata := listedPackages(t, "-export", importPath) + if len(metadata) != 1 || metadata[0].Export == "" { + return nil, fmt.Errorf("export data unavailable for %s", importPath) + } + export = metadata[0].Export + exports[importPath] = export + } + return os.Open(export) + } + + compiled := importer.ForCompiler(token.NewFileSet(), runtime.Compiler, lookup) + pkg, err := compiled.Import(root.ImportPath) + if err != nil { + t.Fatalf("import compiled root package: %v", err) + } + return pkg +} + +func publicSurfaceViolations(pkg *types.Package, modulePath string) []string { + var violations []string + seen := map[types.Type]bool{} + record := func(where string, typ types.Type) { + violations = append(violations, fmt.Sprintf("%s: %s", where, typ.String())) + } + var inspect func(types.Type, string) + var inspectTuple func(*types.Tuple, string) + var inspectTypeParams func(*types.TypeParamList, string) + inspect = func(typ types.Type, where string) { + if typ == nil || seen[typ] { + return + } + seen[typ] = true + + switch value := typ.(type) { + case *types.Named: + if isInternalPackage(value.Obj().Pkg(), modulePath) { + record(where, value) + return + } + if value.Obj().Pkg() == pkg { + inspectTypeParams(value.TypeParams(), where) + inspect(value.Underlying(), where) + } + case *types.Alias: + if isInternalPackage(value.Obj().Pkg(), modulePath) { + record(where, value) + return + } + inspectTypeParams(value.TypeParams(), where) + inspect(types.Unalias(value), where) + case *types.Pointer: + inspect(value.Elem(), where) + case *types.Array: + inspect(value.Elem(), where) + case *types.Slice: + inspect(value.Elem(), where) + case *types.Map: + inspect(value.Key(), where) + inspect(value.Elem(), where) + case *types.Chan: + inspect(value.Elem(), where) + case *types.Struct: + for index := 0; index < value.NumFields(); index++ { + field := value.Field(index) + if field.Exported() { + inspect(field.Type(), where+"."+field.Name()) + } + } + case *types.Interface: + value.Complete() + for index := 0; index < value.NumEmbeddeds(); index++ { + inspect(value.EmbeddedType(index), where) + } + for index := 0; index < value.NumMethods(); index++ { + method := value.Method(index) + if method.Exported() { + inspect(method.Type(), where+"."+method.Name()) + } + } + case *types.Signature: + if value.Recv() != nil { + inspect(value.Recv().Type(), where) + } + inspectTuple(value.Params(), where) + inspectTuple(value.Results(), where) + inspectTypeParams(value.TypeParams(), where) + case *types.TypeParam: + inspect(value.Constraint(), where) + case *types.Union: + for index := 0; index < value.Len(); index++ { + inspect(value.Term(index).Type(), where) + } + } + } + inspectTuple = func(tuple *types.Tuple, where string) { + for index := 0; index < tuple.Len(); index++ { + inspect(tuple.At(index).Type(), where) + } + } + inspectTypeParams = func(params *types.TypeParamList, where string) { + if params == nil { + return + } + for index := 0; index < params.Len(); index++ { + inspect(params.At(index), where) + } + } + + for _, name := range pkg.Scope().Names() { + if !token.IsExported(name) { + continue + } + object := pkg.Scope().Lookup(name) + inspect(object.Type(), name) + typeName, ok := object.(*types.TypeName) + if !ok { + continue + } + named, ok := types.Unalias(typeName.Type()).(*types.Named) + if !ok { + continue + } + for _, methodSet := range []*types.MethodSet{types.NewMethodSet(named), types.NewMethodSet(types.NewPointer(named))} { + for index := 0; index < methodSet.Len(); index++ { + selection := methodSet.At(index) + method := selection.Obj().(*types.Func) + if !method.Exported() { + continue + } + if isInternalPackage(method.Pkg(), modulePath) { + record(name+"."+method.Name(), method.Type()) + continue + } + inspect(method.Type(), name+"."+method.Name()) + } + } + } + sort.Strings(violations) + return compactStrings(violations) +} + +func isInternalPackage(pkg *types.Package, modulePath string) bool { + return pkg != nil && strings.HasPrefix(pkg.Path(), modulePath+"/internal/") +} + +func compactStrings(values []string) []string { + if len(values) == 0 { + return nil + } + unique := values[:1] + for _, value := range values[1:] { + if value != unique[len(unique)-1] { + unique = append(unique, value) + } + } + return unique +} + +func fixturePackage(t *testing.T, source string) *types.Package { + t.Helper() + const rootPath = "example.test/facade" + const internalPath = rootPath + "/internal/hidden" + fset := token.NewFileSet() + internalFile, err := parser.ParseFile(fset, "hidden.go", `package hidden +type Value struct{} +func (Value) Exposed() {} +`, 0) + if err != nil { + t.Fatalf("parse internal fixture: %v", err) + } + internalPkg, err := (&types.Config{}).Check(internalPath, fset, []*ast.File{internalFile}, nil) + if err != nil { + t.Fatalf("type-check internal fixture: %v", err) + } + rootFile, err := parser.ParseFile(fset, "facade.go", source, 0) + if err != nil { + t.Fatalf("parse facade fixture: %v", err) + } + pkg, err := (&types.Config{Importer: fixtureImporter(func(path string) (*types.Package, error) { + if path == internalPath { + return internalPkg, nil + } + return nil, fmt.Errorf("unexpected fixture import %s", path) + })}).Check(rootPath, fset, []*ast.File{rootFile}, nil) + if err != nil { + t.Fatalf("type-check facade fixture: %v", err) + } + return pkg +} + +type fixtureImporter func(string) (*types.Package, error) + +func (f fixtureImporter) Import(path string) (*types.Package, error) { return f(path) }