From 409692cf78ffc5dcca9f239e4b72d61feff1302f Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Mon, 8 Dec 2025 14:36:06 +0100 Subject: [PATCH 1/3] [BUGFIX] Validate guides.xml against XSD schema before loading This adds proper XSD schema validation before the configuration is loaded into Symfony's container. When validation fails, users now see precise error messages with line and column numbers instead of cryptic PHP fatal errors. Example output for invalid guides.xml: Invalid guides.xml configuration Your guides.xml file failed XSD schema validation: Schema validation failed for Documentation/guides.xml Line 3, Column 0: Element 'theme': This element is not expected. The fix: - Validates all guides.xml files against the XSD schema upfront - Shows exact file path, line number, and column for each error - Links to official documentation for correct format - Keeps a fallback catch for any edge cases not covered by XSD This provides much better developer experience compared to the previous "Expected scalar, but got array" fatal error. --- bin/guides | 135 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 133 insertions(+), 2 deletions(-) mode change 100644 => 100755 bin/guides diff --git a/bin/guides b/bin/guides old mode 100644 new mode 100755 index ae0906f82..df06dec63 --- a/bin/guides +++ b/bin/guides @@ -1,7 +1,138 @@ #!/usr/bin/env php true, 'errors' => []]; + } + + if (!file_exists($xsdPath)) { + // XSD not found, skip validation + return ['valid' => true, 'errors' => []]; + } + + libxml_use_internal_errors(true); + + $dom = new DOMDocument(); + if (!$dom->load($xmlPath)) { + $errors[] = sprintf('Failed to load %s as XML', $xmlPath); + foreach (libxml_get_errors() as $error) { + $errors[] = sprintf(' Line %d, Column %d: %s', $error->line, $error->column, trim($error->message)); + } + libxml_clear_errors(); + return ['valid' => false, 'errors' => $errors]; + } + + if (!$dom->schemaValidate($xsdPath)) { + $errors[] = sprintf('Schema validation failed for %s', $xmlPath); + foreach (libxml_get_errors() as $error) { + $errors[] = sprintf(' Line %d, Column %d: %s', $error->line, $error->column, trim($error->message)); + } + libxml_clear_errors(); + return ['valid' => false, 'errors' => $errors]; + } + + libxml_clear_errors(); + return ['valid' => true, 'errors' => []]; +} + +/** + * Outputs an error message to STDERR with formatting. + */ +function outputError(string $message, bool $isHeader = false): void +{ + if ($isHeader) { + fwrite(STDERR, "\033[37;41m " . $message . " \033[0m\n"); + } else { + fwrite(STDERR, $message . "\n"); + } +} + +// Determine which guides.xml files will be loaded (mirrors upstream logic) +$input = new ArgvInput(); +$vendorDir = $root . '/vendor'; +$xsdPath = $vendorDir . '/phpdocumentor/guides-cli/resources/schema/guides.xsd'; + +$configFiles = []; + +// Project-level config +$projectConfig = $vendorDir . '/../guides.xml'; +if (is_file($projectConfig)) { + $realPath = realpath($projectConfig); + if ($realPath !== false) { + $configFiles[] = $realPath; + } +} + +// Local config (from --config or working directory) +$workingDir = $input->getParameterOption(['--working-dir', '-w'], getcwd(), true); +$localConfigDir = $input->getParameterOption(['--config', '-c'], $workingDir, true); +$localConfig = $localConfigDir . '/guides.xml'; + +if (is_file($localConfig)) { + $realLocalConfig = realpath($localConfig); + $realProjectConfig = isset($realPath) ? $realPath : null; + if ($realLocalConfig !== false && $realLocalConfig !== $realProjectConfig) { + $configFiles[] = $realLocalConfig; + } +} + +// Validate all config files against XSD +$hasValidationErrors = false; +$allErrors = []; + +foreach ($configFiles as $configFile) { + $result = validateGuidesXml($configFile, $xsdPath); + if (!$result['valid']) { + $hasValidationErrors = true; + $allErrors = array_merge($allErrors, $result['errors']); + } +} + +if ($hasValidationErrors) { + outputError('Invalid guides.xml configuration', true); + fwrite(STDERR, "\n"); + fwrite(STDERR, "Your guides.xml file failed XSD schema validation:\n"); + fwrite(STDERR, "\n"); + foreach ($allErrors as $error) { + fwrite(STDERR, "\033[33m" . $error . "\033[0m\n"); + } + fwrite(STDERR, "\n"); + fwrite(STDERR, "See: https://docs.typo3.org/m/typo3/docs-how-to-document/main/en-us/GeneralConventions/GuideXml.html\n"); + fwrite(STDERR, "\n"); + exit(1); +} + +// Validation passed, proceed with normal execution +// Keep a fallback catch for any edge cases not covered by XSD +try { + require_once $root . '/vendor/phpdocumentor/guides-cli/bin/guides'; +} catch (InvalidTypeException|InvalidConfigurationException $e) { + outputError('Invalid guides.xml configuration', true); + fwrite(STDERR, "\n"); + fwrite(STDERR, "Your guides.xml file contains a configuration error:\n"); + fwrite(STDERR, " \033[33m" . $e->getMessage() . "\033[0m\n"); + fwrite(STDERR, "\n"); + fwrite(STDERR, "Run \033[33mvendor/bin/typo3-guides lint-guides-xml\033[0m for detailed validation.\n"); + fwrite(STDERR, "See: https://docs.typo3.org/m/typo3/docs-how-to-document/main/en-us/GeneralConventions/GuideXml.html\n"); + fwrite(STDERR, "\n"); + exit(1); +} From d5a8b05fda1515d5ef0ca4b5d9a40ee22e1ba11d Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Mon, 8 Dec 2025 15:01:26 +0100 Subject: [PATCH 2/3] [TASK] Add tests for invalid guides.xml error handling Adds integration tests that verify: - Invalid guides.xml shows helpful error message instead of PHP Fatal error - XSD validation error includes line number - Valid guides.xml files still render successfully --- tests/Integration/InvalidGuidesXmlTest.php | 97 +++++++++++++++++++ .../tests/invalid-guides-xml/input/Index.rst | 5 + .../tests/invalid-guides-xml/input/guides.xml | 5 + 3 files changed, 107 insertions(+) create mode 100644 tests/Integration/InvalidGuidesXmlTest.php create mode 100644 tests/Integration/tests/invalid-guides-xml/input/Index.rst create mode 100644 tests/Integration/tests/invalid-guides-xml/input/guides.xml diff --git a/tests/Integration/InvalidGuidesXmlTest.php b/tests/Integration/InvalidGuidesXmlTest.php new file mode 100644 index 000000000..8993f1385 --- /dev/null +++ b/tests/Integration/InvalidGuidesXmlTest.php @@ -0,0 +1,97 @@ +run(); + + // Should fail with exit code 1, not crash with fatal error + self::assertSame(1, $process->getExitCode(), 'Expected exit code 1 for invalid guides.xml'); + + $stderr = $process->getErrorOutput(); + + // Should contain helpful error message, not PHP fatal error + self::assertStringContainsString('Invalid guides.xml configuration', $stderr); + self::assertStringNotContainsString('PHP Fatal error', $stderr); + self::assertStringNotContainsString('Stack trace', $stderr); + + // Should reference documentation + self::assertStringContainsString('https://docs.typo3.org', $stderr); + } + + public function testInvalidGuidesXmlShowsLineNumber(): void + { + $binPath = dirname(__DIR__, 2) . '/bin/guides'; + + $process = new Process([ + 'php', + $binPath, + 'run', + '--config=' . self::FIXTURES_PATH, + self::FIXTURES_PATH, + ]); + + $process->run(); + + $stderr = $process->getErrorOutput(); + + // Should show XSD validation error with line number + self::assertStringContainsString('Line 3', $stderr); + self::assertStringContainsString('theme', $stderr); + } + + public function testValidGuidesXmlRendersSuccessfully(): void + { + $binPath = dirname(__DIR__, 2) . '/bin/guides'; + $validFixturePath = __DIR__ . '/tests/getting-started/input'; + + // Skip if fixture doesn't exist + if (!is_dir($validFixturePath)) { + self::markTestSkipped('Valid fixture not available'); + } + + $outputPath = sys_get_temp_dir() . '/render-guides-test-' . uniqid(); + + $process = new Process([ + 'php', + $binPath, + 'run', + '--config=' . $validFixturePath, + '--output=' . $outputPath, + $validFixturePath, + ]); + + $process->run(); + + // Clean up + if (is_dir($outputPath)) { + system('rm -rf ' . escapeshellarg($outputPath)); + } + + // Should succeed + self::assertSame(0, $process->getExitCode(), 'Expected exit code 0 for valid guides.xml. Error: ' . $process->getErrorOutput()); + } +} diff --git a/tests/Integration/tests/invalid-guides-xml/input/Index.rst b/tests/Integration/tests/invalid-guides-xml/input/Index.rst new file mode 100644 index 000000000..50a726c9d --- /dev/null +++ b/tests/Integration/tests/invalid-guides-xml/input/Index.rst @@ -0,0 +1,5 @@ +===== +Test +===== + +This should not render due to invalid guides.xml. diff --git a/tests/Integration/tests/invalid-guides-xml/input/guides.xml b/tests/Integration/tests/invalid-guides-xml/input/guides.xml new file mode 100644 index 000000000..88cc4c732 --- /dev/null +++ b/tests/Integration/tests/invalid-guides-xml/input/guides.xml @@ -0,0 +1,5 @@ + + + + + From 501b73362cb0bae5d532588d0d4c1af914dc1da3 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Mon, 8 Dec 2025 15:15:04 +0100 Subject: [PATCH 3/3] Fix CI: move test fixture to avoid lint-guides-xml and data provider conflicts --- tests/Integration/InvalidGuidesXmlTest.php | 32 ++++++++++++++++--- .../invalid-guides-xml}/Index.rst | 0 .../invalid-guides-xml/guides.xml.fixture} | 0 3 files changed, 27 insertions(+), 5 deletions(-) rename tests/{Integration/tests/invalid-guides-xml/input => fixtures/invalid-guides-xml}/Index.rst (100%) rename tests/{Integration/tests/invalid-guides-xml/input/guides.xml => fixtures/invalid-guides-xml/guides.xml.fixture} (100%) diff --git a/tests/Integration/InvalidGuidesXmlTest.php b/tests/Integration/InvalidGuidesXmlTest.php index 8993f1385..ae970b3bd 100644 --- a/tests/Integration/InvalidGuidesXmlTest.php +++ b/tests/Integration/InvalidGuidesXmlTest.php @@ -9,10 +9,32 @@ /** * Tests error handling for invalid guides.xml configurations. + * + * The fixture is stored as guides.xml.fixture to avoid being picked up by lint-guides-xml. + * Tests copy it to a temp directory with the correct name before execution. */ final class InvalidGuidesXmlTest extends TestCase { - private const FIXTURES_PATH = __DIR__ . '/tests/invalid-guides-xml/input'; + private const FIXTURE_SOURCE = __DIR__ . '/../fixtures/invalid-guides-xml'; + + private string $tempDir = ''; + + protected function setUp(): void + { + $this->tempDir = sys_get_temp_dir() . '/render-guides-invalid-test-' . uniqid(); + mkdir($this->tempDir, 0755, true); + + // Copy fixture files to temp directory + copy(self::FIXTURE_SOURCE . '/guides.xml.fixture', $this->tempDir . '/guides.xml'); + copy(self::FIXTURE_SOURCE . '/Index.rst', $this->tempDir . '/Index.rst'); + } + + protected function tearDown(): void + { + if (is_dir($this->tempDir)) { + system('rm -rf ' . escapeshellarg($this->tempDir)); + } + } public function testInvalidGuidesXmlShowsHelpfulErrorMessage(): void { @@ -22,8 +44,8 @@ public function testInvalidGuidesXmlShowsHelpfulErrorMessage(): void 'php', $binPath, 'run', - '--config=' . self::FIXTURES_PATH, - self::FIXTURES_PATH, + '--config=' . $this->tempDir, + $this->tempDir, ]); $process->run(); @@ -50,8 +72,8 @@ public function testInvalidGuidesXmlShowsLineNumber(): void 'php', $binPath, 'run', - '--config=' . self::FIXTURES_PATH, - self::FIXTURES_PATH, + '--config=' . $this->tempDir, + $this->tempDir, ]); $process->run(); diff --git a/tests/Integration/tests/invalid-guides-xml/input/Index.rst b/tests/fixtures/invalid-guides-xml/Index.rst similarity index 100% rename from tests/Integration/tests/invalid-guides-xml/input/Index.rst rename to tests/fixtures/invalid-guides-xml/Index.rst diff --git a/tests/Integration/tests/invalid-guides-xml/input/guides.xml b/tests/fixtures/invalid-guides-xml/guides.xml.fixture similarity index 100% rename from tests/Integration/tests/invalid-guides-xml/input/guides.xml rename to tests/fixtures/invalid-guides-xml/guides.xml.fixture