Add public API boundary checks
This commit is contained in:
334
internal/apicheck/main.go
Normal file
334
internal/apicheck/main.go
Normal file
@@ -0,0 +1,334 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"go/importer"
|
||||
"go/token"
|
||||
"go/types"
|
||||
"io"
|
||||
"os"
|
||||
"os/exec"
|
||||
"sort"
|
||||
"strings"
|
||||
)
|
||||
|
||||
const modulePath = "gitea.maximumdirect.net/eric/ReleaseKit"
|
||||
|
||||
var facades = []string{modulePath, modulePath + "/buildinfo"}
|
||||
|
||||
type listedPackage struct {
|
||||
ImportPath string
|
||||
Export string
|
||||
Standard bool
|
||||
Imports []string
|
||||
}
|
||||
|
||||
func main() {
|
||||
os.Exit(run(context.Background(), ".", os.Stdout, os.Stderr))
|
||||
}
|
||||
|
||||
func run(ctx context.Context, dir string, stdout, stderr io.Writer) int {
|
||||
if err := checkRepository(ctx, dir); err != nil {
|
||||
fmt.Fprintln(stderr, err)
|
||||
return 1
|
||||
}
|
||||
fmt.Fprintln(stdout, "public API and dependency boundaries pass")
|
||||
return 0
|
||||
}
|
||||
|
||||
func checkRepository(ctx context.Context, dir string) error {
|
||||
packages, err := listPackages(ctx, dir)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if err := checkImportDirections(packages); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
lookup := make(map[string]string, len(packages))
|
||||
for _, pkg := range packages {
|
||||
if pkg.Export != "" {
|
||||
lookup[pkg.ImportPath] = pkg.Export
|
||||
}
|
||||
}
|
||||
apiImporter := importer.ForCompiler(token.NewFileSet(), "gc", func(path string) (io.ReadCloser, error) {
|
||||
export, ok := lookup[path]
|
||||
if !ok {
|
||||
return nil, fmt.Errorf("no compiled export data for %q", path)
|
||||
}
|
||||
return os.Open(export)
|
||||
})
|
||||
|
||||
for _, path := range facades {
|
||||
pkg, err := apiImporter.Import(path)
|
||||
if err != nil {
|
||||
return fmt.Errorf("load facade %s: %w", path, err)
|
||||
}
|
||||
if violations := checkExports(pkg); len(violations) != 0 {
|
||||
return fmt.Errorf("facade %s has invalid public contracts:\n %s", path, strings.Join(violations, "\n "))
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func listPackages(ctx context.Context, dir string) ([]listedPackage, error) {
|
||||
cmd := exec.CommandContext(ctx, "go", "list", "-deps", "-export", "-json", "./...")
|
||||
cmd.Dir = dir
|
||||
output, err := cmd.Output()
|
||||
if err != nil {
|
||||
var exitErr *exec.ExitError
|
||||
if errors.As(err, &exitErr) {
|
||||
return nil, fmt.Errorf("go list -deps -export failed: %s", strings.TrimSpace(string(exitErr.Stderr)))
|
||||
}
|
||||
return nil, fmt.Errorf("go list -deps -export: %w", err)
|
||||
}
|
||||
|
||||
decoder := json.NewDecoder(strings.NewReader(string(output)))
|
||||
var packages []listedPackage
|
||||
for {
|
||||
var pkg listedPackage
|
||||
if err := decoder.Decode(&pkg); errors.Is(err, io.EOF) {
|
||||
break
|
||||
} else if err != nil {
|
||||
return nil, fmt.Errorf("decode go list output: %w", err)
|
||||
}
|
||||
packages = append(packages, pkg)
|
||||
}
|
||||
return packages, nil
|
||||
}
|
||||
|
||||
func checkImportDirections(packages []listedPackage) error {
|
||||
byPath := make(map[string]listedPackage, len(packages))
|
||||
for _, pkg := range packages {
|
||||
byPath[pkg.ImportPath] = pkg
|
||||
}
|
||||
var violations []string
|
||||
|
||||
for _, pkg := range packages {
|
||||
if !strings.HasPrefix(pkg.ImportPath, modulePath+"/internal/") {
|
||||
continue
|
||||
}
|
||||
for _, imported := range pkg.Imports {
|
||||
if imported == modulePath || imported == modulePath+"/buildinfo" || strings.HasPrefix(imported, modulePath+"/cmd/") {
|
||||
violations = append(violations, fmt.Sprintf("internal package %s imports public or command package %s", pkg.ImportPath, imported))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
seen := make(map[string]bool)
|
||||
var visit func(string)
|
||||
visit = func(path string) {
|
||||
if seen[path] {
|
||||
return
|
||||
}
|
||||
seen[path] = true
|
||||
pkg, ok := byPath[path]
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
if path != modulePath+"/buildinfo" && !pkg.Standard {
|
||||
if !strings.HasPrefix(path, modulePath+"/internal/") {
|
||||
violations = append(violations, fmt.Sprintf("buildinfo depends on non-runtime package %s", path))
|
||||
} else if forbiddenRuntimeDependency(path) {
|
||||
violations = append(violations, fmt.Sprintf("buildinfo depends on excluded implementation package %s", path))
|
||||
}
|
||||
}
|
||||
for _, imported := range pkg.Imports {
|
||||
visit(imported)
|
||||
}
|
||||
}
|
||||
visit(modulePath + "/buildinfo")
|
||||
|
||||
if len(violations) == 0 {
|
||||
return nil
|
||||
}
|
||||
sort.Strings(violations)
|
||||
return errors.New(strings.Join(violations, "\n"))
|
||||
}
|
||||
|
||||
func forbiddenRuntimeDependency(path string) bool {
|
||||
for _, part := range strings.Split(strings.ToLower(path), "/") {
|
||||
switch part {
|
||||
case "orchestration", "orchestrator", "engine", "release", "process", "build", "builder", "gitea", "delivery", "cli":
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
var allowedExternalTypes = map[string]bool{
|
||||
"context.Context": true,
|
||||
"io.Reader": true,
|
||||
"io.Writer": true,
|
||||
}
|
||||
|
||||
func checkExports(pkg *types.Package) []string {
|
||||
checker := contractChecker{
|
||||
packagePath: pkg.Path(),
|
||||
checkedTypes: make(map[types.Type]bool),
|
||||
allowedNamed: allowedExternalTypes,
|
||||
violationsSet: make(map[string]bool),
|
||||
}
|
||||
for _, name := range pkg.Scope().Names() {
|
||||
if !token.IsExported(name) {
|
||||
continue
|
||||
}
|
||||
object := pkg.Scope().Lookup(name)
|
||||
switch obj := object.(type) {
|
||||
case *types.TypeName:
|
||||
checker.checkType(obj.Type(), name)
|
||||
case *types.Func:
|
||||
checker.checkType(obj.Type(), name)
|
||||
case *types.Var:
|
||||
checker.checkType(obj.Type(), name)
|
||||
case *types.Const:
|
||||
checker.checkType(obj.Type(), name)
|
||||
}
|
||||
}
|
||||
return checker.violations()
|
||||
}
|
||||
|
||||
type contractChecker struct {
|
||||
packagePath string
|
||||
checkedTypes map[types.Type]bool
|
||||
allowedNamed map[string]bool
|
||||
violationsSet map[string]bool
|
||||
}
|
||||
|
||||
func (c *contractChecker) violations() []string {
|
||||
violations := make([]string, 0, len(c.violationsSet))
|
||||
for violation := range c.violationsSet {
|
||||
violations = append(violations, violation)
|
||||
}
|
||||
sort.Strings(violations)
|
||||
return violations
|
||||
}
|
||||
|
||||
func (c *contractChecker) violation(message string) {
|
||||
c.violationsSet[message] = true
|
||||
}
|
||||
|
||||
func (c *contractChecker) checkType(typ types.Type, path string) {
|
||||
if typ == nil || c.checkedTypes[typ] {
|
||||
return
|
||||
}
|
||||
c.checkedTypes[typ] = true
|
||||
|
||||
switch typ := typ.(type) {
|
||||
case *types.Basic:
|
||||
case *types.Alias:
|
||||
if params := typ.TypeParams(); params != nil {
|
||||
for i := 0; i < params.Len(); i++ {
|
||||
c.checkType(params.At(i), path+" type parameter")
|
||||
}
|
||||
}
|
||||
if args := typ.TypeArgs(); args != nil {
|
||||
for i := 0; i < args.Len(); i++ {
|
||||
c.checkType(args.At(i), path+" type argument")
|
||||
}
|
||||
}
|
||||
c.checkType(types.Unalias(typ), path)
|
||||
case *types.Named:
|
||||
object := typ.Obj()
|
||||
if object.Pkg() != nil && object.Pkg().Path() != c.packagePath {
|
||||
key := object.Pkg().Path() + "." + object.Name()
|
||||
if !c.allowedNamed[key] {
|
||||
c.violation(fmt.Sprintf("%s exposes named type %s", path, key))
|
||||
}
|
||||
return
|
||||
}
|
||||
if params := typ.TypeParams(); params != nil {
|
||||
for i := 0; i < params.Len(); i++ {
|
||||
c.checkType(params.At(i), path+" type parameter")
|
||||
}
|
||||
}
|
||||
if args := typ.TypeArgs(); args != nil {
|
||||
for i := 0; i < args.Len(); i++ {
|
||||
c.checkType(args.At(i), path+" type argument")
|
||||
}
|
||||
}
|
||||
c.checkType(typ.Underlying(), path)
|
||||
for _, receiver := range []types.Type{typ, types.NewPointer(typ)} {
|
||||
methodSet := types.NewMethodSet(receiver)
|
||||
for i := 0; i < methodSet.Len(); i++ {
|
||||
method := methodSet.At(i).Obj()
|
||||
if !method.Exported() {
|
||||
continue
|
||||
}
|
||||
function, _ := method.(*types.Func)
|
||||
if method.Pkg() != nil && method.Pkg().Path() != c.packagePath && !c.allowedMethod(function) {
|
||||
c.violation(fmt.Sprintf("%s promotes method %s from %s", path, method.Name(), method.Pkg().Path()))
|
||||
}
|
||||
c.checkType(method.Type(), path+" method "+method.Name())
|
||||
}
|
||||
}
|
||||
case *types.Pointer:
|
||||
c.checkType(typ.Elem(), path)
|
||||
case *types.Array:
|
||||
c.checkType(typ.Elem(), path)
|
||||
case *types.Slice:
|
||||
c.checkType(typ.Elem(), path)
|
||||
case *types.Map:
|
||||
c.checkType(typ.Key(), path)
|
||||
c.checkType(typ.Elem(), path)
|
||||
case *types.Chan:
|
||||
c.checkType(typ.Elem(), path)
|
||||
case *types.Struct:
|
||||
for i := 0; i < typ.NumFields(); i++ {
|
||||
field := typ.Field(i)
|
||||
if field.Exported() || field.Embedded() {
|
||||
c.checkType(field.Type(), path+" field "+field.Name())
|
||||
}
|
||||
}
|
||||
case *types.Interface:
|
||||
typ.Complete()
|
||||
for i := 0; i < typ.NumMethods(); i++ {
|
||||
method := typ.Method(i)
|
||||
if method.Pkg() != nil && method.Pkg().Path() != c.packagePath && !c.allowedMethod(method) {
|
||||
c.violation(fmt.Sprintf("%s exposes method %s from %s", path, method.Name(), method.Pkg().Path()))
|
||||
}
|
||||
c.checkType(method.Type(), path+" method "+method.Name())
|
||||
}
|
||||
for i := 0; i < typ.NumEmbeddeds(); i++ {
|
||||
c.checkType(typ.EmbeddedType(i), path+" embedded interface")
|
||||
}
|
||||
case *types.Signature:
|
||||
if params := typ.TypeParams(); params != nil {
|
||||
for i := 0; i < params.Len(); i++ {
|
||||
c.checkType(params.At(i), path+" type parameter")
|
||||
}
|
||||
}
|
||||
c.checkType(typ.Params(), path+" parameter")
|
||||
c.checkType(typ.Results(), path+" result")
|
||||
if typ.Variadic() && typ.Params().Len() != 0 {
|
||||
c.checkType(typ.Params().At(typ.Params().Len()-1).Type(), path+" variadic parameter")
|
||||
}
|
||||
case *types.Tuple:
|
||||
for i := 0; i < typ.Len(); i++ {
|
||||
c.checkType(typ.At(i).Type(), path)
|
||||
}
|
||||
case *types.TypeParam:
|
||||
c.checkType(typ.Constraint(), path+" constraint")
|
||||
case *types.Union:
|
||||
for i := 0; i < typ.Len(); i++ {
|
||||
c.checkType(typ.Term(i).Type(), path+" constraint term")
|
||||
}
|
||||
default:
|
||||
c.violation(fmt.Sprintf("%s has uninspected type %T", path, typ))
|
||||
}
|
||||
}
|
||||
|
||||
func (c *contractChecker) allowedMethod(method *types.Func) bool {
|
||||
if method.Pkg() == nil {
|
||||
return true
|
||||
}
|
||||
for typeName := range c.allowedNamed {
|
||||
if strings.HasPrefix(typeName, method.Pkg().Path()+".") {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
180
internal/apicheck/main_test.go
Normal file
180
internal/apicheck/main_test.go
Normal file
@@ -0,0 +1,180 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"go/ast"
|
||||
"go/importer"
|
||||
"go/parser"
|
||||
"go/token"
|
||||
"go/types"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestExportedContractBoundaries(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
source string
|
||||
wantError string
|
||||
}{
|
||||
{
|
||||
name: "leaked implementation type",
|
||||
source: `package facade
|
||||
import "example.com/implementation"
|
||||
func New() *implementation.State { return nil }
|
||||
`,
|
||||
wantError: "exposes named type example.com/implementation.State",
|
||||
},
|
||||
{
|
||||
name: "alias to implementation type",
|
||||
source: `package facade
|
||||
import "example.com/implementation"
|
||||
type Alias = implementation.State
|
||||
`,
|
||||
wantError: "exposes named type example.com/implementation.State",
|
||||
},
|
||||
{
|
||||
name: "callback leaks implementation type",
|
||||
source: `package facade
|
||||
import "example.com/implementation"
|
||||
type Callback func() *implementation.State
|
||||
`,
|
||||
wantError: "exposes named type example.com/implementation.State",
|
||||
},
|
||||
{
|
||||
name: "generic constraint leaks implementation type",
|
||||
source: `package facade
|
||||
import "example.com/implementation"
|
||||
type Box[T interface{ Value() *implementation.State }] struct { value T }
|
||||
`,
|
||||
wantError: "exposes named type example.com/implementation.State",
|
||||
},
|
||||
{
|
||||
name: "generic instantiation leaks implementation type",
|
||||
source: `package facade
|
||||
import "example.com/implementation"
|
||||
type Box[T any] struct { value T }
|
||||
func New() Box[implementation.State] { return Box[implementation.State]{} }
|
||||
`,
|
||||
wantError: "exposes named type example.com/implementation.State",
|
||||
},
|
||||
{
|
||||
name: "promoted implementation method",
|
||||
source: `package facade
|
||||
import "example.com/implementation"
|
||||
type state = implementation.State
|
||||
type Handle struct { state }
|
||||
`,
|
||||
wantError: "promotes method Promoted from example.com/implementation",
|
||||
},
|
||||
{
|
||||
name: "promoted pointer implementation method",
|
||||
source: `package facade
|
||||
import "example.com/implementation"
|
||||
type state = implementation.State
|
||||
type Handle struct { state }
|
||||
`,
|
||||
wantError: "promotes method PointerPromoted from example.com/implementation",
|
||||
},
|
||||
}
|
||||
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
violations := checkFixture(t, test.source)
|
||||
if !strings.Contains(strings.Join(violations, "\n"), test.wantError) {
|
||||
t.Fatalf("checkExports() = %v, want an error containing %q", violations, test.wantError)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestPrivateImplementationFieldAndSelectedStandardTypes(t *testing.T) {
|
||||
source := `package facade
|
||||
import (
|
||||
"context"
|
||||
"example.com/implementation"
|
||||
)
|
||||
type Handle struct { state *implementation.State }
|
||||
func Open(context.Context) *Handle { return nil }
|
||||
`
|
||||
if violations := checkFixture(t, source); len(violations) != 0 {
|
||||
t.Fatalf("private implementation field and selected standard type were rejected: %v", violations)
|
||||
}
|
||||
}
|
||||
|
||||
func TestActualRepositoryBoundaries(t *testing.T) {
|
||||
if err := checkRepository(t.Context(), "../.."); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInternalPackagesCannotImportPublicFacades(t *testing.T) {
|
||||
packages := []listedPackage{
|
||||
{ImportPath: modulePath + "/internal/operations", Imports: []string{modulePath}},
|
||||
{ImportPath: modulePath},
|
||||
}
|
||||
err := checkImportDirections(packages)
|
||||
if err == nil || !strings.Contains(err.Error(), "internal package "+modulePath+"/internal/operations imports public or command package "+modulePath) {
|
||||
t.Fatalf("checkImportDirections() error = %v, want internal-to-facade violation", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRuntimeFacadeCannotDependOnExternalPackages(t *testing.T) {
|
||||
packages := []listedPackage{
|
||||
{ImportPath: modulePath + "/buildinfo", Imports: []string{"example.com/runtime"}},
|
||||
{ImportPath: "example.com/runtime"},
|
||||
}
|
||||
err := checkImportDirections(packages)
|
||||
if err == nil || !strings.Contains(err.Error(), "buildinfo depends on non-runtime package example.com/runtime") {
|
||||
t.Fatalf("checkImportDirections() error = %v, want runtime dependency violation", err)
|
||||
}
|
||||
}
|
||||
|
||||
func checkFixture(t *testing.T, facadeSource string) []string {
|
||||
t.Helper()
|
||||
fset := token.NewFileSet()
|
||||
implementation := typecheck(t, fset, "example.com/implementation", `package implementation
|
||||
type State struct{}
|
||||
func (State) Promoted() {}
|
||||
func (*State) PointerPromoted() {}
|
||||
`)
|
||||
fixtureImporter := singlePackageImporter{path: "example.com/implementation", pkg: implementation}
|
||||
facade := typecheckWithImporter(t, fset, "example.com/facade", facadeSource, fixtureImporter)
|
||||
return checkExports(facade)
|
||||
}
|
||||
|
||||
type singlePackageImporter struct {
|
||||
path string
|
||||
pkg *types.Package
|
||||
}
|
||||
|
||||
func (i singlePackageImporter) Import(path string) (*types.Package, error) {
|
||||
if path == i.path {
|
||||
return i.pkg, nil
|
||||
}
|
||||
return importer.Default().Import(path)
|
||||
}
|
||||
|
||||
func typecheck(t *testing.T, fset *token.FileSet, path, source string) *types.Package {
|
||||
t.Helper()
|
||||
return typecheckWithImporter(t, fset, path, source, importer.Default())
|
||||
}
|
||||
|
||||
func typecheckWithImporter(t *testing.T, fset *token.FileSet, path, source string, imports types.Importer) *types.Package {
|
||||
t.Helper()
|
||||
file, err := parser.ParseFile(fset, path+".go", source, 0)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
var errors []error
|
||||
config := types.Config{
|
||||
Importer: imports,
|
||||
Error: func(err error) {
|
||||
errors = append(errors, err)
|
||||
},
|
||||
}
|
||||
pkg, err := config.Check(path, fset, []*ast.File{file}, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("typecheck %s: %v (%v)", path, err, errors)
|
||||
}
|
||||
return pkg
|
||||
}
|
||||
Reference in New Issue
Block a user