Skip to content

Commit

Permalink
feat(docs): add validation of xrefs to docfx command (#6658)
Browse files Browse the repository at this point in the history
  • Loading branch information
bshaffer authored Jun 6, 2024
1 parent 7b748a6 commit 9b419c4
Show file tree
Hide file tree
Showing 6 changed files with 255 additions and 10 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/docs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .kokoro/docs/docker/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
67 changes: 60 additions & 7 deletions dev/src/Command/DocFxCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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.'
)
;
}

Expand All @@ -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 <options=bold;fg=white>%s</>', $componentName));
$xml = $input->getOption('xml');
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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());
Expand All @@ -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);
Expand Down Expand Up @@ -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 = '<options=bold>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("\n<error>Invalid xref in %s: %s</>", $node->getFullname(), $invalidRef));
$valid = false;
}
foreach ($this->getBrokenXrefs($node->getContent()) as $brokenRef) {
$output->writeln(
sprintf('<comment>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;
}
}
8 changes: 8 additions & 0 deletions dev/src/DocFx/Node/ClassNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
114 changes: 114 additions & 0 deletions dev/src/DocFx/XrefValidationTrait.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
<?php
/**
* Copyright 2024 Google Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

namespace Google\Cloud\Dev\DocFx;

use Google\Cloud\Core\Logger\AppEngineFlexFormatter;
use Google\Cloud\Core\Logger\AppEngineFlexFormatterV2;

/**
* @internal
*/
trait XrefValidationTrait
{
/**
* Verifies that all class references and @see tags are properly formatted
* with either an FQSEN (Fully Qualified Structural Element Name), or a URL.
*/
private function getInvalidXrefs(string $description): array
{
$invalidRefs = [];
preg_replace_callback(
'/<xref uid="([^ ]*)"/',
function ($matches) use (&$invalidRefs) {
// Valid external reference
if (0 === strpos($matches[1], 'http')) {
return;
}
// Invalid reference format (not an FQSEN)
if ('\\' !== $matches[1][0]) {
$invalidRefs[] = $matches[1];
return;
}
// Invalid FQSEN (If it contains "\Google\" more than once, it wasn't properly imported)
if (substr_count($matches[1], '\Google\\') > 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(
'/<xref uid="([^ ]*)"/',
function ($matches) use (&$brokenRefs) {
// Valid external reference
if (0 === strpos($matches[1], 'http')) {
return;
}
// We cannot run "class_exists" on these classes because they will throw a fatal error.
if (in_array(
$matches[1],
['\\' . AppEngineFlexFormatter::class, '\\' . AppEngineFlexFormatterV2::class]
)) {
return;
}
// Valid class reference
if (class_exists($matches[1]) || interface_exists($matches[1] || trait_exists($matches[1]))) {
return;
}
// Valid method, magic method, andd constant references
if (false !== strpos($matches[1], '::')) {
if (false !== strpos($matches[1], '()')) {
list($class, $method) = explode('::', str_replace('()', '', $matches[1]));
// Valid method reference
if (method_exists($class, $method)) {
return;
}
// Assume it's a magic Async method
if ('Async' === substr($method, -5)) {
return;
}
} elseif (defined($matches[1])) {
// Valid constant reference
return;
}
}
// Invalid reference!
if ($matches[1] === '\\\\') {
// empty hrefs show up as "\\"
$brokenRefs[] = null;
} else {
$brokenRefs[] = $matches[1];
}
},
$description
);

return $brokenRefs;
}
}
72 changes: 71 additions & 1 deletion dev/tests/Unit/DocFx/NodeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,24 @@

namespace Google\Cloud\Dev\Tests\Unit\DocFx;

use PHPUnit\Framework\TestCase;
use Google\Cloud\Dev\DocFx\Node\ClassNode;
use Google\Cloud\Dev\DocFx\Node\MethodNode;
use Google\Cloud\Dev\DocFx\Node\XrefTrait;
use Google\Cloud\Dev\DocFx\Node\FencedCodeBlockTrait;
use Google\Cloud\Dev\DocFx\XrefValidationTrait;
use PHPUnit\Framework\TestCase;
use Prophecy\Argument;
use Prophecy\PhpUnit\ProphecyTrait;
use Symfony\Component\Console\Output\OutputInterface;
use SimpleXMLElement;

/**
* @group dev
*/
class NodeTest extends TestCase
{
use ProphecyTrait;

public function testNestedParameters()
{
$nestedParamsXml = file_get_contents(__DIR__ . '/../../fixtures/phpdoc/nestedparams.xml');
Expand Down Expand Up @@ -433,4 +439,68 @@ public function provideStatusAndVersionByNamespace()
['Foo', ''],
];
}

/**
* @dataProvider provideInvalidXrefs
*/
public function testInvalidXrefs(string $description, array $invalidXrefs = [])
{
$classXml = '<class><full_name>TestClass</full_name><docblock><description>%s</description></docblock></class>';
$class = new ClassNode(new SimpleXMLElement(sprintf($classXml, $description)));

$validator = new class () {
use XrefValidationTrait {
getInvalidXrefs as public;
}
};

$this->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 = '<class><full_name>TestClass</full_name><docblock><description>%s</description></docblock></class>';
$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
];
}
}

0 comments on commit 9b419c4

Please sign in to comment.