test(architecture): catch missing container bindings statically
Codifies the rule that every class referenced via `$c->get(X::class)` inside a container registrar must itself be bound via `$container->set(X::class, ...)` somewhere — in the same registrar, in core/App/registerContainer.php, or in a module container registrar. Catches the bug pattern fromcd2c9c5: a registrar's factory closure pulled UserSettingsGateway via $c->get() but no matching ->set() ever ran. Unit tests with mocked dependencies passed; the failure only surfaced at runtime ("Service not bound: ...") when the affected admin/settings/user-lifecycle page tried to construct the dashboard service against the live container. Implementation is purely static — token / regex parsing of registrar files plus class_exists() / interface_exists() gating to avoid false positives on non-class identifiers. No factory invocation, so the test does not require DB / session / HTTP and is fast. Verified by reverting thecd2c9c5binding line in the working tree and running the test — it produces the exact "UserSettingsGateway referenced in core/App/Container/Registrars/UserRegistrar.php" finding. Restored after the dry-run. Allowlist starts with two framework-bootstrap classes (AppContainer, ModuleRegistry) that are bound outside the registrar mechanism. Each future addition needs an inline justification. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
220
tests/Architecture/ContainerBindingResolutionContractTest.php
Normal file
220
tests/Architecture/ContainerBindingResolutionContractTest.php
Normal file
@@ -0,0 +1,220 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
namespace MintyPHP\Tests\Architecture;
|
||||||
|
|
||||||
|
use FilesystemIterator;
|
||||||
|
use PHPUnit\Framework\TestCase;
|
||||||
|
use RecursiveDirectoryIterator;
|
||||||
|
use RecursiveIteratorIterator;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Contract: every class referenced via `$c->get(X::class)` inside a container
|
||||||
|
* registrar MUST itself be bound via `$container->set(X::class, ...)` somewhere
|
||||||
|
* (in the same registrar, in core/App/registerContainer.php, or in a module
|
||||||
|
* container registrar) — or be on the framework-bootstrap allowlist.
|
||||||
|
*
|
||||||
|
* Catches the bug pattern from cd2c9c5: a registrar's factory closure pulls
|
||||||
|
* a service via $c->get() but no matching ->set() exists, so the failure only
|
||||||
|
* surfaces at runtime ("Service not bound: ...") when the affected page is
|
||||||
|
* loaded — never inside unit tests, because unit tests mock the dependencies.
|
||||||
|
*
|
||||||
|
* Static-only: parses use-aliases + namespace + ::class references via regex.
|
||||||
|
* No factory invocation — so the test does not require DB / session / HTTP.
|
||||||
|
*
|
||||||
|
* Detection is conservative: when an aggregator binding cannot be statically
|
||||||
|
* located (string-literal keys, dynamic `::class` lookups), the entry is
|
||||||
|
* skipped rather than reported as a false positive. The class-must-exist
|
||||||
|
* gate via class_exists() avoids matching non-class identifiers.
|
||||||
|
*/
|
||||||
|
final class ContainerBindingResolutionContractTest extends TestCase
|
||||||
|
{
|
||||||
|
use ProjectFileAssertionSupport;
|
||||||
|
|
||||||
|
public function testEveryContainerGetTargetIsBoundSomewhere(): void
|
||||||
|
{
|
||||||
|
$registrarFiles = $this->collectRegistrarFiles();
|
||||||
|
self::assertNotEmpty(
|
||||||
|
$registrarFiles,
|
||||||
|
'No registrar files discovered — the registrar-discovery glob is broken.'
|
||||||
|
);
|
||||||
|
|
||||||
|
$boundClasses = [];
|
||||||
|
$referencedClasses = [];
|
||||||
|
|
||||||
|
foreach ($registrarFiles as $file) {
|
||||||
|
$content = $this->readProjectFile($file);
|
||||||
|
$aliases = $this->parseUseAliases($content);
|
||||||
|
$namespace = $this->parseNamespace($content);
|
||||||
|
|
||||||
|
foreach ($this->matchClassRefs('->set\(', $content) as $ref) {
|
||||||
|
$fqn = $this->resolveFqn($ref, $aliases, $namespace);
|
||||||
|
if ($fqn !== null && (class_exists($fqn) || interface_exists($fqn))) {
|
||||||
|
$boundClasses[$fqn] = true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
foreach ($this->matchClassRefs('->get\(', $content) as $ref) {
|
||||||
|
$fqn = $this->resolveFqn($ref, $aliases, $namespace);
|
||||||
|
if ($fqn === null) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
if (!class_exists($fqn) && !interface_exists($fqn)) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
if (!isset($referencedClasses[$fqn])) {
|
||||||
|
$referencedClasses[$fqn] = $file;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
$allowlist = $this->frameworkBoundAllowlist();
|
||||||
|
|
||||||
|
$missing = [];
|
||||||
|
foreach ($referencedClasses as $class => $file) {
|
||||||
|
if (isset($boundClasses[$class])) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
if (in_array($class, $allowlist, true)) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
$missing[] = sprintf('%s — referenced in %s', $class, $file);
|
||||||
|
}
|
||||||
|
sort($missing);
|
||||||
|
|
||||||
|
self::assertSame(
|
||||||
|
[],
|
||||||
|
$missing,
|
||||||
|
"Container ->get() targets without matching ->set() — these will fail with"
|
||||||
|
. " 'Service not bound: ...' at runtime:\n "
|
||||||
|
. implode("\n ", $missing)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Match either `->set(Class::class,` or `->get(Class::class)` based on the
|
||||||
|
* provided opening pattern, and return the captured class identifier
|
||||||
|
* strings (still potentially aliased — caller resolves to FQN).
|
||||||
|
*
|
||||||
|
* @return list<string>
|
||||||
|
*/
|
||||||
|
private function matchClassRefs(string $callOpenPattern, string $content): array
|
||||||
|
{
|
||||||
|
$regex = '/' . $callOpenPattern . '\s*([\\\\A-Za-z_][\\\\A-Za-z0-9_]*)::class\s*[,)]/';
|
||||||
|
if (!preg_match_all($regex, $content, $m)) {
|
||||||
|
return [];
|
||||||
|
}
|
||||||
|
return $m[1];
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @return list<string>
|
||||||
|
*/
|
||||||
|
private function collectRegistrarFiles(): array
|
||||||
|
{
|
||||||
|
$root = $this->projectRootPath();
|
||||||
|
$files = ['core/App/registerContainer.php'];
|
||||||
|
|
||||||
|
$coreGlob = glob($root . '/core/App/Container/Registrars/*Registrar.php');
|
||||||
|
foreach ($coreGlob ?: [] as $f) {
|
||||||
|
$files[] = str_replace($root . '/', '', $f);
|
||||||
|
}
|
||||||
|
|
||||||
|
$modulesDir = $root . '/modules';
|
||||||
|
if (is_dir($modulesDir)) {
|
||||||
|
$iterator = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($modulesDir, FilesystemIterator::SKIP_DOTS));
|
||||||
|
foreach ($iterator as $file) {
|
||||||
|
if (!$file->isFile() || $file->getExtension() !== 'php') {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
if (!str_ends_with($file->getFilename(), 'ContainerRegistrar.php')) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
$files[] = str_replace($root . '/', '', $file->getPathname());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
sort($files);
|
||||||
|
return array_values(array_unique($files));
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @return array<string, string>
|
||||||
|
*/
|
||||||
|
private function parseUseAliases(string $content): array
|
||||||
|
{
|
||||||
|
$aliases = [];
|
||||||
|
if (!preg_match_all('/^use\s+([^;]+);/m', $content, $m)) {
|
||||||
|
return $aliases;
|
||||||
|
}
|
||||||
|
foreach ($m[1] as $useStmt) {
|
||||||
|
$useStmt = trim($useStmt);
|
||||||
|
// Skip group-uses for simplicity ("use A\{B, C}") — registrar files don't use them.
|
||||||
|
if (str_contains($useStmt, '{')) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
if (preg_match('/^(.+?)\s+as\s+(.+)$/i', $useStmt, $aliasM)) {
|
||||||
|
$fqn = trim($aliasM[1]);
|
||||||
|
$alias = trim($aliasM[2]);
|
||||||
|
} else {
|
||||||
|
$fqn = $useStmt;
|
||||||
|
$lastBackslash = strrpos($fqn, '\\');
|
||||||
|
$alias = $lastBackslash === false ? $fqn : substr($fqn, $lastBackslash + 1);
|
||||||
|
}
|
||||||
|
$aliases[$alias] = ltrim($fqn, '\\');
|
||||||
|
}
|
||||||
|
return $aliases;
|
||||||
|
}
|
||||||
|
|
||||||
|
private function parseNamespace(string $content): string
|
||||||
|
{
|
||||||
|
if (preg_match('/^namespace\s+([^;]+);/m', $content, $m)) {
|
||||||
|
return trim($m[1]);
|
||||||
|
}
|
||||||
|
return '';
|
||||||
|
}
|
||||||
|
|
||||||
|
private function resolveFqn(string $ref, array $aliases, string $namespace): ?string
|
||||||
|
{
|
||||||
|
// FQN with leading backslash → strip and use directly
|
||||||
|
if (str_starts_with($ref, '\\')) {
|
||||||
|
return ltrim($ref, '\\');
|
||||||
|
}
|
||||||
|
|
||||||
|
// First segment is either an alias or a namespace prefix
|
||||||
|
$parts = explode('\\', $ref);
|
||||||
|
$first = $parts[0];
|
||||||
|
|
||||||
|
if (isset($aliases[$first])) {
|
||||||
|
$rest = array_slice($parts, 1);
|
||||||
|
return $rest === [] ? $aliases[$first] : $aliases[$first] . '\\' . implode('\\', $rest);
|
||||||
|
}
|
||||||
|
|
||||||
|
// No alias match — try as same-namespace ref (rare in registrars)
|
||||||
|
if ($namespace !== '') {
|
||||||
|
return $namespace . '\\' . $ref;
|
||||||
|
}
|
||||||
|
|
||||||
|
return $ref;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Classes bound by the MintyPHP framework or PHP-FPM bootstrap before
|
||||||
|
* any project registrar runs. Each entry needs a one-line justification.
|
||||||
|
*
|
||||||
|
* Add to this list ONLY when you have verified that the class genuinely
|
||||||
|
* cannot be discovered statically from registrars — never to silence a
|
||||||
|
* legitimate "Service not bound" warning.
|
||||||
|
*
|
||||||
|
* @return list<string>
|
||||||
|
*/
|
||||||
|
private function frameworkBoundAllowlist(): array
|
||||||
|
{
|
||||||
|
return [
|
||||||
|
// App container itself — bound by registerContainer.php's `new AppContainer()`,
|
||||||
|
// not via $container->set() (it IS the container).
|
||||||
|
'MintyPHP\\App\\AppContainer',
|
||||||
|
// Module registry — bound dynamically after module discovery.
|
||||||
|
'MintyPHP\\App\\Module\\ModuleRegistry',
|
||||||
|
];
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user