From 6146b0bd0001bc6bc55d17fc81b14eecd0712303 Mon Sep 17 00:00:00 2001 From: fs Date: Mon, 27 Apr 2026 08:54:21 +0200 Subject: [PATCH] test(architecture): catch missing container bindings statically MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 from cd2c9c5: 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 the cd2c9c5 binding 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) --- ...ContainerBindingResolutionContractTest.php | 220 ++++++++++++++++++ 1 file changed, 220 insertions(+) create mode 100644 tests/Architecture/ContainerBindingResolutionContractTest.php diff --git a/tests/Architecture/ContainerBindingResolutionContractTest.php b/tests/Architecture/ContainerBindingResolutionContractTest.php new file mode 100644 index 0000000..5ca615f --- /dev/null +++ b/tests/Architecture/ContainerBindingResolutionContractTest.php @@ -0,0 +1,220 @@ +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 + */ + 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 + */ + 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 + */ + 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 + */ + 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', + ]; + } +}