From 9daa2a3eaaa5d4c50557fe25937846b8ce06a0df Mon Sep 17 00:00:00 2001 From: "lina.wolf" Date: Sun, 17 Mar 2024 12:55:45 +0100 Subject: [PATCH 1/2] [TASK] Warn about duplicate link targets releases: main, 1.0 --- .../CollectLinkTargetsTransformer.php | 46 +++++++++++++++---- packages/guides/src/Nodes/ProjectNode.php | 5 ++ .../section-nesting/section-nesting.html | 4 +- .../tests/section-nesting/section-nesting.rst | 4 +- 4 files changed, 47 insertions(+), 12 deletions(-) diff --git a/packages/guides/src/Compiler/NodeTransformers/CollectLinkTargetsTransformer.php b/packages/guides/src/Compiler/NodeTransformers/CollectLinkTargetsTransformer.php index a0dd7af0e..13f507103 100644 --- a/packages/guides/src/Compiler/NodeTransformers/CollectLinkTargetsTransformer.php +++ b/packages/guides/src/Compiler/NodeTransformers/CollectLinkTargetsTransformer.php @@ -23,9 +23,12 @@ use phpDocumentor\Guides\Nodes\Node; use phpDocumentor\Guides\Nodes\SectionNode; use phpDocumentor\Guides\ReferenceResolvers\AnchorNormalizer; +use Psr\Log\LoggerInterface; use SplStack; use Webmozart\Assert\Assert; +use function sprintf; + /** @implements NodeTransformer */ final class CollectLinkTargetsTransformer implements NodeTransformer { @@ -34,6 +37,7 @@ final class CollectLinkTargetsTransformer implements NodeTransformer public function __construct( private readonly AnchorNormalizer $anchorReducer, + private LoggerInterface|null $logger = null, ) { /* * TODO: remove stack here, as we should not have sub documents in this way, sub documents are @@ -68,17 +72,43 @@ public function enterNode(Node $node, CompilerContext $compilerContext): Node $currentDocument = $this->documentStack->top(); Assert::notNull($currentDocument); $anchor = $node->getId(); - $compilerContext->getProjectNode()->addLinkTarget( - $anchor, - new InternalTarget( - $currentDocument->getFilePath(), + if ($compilerContext->getProjectNode()->hasInternalTarget($anchor, $node->getLinkType())) { + $this->logger?->warning( + sprintf( + 'Duplicate anchor "%s" for link type "%s" in document "%s". The anchor is already used at "%s"', + $anchor, + $node->getLinkType(), + $compilerContext->getDocumentNode()->getFilePath(), + $compilerContext->getProjectNode()->getInternalTarget($anchor, $node->getLinkType())?->getDocumentPath(), + ), + $compilerContext->getLoggerInformation(), + ); + } else { + $compilerContext->getProjectNode()->addLinkTarget( $anchor, - $node->getLinkText(), - $node->getLinkType(), - ), - ); + new InternalTarget( + $currentDocument->getFilePath(), + $anchor, + $node->getLinkText(), + $node->getLinkType(), + ), + ); + } if ($node instanceof MultipleLinkTargetsNode) { foreach ($node->getAdditionalIds() as $id) { + if ($compilerContext->getProjectNode()->hasInternalTarget($id, $node->getLinkType())) { + $this->logger?->warning( + sprintf( + 'Duplicate anchor "%s" for link type "%s" in document "%s". The anchor is already used at "%s"', + $anchor, + $node->getLinkType(), + $compilerContext->getDocumentNode()->getFilePath(), + $compilerContext->getProjectNode()->getInternalTarget($anchor, $node->getLinkType())?->getDocumentPath(), + ), + $compilerContext->getLoggerInformation(), + ); + } + $compilerContext->getProjectNode()->addLinkTarget( $id, new InternalTarget( diff --git a/packages/guides/src/Nodes/ProjectNode.php b/packages/guides/src/Nodes/ProjectNode.php index 2bdb5f3d8..3ce4bc731 100644 --- a/packages/guides/src/Nodes/ProjectNode.php +++ b/packages/guides/src/Nodes/ProjectNode.php @@ -152,6 +152,11 @@ public function addLinkTarget(string $anchorName, InternalTarget $target): void $this->internalLinkTargets[$target->getLinkType()][$anchorName] = $target; } + public function hasInternalTarget(string $anchorName, string $linkType = SectionNode::STD_LABEL): bool + { + return isset($this->internalLinkTargets[$linkType][$anchorName]); + } + public function getInternalTarget(string $anchorName, string $linkType = SectionNode::STD_LABEL): InternalTarget|null { return $this->internalLinkTargets[$linkType][$anchorName] ?? null; diff --git a/tests/Functional/tests/section-nesting/section-nesting.html b/tests/Functional/tests/section-nesting/section-nesting.html index ce0dcfccf..327012698 100644 --- a/tests/Functional/tests/section-nesting/section-nesting.html +++ b/tests/Functional/tests/section-nesting/section-nesting.html @@ -37,9 +37,9 @@

Level 1 Test 4

-
+

- Level 2 Test 3 + Level 2 Test 3b

diff --git a/tests/Functional/tests/section-nesting/section-nesting.rst b/tests/Functional/tests/section-nesting/section-nesting.rst index d18d959d6..9a371a583 100644 --- a/tests/Functional/tests/section-nesting/section-nesting.rst +++ b/tests/Functional/tests/section-nesting/section-nesting.rst @@ -22,8 +22,8 @@ Level 1 Test 3 Level 1 Test 4 ============== -Level 2 Test 3 --------------- +Level 2 Test 3b +--------------- Level 3 Test 1 ************** From 17c3a07222db303db6432f988ad21ab2873aa67a Mon Sep 17 00:00:00 2001 From: "lina.wolf" Date: Sun, 17 Mar 2024 18:11:37 +0100 Subject: [PATCH 2/2] [TASK] Warn about duplicate anchors in LinkTargetNode Exclude section for now as this is a bit more tricky. releases: main, 1.0 --- .../Directives/ConfvalDirective.php | 9 +- .../CollectLinkTargetsTransformer.php | 104 +++++++++++------- .../expected/another.html} | 11 +- .../confval/confval-name/expected/index.html | 39 +++++++ .../confval-name/expected/objects.inv.json | 38 +++++++ .../confval/confval-name/input/another.rst | 15 +++ .../confval/confval-name/input/index.rst | 19 ++++ .../confval-warning/expected/another.html | 32 ++++++ .../confval-warning/expected/index.html | 39 +++++++ .../confval-warning/expected/logs/warning.log | 1 + .../expected/objects.inv.json | 8 +- .../input/another.rst} | 0 .../confval/confval-warning/input/index.rst | 19 ++++ 13 files changed, 290 insertions(+), 44 deletions(-) rename tests/Integration/tests/confval/{expected/index.html => confval-name/expected/another.html} (85%) create mode 100644 tests/Integration/tests/confval/confval-name/expected/index.html create mode 100644 tests/Integration/tests/confval/confval-name/expected/objects.inv.json create mode 100644 tests/Integration/tests/confval/confval-name/input/another.rst create mode 100644 tests/Integration/tests/confval/confval-name/input/index.rst create mode 100644 tests/Integration/tests/confval/confval-warning/expected/another.html create mode 100644 tests/Integration/tests/confval/confval-warning/expected/index.html create mode 100644 tests/Integration/tests/confval/confval-warning/expected/logs/warning.log rename tests/Integration/tests/confval/{ => confval-warning}/expected/objects.inv.json (73%) rename tests/Integration/tests/confval/{input/index.rst => confval-warning/input/another.rst} (100%) create mode 100644 tests/Integration/tests/confval/confval-warning/input/index.rst diff --git a/packages/guides-restructured-text/src/RestructuredText/Directives/ConfvalDirective.php b/packages/guides-restructured-text/src/RestructuredText/Directives/ConfvalDirective.php index 3b64b256c..9c978f6d4 100644 --- a/packages/guides-restructured-text/src/RestructuredText/Directives/ConfvalDirective.php +++ b/packages/guides-restructured-text/src/RestructuredText/Directives/ConfvalDirective.php @@ -61,7 +61,12 @@ protected function processSub( CollectionNode $collectionNode, Directive $directive, ): Node|null { - $id = $this->anchorReducer->reduceAnchor($directive->getData()); + $id = $directive->getData(); + if ($directive->hasOption('name')) { + $id = (string) $directive->getOption('name')->getValue(); + } + + $id = $this->anchorReducer->reduceAnchor($id); $type = null; $required = false; $default = null; @@ -79,7 +84,7 @@ protected function processSub( } foreach ($directive->getOptions() as $option) { - if (in_array($option->getName(), ['type', 'required', 'default', 'noindex'], true)) { + if (in_array($option->getName(), ['type', 'required', 'default', 'noindex', 'name'], true)) { continue; } diff --git a/packages/guides/src/Compiler/NodeTransformers/CollectLinkTargetsTransformer.php b/packages/guides/src/Compiler/NodeTransformers/CollectLinkTargetsTransformer.php index 13f507103..86a5187ca 100644 --- a/packages/guides/src/Compiler/NodeTransformers/CollectLinkTargetsTransformer.php +++ b/packages/guides/src/Compiler/NodeTransformers/CollectLinkTargetsTransformer.php @@ -51,7 +51,11 @@ public function enterNode(Node $node, CompilerContext $compilerContext): Node { if ($node instanceof DocumentNode) { $this->documentStack->push($node); - } elseif ($node instanceof AnchorNode) { + + return $node; + } + + if ($node instanceof AnchorNode) { $currentDocument = $compilerContext->getDocumentNode(); $parentSection = $compilerContext->getShadowTree()->getParent()?->getNode(); $title = null; @@ -68,52 +72,48 @@ public function enterNode(Node $node, CompilerContext $compilerContext): Node $title, ), ); - } elseif ($node instanceof LinkTargetNode) { + + return $node; + } + + if ($node instanceof SectionNode) { + $currentDocument = $this->documentStack->top(); + Assert::notNull($currentDocument); + $anchorName = $node->getId(); + $compilerContext->getProjectNode()->addLinkTarget( + $anchorName, + new InternalTarget( + $currentDocument->getFilePath(), + $anchorName, + $node->getLinkText(), + $node->getLinkType(), + ), + ); + + return $node; + } + + if ($node instanceof LinkTargetNode) { $currentDocument = $this->documentStack->top(); Assert::notNull($currentDocument); - $anchor = $node->getId(); - if ($compilerContext->getProjectNode()->hasInternalTarget($anchor, $node->getLinkType())) { - $this->logger?->warning( - sprintf( - 'Duplicate anchor "%s" for link type "%s" in document "%s". The anchor is already used at "%s"', - $anchor, - $node->getLinkType(), - $compilerContext->getDocumentNode()->getFilePath(), - $compilerContext->getProjectNode()->getInternalTarget($anchor, $node->getLinkType())?->getDocumentPath(), - ), - $compilerContext->getLoggerInformation(), - ); - } else { - $compilerContext->getProjectNode()->addLinkTarget( + $anchor = $this->anchorReducer->reduceAnchor($node->getId()); + $this->addLinkTargetToProject( + $compilerContext, + new InternalTarget( + $currentDocument->getFilePath(), $anchor, - new InternalTarget( - $currentDocument->getFilePath(), - $anchor, - $node->getLinkText(), - $node->getLinkType(), - ), - ); - } + $node->getLinkText(), + $node->getLinkType(), + ), + ); if ($node instanceof MultipleLinkTargetsNode) { foreach ($node->getAdditionalIds() as $id) { - if ($compilerContext->getProjectNode()->hasInternalTarget($id, $node->getLinkType())) { - $this->logger?->warning( - sprintf( - 'Duplicate anchor "%s" for link type "%s" in document "%s". The anchor is already used at "%s"', - $anchor, - $node->getLinkType(), - $compilerContext->getDocumentNode()->getFilePath(), - $compilerContext->getProjectNode()->getInternalTarget($anchor, $node->getLinkType())?->getDocumentPath(), - ), - $compilerContext->getLoggerInformation(), - ); - } - - $compilerContext->getProjectNode()->addLinkTarget( - $id, + $anchor = $this->anchorReducer->reduceAnchor($id); + $this->addLinkTargetToProject( + $compilerContext, new InternalTarget( $currentDocument->getFilePath(), - $id, + $anchor, $node->getLinkText(), $node->getLinkType(), ), @@ -144,4 +144,28 @@ public function getPriority(): int // After MetasPass return 5000; } + + private function addLinkTargetToProject(CompilerContext $compilerContext, InternalTarget $internalTarget): void + { + if ($compilerContext->getProjectNode()->hasInternalTarget($internalTarget->getAnchor(), $internalTarget->getLinkType())) { + $otherLink = $compilerContext->getProjectNode()->getInternalTarget($internalTarget->getAnchor(), $internalTarget->getLinkType()); + $this->logger?->warning( + sprintf( + 'Duplicate anchor "%s" for link type "%s" in document "%s". The anchor is already used at "%s"', + $internalTarget->getAnchor(), + $internalTarget->getLinkType(), + $compilerContext->getDocumentNode()->getFilePath(), + $otherLink?->getDocumentPath(), + ), + $compilerContext->getLoggerInformation(), + ); + + return; + } + + $compilerContext->getProjectNode()->addLinkTarget( + $internalTarget->getAnchor(), + $internalTarget, + ); + } } diff --git a/tests/Integration/tests/confval/expected/index.html b/tests/Integration/tests/confval/confval-name/expected/another.html similarity index 85% rename from tests/Integration/tests/confval/expected/index.html rename to tests/Integration/tests/confval/confval-name/expected/another.html index e89a6a525..f3a2925e9 100644 --- a/tests/Integration/tests/confval/expected/index.html +++ b/tests/Integration/tests/confval/confval-name/expected/another.html @@ -1,9 +1,16 @@ + + + + Confval directive + + +

Confval directive

-
+
demo
@@ -21,3 +28,5 @@

Confval directive

+ + diff --git a/tests/Integration/tests/confval/confval-name/expected/index.html b/tests/Integration/tests/confval/confval-name/expected/index.html new file mode 100644 index 000000000..0eabef45f --- /dev/null +++ b/tests/Integration/tests/confval/confval-name/expected/index.html @@ -0,0 +1,39 @@ + + + + Confval directive + + + + +
+

Confval directive

+ +
+
+ demo
+
+
+
Type: "Hello World"
+
Required: true
+
Custom Info: custom
+ +
+
+

This is the confval demo content!

Another paragraph.

+
+
+
+

See option demo.

+
+ +
+ +
+ + + + diff --git a/tests/Integration/tests/confval/confval-name/expected/objects.inv.json b/tests/Integration/tests/confval/confval-name/expected/objects.inv.json new file mode 100644 index 000000000..98ada9b4d --- /dev/null +++ b/tests/Integration/tests/confval/confval-name/expected/objects.inv.json @@ -0,0 +1,38 @@ +{ + "std:doc": { + "another": [ + "-", + "-", + "another.html", + "Confval directive" + ], + "index": [ + "-", + "-", + "index.html", + "Confval directive" + ] + }, + "std:label": { + "confval-directive": [ + "-", + "-", + "index.html#confval-directive", + "Confval directive" + ] + }, + "std:confval": { + "another-demo": [ + "-", + "-", + "another.html#another-demo", + "demo" + ], + "demo": [ + "-", + "-", + "index.html#demo", + "demo" + ] + } +} \ No newline at end of file diff --git a/tests/Integration/tests/confval/confval-name/input/another.rst b/tests/Integration/tests/confval/confval-name/input/another.rst new file mode 100644 index 000000000..56b34e298 --- /dev/null +++ b/tests/Integration/tests/confval/confval-name/input/another.rst @@ -0,0 +1,15 @@ +Confval directive +================= + +.. confval:: demo + :name: another-demo + :type: :php:`string` + :default: ``"Hello World"`` + :required: true + :Custom Info: **custom** + + This is the confval ``demo`` content! + + Another paragraph. + +See option :confval:`demo`. \ No newline at end of file diff --git a/tests/Integration/tests/confval/confval-name/input/index.rst b/tests/Integration/tests/confval/confval-name/input/index.rst new file mode 100644 index 000000000..68c08f569 --- /dev/null +++ b/tests/Integration/tests/confval/confval-name/input/index.rst @@ -0,0 +1,19 @@ +Confval directive +================= + +.. confval:: demo + :type: :php:`string` + :default: ``"Hello World"`` + :required: true + :Custom Info: **custom** + + This is the confval ``demo`` content! + + Another paragraph. + +See option :confval:`demo`. + +.. toctree:: + :glob: + + * diff --git a/tests/Integration/tests/confval/confval-warning/expected/another.html b/tests/Integration/tests/confval/confval-warning/expected/another.html new file mode 100644 index 000000000..71ebd17c1 --- /dev/null +++ b/tests/Integration/tests/confval/confval-warning/expected/another.html @@ -0,0 +1,32 @@ + + + + Confval directive + + + + +
+

Confval directive

+ +
+
+ demo
+
+
+
Type: "Hello World"
+
Required: true
+
Custom Info: custom
+ +
+
+

This is the confval demo content!

Another paragraph.

+
+
+
+

See option demo.

+
+ + + + diff --git a/tests/Integration/tests/confval/confval-warning/expected/index.html b/tests/Integration/tests/confval/confval-warning/expected/index.html new file mode 100644 index 000000000..4af893577 --- /dev/null +++ b/tests/Integration/tests/confval/confval-warning/expected/index.html @@ -0,0 +1,39 @@ + + + + Confval directive + + + + +
+

Confval directive

+ +
+
+ demo
+
+
+
Type: "Hello World"
+
Required: true
+
Custom Info: custom
+ +
+
+

This is the confval demo content!

Another paragraph.

+
+
+
+

See option demo.

+
+ +
+ +
+ + + + diff --git a/tests/Integration/tests/confval/confval-warning/expected/logs/warning.log b/tests/Integration/tests/confval/confval-warning/expected/logs/warning.log new file mode 100644 index 000000000..27f3fddd3 --- /dev/null +++ b/tests/Integration/tests/confval/confval-warning/expected/logs/warning.log @@ -0,0 +1 @@ +app.WARNING: Duplicate anchor "demo" for link type "std:confval" in document "index". The anchor is already used at "another" \ No newline at end of file diff --git a/tests/Integration/tests/confval/expected/objects.inv.json b/tests/Integration/tests/confval/confval-warning/expected/objects.inv.json similarity index 73% rename from tests/Integration/tests/confval/expected/objects.inv.json rename to tests/Integration/tests/confval/confval-warning/expected/objects.inv.json index 2dc5f234b..9692b5c17 100644 --- a/tests/Integration/tests/confval/expected/objects.inv.json +++ b/tests/Integration/tests/confval/confval-warning/expected/objects.inv.json @@ -1,5 +1,11 @@ { "std:doc": { + "another": [ + "-", + "-", + "another.html", + "Confval directive" + ], "index": [ "-", "-", @@ -19,7 +25,7 @@ "demo": [ "-", "-", - "index.html#demo", + "another.html#demo", "demo" ] } diff --git a/tests/Integration/tests/confval/input/index.rst b/tests/Integration/tests/confval/confval-warning/input/another.rst similarity index 100% rename from tests/Integration/tests/confval/input/index.rst rename to tests/Integration/tests/confval/confval-warning/input/another.rst diff --git a/tests/Integration/tests/confval/confval-warning/input/index.rst b/tests/Integration/tests/confval/confval-warning/input/index.rst new file mode 100644 index 000000000..68c08f569 --- /dev/null +++ b/tests/Integration/tests/confval/confval-warning/input/index.rst @@ -0,0 +1,19 @@ +Confval directive +================= + +.. confval:: demo + :type: :php:`string` + :default: ``"Hello World"`` + :required: true + :Custom Info: **custom** + + This is the confval ``demo`` content! + + Another paragraph. + +See option :confval:`demo`. + +.. toctree:: + :glob: + + *