Make it easier to support other link disambiguation in the future (#824)

d-ronnqvist · web-flow · commit f0a3d7a1e2a0 · 2024-02-14T19:34:51.000+01:00
* Make it easier to support other types of disambiguation in the future

* Don't globally register the extended symbol kinds
diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Error.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Error.swift
@@ -1,7 +1,7 @@
 /*
  This source file is part of the Swift.org open source project
 
- Copyright (c) 2023 Apple Inc. and the Swift project authors
+ Copyright (c) 2023-2024 Apple Inc. and the Swift project authors
  Licensed under Apache License v2.0 with Runtime Library Exception
 
  See https://swift.org/LICENSE.txt for license information
@@ -169,7 +169,7 @@ extension PathHierarchy.Error {
             // Use the authored disambiguation to try and reduce the possible near misses. For example, if the link was disambiguated with `-struct` we should
             // only make suggestions for similarly spelled structs.
             let filteredNearMisses = nearMisses.filter { name in
-                (try? partialResult.node.children[name]?.find(nextPathComponent.kind.map(String.init), nextPathComponent.hash.map(String.init))) != nil
+                (try? partialResult.node.children[name]?.find(nextPathComponent.disambiguation)) != nil
             }
 
             let pathPrefix = partialResult.pathPrefix
diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Find.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Find.swift
@@ -1,7 +1,7 @@
 /*
  This source file is part of the Swift.org open source project
 
- Copyright (c) 2023 Apple Inc. and the Swift project authors
+ Copyright (c) 2023-2024 Apple Inc. and the Swift project authors
  Licensed under Apache License v2.0 with Runtime Library Exception
 
  See https://swift.org/LICENSE.txt for license information
@@ -206,7 +206,7 @@ extension PathHierarchy {
             let (children, pathComponent) = try findChildContainer(node: &node, remaining: remaining, rawPathForError: rawPathForError)
             
             do {
-                guard let child = try children.find(pathComponent.kind, pathComponent.hash) else {
+                guard let child = try children.find(pathComponent.disambiguation) else {
                     // The search has ended with a node that doesn't have a child matching the next path component.
                     throw makePartialResultError(node: node, remaining: remaining, rawPathForError: rawPathForError)
                 }
@@ -252,7 +252,7 @@ extension PathHierarchy {
                 // Look ahead one path component to narrow down the list of collisions. 
                 // For each collision where the next path component can be found unambiguously, return that matching node one level down.
                 let possibleMatchesOneLevelDown = collisions.compactMap {
-                    return try? $0.node.children[String(nextPathComponent.name)]?.find(nextPathComponent.kind, nextPathComponent.hash)
+                    return try? $0.node.children[String(nextPathComponent.name)]?.find(nextPathComponent.disambiguation)
                 }
                 let onlyPossibleMatch: Node?
                 
@@ -382,8 +382,7 @@ extension PathHierarchy {
         if let match = node.children[pathComponent.full] {
             // The path component parsing may treat dash separated words as disambiguation information.
             // If the parsed name didn't match, also try the original.
-            pathComponent.kind = nil
-            pathComponent.hash = nil
+            pathComponent.disambiguation = nil
             return (match, pathComponent)
         } else if let match = node.children[String(pathComponent.name)] {
             return (match, pathComponent)
@@ -404,16 +403,24 @@ extension PathHierarchy.DisambiguationContainer {
         case lookupCollision([(node: PathHierarchy.Node, disambiguation: String)])
     }
     
-    fileprivate func find(_ kind: Substring?, _ hash: Substring?) throws -> PathHierarchy.Node? {
-        try find(kind.map(String.init), hash.map(String.init))
-    }
     /// Attempts to find a value in the disambiguation tree based on partial disambiguation information.
     ///
     /// There are 3 possible results:
     ///  - No match is found; indicated by a `nil` return value.
     ///  - Exactly one match is found; indicated by a non-nil return value.
     ///  - More than one match is found; indicated by a raised error listing the matches and their missing disambiguation.
-    func find(_ kind: String?, _ hash: String?) throws -> PathHierarchy.Node? {
+    func find(_ disambiguation: PathHierarchy.PathComponent.Disambiguation?) throws -> PathHierarchy.Node? {
+        var kind: String?
+        var hash: String?
+        switch disambiguation {
+        case .kindAndHash(kind: let maybeKind, hash: let maybeHash):
+            kind = maybeKind.map(String.init)
+            hash = maybeHash.map(String.init)
+        case nil:
+            kind = nil
+            hash = nil
+        }
+        
         if let kind = kind {
             // Need to match the provided kind
             guard let subtree = storage[kind] else { return nil }
@@ -480,17 +487,13 @@ private extension PathHierarchy.Node {
             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
+        else if let symbol = symbol, let disambiguation = component.disambiguation {
+            switch disambiguation {
+            case .kindAndHash(let kind, let hash):
+                return name == component.name
+                    && (kind == nil || kind! == symbol.kind.identifier.identifier)
+                    && (hash == nil || hash! == symbol.identifier.precise.stableHashString)
             }
-            if let kind = component.kind, kind != symbol.kind.identifier.identifier {
-                return false
-            }
-            if let hash = component.hash, hash != symbol.identifier.precise.stableHashString {
-                return false
-            }
-            return true
         }
         
         return false
diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+PathComponent.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+PathComponent.swift
@@ -1,7 +1,7 @@
 /*
  This source file is part of the Swift.org open source project
 
- Copyright (c) 2023 Apple Inc. and the Swift project authors
+ Copyright (c) 2023-2024 Apple Inc. and the Swift project authors
  Licensed under Apache License v2.0 with Runtime Library Exception
 
  See https://swift.org/LICENSE.txt for license information
@@ -14,16 +14,16 @@ import SymbolKit
 ///
 /// This is used to identify parsed path components as kind information.
 private let knownSymbolKinds: Set<String> = {
-    // There's nowhere else that registers these extended symbol kinds and we need to know them in this list.
-    SymbolGraph.Symbol.KindIdentifier.register(
+    // We don't want to register these extended symbol kinds because that makes them available for decoding from symbol graphs which is unexpected.
+    let knownKinds = SymbolGraph.Symbol.KindIdentifier.allCases + [
         .extendedProtocol,
         .extendedStructure,
         .extendedClass,
         .extendedEnumeration,
         .unknownExtendedType,
         .extendedModule
-    )
-    return Set(SymbolGraph.Symbol.KindIdentifier.allCases.map(\.identifier))
+    ]
+    return Set(knownKinds.map(\.identifier))
 }()
 
 /// All known source language identifiers.
@@ -38,10 +38,13 @@ extension PathHierarchy {
         let full: String
         /// The parsed entity name
         let name: Substring
-        /// The parsed entity kind, if any.
-        var kind: Substring?
-        /// The parsed entity hash, if any.
-        var hash: Substring?
+        /// The parsed disambiguation information, if any.
+        var disambiguation: Disambiguation?
+        
+        enum Disambiguation {
+            /// This path component uses a combination of kind and hash disambiguation
+            case kindAndHash(kind: Substring?, hash: Substring?)
+        }
     }
     
     enum PathParser {
@@ -65,13 +68,14 @@ extension PathHierarchy.PathParser {
     static func parse(pathComponent original: Substring) -> PathComponent {
         let full = String(original)
         guard let dashIndex = original.lastIndex(of: "-") else {
-            return PathComponent(full: full, name: full[...], kind: nil, hash: nil)
+            return PathComponent(full: full, name: full[...], disambiguation: nil)
         }
         
         let hash = original[dashIndex...].dropFirst()
         let name = original[..<dashIndex]
         
         func isValidHash(_ hash: Substring) -> Bool {
+            // Checks if a string looks like a truncated, lowercase FNV-1 hash string.
             var index: UInt8 = 0
             for char in hash.utf8 {
                 guard index <= 5, (48...57).contains(char) || (97...122).contains(char) else { return false }
@@ -82,28 +86,28 @@ extension PathHierarchy.PathParser {
         
         if knownSymbolKinds.contains(String(hash)) {
             // The parsed hash value is a symbol kind
-            return PathComponent(full: full, name: name, kind: hash, hash: nil)
+            return PathComponent(full: full, name: name, disambiguation: .kindAndHash(kind: hash, hash: nil))
         }
         if let languagePrefix = knownLanguagePrefixes.first(where: { hash.starts(with: $0) }) {
             // The hash is actually a symbol kind with a language prefix
-            return PathComponent(full: full, name: name, kind: hash.dropFirst(languagePrefix.count), hash: nil)
+            return PathComponent(full: full, name: name, disambiguation: .kindAndHash(kind: hash.dropFirst(languagePrefix.count), hash: nil))
         }
         if !isValidHash(hash) {
             // The parsed hash is neither a symbol not a valid hash. It's probably a hyphen-separated name.
-            return PathComponent(full: full, name: full[...], kind: nil, hash: nil)
+            return PathComponent(full: full, name: full[...], disambiguation: nil)
         }
         
         if let dashIndex = name.lastIndex(of: "-") {
             let kind = name[dashIndex...].dropFirst()
             let name = name[..<dashIndex]
             if knownSymbolKinds.contains(String(kind)) {
-                return PathComponent(full: full, name: name, kind: kind, hash: hash)
+                return PathComponent(full: full, name: name, disambiguation: .kindAndHash(kind: kind, hash: hash))
             } else if let languagePrefix = knownLanguagePrefixes.first(where: { kind.starts(with: $0) }) {
                 let kindWithoutLanguage = kind.dropFirst(languagePrefix.count)
-                return PathComponent(full: full, name: name, kind: kindWithoutLanguage, hash: hash)
+                return PathComponent(full: full, name: name, disambiguation: .kindAndHash(kind: kindWithoutLanguage, hash: hash))
             }
         }
-        return PathComponent(full: full, name: name, kind: nil, hash: hash)
+        return PathComponent(full: full, name: name, disambiguation: .kindAndHash(kind: nil, hash: hash))
     }
     
     static func split(_ path: String) -> (componentSubstrings: [Substring], isAbsolute: Bool) {
diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift
@@ -198,7 +198,7 @@ struct PathHierarchy {
                         return original
                     }
                 }(node.symbol!)[...].dropLast()
-                while !components.isEmpty, let child = try? parent.children[components.first!]?.find(nil, nil) {
+                while !components.isEmpty, let child = try? parent.children[components.first!]?.find(nil) {
                     parent = child
                     components = components.dropFirst()
                 }
@@ -210,7 +210,12 @@ struct PathHierarchy {
                     let component = PathParser.parse(pathComponent: component[...])
                     let nodeWithoutSymbol = Node(name: String(component.name))
                     nodeWithoutSymbol.isDisfavoredInCollision = true
-                    parent.add(child: nodeWithoutSymbol, kind: component.kind.map(String.init), hash: component.hash.map(String.init))
+                    switch component.disambiguation {
+                    case .kindAndHash(kind: let kind, hash: let hash):
+                        parent.add(child: nodeWithoutSymbol, kind: kind.map(String.init), hash: hash.map(String.init))
+                    case nil:
+                        parent.add(child: nodeWithoutSymbol, kind: nil, hash: nil)
+                    }
                     parent = nodeWithoutSymbol
                 }
                 parent.add(symbolChild: node)
@@ -425,7 +430,7 @@ extension PathHierarchy {
         fileprivate func add(child: Node, kind: String?, hash: String?) {
             guard child.parent !== self else { 
                 assert(
-                    (try? children[child.name]?.find(kind, hash)) === child,
+                    (try? children[child.name]?.find(.kindAndHash(kind: kind?[...], hash: hash?[...]))) === child,
                     "If the new child node already has this node as its parent it should already exist among this node's children."
                 )
                 return
diff --git a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift
