Add public facade boundary checks
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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.
|
||||
|
||||
346
public_boundary_test.go
Normal file
346
public_boundary_test.go
Normal file
@@ -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) }
|
||||
Reference in New Issue
Block a user