From 8c09d74f3cf974ad26e73e993c9bb8f929454e77 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Mon, 19 Aug 2024 12:19:04 -0700 Subject: [PATCH 1/7] WIP --- .kokoro/docs/publish.sh | 13 +++++-- dev/src/Command/DocFxCommand.php | 49 ++++++++++++++++++++++----- dev/src/DocFx/Node/ClassNode.php | 19 +++++++++++ dev/src/DocFx/Node/ConstantNode.php | 5 +++ dev/src/DocFx/Node/MethodNode.php | 6 ++++ dev/src/DocFx/XrefValidationTrait.php | 2 ++ 6 files changed, 83 insertions(+), 11 deletions(-) diff --git a/.kokoro/docs/publish.sh b/.kokoro/docs/publish.sh index 449c16b592ce..698051844a6f 100755 --- a/.kokoro/docs/publish.sh +++ b/.kokoro/docs/publish.sh @@ -21,6 +21,11 @@ fi if [ "$STAGING_BUCKET" != "" ]; then echo "Using staging bucket ${STAGING_BUCKET}..." fi +VERBOSITY=""; +if [ "$GCLOUD_DEBUG" -eq 1 ]; then + echo "Setting verbosity to VERBOSE..."; + VERBOSITY=" -v"; +fi find $PROJECT_DIR/* -mindepth 1 -maxdepth 1 -name 'composer.json' -not -path '*vendor/*' -regex "$PROJECT_DIR/[A-Z].*" -exec dirname {} \; | while read DIR do @@ -31,13 +36,15 @@ do --component $COMPONENT \ --out $DIR/out \ --metadata-version $VERSION \ - --staging-bucket $STAGING_BUCKET + --staging-bucket $STAGING_BUCKET \ + $VERBOSITY else # dry run $PROJECT_DIR/dev/google-cloud docfx \ --component $COMPONENT \ --out $DIR/out \ - --metadata-version $VERSION + --metadata-version $VERSION \ + $VERBOSITY fi done @@ -77,4 +84,4 @@ if [ "$KOKORO_GITHUB_COMMIT" != "" ]; then composer update -d "$module" cp "$module/composer.lock" "pkg/$module/composer.lock" done -fi \ No newline at end of file +fi diff --git a/dev/src/Command/DocFxCommand.php b/dev/src/Command/DocFxCommand.php index fb9149ba6f87..28f79857b032 100644 --- a/dev/src/Command/DocFxCommand.php +++ b/dev/src/Command/DocFxCommand.php @@ -38,6 +38,7 @@ class DocFxCommand extends Command { use XrefValidationTrait; + private string $componentName; private array $composerJson; private array $repoMetadataJson; @@ -73,9 +74,9 @@ protected function execute(InputInterface $input, OutputInterface $output) throw new RuntimeException('This command must be run on PHP 8.0 or above'); } - $componentName = rtrim($input->getOption('component'), '/') ?: basename(getcwd()); - $component = new Component($componentName, $input->getOption('component-path')); - $output->writeln(sprintf('Generating documentation for %s', $componentName)); + $this->componentName = rtrim($input->getOption('component'), '/') ?: basename(getcwd()); + $component = new Component($this->componentName, $input->getOption('component-path')); + $output->writeln(sprintf('Generating documentation for %s', $this->componentName)); $xml = $input->getOption('xml'); $outDir = $input->getOption('out'); if (empty($xml)) { @@ -225,6 +226,7 @@ private function validate(ClassNode $class, OutputInterface $output): bool $valid = true; $emptyRef = 'empty'; $isGenerated = $class->isProtobufMessageClass() || $class->isProtobufEnumClass() || $class->isServiceClass(); + $warnings = []; foreach (array_merge([$class], $class->getMethods(), $class->getConstants()) as $node) { foreach ($this->getInvalidXrefs($node->getContent()) as $invalidRef) { if (isset(self::$allowedReferenceFailures[$node->getFullname()]) @@ -236,10 +238,9 @@ private function validate(ClassNode $class, OutputInterface $output): bool $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 - ); + $nodePath = $isGenerated ? $node->getProtoPath($class->getProtoPath()) : $node->getFullname(); + $brokenRef = $isGenerated ? $this->classnameToProtobufPath((string) $brokenRef) : $brokenRef; + $warnings[] = sprintf('[%s] Broken xref in %s: %s', $this->componentName, $nodePath, $brokenRef ?: $emptyRef); // generated classes are allowed to have broken xrefs if ($isGenerated) { continue; @@ -247,9 +248,41 @@ private function validate(ClassNode $class, OutputInterface $output): bool $valid = false; } } - if (!$valid) { + if (!$valid || count($warnings) > 0) { $output->writeln(''); } + foreach ($warnings as $warning) { + $output->writeln($warning, $isGenerated ? OutputInterface::VERBOSITY_VERBOSE : OutputInterface::VERBOSITY_NORMAL); + } return $valid; } + + private function classnameToProtobufPath(string $ref): string + { + // remove leading and trailing slashes and parentheses + $ref = trim(trim($ref, '\\'), '()'); + // convert methods to snake case + if (strpos($ref, '::set') !== false || strpos($ref, '::get') !== false) { + $parts = explode('::', $ref); + $ref = $parts[0] . '.' . strtolower(preg_replace('/(? $part) { + if (preg_match(Component::VERSION_REGEX, $part) || $part === 'Master') { + for ($j = 0; $j <= $i; $j++) { + $parts[$j] = strtolower($parts[$j]); + } + $ref = implode('.', $parts); + break; + } + } + + // convert namespace to lowercase + return false === strpos($ref, '.') ? strtolower($ref) : $ref; + } } diff --git a/dev/src/DocFx/Node/ClassNode.php b/dev/src/DocFx/Node/ClassNode.php index 9ec6b0ad16eb..150837b32950 100644 --- a/dev/src/DocFx/Node/ClassNode.php +++ b/dev/src/DocFx/Node/ClassNode.php @@ -246,6 +246,25 @@ public function getProtoPackage(): ?string return null; } + public function getProtoPath(): ?string + { + if ($this->isProtobufMessageClass()) { + if ($generatedFrom = (string) $this->xmlNode?->docblock?->{'long-description'}) { + if (preg_match('/Generated from protobuf message (.*)<\/code>/', $generatedFrom, $matches)) { + return $matches[1]; + } + } + + return null; + } + + if ($this->isServiceClass()) { + return $this->getProtoPackage() . '.' . $this->getName(); + } + + return null; + } + public function setProtoPackages(array $protoPackages) { $this->protoPackages = $protoPackages; diff --git a/dev/src/DocFx/Node/ConstantNode.php b/dev/src/DocFx/Node/ConstantNode.php index f522167ce61d..19f0c9e6d343 100644 --- a/dev/src/DocFx/Node/ConstantNode.php +++ b/dev/src/DocFx/Node/ConstantNode.php @@ -42,4 +42,9 @@ public function getValue(): string { return $this->xmlNode->value; } + + public function getProtoPath(string $package = null): string + { + return ($package ? $package . '.' : '') . $this->getName(); + } } diff --git a/dev/src/DocFx/Node/MethodNode.php b/dev/src/DocFx/Node/MethodNode.php index 8cf2fce5f248..5d5afcfb17af 100644 --- a/dev/src/DocFx/Node/MethodNode.php +++ b/dev/src/DocFx/Node/MethodNode.php @@ -148,6 +148,12 @@ public function getDisplayName(): string return $this->isStatic() ? 'static::' . $this->getName() : $this->getName(); } + public function getProtoPath(string $package = null): string + { + return ($package ? $package . '.' : '') + . strtolower(preg_replace('/(?getName(), 3))); + } + public function getContent(): string { $content = $this->getDocblockContent(); diff --git a/dev/src/DocFx/XrefValidationTrait.php b/dev/src/DocFx/XrefValidationTrait.php index caf5790fb438..61dffcead0cc 100644 --- a/dev/src/DocFx/XrefValidationTrait.php +++ b/dev/src/DocFx/XrefValidationTrait.php @@ -89,10 +89,12 @@ function ($matches) use (&$brokenRefs) { 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; From 232e39fd714114f295a67bebe9298d2f470668da Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Mon, 19 Aug 2024 14:53:49 -0700 Subject: [PATCH 2/7] WIP and fix a few bugs --- .../src/Admin/V1/FirestoreAdminClient.php | 10 ++-- Firestore/src/V1/FirestoreClient.php | 2 +- PubSub/src/PubSubClient.php | 2 +- dev/composer.json | 15 +++++- dev/src/Command/DocFxCommand.php | 30 +++++++----- dev/src/DocFx/Node/ClassNode.php | 46 ++++++++++++++++++- dev/src/DocFx/XrefValidationTrait.php | 11 +++-- 7 files changed, 90 insertions(+), 26 deletions(-) diff --git a/Firestore/src/Admin/V1/FirestoreAdminClient.php b/Firestore/src/Admin/V1/FirestoreAdminClient.php index 132569cbeb2d..5c179ce86e84 100644 --- a/Firestore/src/Admin/V1/FirestoreAdminClient.php +++ b/Firestore/src/Admin/V1/FirestoreAdminClient.php @@ -18,7 +18,7 @@ /* * GENERATED CODE WARNING * Generated by gapic-generator-php from the file - * https://github.com/google/googleapis/blob/master/google/firestore/admin/v1/firestore_admin.proto + * https://github.com/googleapis/googleapis/blob/master/google/firestore/admin/v1/firestore_admin.proto * Updates to the above are reflected here through a refresh process. */ @@ -31,7 +31,7 @@ class FirestoreAdminClient extends FirestoreAdminGapicClient { /** - * @see FirestoreAdminClient::createIndexLRO + * @see FirestoreAdminClient::createIndexLRO() * @return \Google\LongRunning\Operation * @deprecated use createIndexLRO instead */ @@ -41,7 +41,7 @@ public function createIndex($parent, $index, array $optionalArgs = []) } /** - * @see FirestoreAdminClient::exportDocumentsLRO + * @see FirestoreAdminClient::exportDocumentsLRO() * @return \Google\LongRunning\Operation * @deprecated use exportDocumentsLRO instead */ @@ -51,7 +51,7 @@ public function exportDocuments($name, array $optionalArgs = []) } /** - * @see FirestoreAdminClient::importDocumentsLRO + * @see FirestoreAdminClient::importDocumentsLRO() * @return \Google\LongRunning\Operation * @deprecated use importDocumentsLRO instead */ @@ -63,7 +63,7 @@ public function importDocuments($name, array $optionalArgs = []) /** * @return \Google\LongRunning\Operation * @deprecated use updateFieldLRO instead - * @see FirestoreAdminClient::updateFieldLRO + * @see FirestoreAdminClient::updateFieldLRO() */ public function updateField($field, array $optionalArgs = []) { diff --git a/Firestore/src/V1/FirestoreClient.php b/Firestore/src/V1/FirestoreClient.php index a4cee8168dd1..cbd4889f5af2 100644 --- a/Firestore/src/V1/FirestoreClient.php +++ b/Firestore/src/V1/FirestoreClient.php @@ -18,7 +18,7 @@ /* * GENERATED CODE WARNING * This file was generated from the file - * https://github.com/google/googleapis/blob/master/google/firestore/v1/firestore.proto + * https://github.com/googleapis/googleapis/blob/master/google/firestore/v1/firestore.proto * and updates to that file get reflected here through a refresh process. * * @experimental diff --git a/PubSub/src/PubSubClient.php b/PubSub/src/PubSubClient.php index d40e736a53fc..bdaddd2d94c3 100644 --- a/PubSub/src/PubSubClient.php +++ b/PubSub/src/PubSubClient.php @@ -262,7 +262,7 @@ public function createTopic($name, array $options = []) * Lazily instantiate a topic with a topic name. * * No API requests are made by this method. If you want to create a new - * topic, use {@see Topic::createTopic()}. + * topic, use {@see Topic::create()}. * * Example: * ``` diff --git a/dev/composer.json b/dev/composer.json index 45d6640be076..4efd6b8e8a53 100644 --- a/dev/composer.json +++ b/dev/composer.json @@ -22,7 +22,8 @@ "require-dev": { "phpunit/phpunit": "^9.0", "phpspec/prophecy-phpunit": "^2.0", - "swaggest/json-schema": "^0.12.0" + "swaggest/json-schema": "^0.12.0", + "googleapis/googleapis": "dev-master" }, "repositories": { "google-cloud": { @@ -33,6 +34,18 @@ "google/cloud": "dev-parent" } } + }, + "googleapis/googleapis": { + "type": "package", + "package": { + "name": "googleapis/googleapis", + "version": "dev-master", + "source": { + "url": "https://github.com/googleapis/googleapis.git", + "type": "git", + "reference": "master" + } + } } } } diff --git a/dev/src/Command/DocFxCommand.php b/dev/src/Command/DocFxCommand.php index 28f79857b032..3448d9413b98 100644 --- a/dev/src/Command/DocFxCommand.php +++ b/dev/src/Command/DocFxCommand.php @@ -98,7 +98,7 @@ protected function execute(InputInterface $input, OutputInterface $output) } } - $output->write(sprintf('Writing output to %s... ', $outDir)); + $output->writeln(sprintf('Writing output to %s... ', $outDir)); // YAML dump configuration $inline = 11; // The level where you switch to inline YAML @@ -136,6 +136,7 @@ protected function execute(InputInterface $input, OutputInterface $output) // exit early if the docs aren't valid if (!$valid) { + $output->writeln('Docs validation failed - invalid reference'); return 1; } @@ -160,7 +161,6 @@ protected function execute(InputInterface $input, OutputInterface $output) $tocYaml = Yaml::dump([$componentToc], $inline, $indent, $flags); $outFile = sprintf('%s/toc.yml', $outDir); file_put_contents($outFile, $tocYaml); - $output->writeln('Done.'); if ($metadataVersion = $input->getOption('metadata-version')) { @@ -237,10 +237,17 @@ private function validate(ClassNode $class, OutputInterface $output): bool $output->write(sprintf("\nInvalid xref in %s: %s", $node->getFullname(), $invalidRef)); $valid = false; } - foreach ($this->getBrokenXrefs($node->getContent()) as $brokenRef) { - $nodePath = $isGenerated ? $node->getProtoPath($class->getProtoPath()) : $node->getFullname(); - $brokenRef = $isGenerated ? $this->classnameToProtobufPath((string) $brokenRef) : $brokenRef; - $warnings[] = sprintf('[%s] Broken xref in %s: %s', $this->componentName, $nodePath, $brokenRef ?: $emptyRef); + foreach ($this->getBrokenXrefs($node->getContent()) as [$brokenRef, $brokenRefText]) { + $brokenRef = $isGenerated ? $this->classnameToProtobufPath((string) $brokenRef, $brokenRefText) : $brokenRef; + $nodePath = $isGenerated + ? $class->getProtoFileName($brokenRef) . ' (' . $node->getProtoPath($class->getName()) . ')' + : $node->getFullname(); + $warnings[] = sprintf( + '[%s] Broken xref in %s: %s', + $this->componentName, + $nodePath, + str_replace("\n", '', $brokenRef) ?: $emptyRef + ); // generated classes are allowed to have broken xrefs if ($isGenerated) { continue; @@ -248,16 +255,13 @@ private function validate(ClassNode $class, OutputInterface $output): bool $valid = false; } } - if (!$valid || count($warnings) > 0) { - $output->writeln(''); - } - foreach ($warnings as $warning) { + foreach (array_unique($warnings) as $warning) { $output->writeln($warning, $isGenerated ? OutputInterface::VERBOSITY_VERBOSE : OutputInterface::VERBOSITY_NORMAL); } return $valid; } - private function classnameToProtobufPath(string $ref): string + private function classnameToProtobufPath(string $ref, string $text): string { // remove leading and trailing slashes and parentheses $ref = trim(trim($ref, '\\'), '()'); @@ -283,6 +287,8 @@ private function classnameToProtobufPath(string $ref): string } // convert namespace to lowercase - return false === strpos($ref, '.') ? strtolower($ref) : $ref; + $ref = false === strpos($ref, '.') ? strtolower($ref) : $ref; + + return sprintf('[%s][%s]', $text, $ref); } } diff --git a/dev/src/DocFx/Node/ClassNode.php b/dev/src/DocFx/Node/ClassNode.php index 150837b32950..4d1a6a287ed5 100644 --- a/dev/src/DocFx/Node/ClassNode.php +++ b/dev/src/DocFx/Node/ClassNode.php @@ -68,6 +68,49 @@ public function isProtobufMessageClass(): bool return false; } + public function getProtoFileName(string $ref = null): string|null + { + if (!$this->isProtobufMessageClass() + && !$this->isProtobufEnumClass() + && !$this->isServiceClass() + ) { + return null; + } + + $filename = (new \ReflectionClass($this->getFullName()))->getFileName(); + + if ($this->isProtobufMessageClass() || $this->isProtobufEnumClass()) { + $lines = explode("\n", file_get_contents($filename)); + $proto = str_replace('# source: ', '', $lines[2]); + } else { + $lines = explode("\n", file_get_contents($filename)); + $proto = str_replace(' * https://github.com/googleapis/googleapis/blob/master/', '', $lines[20]); + } + + $vendor = __DIR__ . '/../../../vendor/googleapis/googleapis/'; + if (!$ref || !file_exists($vendor . $proto)) { + return $proto; + } + + $lines = explode("\n", file_get_contents($vendor . $proto)); + $ref1 = $ref2 = null; + if (false !== strpos($ref, "\n")) { + [$ref1, $ref2] = explode("\n", $ref); + } + foreach ($lines as $i => $line) { + if ($ref1 && $ref2) { + if (false !== stripos($line, $ref1) + && false !== stripos($lines[$i+1], $ref2)) { + return $proto . '#' . ($i + 1); + } + } elseif (false !== stripos($line, $ref)) { + return $proto . '#' . ($i + 1); + } + } + + return $proto; + } + public function isGapicEnumClass(): bool { // returns true if the class extends \Google\Protobuf\Internal\Message @@ -98,10 +141,11 @@ public function isServiceBaseClass(): bool public function isV2ServiceClass(): bool { // returns true if the class does not extend another class and isn't a - // base class + // base class and it contains a "Client" namespace if (!$this->getExtends() && !$this->isServiceBaseClass() && 'Client' === substr($this->getName(), -6) + && false !== strpos($this->getFullName(), '\\Client\\') ) { return true; } diff --git a/dev/src/DocFx/XrefValidationTrait.php b/dev/src/DocFx/XrefValidationTrait.php index 61dffcead0cc..f3f6a3c1dc1b 100644 --- a/dev/src/DocFx/XrefValidationTrait.php +++ b/dev/src/DocFx/XrefValidationTrait.php @@ -64,7 +64,7 @@ private function getBrokenXrefs(string $description): array { $brokenRefs = []; preg_replace_callback( - '/([^ ]*)<\/xref>/', function ($matches) use (&$brokenRefs) { // Valid external reference if (0 === strpos($matches[1], 'http')) { @@ -78,10 +78,11 @@ function ($matches) use (&$brokenRefs) { return; } // Valid class reference - if (class_exists($matches[1]) || interface_exists($matches[1] || trait_exists($matches[1]))) { + if (class_exists($matches[1]) || interface_exists($matches[1]) || trait_exists($matches[1])) { return; } - // Valid method, magic method, andd constant references + + // Valid method, magic method, and constant references if (false !== strpos($matches[1], '::')) { if (false !== strpos($matches[1], '()')) { list($class, $method) = explode('::', str_replace('()', '', $matches[1])); @@ -103,9 +104,9 @@ function ($matches) use (&$brokenRefs) { // Invalid reference! if ($matches[1] === '\\\\') { // empty hrefs show up as "\\" - $brokenRefs[] = null; + $brokenRefs[] = [null, $matches[2]]; } else { - $brokenRefs[] = $matches[1]; + $brokenRefs[] = [$matches[1], $matches[2]]; } }, $description From d4a83a86851c282b952e9615285a17272ccf794a Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Mon, 7 Oct 2024 11:13:24 -0700 Subject: [PATCH 3/7] move classnameToProtobufPath into XrefValidationTrait --- dev/src/Command/DocFxCommand.php | 31 --------------------------- dev/src/DocFx/XrefValidationTrait.php | 31 +++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/dev/src/Command/DocFxCommand.php b/dev/src/Command/DocFxCommand.php index cc19acbe827c..59aa68af5aa0 100644 --- a/dev/src/Command/DocFxCommand.php +++ b/dev/src/Command/DocFxCommand.php @@ -270,35 +270,4 @@ private function validate(ClassNode $class, OutputInterface $output): bool } return $valid; } - - private function classnameToProtobufPath(string $ref, string $text): string - { - // remove leading and trailing slashes and parentheses - $ref = trim(trim($ref, '\\'), '()'); - // convert methods to snake case - if (strpos($ref, '::set') !== false || strpos($ref, '::get') !== false) { - $parts = explode('::', $ref); - $ref = $parts[0] . '.' . strtolower(preg_replace('/(? $part) { - if (preg_match(Component::VERSION_REGEX, $part) || $part === 'Master') { - for ($j = 0; $j <= $i; $j++) { - $parts[$j] = strtolower($parts[$j]); - } - $ref = implode('.', $parts); - break; - } - } - - // convert namespace to lowercase - $ref = false === strpos($ref, '.') ? strtolower($ref) : $ref; - - return sprintf('[%s][%s]', $text, $ref); - } } diff --git a/dev/src/DocFx/XrefValidationTrait.php b/dev/src/DocFx/XrefValidationTrait.php index f3f6a3c1dc1b..7ec63deba4a2 100644 --- a/dev/src/DocFx/XrefValidationTrait.php +++ b/dev/src/DocFx/XrefValidationTrait.php @@ -114,4 +114,35 @@ function ($matches) use (&$brokenRefs) { return $brokenRefs; } + + private function classnameToProtobufPath(string $ref, string $text): string + { + // remove leading and trailing slashes and parentheses + $ref = trim(trim($ref, '\\'), '()'); + // convert methods to snake case + if (strpos($ref, '::set') !== false || strpos($ref, '::get') !== false) { + $parts = explode('::', $ref); + $ref = $parts[0] . '.' . strtolower(preg_replace('/(? $part) { + if (preg_match(Component::VERSION_REGEX, $part) || $part === 'Master') { + for ($j = 0; $j <= $i; $j++) { + $parts[$j] = strtolower($parts[$j]); + } + $ref = implode('.', $parts); + break; + } + } + + // convert namespace to lowercase + $ref = false === strpos($ref, '.') ? strtolower($ref) : $ref; + + return sprintf('[%s][%s]', $text, $ref); + } } From 995c591c2a845b7f923fe9a44537a4d572e75c54 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Mon, 7 Oct 2024 14:54:06 -0700 Subject: [PATCH 4/7] add command to list broken xrefs --- dev/google-cloud | 2 + dev/src/Command/ListBrokenXrefsCommand.php | 67 ++++++++++++++++++++++ dev/src/DocFx/Node/ClassNode.php | 4 +- dev/src/DocFx/XrefValidationTrait.php | 1 + 4 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 dev/src/Command/ListBrokenXrefsCommand.php diff --git a/dev/google-cloud b/dev/google-cloud index 4683cd760b98..806aeb71a794 100755 --- a/dev/google-cloud +++ b/dev/google-cloud @@ -25,6 +25,7 @@ require __DIR__ . '/vendor/autoload.php'; use Google\Cloud\Dev\Command\AddComponentCommand; use Google\Cloud\Dev\Command\ComponentInfoCommand; use Google\Cloud\Dev\Command\DocFxCommand; +use Google\Cloud\Dev\Command\ListBrokenXrefsCommand; use Google\Cloud\Dev\Command\RepoInfoCommand; use Google\Cloud\Dev\Command\ReleaseInfoCommand; use Google\Cloud\Dev\Command\SplitCommand; @@ -45,6 +46,7 @@ $app = new Application; $app->add(new AddComponentCommand($rootDirectory)); $app->add(new ComponentInfoCommand()); $app->add(new DocFxCommand()); +$app->add(new ListBrokenXrefsCommand()); $app->add(new RepoInfoCommand()); $app->add(new ReleaseInfoCommand()); $app->add(new SplitCommand($rootDirectory)); diff --git a/dev/src/Command/ListBrokenXrefsCommand.php b/dev/src/Command/ListBrokenXrefsCommand.php new file mode 100644 index 000000000000..703a1f4d1c29 --- /dev/null +++ b/dev/src/Command/ListBrokenXrefsCommand.php @@ -0,0 +1,67 @@ +setName('list-broken-xrefs') + ->setDescription('List all the broken xrefs in the documentation using ".kokoro/docs/publish.sh"') + ; + } + + protected function execute(InputInterface $input, OutputInterface $output) + { + foreach (Component::getComponents() as $component) { + $input = new ArrayInput($f = [ + 'command' => 'docfx', + '--component' => $component->getName(), + ]); + + $buffer = new BufferedOutput(OutputInterface::VERBOSITY_DEBUG); + $returnCode = $this->getApplication()->doRun($input, $buffer); + foreach (explode("\n", $buffer->fetch()) as $line) { + if (preg_match(self::BROKEN_REFS_REGEX, $line, $matches)) { + list(, $component, $file, $ref, $brokenText) = $matches; + $link = sprintf( + '=HYPERLINK("https://github.com/googleapis/googleapis/blob/master/%s", "%s")', + $file, + $file + ); + $output->writeln(implode(',', [$component, $link, $ref, $brokenText])); + } + } + } + + return 0; + } +} diff --git a/dev/src/DocFx/Node/ClassNode.php b/dev/src/DocFx/Node/ClassNode.php index 4d1a6a287ed5..82644fb30325 100644 --- a/dev/src/DocFx/Node/ClassNode.php +++ b/dev/src/DocFx/Node/ClassNode.php @@ -101,10 +101,10 @@ public function getProtoFileName(string $ref = null): string|null if ($ref1 && $ref2) { if (false !== stripos($line, $ref1) && false !== stripos($lines[$i+1], $ref2)) { - return $proto . '#' . ($i + 1); + return $proto . '#L' . ($i + 1); } } elseif (false !== stripos($line, $ref)) { - return $proto . '#' . ($i + 1); + return $proto . '#L' . ($i + 1); } } diff --git a/dev/src/DocFx/XrefValidationTrait.php b/dev/src/DocFx/XrefValidationTrait.php index 7ec63deba4a2..570deaea734c 100644 --- a/dev/src/DocFx/XrefValidationTrait.php +++ b/dev/src/DocFx/XrefValidationTrait.php @@ -19,6 +19,7 @@ use Google\Cloud\Core\Logger\AppEngineFlexFormatter; use Google\Cloud\Core\Logger\AppEngineFlexFormatterV2; +use Google\Cloud\Dev\Component; /** * @internal From dafb98ef231e846c822c326d78459e76ae1fcfdd Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Tue, 8 Oct 2024 10:35:58 -0700 Subject: [PATCH 5/7] fix hyperlink --- dev/src/Command/ListBrokenXrefsCommand.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/src/Command/ListBrokenXrefsCommand.php b/dev/src/Command/ListBrokenXrefsCommand.php index 703a1f4d1c29..1033f8acbbf1 100644 --- a/dev/src/Command/ListBrokenXrefsCommand.php +++ b/dev/src/Command/ListBrokenXrefsCommand.php @@ -53,7 +53,7 @@ protected function execute(InputInterface $input, OutputInterface $output) if (preg_match(self::BROKEN_REFS_REGEX, $line, $matches)) { list(, $component, $file, $ref, $brokenText) = $matches; $link = sprintf( - '=HYPERLINK("https://github.com/googleapis/googleapis/blob/master/%s", "%s")', + '"=HYPERLINK(""https://github.com/googleapis/googleapis/blob/master/%s"", ""%s"")"', $file, $file ); From db0e88131a1b42db9957bcdb41615febd68e9b62 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Tue, 8 Oct 2024 20:22:47 -0700 Subject: [PATCH 6/7] add bug template --- dev/src/Command/ListBrokenXrefsCommand.php | 103 +++++++++++++++++++-- 1 file changed, 96 insertions(+), 7 deletions(-) diff --git a/dev/src/Command/ListBrokenXrefsCommand.php b/dev/src/Command/ListBrokenXrefsCommand.php index 1033f8acbbf1..90908218d93f 100644 --- a/dev/src/Command/ListBrokenXrefsCommand.php +++ b/dev/src/Command/ListBrokenXrefsCommand.php @@ -20,6 +20,7 @@ use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\ArrayInput; use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\BufferedOutput; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Process\Process; @@ -31,16 +32,45 @@ class ListBrokenXrefsCommand extends Command { const BROKEN_REFS_REGEX = '/\[(\w+)\] Broken xref in (.*) \((.*)\): (.*)/'; + const MIN_REFS_PER_BUG = 10; + const BUG_TEMPLATE=<<setName('list-broken-xrefs') ->setDescription('List all the broken xrefs in the documentation using ".kokoro/docs/publish.sh"') + ->addOption('write-bugs', null, InputOption::VALUE_REQUIRED, 'write the bug to the given directory') ; } protected function execute(InputInterface $input, OutputInterface $output) { + $bugDir = $input->getOption('write-bugs'); + if ($bugDir) { + $this->sha = $this->determineGoogleapisSha(); + } + $brokenReferences = []; foreach (Component::getComponents() as $component) { $input = new ArrayInput($f = [ 'command' => 'docfx', @@ -49,19 +79,78 @@ protected function execute(InputInterface $input, OutputInterface $output) $buffer = new BufferedOutput(OutputInterface::VERBOSITY_DEBUG); $returnCode = $this->getApplication()->doRun($input, $buffer); + $componentBrokenRefs = []; foreach (explode("\n", $buffer->fetch()) as $line) { if (preg_match(self::BROKEN_REFS_REGEX, $line, $matches)) { - list(, $component, $file, $ref, $brokenText) = $matches; - $link = sprintf( - '"=HYPERLINK(""https://github.com/googleapis/googleapis/blob/master/%s"", ""%s"")"', - $file, - $file - ); - $output->writeln(implode(',', [$component, $link, $ref, $brokenText])); + list(, $componentName, $file, $ref, $brokenText) = $matches; + if (false === strpos($file, '#L')) { + // If there are no line numbers, assume this is a PHP bug, and skip it + continue; + } + if ($bugDir) { + $componentBrokenRefs[] = ['file' => $file, 'text' => $brokenText]; + } else { + $link = sprintf( + '"=HYPERLINK(""https://github.com/googleapis/googleapis/blob/master/%s"", ""%s"")"', + $file, + $file + ); + $output->writeln(implode(',', [$componentName, $link, $brokenText])); + } + } + } + if ($bugDir) { + if (count($componentBrokenRefs) > self::MIN_REFS_PER_BUG) { + $file = $this->writeBuggerFile([$component->getName() => $componentBrokenRefs], $bugDir); + $output->writeln(sprintf('Wrote %s references to %s', count($componentBrokenRefs), $file)); + } else { + if (count($componentBrokenRefs) > 0) { + $brokenReferences[$component->getName()] = $componentBrokenRefs; + } + if (self::MIN_REFS_PER_BUG < $count = array_sum(array_map('count', $brokenReferences))) { + $file = $this->writeBuggerFile($brokenReferences, $bugDir); + $output->writeln(sprintf('Wrote %s references to %s', $count, $file)); + // reset broken references + $brokenReferences = []; + } } } } return 0; } + + private function writeBuggerFile(array $brokenReferences, string $bugDir): string + { + $components = array_keys($brokenReferences); + if (strlen($componentNames = implode(', ', $components)) > 80) { + $componentNames = substr($componentNames, 0, 78) . '...'; + } + $references = array_merge(...array_values($brokenReferences)); + $bugFile = sprintf('%s/broken-refs-%s.txt', $bugDir, implode('-', $components)); + $bugText = sprintf( + self::BUG_TEMPLATE, + $componentNames, + implode("\n", array_map( + fn ($ref) => sprintf( + self::BROKEN_REF_TEMPLATE, + $ref['file'], + $this->sha, + $ref['file'], + $ref['text'] + ), + $references + )) + ); + file_put_contents($bugFile, $bugText); + + return $bugFile; + } + + private function determineGoogleapisSha(): string + { + $process = new Process(['git', 'rev-parse', 'HEAD'], realpath(__DIR__ . '/../../vendor/googleapis/googleapis')); + $process->run(); + return trim($process->getOutput()); + } } From e1cc1fa08263b58baf065a2872254c8f2a156f72 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Tue, 23 Dec 2025 09:43:35 -0800 Subject: [PATCH 7/7] code cleanup checkpoint --- dev/composer.json | 13 ----- dev/src/Command/DocFxCommand.php | 83 ++++++++++++++++++++++++++- dev/src/DocFx/Node/ClassNode.php | 43 -------------- dev/src/DocFx/XrefValidationTrait.php | 34 ----------- 4 files changed, 82 insertions(+), 91 deletions(-) diff --git a/dev/composer.json b/dev/composer.json index 735dc7404cb6..4d8780f84a36 100644 --- a/dev/composer.json +++ b/dev/composer.json @@ -23,7 +23,6 @@ "phpunit/phpunit": "^9.0", "phpspec/prophecy-phpunit": "^2.0", "swaggest/json-schema": "^0.12.0", - "googleapis/googleapis": "dev-master", "phpdocumentor/reflection": "^6.3", "erusev/parsedown": "^1.7", "dg/bypass-finals": "^1.7" @@ -37,18 +36,6 @@ "google/cloud": "dev-parent" } } - }, - "googleapis/googleapis": { - "type": "package", - "package": { - "name": "googleapis/googleapis", - "version": "dev-master", - "source": { - "url": "https://github.com/googleapis/googleapis.git", - "type": "git", - "reference": "master" - } - } } } } diff --git a/dev/src/Command/DocFxCommand.php b/dev/src/Command/DocFxCommand.php index fbfa3a511360..d710037ec36a 100644 --- a/dev/src/Command/DocFxCommand.php +++ b/dev/src/Command/DocFxCommand.php @@ -25,6 +25,8 @@ use Symfony\Component\Yaml\Yaml; use RuntimeException; use Google\Auth\Cache\FileSystemCacheItemPool; +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; @@ -302,7 +304,7 @@ private function validate(ClassNode $class, OutputInterface $output): bool foreach ($this->getBrokenXrefs($node->getContent()) as [$brokenRef, $brokenRefText]) { $brokenRef = $isGenerated ? $this->classnameToProtobufPath((string) $brokenRef, $brokenRefText) : $brokenRef; $nodePath = $isGenerated - ? $class->getProtoFileName($brokenRef) . ' (' . $node->getProtoPath($class->getName()) . ')' + ? $this->getProtoFileName($class, $brokenRef) . ' (' . $node->getProtoPath($class->getName()) . ')' : $node->getFullname(); $warnings[] = sprintf( '[%s] Broken xref in %s: %s', @@ -356,4 +358,83 @@ private function uploadToStagingBucket(string $outDir, string $stagingBucket): v ]); $process->mustRun(); } + + private function classnameToProtobufPath(string $ref, string $text): string + { + // remove leading and trailing slashes and parentheses + $ref = trim(trim($ref, '\\'), '()'); + // convert methods to snake case + if (strpos($ref, '::set') !== false || strpos($ref, '::get') !== false) { + $parts = explode('::', $ref); + $ref = $parts[0] . '.' . strtolower(preg_replace('/(? $part) { + if (preg_match(Component::VERSION_REGEX, $part) || $part === 'Master') { + for ($j = 0; $j <= $i; $j++) { + $parts[$j] = strtolower($parts[$j]); + } + $ref = implode('.', $parts); + break; + } + } + + // convert namespace to lowercase + $ref = false === strpos($ref, '.') ? strtolower($ref) : $ref; + + return sprintf('[%s][%s]', $text, $ref); + } + + public function getProtoFileName(ClassNode $node, string $ref = null): string|null + { + if (!$node->isProtobufMessageClass() + && !$node->isProtobufEnumClass() + && !$node->isServiceClass() + ) { + return null; + } + + $filename = (new \ReflectionClass($node->getFullName()))->getFileName(); + + if ($node->isProtobufMessageClass() || $node->isProtobufEnumClass()) { + $lines = explode("\n", file_get_contents($filename)); + $proto = str_replace('# source: ', '', $lines[2]); + } else { + $lines = explode("\n", file_get_contents($filename)); + $proto = str_replace(' * https://github.com/googleapis/googleapis/blob/master/', '', $lines[20]); + } + + if (!$ref) { + return $proto; + } + + $protoUrl = 'https://github.com/googleapis/googleapis/blob/master/' . $proto; + if (!$protoContents = file_get_contents($protoUrl)) { + // gracefully fail to retrieve proto contents + return $proto; + } + + $lines = explode("\n", $protoContents); + $ref1 = $ref2 = null; + if (false !== strpos($ref, "\n")) { + [$ref1, $ref2] = explode("\n", $ref); + } + foreach ($lines as $i => $line) { + if ($ref1 && $ref2) { + if (false !== stripos($line, $ref1) + && false !== stripos($lines[$i+1], $ref2)) { + return $proto . '#L' . ($i + 1); + } + } elseif (false !== stripos($line, $ref)) { + return $proto . '#L' . ($i + 1); + } + } + + return $proto; + } } diff --git a/dev/src/DocFx/Node/ClassNode.php b/dev/src/DocFx/Node/ClassNode.php index cc84717a5a16..d29c1599f972 100644 --- a/dev/src/DocFx/Node/ClassNode.php +++ b/dev/src/DocFx/Node/ClassNode.php @@ -70,49 +70,6 @@ public function isProtobufMessageClass(): bool return false; } - public function getProtoFileName(string $ref = null): string|null - { - if (!$this->isProtobufMessageClass() - && !$this->isProtobufEnumClass() - && !$this->isServiceClass() - ) { - return null; - } - - $filename = (new \ReflectionClass($this->getFullName()))->getFileName(); - - if ($this->isProtobufMessageClass() || $this->isProtobufEnumClass()) { - $lines = explode("\n", file_get_contents($filename)); - $proto = str_replace('# source: ', '', $lines[2]); - } else { - $lines = explode("\n", file_get_contents($filename)); - $proto = str_replace(' * https://github.com/googleapis/googleapis/blob/master/', '', $lines[20]); - } - - $vendor = __DIR__ . '/../../../vendor/googleapis/googleapis/'; - if (!$ref || !file_exists($vendor . $proto)) { - return $proto; - } - - $lines = explode("\n", file_get_contents($vendor . $proto)); - $ref1 = $ref2 = null; - if (false !== strpos($ref, "\n")) { - [$ref1, $ref2] = explode("\n", $ref); - } - foreach ($lines as $i => $line) { - if ($ref1 && $ref2) { - if (false !== stripos($line, $ref1) - && false !== stripos($lines[$i+1], $ref2)) { - return $proto . '#L' . ($i + 1); - } - } elseif (false !== stripos($line, $ref)) { - return $proto . '#L' . ($i + 1); - } - } - - return $proto; - } - 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 index 570deaea734c..fb53cefa1a39 100644 --- a/dev/src/DocFx/XrefValidationTrait.php +++ b/dev/src/DocFx/XrefValidationTrait.php @@ -17,10 +17,6 @@ namespace Google\Cloud\Dev\DocFx; -use Google\Cloud\Core\Logger\AppEngineFlexFormatter; -use Google\Cloud\Core\Logger\AppEngineFlexFormatterV2; -use Google\Cloud\Dev\Component; - /** * @internal */ @@ -116,34 +112,4 @@ function ($matches) use (&$brokenRefs) { return $brokenRefs; } - private function classnameToProtobufPath(string $ref, string $text): string - { - // remove leading and trailing slashes and parentheses - $ref = trim(trim($ref, '\\'), '()'); - // convert methods to snake case - if (strpos($ref, '::set') !== false || strpos($ref, '::get') !== false) { - $parts = explode('::', $ref); - $ref = $parts[0] . '.' . strtolower(preg_replace('/(? $part) { - if (preg_match(Component::VERSION_REGEX, $part) || $part === 'Master') { - for ($j = 0; $j <= $i; $j++) { - $parts[$j] = strtolower($parts[$j]); - } - $ref = implode('.', $parts); - break; - } - } - - // convert namespace to lowercase - $ref = false === strpos($ref, '.') ? strtolower($ref) : $ref; - - return sprintf('[%s][%s]', $text, $ref); - } }