Index external entities after local entities

Sources/SwiftDocC/Infrastructure/ConvertActionConverter.swift

@@ -100,16 +100,7 @@ package enum ConvertActionConverter {
 
         let resultsSyncQueue = DispatchQueue(label: "Convert Serial Queue", qos: .unspecified, attributes: [])
         let resultsGroup = DispatchGroup()
-
-        // Consume external links and add them into the sidebar.
-        for externalLink in context.externalCache {
-            // Here we're associating the external node with the **current** bundle's bundle ID.
-            // This is needed because nodes are only considered children if the parent and child's bundle ID match.
-            // Otherwise, the node will be considered as a separate root node and displayed separately.
-            let externalRenderNode = ExternalRenderNode(externalEntity: externalLink.value, bundleIdentifier: context.inputs.id)
-            try outputConsumer.consume(externalRenderNode: externalRenderNode)
-        }
-
+
         let renderSignpostHandle = signposter.beginInterval("Render", id: signposter.makeSignpostID(), "Render \(context.knownPages.count) pages")
 
         var conversionProblems: [Problem] = context.knownPages.concurrentPerform { identifier, results in
@@ -172,7 +163,27 @@ package enum ConvertActionConverter {
         signposter.endInterval("Render", renderSignpostHandle)
 
         guard !Task.isCancelled else { return [] }
-
+
+        // Consumes all external links and adds them into the sidebar.
+        // This consumes all external links referenced across all content, and indexes them so they're available for reference in the navigator.
+        // This is not ideal as it means that links outside of the Topics section can impact the content of the navigator.
+        // TODO: It would be more correct to only index external links which have been curated as part of the Topics section.
+        //
+        // This has to run after all local nodes have been indexed because we're associating the external node with the **current** bundle's bundle ID,
+        // which makes it possible for there be clashes between local and external render nodes.
+        // When there are duplicate nodes, only the first one will be indexed,
+        // so in order to prefer local entities whenever there are any clashes, we have to index external nodes second.
+        // TODO: External render nodes should be associated with the correct bundle identifier.
+        try signposter.withIntervalSignpost("Index external links", id: signposter.makeSignpostID()) {
+            for externalLink in context.externalCache {
+                // Here we're associating the external node with the **current** bundle's bundle ID.
+                // This is needed because nodes are only considered children if the parent and child's bundle ID match.
+                // Otherwise, the node will be considered as a separate root node and displayed separately.
+                let externalRenderNode = ExternalRenderNode(externalEntity: externalLink.value, bundleIdentifier: context.inputs.id)
+                try outputConsumer.consume(externalRenderNode: externalRenderNode)
+            }
+        }
+
         // Write various metadata
         if emitDigest {
             signposter.withIntervalSignpost("Emit digest", id: signposter.makeSignpostID()) {

Tests/CommandLineTests/ConvertActionTests.swift

@@ -3186,6 +3186,160 @@ class ConvertActionTests: XCTestCase {
             "/images/unit-test/image-name~dark@2x.png"
         ])
     }
+
+    func testExternalLinksInContentDontAffectNavigatorIndex() async throws {
+        let temporaryFolder = try createTemporaryDirectory()
+
+        let resolver: OutOfProcessReferenceResolver
+        do {
+            let executableLocation = temporaryFolder.appendingPathComponent("link-resolver-executable")
+
+            let externalBundleID = "com.external.test"
+            let externalSummary = LinkDestinationSummary(
+                kind: .class,
+                language: .swift,
+                relativePresentationURL: URL(string: "/documentation/TestBundle/SampleClass")!,
+                referenceURL: URL(string: "doc://\(externalBundleID)/documentation/TestBundle/SampleClass")!,
+                title: "SampleClass",
+                abstract: [.text("External abstract")],
+                availableLanguages: [.swift],
+                references: [],
+                variants: []
+            )
+            let encodedResponse = try String(decoding: JSONEncoder().encode(OutOfProcessReferenceResolver.ResponseV2.resolved(externalSummary)), as: UTF8.self)
+
+            try """
+        #!/bin/bash
+        echo '{"bundleIdentifier":"\(externalBundleID)","capabilities":0}'        # Write this resolver's bundle identifier
+        read                                                                    # Wait for docc to send a symbol USR
+        echo '\(encodedResponse)'                                             # Respond with the resolved link summary
+        """.write(to: executableLocation, atomically: true, encoding: .utf8)
+
+            // `0o0700` is `-rwx------` (read, write, & execute only for owner)
+            try FileManager.default.setAttributes([.posixPermissions: 0o0700], ofItemAtPath: executableLocation.path)
+            XCTAssert(FileManager.default.isExecutableFile(atPath: executableLocation.path))
+
+            resolver = try OutOfProcessReferenceResolver(processLocation: executableLocation, errorOutputHandler: { _ in })
+        }
+
+        let catalog = Folder(name: "unit-test.docc", content: [
+            InfoPlist(displayName: "TestBundle", identifier: "com.test.example"),
+            TextFile(name: "Article.md", utf8Content: """
+            # Article
+
+            This is an internal article with an external link <doc://com.external.test/documentation/TestBundle/SampleClass> which clashes with the curated local link.
+
+            External links in content should not affect the navigator.
+
+            ## Topics
+
+            - ``SampleClass``
+            """),
+            TextFile(name: "SampleClass.md", utf8Content: """
+            # ``SampleClass``
+
+            This extends the documentation for this symbol.
+
+            ## Topics
+
+            - <doc:ChildArticleA>
+            - <doc:ChildArticleB>
+            """),
+            TextFile(name: "ChildArticleA.md", utf8Content: """
+            # ChildArticleA
+
+            A child article.
+            """),
+            TextFile(name: "ChildArticleB.md", utf8Content: """
+            # ChildArticleB
+
+            A child article.
+            """),
+            // Symbol graph with a class that matches an external link path
+            JSONFile(name: "TestBundle.symbols.json", content: makeSymbolGraph(moduleName: "TestBundle", symbols: [
+                makeSymbol(id: "some-symbol-id", language: .swift, kind: .class, pathComponents: ["SampleClass"])
+            ])),
+        ])
+
+        let catalogURL = try catalog.write(inside: temporaryFolder)
+        let targetURL = temporaryFolder.appendingPathComponent("Output.doccarchive", isDirectory: true)
+
+        let action = try ConvertAction(
+            documentationBundleURL: catalogURL,
+            outOfProcessResolver: resolver,
+            analyze: false,
+            targetDirectory: targetURL,
+            htmlTemplateDirectory: nil,
+            emitDigest: false,
+            currentPlatforms: nil,
+            fileManager: FileManager.default,
+            temporaryDirectory: createTemporaryDirectory()
+        )
+
+        let result = try await action.perform(logHandle: .none)
+
+        XCTAssertEqual(result.outputs, [targetURL])
+
+        let renderIndexURL = targetURL.appendingPathComponent("index", isDirectory: true)
+            .appendingPathComponent("index.json", isDirectory: false)
+
+        XCTAssertEqual(
+            try RenderIndex.fromURL(renderIndexURL),
+            try RenderIndex.fromString(
+                """
+                {
+                  "includedArchiveIdentifiers": [
+                    "com.test.example"
+                  ],
+                  "interfaceLanguages": {
+                    "swift": [
+                      {
+                        "children": [
+                          {
+                            "title": "Articles",
+                            "type": "groupMarker"
+                          },
+                          {
+                            "children": [
+                              {
+                                "children": [
+                                  {
+                                    "path": "/documentation/testbundle/childarticlea",
+                                    "title": "ChildArticleA",
+                                    "type": "article"
+                                  },
+                                  {
+                                    "path": "/documentation/testbundle/childarticleb",
+                                    "title": "ChildArticleB",
+                                    "type": "article"
+                                  }
+                                ],
+                                "path": "/documentation/testbundle/sampleclass",
+                                "title": "SampleClass",
+                                "type": "class"
+                              }
+                            ],
+                            "path": "/documentation/testbundle/article",
+                            "title": "Article",
+                            "type": "symbol"
+                          }
+                        ],
+                        "path": "/documentation/testbundle",
+                        "title": "TestBundle",
+                        "type": "module"
+                      }
+                    ]
+                  },
+                  "schemaVersion": {
+                    "major": 0,
+                    "minor": 1,
+                    "patch": 2
+                  }
+                }
+                """
+            )
+        )
+    }
 
     #endif
 }
@@ -3280,3 +3434,15 @@ private extension ConvertAction {
         return try await perform(logHandle: &logHandle)
     }
 }
+
+extension RenderIndex {
+    static func fromString(_ string: String) throws -> RenderIndex {
+        let decoder = JSONDecoder()
+        return try decoder.decode(RenderIndex.self, from: Data(string.utf8))
+    }
+
+    static func fromURL(_ url: URL) throws -> RenderIndex? {
+        let data = try Data(contentsOf: url)
+        return try JSONDecoder().decode(RenderIndex.self, from: data)
+    }
+}