Prevent a manual technology root from overriding the module node with the same name (#770)

* Don't override the module node with a same-name manual technology root

rdar://119389007

* Preserve symbol kinds when deserializing a path hierarchy from another archive

* Add comments to describe why symbol kinds are filled in after creation

Author: d-ronnqvist

Sources/SwiftDocC/Infrastructure/Link Resolution/LinkResolver.swift

@@ -32,8 +32,8 @@ public class LinkResolver {
             try ExternalPathHierarchyResolver(dependencyArchive: $0)
         }
         for resolver in resolvers {
-            for moduleName in resolver.pathHierarchy.modules.keys {
-                self.externalResolvers[moduleName] = resolver
+            for moduleNode in resolver.pathHierarchy.modules {
+                self.externalResolvers[moduleNode.name] = resolver
             }
         }
     }

Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+DisambiguatedPaths.swift

@@ -68,10 +68,10 @@ extension PathHierarchy {
 
         var gathered: [(String, (String, Bool))] = []
 
-        for (moduleName, node) in modules {
-            let path = "/" + moduleName
+        for node in modules {
+            let path = "/" + node.name
             gathered.append(
-                (moduleName, (path, node.symbol == nil || node.symbol!.identifier.interfaceLanguage == "swift"))
+                (node.name, (path, node.symbol == nil || node.symbol!.identifier.interfaceLanguage == "swift"))
             )
             gathered += descend(node, accumulatedPath: path)
         }

Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Dump.swift

@@ -47,7 +47,7 @@ private extension PathHierarchy.Node {
 extension PathHierarchy {
     /// Creates a visual representation or the path hierarchy for debugging.
     func dump() -> String {
-        var children = modules.sorted(by: \.key).map { $0.value.dumpableNode() }
+        var children = modules.sorted(by: \.name).map { $0.dumpableNode() }
         if articlesContainer.symbol == nil {
             children.append(articlesContainer.dumpableNode()) // The article parent can be the same node as the module
         }

Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Find.swift

@@ -85,12 +85,12 @@ extension PathHierarchy {
         // A function to avoid repeating the
         func searchForNodeInModules() throws -> Node {
             // Note: This captures `parentID`, `remaining`, and `rawPathForError`.
-            if let moduleMatch = modules[firstComponent.full]  ?? modules[String(firstComponent.name)] {
+            if let moduleMatch = modules.first(where: { $0.matches(firstComponent) }) {
                 return try searchForNode(descendingFrom: moduleMatch, pathComponents: remaining.dropFirst(), onlyFindSymbols: onlyFindSymbols, rawPathForError: rawPath)
             }
             if modules.count == 1 {
                 do {
-                    return try searchForNode(descendingFrom: modules.first!.value, pathComponents: remaining, onlyFindSymbols: onlyFindSymbols, rawPathForError: rawPath)
+                    return try searchForNode(descendingFrom: modules.first!, pathComponents: remaining, onlyFindSymbols: onlyFindSymbols, rawPathForError: rawPath)
                 } catch let error as PathHierarchy.Error {
                     switch error {
                     case .notFound:
@@ -115,13 +115,13 @@ extension PathHierarchy {
                     }
                 }
             }
-            let topLevelNames = Set(modules.keys + [articlesContainer.name, tutorialContainer.name])
+            let topLevelNames = Set(modules.map(\.name) + [articlesContainer.name, tutorialContainer.name])
 
             if isAbsolute, FeatureFlags.current.isExperimentalLinkHierarchySerializationEnabled {
                 throw Error.moduleNotFound(
                     pathPrefix: pathForError(of: rawPath, droppingLast: remaining.count),
                     remaining: Array(remaining),
-                    availableChildren: Set(modules.keys)
+                    availableChildren: Set(modules.map(\.name))
                 )
             } else {
                 throw Error.notFound(
@@ -475,7 +475,12 @@ private extension Sequence {
 
 private extension PathHierarchy.Node {
     func matches(_ component: PathHierarchy.PathComponent) -> Bool {
-        if let symbol = symbol {
+        // Check the full path component first in case the node's name has a suffix that could be mistaken for a hash disambiguation.
+        if name == component.full {
+            return true
+        }
+        // Otherwise, check if the node's symbol matches the provided disambiguation
+        else if let symbol = symbol {
             guard name == component.name else {
                 return false
             }
@@ -486,9 +491,9 @@ private extension PathHierarchy.Node {
                 return false
             }
             return true
-        } else {
-            return name == component.full
         }
+        
+        return false
     }
 
     func anyChildMatches(_ component: PathHierarchy.PathComponent) -> Bool {

Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Serialization.swift

@@ -60,7 +60,7 @@ extension PathHierarchy.FileRepresentation {
         }
 
         self.nodes = nodes
-        self.modules = pathHierarchy.modules.mapValues({ identifierMap[$0.identifier]! })
+        self.modules = pathHierarchy.modules.map({ identifierMap[$0.identifier]! })
         self.articlesContainer = identifierMap[pathHierarchy.articlesContainer.identifier]!
         self.tutorialContainer = identifierMap[pathHierarchy.tutorialContainer.identifier]!
         self.tutorialOverviewContainer = identifierMap[pathHierarchy.tutorialOverviewContainer.identifier]!
@@ -93,7 +93,7 @@ extension PathHierarchy {
         var nodes: [Node]
 
         /// The module nodes in this hierarchy.
-        var modules: [String: Int]
+        var modules: [Int]
         /// The container for articles and reference documentation.
         var articlesContainer: Int
         /// The container of tutorials.
@@ -174,7 +174,7 @@ extension PathHierarchyBasedLinkResolver {
         }
 
         return SerializableLinkResolutionInformation(
-            version: .init(major: 0, minor: 0, patch: 1), // This is still in development
+            version: .init(major: 0, minor: 1, patch: 0), // This is still in development
             bundleID: bundleID,
             pathHierarchy: hierarchyFileRepresentation,
             nonSymbolPaths: nonSymbolPaths

Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift

@@ -37,8 +37,8 @@ struct ResolvedIdentifier: Equatable, Hashable {
 /// After a path hierarchy has been fully created — with both symbols and non-symbols — it can be used to find elements in the hierarchy and to determine the least disambiguated paths for all elements.
 struct PathHierarchy {
 
-    /// A map of module names to module nodes.
-    private(set) var modules: [String: Node]
+    /// The list of module nodes.
+    private(set) var modules: [Node]
     /// The container of top-level articles in the documentation hierarchy.
     let articlesContainer: Node
     /// The container of tutorials in the documentation hierarchy.
@@ -295,7 +295,7 @@ struct PathHierarchy {
             """
         )
 
-        self.modules = roots
+        self.modules = Array(roots.values)
         self.lookup = lookup
 
         assert(topLevelSymbols().allSatisfy({ lookup[$0] != nil }))
@@ -352,7 +352,7 @@ struct PathHierarchy {
         newNode.identifier = newReference
         self.lookup[newReference] = newNode
 
-        modules[name] = newNode
+        modules.append(newNode)
 
         return newReference
     }
@@ -377,7 +377,7 @@ extension PathHierarchy {
 
         fileprivate(set) unowned var parent: Node?
         /// The symbol, if a node has one.
-        private(set) var symbol: SymbolGraph.Symbol?
+        fileprivate(set) var symbol: SymbolGraph.Symbol?
 
         /// If the path hierarchy should disfavor this node in a link collision.
         ///
@@ -452,7 +452,7 @@ extension PathHierarchy {
     func topLevelSymbols() -> [ResolvedIdentifier] {
         var result: Set<ResolvedIdentifier> = []
         // Roots represent modules and only have direct symbol descendants.
-        for root in modules.values {
+        for root in modules {
             for (_, tree) in root.children {
                 for subtree in tree.storage.values {
                     for node in subtree.values where node.symbol != nil {
@@ -461,7 +461,7 @@ extension PathHierarchy {
                 }
             }
         }
-        return Array(result) + modules.values.map { $0.identifier }
+        return Array(result) + modules.map { $0.identifier }
     }
 }
 
@@ -566,6 +566,8 @@ extension PathHierarchy {
                     pathComponents: [],
                     docComment: nil,
                     accessLevel: .public,
+                    // To make the file format smaller we don't store the symbol kind identifiers with each node. Instead, the kind identifier is stored
+                    // as disambiguation and is filled in while building up the hierarchy below.
                     kind: SymbolGraph.Symbol.Kind(rawIdentifier: "", displayName: ""),
                     mixins: [:]
                 )
@@ -584,11 +586,21 @@ extension PathHierarchy {
                 let childNode = lookup[identifiers[child.nodeID]]!
                 // Even if this is a symbol node, explicitly pass the kind and hash disambiguation.
                 node.add(child: childNode, kind: child.kind, hash: child.hash)
+                if let kind = child.kind {
+                    // Since the symbol was created with an empty symbol kind, fill in its kind identifier here.
+                    childNode.symbol?.kind.identifier = .init(identifier: kind)
+                }
             }
         }
 
         self.lookup = lookup
-        self.modules = fileRepresentation.modules.mapValues({ lookup[identifiers[$0]]! })
+        let modules = fileRepresentation.modules.map({ lookup[identifiers[$0]]! })
+        // Fill in the symbol kind of all modules. This is needed since the modules were created with empty symbol kinds and since no other symbol has a 
+        // module as its child, so the modules didn't get their symbol kind set when building up the hierarchy above.
+        for node in modules {
+            node.symbol?.kind.identifier = .module
+        }
+        self.modules = modules
         self.articlesContainer = lookup[identifiers[fileRepresentation.articlesContainer]]!
         self.tutorialContainer = lookup[identifiers[fileRepresentation.tutorialContainer]]!
         self.tutorialOverviewContainer = lookup[identifiers[fileRepresentation.tutorialOverviewContainer]]!

Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchyBasedLinkResolver.swift

@@ -65,7 +65,7 @@ final class PathHierarchyBasedLinkResolver {
 
     /// Returns a list of all module symbols.
     func modules() -> [ResolvedTopicReference] {
-        return pathHierarchy.modules.values.map { resolvedReferenceMap[$0.identifier]! }
+        return pathHierarchy.modules.map { resolvedReferenceMap[$0.identifier]! }
     }
 
     // MARK: - Adding non-symbols

Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift

@@ -1755,6 +1755,37 @@ class PathHierarchyTests: XCTestCase {
         XCTAssertEqual(paths[memberID], "/ModuleName/ContainerName-qwwf/MemberName1")
     }
 
+    func testModuleAndCollidingTechnologyRootHasPathsForItsSymbols() throws {
+        let symbolID = "some-symbol-id"
+        
+        let exampleDocumentation = Folder(name: "unit-test.docc", content: [
+            JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(
+                moduleName: "ModuleName",
+                symbols: [
+                    (symbolID, .swift, ["SymbolName"]),
+                ],
+                relationships: []
+            )),
+            
+            TextFile(name: "ModuleName.md", utf8Content: """
+            # Manual Technology Root
+            
+            @Metadata {
+              @TechnologyRoot
+            }
+            
+            A technology root with the same file name as the module name.
+            """)
+        ])
+        
+        let tempURL = try createTempFolder(content: [exampleDocumentation])
+        let (_, _, context) = try loadBundle(from: tempURL)
+        let tree = try XCTUnwrap(context.linkResolver.localResolver?.pathHierarchy)
+        
+        let paths = tree.caseInsensitiveDisambiguatedPaths(includeDisambiguationForUnambiguousChildren: true)
+        XCTAssertEqual(paths[symbolID], "/ModuleName/SymbolName")
+    }
+    
     func testMultiPlatformModuleWithExtension() throws {
         let (_, context) = try testBundleAndContext(named: "MultiPlatformModuleWithExtension")
         let tree = try XCTUnwrap(context.linkResolver.localResolver?.pathHierarchy)