/_form.phtml * modules/ * /pages//_form.phtml * MUST satisfy at least one of: * 1. The variable is initialized/assigned earlier in the same partial. * 2. The read is itself defensively guarded (`$x ?? …`, `isset($x)`, * `empty($x)`, `!empty($x)`, `array_key_exists(…, $x)`). * 3. Every consuming view (`/(default).phtml`, * `/(none).phtml`, …) defines the variable BEFORE the * `require __DIR__ . '/_form.phtml'` (or `require __DIR__.'/_form.phtml'`). * * --- Allowlist --- * Documented exceptions where a partial reads a variable that no consumer * provides, but the partial is not actually entered on that consumer's path * (e.g. behind an `if ($showSsoTab)` whose flag is false on that consumer). * Today: empty. * * --- Static-analysis caveat --- * The scanner is conservative. When it cannot statically classify a * construct (e.g. `extract($vars)`, dynamic include paths, eval), the test * fails explicitly with "cannot statically verify, please review manually" * rather than passing silently. This mirrors the fail-loud behaviour of * DetailDrawerFragmentContractTest and ActionContextCsrfPairingContractTest. * * --- Coverage guarantee --- * A second test asserts that the scanner sees exactly the seven partials * known at authoring time. If a new partial is added, that test fails until * the count is bumped, ensuring the contract is reviewed for new call sites. */ class FormPartialDefensiveReadsContractTest extends TestCase { use ProjectFileAssertionSupport; /** * Per-partial allowlist of variable names that are read without * defensive guard but are deliberately not provided by any consumer. * Example entry would be: * 'pages/admin/tenants/_form.phtml' => ['someVar' => 'reason / proof'], * Today: empty (all 7 partials are clean). * * @var array> */ private const ALLOWLIST = []; /** * Authoring-time count of shared form partials. Bumping this is a * deliberate review checkpoint: when adding a new partial, audit * its read patterns against the convention before raising the count. */ private const EXPECTED_PARTIAL_COUNT = 7; public function testSharedFormPartialsHaveDefensiveReadsOrCallerProvidesVariables(): void { $partials = $this->locateFormPartials(); $this->assertNotEmpty($partials, 'No shared form partials found — scanner is broken or layout changed.'); $violations = []; foreach ($partials as $partialRel) { $partialAbs = $this->projectRootPath() . '/' . $partialRel; $partialContent = (string) file_get_contents($partialAbs); try { $analysis = $this->analysePartial($partialContent); } catch (\RuntimeException $e) { $violations[] = sprintf( "Partial %s: cannot statically verify reads — %s. Please review manually.", $partialRel, $e->getMessage() ); continue; } if ($analysis['unverifiable']) { $violations[] = sprintf( "Partial %s: contains construct(s) the scanner cannot classify (%s). Cannot statically verify, please review manually.", $partialRel, implode(', ', $analysis['unverifiable']) ); continue; } if ($analysis['unguardedReads'] === []) { continue; } $consumers = $this->locateConsumingTemplates($partialRel); // Collect each consumer's pre-require variable definitions once. $consumerDefinitions = []; foreach ($consumers as $consumerRel) { $consumerAbs = $this->projectRootPath() . '/' . $consumerRel; $consumerContent = (string) file_get_contents($consumerAbs); try { $consumerDefinitions[$consumerRel] = $this->collectConsumerDefinitions( $consumerContent, $partialRel ); } catch (\RuntimeException $e) { $violations[] = sprintf( "Consumer %s of partial %s: cannot statically verify pre-require definitions — %s. Please review manually.", $consumerRel, $partialRel, $e->getMessage() ); $consumerDefinitions[$consumerRel] = null; } } // @phpstan-ignore-next-line nullCoalesce.offset (ALLOWLIST is intentionally empty today) $allowlist = self::ALLOWLIST[$partialRel] ?? []; foreach ($analysis['unguardedReads'] as $varName => $line) { // @phpstan-ignore-next-line function.impossibleType (allowlist is empty today; check guards future entries) if (array_key_exists($varName, $allowlist)) { continue; } $missingFrom = []; foreach ($consumerDefinitions as $consumerRel => $definitions) { if ($definitions === null) { // Already recorded as "cannot verify". continue; } if (!in_array($varName, $definitions, true)) { $missingFrom[] = $consumerRel; } } if ($missingFrom === []) { continue; } $violations[] = sprintf( "Form partial '%s' line %d reads \$%s without defensive guard, but consuming template(s) do not define it before the require:\n %s\nEither:\n - Define the variable in each consuming template before `require __DIR__ . '/_form.phtml';`, OR\n - Add a defensive read in the partial (e.g. `\$%s ?? false` / `isset(\$%s)`).", $partialRel, $line, $varName, implode("\n ", $missingFrom), $varName, $varName ); } } $this->assertSame( [], $violations, "Form-partial defensive-reads contract violations:\n\n" . implode("\n\n", $violations) ); } public function testScannerSeesExpectedNumberOfPartials(): void { $partials = $this->locateFormPartials(); $this->assertCount( self::EXPECTED_PARTIAL_COUNT, $partials, sprintf( "Expected exactly %d shared form partials at authoring time but found %d:\n %s\n" . "If a new partial was added intentionally, audit it against the convention then bump EXPECTED_PARTIAL_COUNT.", self::EXPECTED_PARTIAL_COUNT, count($partials), implode("\n ", $partials) ) ); } /** * @return list list of project-relative paths */ private function locateFormPartials(): array { $root = $this->projectRootPath(); $partials = []; // Core admin form partials: pages/admin//_form.phtml $adminBase = $root . '/pages/admin'; if (is_dir($adminBase)) { foreach (scandir($adminBase) ?: [] as $entry) { if ($entry === '.' || $entry === '..') { continue; } $entryPath = $adminBase . '/' . $entry; if (!is_dir($entryPath)) { continue; } $candidate = $entryPath . '/_form.phtml'; if (is_file($candidate)) { $partials[] = 'pages/admin/' . $entry . '/_form.phtml'; } } } // Module form partials: modules//pages/**/_form.phtml $modulesBase = $root . '/modules'; if (is_dir($modulesBase)) { foreach (scandir($modulesBase) ?: [] as $module) { if ($module === '.' || $module === '..') { continue; } $modulePages = $modulesBase . '/' . $module . '/pages'; if (!is_dir($modulePages)) { continue; } $iterator = new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($modulePages, \FilesystemIterator::SKIP_DOTS)); /** @var \SplFileInfo $file */ foreach ($iterator as $file) { if ($file->isFile() && $file->getFilename() === '_form.phtml') { $partials[] = str_replace($root . '/', '', $file->getPathname()); } } } } sort($partials); return $partials; } /** * Analyse a partial's PHP source via token_get_all. * * @return array{unguardedReads: array, unverifiable: list} * unguardedReads — name => first-line where the unguarded read appears * unverifiable — list of construct names the scanner refused to classify */ private function analysePartial(string $source): array { $tokens = token_get_all($source); // PHP superglobals — always defined. $defined = [ 'GLOBALS' => true, '_GET' => true, '_POST' => true, '_REQUEST' => true, '_SESSION' => true, '_SERVER' => true, '_FILES' => true, '_COOKIE' => true, '_ENV' => true, 'this' => true, ]; $unguarded = []; // name => line $unverifiable = []; $count = count($tokens); // Sticky context flags. // A simple scope stack to detect inside-function-parameter-list etc. // For form partials this is rarely needed but we keep it robust. $parenDepth = 0; // Tracks whether the *current* T_VARIABLE token sits inside the args // of a guard call like isset(/empty(/array_key_exists(/!empty( . When // the call closes we mark all variables read inside as defined. // Implementation: when we see the guard-opening token + its `(`, // remember the paren-depth of that open paren. Until we close back // through it, any T_VARIABLE we read is treated as guarded. $guardOpenDepths = []; // list for ($i = 0; $i < $count; $i++) { $tok = $tokens[$i]; if (is_array($tok)) { [$id, $text, $line] = $tok; // Detect extract(...) — we cannot statically know what becomes defined. if ($id === T_STRING && strtolower($text) === 'extract') { $next = $this->nextSignificant($tokens, $i); if ($next !== null && is_string($tokens[$next]) && $tokens[$next] === '(') { $unverifiable[] = sprintf("extract() at line %d", $line); } } // catch (\Exception $e) — define $e. if ($id === T_CATCH) { // Find the `(` then collect T_VARIABLE up to the matching `)`. $parenPos = null; for ($j = $i + 1; $j < $count; $j++) { if (is_string($tokens[$j]) && $tokens[$j] === '(') { $parenPos = $j; break; } } if ($parenPos !== null) { $depth = 1; for ($j = $parenPos + 1; $j < $count; $j++) { $jt = $tokens[$j]; if (is_string($jt)) { if ($jt === '(') { $depth++; } elseif ($jt === ')') { $depth--; if ($depth === 0) { break; } } } elseif ($jt[0] === T_VARIABLE) { $defined[ltrim($jt[1], '$')] = true; } } } continue; } // foreach (... as $key => $value) — define $key and $value. if ($id === T_FOREACH) { $asPos = $this->findTokenAfter($tokens, $i, T_AS); if ($asPos !== null) { // Collect T_VARIABLE tokens between T_AS and the matching ')' // (the `)` that closes this foreach header). $depth = 1; for ($j = $asPos + 1; $j < $count; $j++) { $jt = $tokens[$j]; if (is_string($jt)) { if ($jt === '(') { $depth++; } elseif ($jt === ')') { $depth--; if ($depth === 0) { break; } } } elseif ($jt[0] === T_VARIABLE) { $defined[ltrim($jt[1], '$')] = true; } } } continue; } // function (...) { ... } — define parameter variables. // Anonymous + named functions handled the same way. if ($id === T_FUNCTION || $id === T_FN) { // Find the `(` that opens the parameter list. $parenPos = null; for ($j = $i + 1; $j < $count; $j++) { if (is_string($tokens[$j]) && $tokens[$j] === '(') { $parenPos = $j; break; } // If we hit `{` first, it's not a parameter list. if (is_string($tokens[$j]) && $tokens[$j] === '{') { break; } } if ($parenPos !== null) { $depth = 1; for ($j = $parenPos + 1; $j < $count; $j++) { $jt = $tokens[$j]; if (is_string($jt)) { if ($jt === '(') { $depth++; } elseif ($jt === ')') { $depth--; if ($depth === 0) { break; } } } elseif ($jt[0] === T_VARIABLE) { $defined[ltrim($jt[1], '$')] = true; } } } continue; } // isset(...) / empty(...) — variables read inside count as guarded. if ($id === T_ISSET || $id === T_EMPTY) { // The next significant token is the `(`. $next = $this->nextSignificant($tokens, $i); if ($next !== null && is_string($tokens[$next]) && $tokens[$next] === '(') { // Mark all T_VARIABLE inside as defined (and skip them // in main read-detection by continuing past them). $depth = 1; for ($j = $next + 1; $j < $count; $j++) { $jt = $tokens[$j]; if (is_string($jt)) { if ($jt === '(') { $depth++; } elseif ($jt === ')') { $depth--; if ($depth === 0) { $i = $j; // skip ahead break; } } } elseif ($jt[0] === T_VARIABLE) { $defined[ltrim($jt[1], '$')] = true; } } } continue; } // array_key_exists(KEY, $var) — second arg is guarded. if ($id === T_STRING && strtolower($text) === 'array_key_exists') { $next = $this->nextSignificant($tokens, $i); if ($next !== null && is_string($tokens[$next]) && $tokens[$next] === '(') { $depth = 1; for ($j = $next + 1; $j < $count; $j++) { $jt = $tokens[$j]; if (is_string($jt)) { if ($jt === '(') { $depth++; } elseif ($jt === ')') { $depth--; if ($depth === 0) { $i = $j; break; } } } elseif ($jt[0] === T_VARIABLE) { $defined[ltrim($jt[1], '$')] = true; } } } continue; } if ($id === T_VARIABLE) { $name = ltrim($text, '$'); // Skip static class property access: ClassName::$prop. // The previous significant token would be T_DOUBLE_COLON. $prev = $this->prevSignificant($tokens, $i); if ($prev !== null) { $pt = $tokens[$prev]; if (is_array($pt) && $pt[0] === T_DOUBLE_COLON) { continue; } // Object property: $obj->$dynamicProp (T_OBJECT_OPERATOR). // Treat as a property read, not a variable read. if (is_array($pt) && $pt[0] === T_OBJECT_OPERATOR) { continue; } } // Look at next significant token to detect assignment. $next = $this->nextSignificant($tokens, $i); $isAssignment = false; if ($next !== null) { $nt = $tokens[$next]; if (is_string($nt) && $nt === '=') { $isAssignment = true; } // T_DOUBLE_ARROW (=>) is array key, not assignment. // T_IS_EQUAL / T_IS_IDENTICAL are tokens, so `==`/`===` // do NOT match the literal '=' branch above. } // Look at following token chain for `??` (null-coalesce) // operator on this variable, possibly through array // subscripts and property accesses, e.g. // $form['key'] ?? null // $obj->prop ?? null // are all defensive reads of the BASE variable. $isCoalescedRead = false; if ($next !== null) { $afterChain = $this->skipMemberChain($tokens, $next); if ($afterChain !== null) { $ct = $tokens[$afterChain]; if (is_array($ct) && $ct[0] === T_COALESCE) { $isCoalescedRead = true; } } } // Look at preceding significant token for `&` (by-ref) // pattern in foreach/function — already handled, skip. // If this is a list/array destructuring on LHS like // `[$a, $b] = …` or `list($a) = …` we'd need broader // analysis; fall through to "read" — partials don't use // these patterns. The existence test guards against drift. if ($isAssignment) { $defined[$name] = true; // Continue — the RHS may still contain reads, and the // tokenizer will visit them on subsequent iterations. continue; } if ($isCoalescedRead) { // Defensive read; do not require pre-definition. // The variable does NOT become "defined" though, // because subsequent unguarded reads must still be // verified. (Common pattern: `$x = $x ?? default;` // would already match the assignment branch.) continue; } if (isset($defined[$name])) { continue; } // Unguarded read of a not-yet-defined variable. if (!isset($unguarded[$name])) { $unguarded[$name] = $line; } continue; } } // String tokens like '(' / ')' — depth tracking not strictly // needed for the patterns above; they each scan their own // matching `)`. Keep the loop simple. } return [ 'unguardedReads' => $unguarded, 'unverifiable' => $unverifiable, ]; } /** * Walk a consumer template and collect every variable name assigned * BEFORE the first `require … _form.phtml` statement. Reads are not * collected — only assignments (LHS). * * Throws RuntimeException when no require-of-_form is found (the * caller used a non-static include the test cannot follow). * * @return list */ private function collectConsumerDefinitions(string $source, string $partialRel): array { $tokens = token_get_all($source); $count = count($tokens); // Find the offset of `require __DIR__ . '/_form.phtml'` (or similar // relative require to the same directory's _form.phtml). $requireOffset = null; for ($i = 0; $i < $count; $i++) { $t = $tokens[$i]; if (!is_array($t)) { continue; } if ($t[0] !== T_REQUIRE && $t[0] !== T_REQUIRE_ONCE && $t[0] !== T_INCLUDE && $t[0] !== T_INCLUDE_ONCE) { continue; } // Look ahead until the statement-terminating `;` and check if // the path string contains `_form.phtml`. $stmt = ''; for ($j = $i + 1; $j < $count; $j++) { $jt = $tokens[$j]; if (is_string($jt) && $jt === ';') { break; } $stmt .= is_array($jt) ? $jt[1] : $jt; } if (str_contains($stmt, '_form.phtml')) { $requireOffset = $i; break; } } if ($requireOffset === null) { throw new \RuntimeException(sprintf( "no `require … _form.phtml` statement found while scanning consumer for partial %s", $partialRel )); } // Collect $x in `$x = …` patterns up to (but excluding) requireOffset. // Also recognise foreach-binding variables defined before the require // (rare in practice for consumers, but cheap to support). $defined = []; for ($i = 0; $i < $requireOffset; $i++) { $t = $tokens[$i]; // foreach (...) — collect bound variables. if (is_array($t) && $t[0] === T_FOREACH) { $asPos = $this->findTokenAfter($tokens, $i, T_AS); if ($asPos !== null) { $depth = 1; for ($j = $asPos + 1; $j < $count; $j++) { $jt = $tokens[$j]; if (is_string($jt)) { if ($jt === '(') { $depth++; } elseif ($jt === ')') { $depth--; if ($depth === 0) { break; } } } elseif ($jt[0] === T_VARIABLE) { $defined[ltrim($jt[1], '$')] = true; } } } continue; } if (!is_array($t) || $t[0] !== T_VARIABLE) { continue; } $next = $this->nextSignificant($tokens, $i); if ($next === null) { continue; } $nt = $tokens[$next]; if (is_string($nt) && $nt === '=') { $defined[ltrim($t[1], '$')] = true; } } return array_keys($defined); } /** * @return list list of project-relative paths to consuming templates */ private function locateConsumingTemplates(string $partialRel): array { $root = $this->projectRootPath(); $partialDir = dirname($partialRel); $absDir = $root . '/' . $partialDir; $consumers = []; if (!is_dir($absDir)) { return []; } // Direct directory scan: all .phtml files in the same directory that // require _form.phtml. We rely on the convention that consumers live // alongside the partial. foreach (scandir($absDir) ?: [] as $entry) { if ($entry === '.' || $entry === '..' || $entry === '_form.phtml') { continue; } $abs = $absDir . '/' . $entry; if (!is_file($abs) || !str_ends_with($entry, '.phtml')) { continue; } $content = (string) file_get_contents($abs); // Match either `require __DIR__ . '/_form.phtml'` or an explicit // relative path that contains `_form.phtml`. if (preg_match('/\b(?:require|require_once|include|include_once)\b[^;]*_form\.phtml/', $content) === 1) { $consumers[] = $partialDir . '/' . $entry; } } sort($consumers); return $consumers; } /** * @param array $tokens */ private function nextSignificant(array $tokens, int $from): ?int { $count = count($tokens); for ($i = $from + 1; $i < $count; $i++) { $t = $tokens[$i]; if (is_array($t)) { if (in_array($t[0], [T_WHITESPACE, T_COMMENT, T_DOC_COMMENT], true)) { continue; } } return $i; } return null; } /** * @param array $tokens */ private function prevSignificant(array $tokens, int $from): ?int { for ($i = $from - 1; $i >= 0; $i--) { $t = $tokens[$i]; if (is_array($t)) { if (in_array($t[0], [T_WHITESPACE, T_COMMENT, T_DOC_COMMENT], true)) { continue; } } return $i; } return null; } /** * Starting from $from (an offset already pointing at a `[` or `->` or * `?->` token following a variable), walk past the entire member-access * chain and return the offset of the next significant token *after* the * chain. Returns the original $from when no chain follows. * * Example token stream for `$form['key'] ?? null`: * $form [ 'key' ] ?? null * ^from points at `[`; this returns the offset of `??`. * * @param array $tokens */ private function skipMemberChain(array $tokens, int $from): ?int { $count = count($tokens); $i = $from; while ($i < $count) { $t = $tokens[$i]; // Bracket: skip the matching `]`. if (is_string($t) && $t === '[') { $depth = 1; $j = $i + 1; while ($j < $count && $depth > 0) { $jt = $tokens[$j]; if (is_string($jt)) { if ($jt === '[') { $depth++; } elseif ($jt === ']') { $depth--; } } $j++; } $i = $j; $next = $this->nextSignificant($tokens, $i - 1); if ($next === null) { return null; } $i = $next; continue; } // Object operator (->) or nullsafe (?->) — skip the operator // plus its identifier (T_STRING) or expression token, then loop. if (is_array($t) && ($t[0] === T_OBJECT_OPERATOR || (defined('T_NULLSAFE_OBJECT_OPERATOR') && $t[0] === T_NULLSAFE_OBJECT_OPERATOR))) { $next = $this->nextSignificant($tokens, $i); if ($next === null) { return null; } $i = $this->nextSignificant($tokens, $next) ?? $count; continue; } // Anything else terminates the chain — return current offset. return $i; } return null; } /** * Return offset of the next token with the given type, after $from. * * @param array $tokens */ private function findTokenAfter(array $tokens, int $from, int $type): ?int { $count = count($tokens); for ($i = $from + 1; $i < $count; $i++) { $t = $tokens[$i]; if (is_array($t) && $t[0] === $type) { return $i; } // Stop at the closing ')' of the immediate construct so we do not // wander into an unrelated `as` token elsewhere. For T_FOREACH the // T_AS we want lives inside the very first parenthesis group; if we // hit a `;` we have left the foreach header entirely. if (is_string($t) && $t === ';') { return null; } } return null; } }