diff --git a/.github/workflows/docs.yaml b/.github/workflows/docs.yaml index 1fae6ba569b..4bff62481b2 100644 --- a/.github/workflows/docs.yaml +++ b/.github/workflows/docs.yaml @@ -26,7 +26,7 @@ jobs: id: extract uses: shrink/actions-docker-extract@v2 with: - image: "phpdoc/phpdoc:20230522201300af6fb5" + image: "phpdoc/phpdoc:3.4.1" path: "/opt/phpdoc/." - name: Symlink phpDocumentor run: ln -s $(pwd)/${{ steps.extract.outputs.destination }}/bin/phpdoc /usr/local/bin/phpdoc diff --git a/.kokoro/docs/docker/Dockerfile b/.kokoro/docs/docker/Dockerfile index 71229ad4442..d8c2f0f9ee8 100644 --- a/.kokoro/docs/docker/Dockerfile +++ b/.kokoro/docs/docker/Dockerfile @@ -94,7 +94,7 @@ RUN php -r "copy('https://getcomposer.org/installer', 'composer-setup.php');" && # Use phpDocumentor from HEAD until they create a new release # TODO: Remove once phpDocumentor tags a new release # @see https://github.com/phpDocumentor/phpDocumentor/issues/3434 -COPY --from=phpdoc/phpdoc:20230522201300af6fb5 /opt/phpdoc /opt/phpdoc +COPY --from=phpdoc/phpdoc:3.4.1 /opt/phpdoc /opt/phpdoc RUN ln -s /opt/phpdoc/bin/phpdoc /usr/local/bin/phpdoc ENV PHPDOC_ENV=prod diff --git a/dev/src/Command/DocFxCommand.php b/dev/src/Command/DocFxCommand.php index 42875db691d..fb9149ba6f8 100644 --- a/dev/src/Command/DocFxCommand.php +++ b/dev/src/Command/DocFxCommand.php @@ -25,18 +25,30 @@ use Symfony\Component\Process\Process; use Symfony\Component\Yaml\Yaml; use RuntimeException; +use Google\Cloud\Dev\Component; +use Google\Cloud\Dev\DocFx\Node\ClassNode; use Google\Cloud\Dev\DocFx\Page\PageTree; use Google\Cloud\Dev\DocFx\Page\OverviewPage; -use Google\Cloud\Dev\Component; +use Google\Cloud\Dev\DocFx\XrefValidationTrait; /** * @internal */ class DocFxCommand extends Command { + use XrefValidationTrait; + private array $composerJson; private array $repoMetadataJson; + // these links are inexplicably broken in phpdoc generation, and will require more investigation + private static array $allowedReferenceFailures = [ + '\Google\Cloud\ResourceManager\V3\Client\ProjectsClient::testIamPermissions()' + => 'ProjectsClient::testIamPermissionsAsync()', + '\Google\Cloud\Logging\V2\Client\ConfigServiceV2Client::getView()' + => 'ConfigServiceV2Client::getViewAsync()' + ]; + protected function configure() { $this->setName('docfx') @@ -50,8 +62,8 @@ protected function configure() 'component-path', '', InputOption::VALUE_OPTIONAL, - 'Specify the path of the desired component. Please note, this option is only intended for testing purposes. - ') + 'Specify the path of the desired component. Please note, this option is only intended for testing purposes.' + ) ; } @@ -61,7 +73,7 @@ protected function execute(InputInterface $input, OutputInterface $output) throw new RuntimeException('This command must be run on PHP 8.0 or above'); } - $componentName = $input->getOption('component') ?: basename(getcwd()); + $componentName = rtrim($input->getOption('component'), '/') ?: basename(getcwd()); $component = new Component($componentName, $input->getOption('component-path')); $output->writeln(sprintf('Generating documentation for %s', $componentName)); $xml = $input->getOption('xml'); @@ -92,8 +104,10 @@ protected function execute(InputInterface $input, OutputInterface $output) $indent = 2; // The amount of spaces to use for indentation of nested nodes $flags = Yaml::DUMP_MULTI_LINE_LITERAL_BLOCK; + $valid = true; $tocItems = []; $packageDescription = $component->getDescription(); + $isBeta = 'stable' !== $component->getReleaseLevel(); foreach ($component->getNamespaces() as $namespace => $dir) { $pageTree = new PageTree( $xml, @@ -103,6 +117,8 @@ protected function execute(InputInterface $input, OutputInterface $output) ); foreach ($pageTree->getPages() as $page) { + // validate the docs page. this will fail the job if it's false + $valid = $this->validate($page->getClassNode(), $output) && $valid; $docFxArray = ['items' => $page->getItems()]; // Dump the YAML for the class node @@ -117,11 +133,15 @@ protected function execute(InputInterface $input, OutputInterface $output) $tocItems = array_merge($tocItems, $pageTree->getTocItems()); } - $releaseLevel = $component->getReleaseLevel(); + // exit early if the docs aren't valid + if (!$valid) { + return 1; + } + if (file_exists($overviewFile = sprintf('%s/README.md', $component->getPath()))) { $overview = new OverviewPage( file_get_contents($overviewFile), - $releaseLevel !== 'stable' + $isBeta ); $outFile = sprintf('%s/%s', $outDir, $overview->getFilename()); file_put_contents($outFile, $overview->getContents()); @@ -133,7 +153,7 @@ protected function execute(InputInterface $input, OutputInterface $output) $componentToc = array_filter([ 'uid' => $component->getReferenceDocumentationUid(), 'name' => $component->getPackageName(), - 'status' => $releaseLevel !== 'stable' ? 'beta' : '', + 'status' => $isBeta ? 'beta' : '', 'items' => $tocItems, ]); $tocYaml = Yaml::dump([$componentToc], $inline, $indent, $flags); @@ -199,4 +219,37 @@ public static function getPhpDocCommand(string $componentPath, string $outDir): return $process; } + + private function validate(ClassNode $class, OutputInterface $output): bool + { + $valid = true; + $emptyRef = 'empty'; + $isGenerated = $class->isProtobufMessageClass() || $class->isProtobufEnumClass() || $class->isServiceClass(); + foreach (array_merge([$class], $class->getMethods(), $class->getConstants()) as $node) { + foreach ($this->getInvalidXrefs($node->getContent()) as $invalidRef) { + if (isset(self::$allowedReferenceFailures[$node->getFullname()]) + && self::$allowedReferenceFailures[$node->getFullname()] == $invalidRef) { + // these links are inexplicably broken in phpdoc generation, and will require more investigation + continue; + } + $output->write(sprintf("\nInvalid xref in %s: %s", $node->getFullname(), $invalidRef)); + $valid = false; + } + foreach ($this->getBrokenXrefs($node->getContent()) as $brokenRef) { + $output->writeln( + sprintf('Broken xref in %s: %s', $node->getFullname(), $brokenRef ?: $emptyRef), + $isGenerated ? OutputInterface::VERBOSITY_VERBOSE : OutputInterface::VERBOSITY_NORMAL + ); + // generated classes are allowed to have broken xrefs + if ($isGenerated) { + continue; + } + $valid = false; + } + } + if (!$valid) { + $output->writeln(''); + } + return $valid; + } } diff --git a/dev/src/DocFx/Node/ClassNode.php b/dev/src/DocFx/Node/ClassNode.php index 0a50ae6c826..9ec6b0ad16e 100644 --- a/dev/src/DocFx/Node/ClassNode.php +++ b/dev/src/DocFx/Node/ClassNode.php @@ -60,6 +60,14 @@ public function isProtobufEnumClass(): bool return 0 === strpos($lastDescriptionLine, 'Protobuf type'); } + public function isProtobufMessageClass(): bool + { + if ($extends = $this->getExtends()) { + return '\Google\Protobuf\Internal\Message' === $extends; + } + return false; + } + public function isGapicEnumClass(): bool { // returns true if the class extends \Google\Protobuf\Internal\Message diff --git a/dev/src/DocFx/XrefValidationTrait.php b/dev/src/DocFx/XrefValidationTrait.php new file mode 100644 index 00000000000..caf5790fb43 --- /dev/null +++ b/dev/src/DocFx/XrefValidationTrait.php @@ -0,0 +1,114 @@ + 1) { + $invalidRefs[] = $matches[1]; + return; + } + }, + $description + ); + + return $invalidRefs; + } + + /** + * Verifies that all class references and @see tags contain references to classes, methods, and + * constants which actually exist. + */ + private function getBrokenXrefs(string $description): array + { + $brokenRefs = []; + preg_replace_callback( + '/assertEquals($invalidXrefs, $validator->getInvalidXrefs($class->getContent())); + } + + public function provideInvalidXrefs() + { + return [ + ['{@see \DoesntExist}'], // class doesn't exist, but is still a valid xref + ['{@see \SimpleXMLElement::method()}'], // method doesn't exist, but is still a valid xref + ['{@see \SimpleXMLElement::addAttribute()}'], // valid method + ['{@see \SimpleXMLElement::OUTPUT_FOO}'], // constant doesn't exist, but is still a valid xref + [sprintf('{@see \%s::OUTPUT_NORMAL}', OutputInterface::class)], // valid constant + ['Take a look at {@see https://foo.bar} for more.'], // valid + ['{@see Foo\Bar}', ['Foo\Bar']], // invalid + ['Take a look at {@see Foo\Bar} for more.', ['Foo\Bar']], // invalid + [ + '{@see \Google\Cloud\PubSub\Google\Cloud\PubSub\Foo}', + ['\Google\Cloud\PubSub\Google\Cloud\PubSub\Foo'] + ], // invalid + ]; + } + + /** + * @dataProvider provideBrokenXrefs + */ + public function testBrokenXrefs(string $description, array $brokenXrefs = []) + { + $classXml = 'TestClass%s'; + $class = new ClassNode(new SimpleXMLElement(sprintf($classXml, $description))); + + $validator = new class () { + use XrefValidationTrait { + getBrokenXrefs as public; + } + }; + + $this->assertEquals($brokenXrefs, $validator->getBrokenXrefs($class->getContent())); + } + + public function provideBrokenXrefs() + { + return [ + ['{@see \OutputInterface}', ['\OutputInterface']], // invalid class (doesn't exist) + ['{@see \SimpleXMLElement}.'], // valid class + ['{@see \SimpleXMLElement::method()}', ['\SimpleXMLElement::method()']], // invalid method (doesn't exist) + ['{@see \SimpleXMLElement::addAttribute()}'], // valid method + ['{@see \SimpleXMLElement::OUTPUT_FOO}', ['\SimpleXMLElement::OUTPUT_FOO']], // invalid constant (doesn't exist) + [sprintf('{@see \%s::OUTPUT_NORMAL}', OutputInterface::class)], // valid constant + ]; + } }