diff --git a/dev/src/Command/DocFxCommand.php b/dev/src/Command/DocFxCommand.php index c6542b202240..93f209399b2a 100644 --- a/dev/src/Command/DocFxCommand.php +++ b/dev/src/Command/DocFxCommand.php @@ -25,23 +25,22 @@ use Symfony\Component\Process\Process; use Symfony\Component\Yaml\Yaml; use RuntimeException; -use Google\Cloud\Core\Logger\AppEngineFlexFormatter; -use Google\Cloud\Core\Logger\AppEngineFlexFormatterV2; +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; - private const XREF_REGEX = '/setName('docfx') @@ -226,7 +225,11 @@ private function validate(ClassNode $class, OutputInterface $output): bool $hadWarning = true; } foreach ($this->getBrokenXrefs($node->getContent()) as $brokenRef) { - $output->write(sprintf("\nBroken xref in %s: %s", $node->getFullname(), $brokenRef)); + $output->write(sprintf( + "\nBroken xref in %s: %s", + $node->getFullname(), + $brokenRef ?: 'empty') + ); $valid = $valid && (false || $isGenerated); $hadWarning = true; } @@ -236,75 +239,4 @@ private function validate(ClassNode $class, OutputInterface $output): bool } return $valid; } - - /** - * Verifies that protobuf references and @see tags are properly formatted. - */ - private function getInvalidXrefs(string $description): array - { - $invalidRefs = []; - preg_replace_callback( - self::XREF_REGEX, - function ($matches) use (&$invalidRefs) { - // Valid external reference - if (0 === strpos($matches[1], 'http')) { - return; - } - // Invalid reference format - if ('\\' !== $matches[1][0] || substr_count($matches[1], '\Google\\') > 1) { - $invalidRefs[] = $matches[1]; - } - }, - $description - ); - - return $invalidRefs; - } - - /** - * Verifies that protobuf references and @see tags reference classes that exist. - */ - private function getBrokenXrefs(string $description): array - { - $brokenRefs = []; - preg_replace_callback( - self::XREF_REGEX, - function ($matches) use (&$brokenRefs) { - if (0 === strpos($matches[1], 'http')) { - return; // Valid external reference - } - if (in_array( - $matches[1], - ['\\' . AppEngineFlexFormatter::class, '\\' . AppEngineFlexFormatterV2::class]) - ) { - return; // We cannot run "class_exists" on these because they will throw a fatal error. - } - if (class_exists($matches[1]) || interface_exists($matches[1] || trait_exists($matches[1]))) { - return; // Valid class reference - } - if (false !== strpos($matches[1], '::')) { - if (false !== strpos($matches[1], '()')) { - list($class, $method) = explode('::', str_replace('()', '', $matches[1])); - if (method_exists($class, $method)) { - return; // Valid method reference - } - if ('Async' === substr($method, -5)) { - return; // Skip magic Async methods - } - } elseif (defined($matches[1])) { - return; // Valid constant reference - } - } - // empty hrefs show up as "\\" - if ($matches[1] === '\\\\') { - $brokenRefs[] = 'empty'; - } else { - $brokenRefs[] = $matches[1]; - } - }, - $description - ); - - return $brokenRefs; - } } diff --git a/dev/src/DocFx/Node/ClassNode.php b/dev/src/DocFx/Node/ClassNode.php index d2e787358e35..731011b5b56e 100644 --- a/dev/src/DocFx/Node/ClassNode.php +++ b/dev/src/DocFx/Node/ClassNode.php @@ -171,16 +171,16 @@ public function getMethods(): array foreach ($this->xmlNode->method as $methodNode) { $method = new MethodNode($methodNode, $this->protoPackages); if ($method->isPublic() && !$method->isInherited() && !$method->isExcludedMethod()) { - // // This is to fix an issue in phpdocumentor where magic methods do not have - // // "inhereted_from" set as expected. - // // TODO: Remove this once the above issue is fixed. - // // @see https://github.com/phpDocumentor/phpDocumentor/pull/3520 - // if (false !== strpos($method->getFullname(), 'Async()')) { - // list($class, $_) = explode('::', $method->getFullname()); - // if ($class !== $this->getFullName()) { - // continue; - // } - // } + // This is to fix an issue in phpdocumentor where magic methods do not have + // "inhereted_from" set as expected. + // TODO: Remove this once the above issue is fixed. + // @see https://github.com/phpDocumentor/phpDocumentor/pull/3520 + if (false !== strpos($method->getFullname(), 'Async()')) { + list($class, $_) = explode('::', $method->getFullname()); + if ($class !== $this->getFullName()) { + continue; + } + } $methods[] = $method; } } diff --git a/dev/src/DocFx/XrefValidationTrait.php b/dev/src/DocFx/XrefValidationTrait.php new file mode 100644 index 000000000000..6f8e349944e2 --- /dev/null +++ b/dev/src/DocFx/XrefValidationTrait.php @@ -0,0 +1,98 @@ + 1) { + $invalidRefs[] = $matches[1]; + } + }, + $description + ); + + return $invalidRefs; + } + + /** + * Verifies that protobuf references and @see tags reference classes that exist. + */ + private function getBrokenXrefs(string $description): array + { + $brokenRefs = []; + preg_replace_callback( + '/prophesize(OutputInterface::class); - if (is_null($errorMessage)) { - $output->writeln(Argument::any())->shouldNotBeCalled(); - $shouldBeValid = true; - } else { - $output->writeln($errorMessage)->shouldBeCalledOnce(); - $shouldBeValid = false; - } - $classXml = 'TestClass%s'; - $xref = new ClassNode(new SimpleXMLElement(sprintf($classXml, $description))); + $class = new ClassNode(new SimpleXMLElement(sprintf($classXml, $description))); + + $validator = new class () { + use XrefValidationTrait { + getInvalidXrefs as public; + } + }; - $this->assertEquals($shouldBeValid, $xref->validate($output->reveal())); + $this->assertEquals($invalidXrefs, $validator->getInvalidXrefs($class->getContent())); } - public function provideValidateXrefs() + public function provideInvalidXrefs() { return [ - ['{@see \OutputInterface}', 'Invalid xref in TestClass: \OutputInterface'], // invalid class (doesn't exist) - ['{@see \SimpleXMLElement}.'], // valid class - ['{@see \SimpleXMLElement::method()}', 'Invalid xref in TestClass: \SimpleXMLElement::method()'], // invalid method (doesn't exist) + ['{@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}', 'Invalid xref in TestClass: \SimpleXMLElement::OUTPUT_FOO'], // invalid constant (doesn't exist) + ['{@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}', 'Xref not rendered propery in TestClass: Foo\Bar'], // invalid - ['Take a look at {@see Foo\Bar} for more.', 'Xref not rendered propery in TestClass: Foo\Bar'], // invalid + ['{@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}', - 'Xref not rendered propery in TestClass: \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 + ]; + } }