From d0cff99d3d760f22d89850a52e68668437521261 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Tue, 18 Aug 2026 08:10:40 +0200 Subject: [PATCH] test(adr): check the code citations in the ADR corpus against the tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 74 citations of the form File.php:NNN across Documentation/Adr, and nothing read them. AdrLifecycleTest checks form inside the corpus, AdrReferenceIntegrityTest checks that ADR filenames named elsewhere resolve, and its docblock says outright that a drifted line number still points at a line that exists. Three assertions, each seen to fail before being trusted: - a cited file that is in neither the tree nor the declared list - a citation past the end of its file, or at a blank line - a change to the set of citations that resolve to nothing here The census turned up something the issue did not: 21 of the 74 point into .Build/vendor — TYPO3 core, cms-install, nr-vault. That code is not committed and its line numbers move with every patch release of a dependency this repository does not pin, so no test here can check them. They are listed by hand instead, which makes adding one deliberate rather than something the resolver quietly skips. What it does not catch is in the docblock: a line that moved onto different but non-blank code. That is the common case and the dangerous one, and nothing mechanical can tell the difference. The rot #793 names — ADR-171 citing ResumeCoordinator.php:204 for a check that d3a8d718 pushed to :205 — is already repaired on main. This test does not fix a live defect; it stops the next one from being invisible. Refs #793 Signed-off-by: Sebastian Mendel --- Tests/Unit/AdrCodeCitationTest.php | 272 +++++++++++++++++++++++++++++ 1 file changed, 272 insertions(+) create mode 100644 Tests/Unit/AdrCodeCitationTest.php diff --git a/Tests/Unit/AdrCodeCitationTest.php b/Tests/Unit/AdrCodeCitationTest.php new file mode 100644 index 000000000..fed102428 --- /dev/null +++ b/Tests/Unit/AdrCodeCitationTest.php @@ -0,0 +1,272 @@ + list of cited paths. + * + * Every entry is TYPO3 core or a sibling extension, read from + * .Build/vendor while the record was written. Nothing here is verifiable + * by this suite; the list exists so a NEW unverifiable citation fails + * rather than passing as one of these. + * + * @var array> + */ + private const CITATIONS_OUTSIDE_THE_REPOSITORY = [ + 'Adr140EffectivePolicyReadoutWithoutApplyPath.rst' => [ + 'ExtensionConfiguration.php', + ], + 'Adr169RecordManagementUsesTypo3Permissions.rst' => [ + 'BackendUtility.php', + 'Clipboard.php', + 'DataHandler.php', + 'DatabaseUserPermissionCheck.php', + 'ElementHistoryController.php', + 'ElementInformationController.php', + 'RecordHistory.php', + 'RecordListController.php', + 'RootLevelCapability.php', + 'SuggestWizardController.php', + 'TcaItemsProcessorFunctions.php', + 'Typo3Version.php', + 'VaultFieldHelper.php', + 'cms-install/Configuration/Backend/Modules.php', + 'nr-vault/Configuration/Backend/Modules.php', + ], + 'Adr171PersonasTheCodeAlreadyAssumes.rst' => [ + 'cms-install/Configuration/Backend/Modules.php', + ], + ]; + + private function repositoryRoot(): string + { + return dirname(__DIR__, 2); + } + + /** + * Every `File.php:NNN` or `File.php:NNN-MMM` in the corpus. + * + * @return list + */ + private function citations(): array + { + $files = glob($this->repositoryRoot() . '/Documentation/Adr/Adr*.rst'); + self::assertIsArray($files); + self::assertNotSame([], $files); + + $citations = []; + foreach ($files as $file) { + $lines = explode("\n", (string)file_get_contents($file)); + foreach ($lines as $index => $line) { + preg_match_all('#([A-Za-z0-9_/.-]+\.php):(\d+)(?:-(\d+))?#', $line, $matches, PREG_SET_ORDER); + foreach ($matches as $match) { + $citations[] = [ + 'adr' => basename($file), + 'adrLine' => $index + 1, + 'path' => $match[1], + 'from' => (int)$match[2], + 'to' => (int)($match[3] ?? $match[2]), + ]; + } + } + } + + return $citations; + } + + /** + * Basename => every path under the repository root carrying it. + * + * .Build holds the composer install, landingpage a generator with its own + * tree; neither is what an ADR means when it names a file. + * + * @return array> + */ + private function basenameIndex(): array + { + $root = $this->repositoryRoot(); + $skip = ['.Build', '.git', 'landingpage', 'node_modules', 'var', '.ddev']; + $index = []; + $iterator = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($root, RecursiveDirectoryIterator::SKIP_DOTS), + RecursiveIteratorIterator::SELF_FIRST, + ); + + /** @var SplFileInfo $entry */ + foreach ($iterator as $entry) { + $relative = ltrim(str_replace($root, '', $entry->getPathname()), '/'); + $segment = explode('/', $relative)[0]; + if (in_array($segment, $skip, true)) { + continue; + } + + if (!$entry->isFile() || $entry->getExtension() !== 'php') { + continue; + } + + $index[$entry->getBasename()][] = $relative; + } + + return $index; + } + + /** + * The repository file a citation names, or null when it names none. + * + * @param array> $index + */ + private function resolve(string $path, array $index, string $where): ?string + { + if (str_contains($path, '/')) { + return is_file($this->repositoryRoot() . '/' . $path) ? $path : null; + } + + $hits = $index[$path] ?? []; + self::assertLessThan( + 2, + count($hits), + sprintf( + '%s cites "%s" by basename alone and the repository now holds %d files with that name (%s). ' + . 'Cite it with its path so the reference names one file.', + $where, + $path, + count($hits), + implode(', ', $hits), + ), + ); + + return $hits[0] ?? null; + } + + #[Test] + public function everyCitationNamesAFileThatStillExists(): void + { + $index = $this->basenameIndex(); + $declared = self::CITATIONS_OUTSIDE_THE_REPOSITORY; + $orphans = []; + + foreach ($this->citations() as $citation) { + $where = sprintf('%s:%d', $citation['adr'], $citation['adrLine']); + if ($this->resolve($citation['path'], $index, $where) !== null) { + continue; + } + + if (in_array($citation['path'], $declared[$citation['adr']] ?? [], true)) { + continue; + } + + $orphans[] = sprintf('%s cites %s, which is in neither the tree nor the declared list', $where, $citation['path']); + } + + self::assertSame([], $orphans, implode("\n", $orphans)); + } + + #[Test] + public function noCitationPointsPastTheEndOfItsFileOrAtABlankLine(): void + { + $index = $this->basenameIndex(); + $stale = []; + + foreach ($this->citations() as $citation) { + $where = sprintf('%s:%d', $citation['adr'], $citation['adrLine']); + $resolved = $this->resolve($citation['path'], $index, $where); + if ($resolved === null) { + continue; + } + + $lines = explode("\n", (string)file_get_contents($this->repositoryRoot() . '/' . $resolved)); + foreach ([$citation['from'], $citation['to']] as $number) { + if ($number > count($lines)) { + $stale[] = sprintf('%s cites %s:%d; the file ends at line %d', $where, $resolved, $number, count($lines)); + continue; + } + + if (trim($lines[$number - 1]) === '') { + $stale[] = sprintf('%s cites %s:%d, which is blank', $where, $resolved, $number); + } + } + } + + self::assertSame([], $stale, implode("\n", $stale)); + } + + #[Test] + public function theListOfUncheckableCitationsMatchesTheCorpus(): void + { + $index = $this->basenameIndex(); + $actual = []; + + foreach ($this->citations() as $citation) { + $where = sprintf('%s:%d', $citation['adr'], $citation['adrLine']); + if ($this->resolve($citation['path'], $index, $where) !== null) { + continue; + } + + $actual[$citation['adr']][$citation['path']] = true; + } + + $normalised = []; + foreach ($actual as $adr => $paths) { + $names = array_keys($paths); + sort($names); + $normalised[$adr] = $names; + } + + ksort($normalised); + + $declared = self::CITATIONS_OUTSIDE_THE_REPOSITORY; + ksort($declared); + + self::assertSame( + $declared, + $normalised, + 'The citations that resolve to no file in this repository have changed. Every one of them is ' + . 'unverifiable by this suite, so the list is maintained by hand: add the new entry deliberately, ' + . 'or drop one that no longer appears.', + ); + } +}