fix some review comments

Author: matthewbastien

Package.swift

@@ -223,6 +223,7 @@ var targets: [Target] = [
       "BuildSystemIntegration",
       "LanguageServerProtocol",
       "SemanticIndex",
+      "SKLogging",
       "SwiftExtensions",
       .product(name: "IndexStoreDB", package: "indexstore-db"),
       .product(name: "SwiftDocC", package: "swift-docc"),

Sources/DocCDocumentation/BuildSystemIntegrationExtensions.swift

@@ -10,16 +10,17 @@
 //
 //===----------------------------------------------------------------------===//
 
-import BuildServerProtocol
+package import BuildServerProtocol
 package import BuildSystemIntegration
 package import Foundation
-package import LanguageServerProtocol
+import LanguageServerProtocol
 
 package extension BuildSystemManager {
-  func moduleName(for document: DocumentURI) async -> String? {
-    guard let target = await canonicalTarget(for: document) else {
-      return nil
-    }
+  /// Retrieves the name of the Swift module for a given target.
+  ///
+  /// - Parameter target: The build target identifier
+  /// - Returns: The name of the Swift module or nil if it could not be determined
+  func moduleName(for target: BuildTargetIdentifier) async -> String? {
     let sourceFiles = (try? await sourceFiles(in: [target]).flatMap(\.sources)) ?? []
     for sourceFile in sourceFiles {
       let language = await defaultLanguage(for: sourceFile.uri, in: target)
@@ -33,10 +34,11 @@ package extension BuildSystemManager {
     return nil
   }
 
-  func doccCatalog(for document: DocumentURI) async -> URL? {
-    guard let target = await canonicalTarget(for: document) else {
-      return nil
-    }
+  /// Finds the SwiftDocC documentation catalog associated with a target, if any.
+  ///
+  /// - Parameter target: The build target identifier
+  /// - Returns: The URL of the documentation catalog or nil if one could not be found
+  func doccCatalog(for target: BuildTargetIdentifier) async -> URL? {
     let sourceFiles = (try? await sourceFiles(in: [target]).flatMap(\.sources)) ?? []
     let catalogURLs = sourceFiles.compactMap { sourceItem -> URL? in
       guard sourceItem.dataKind == .sourceKit,

Sources/DocCDocumentation/DocCCatalogIndexManager.swift

@@ -15,26 +15,20 @@ package import Foundation
 
 final actor DocCCatalogIndexManager {
   private let server: DocCServer
-  private var catalogToIndexMap = [URL: Result<DocCCatalogIndex, DocCIndexError>]()
+  private var catalogToIndexMap: [URL: Result<DocCCatalogIndex, DocCIndexError>] = [:]
 
   init(server: DocCServer) {
     self.server = server
   }
 
-  func invalidate(catalogURLs: some Collection<URL>) {
-    guard catalogURLs.count > 0 else {
-      return
-    }
-    for catalogURL in catalogURLs {
-      catalogToIndexMap.removeValue(forKey: catalogURL)
-    }
+  func invalidate(_ url: URL) {
+    catalogToIndexMap.removeValue(forKey: url)
   }
 
   func index(for catalogURL: URL) async throws(DocCIndexError) -> DocCCatalogIndex {
     if let existingCatalog = catalogToIndexMap[catalogURL] {
       return try existingCatalog.get()
     }
-    let catalogIndexResult: Result<DocCCatalogIndex, DocCIndexError>
     do {
       let convertResponse = try await server.convert(
         externalIDsToConvert: [],
@@ -49,36 +43,28 @@ final actor DocCCatalogIndexManager {
         tutorialFiles: [],
         convertRequestIdentifier: UUID().uuidString
       )
-      catalogIndexResult = Result { convertResponse }
-        .flatMap { convertResponse in
-          guard let renderReferenceStoreData = convertResponse.renderReferenceStore else {
-            return .failure(.unexpectedlyNilRenderReferenceStore)
-          }
-          return .success(renderReferenceStoreData)
-        }
-        .flatMap { renderReferenceStoreData in
-          Result { try JSONDecoder().decode(RenderReferenceStore.self, from: renderReferenceStoreData) }
-            .flatMapError { .failure(.decodingFailure($0)) }
-        }
-        .map { DocCCatalogIndex(from: $0) }
+      guard let renderReferenceStoreData = convertResponse.renderReferenceStore else {
+        throw DocCIndexError.unexpectedlyNilRenderReferenceStore
+      }
+      let renderReferenceStore = try JSONDecoder().decode(RenderReferenceStore.self, from: renderReferenceStoreData)
+      let catalogIndex = DocCCatalogIndex(from: renderReferenceStore)
+      catalogToIndexMap[catalogURL] = .success(catalogIndex)
+      return catalogIndex
     } catch {
-      catalogIndexResult = .failure(.internalError(error))
+      let internalError = error as? DocCIndexError ?? DocCIndexError.internalError(error)
+      catalogToIndexMap[catalogURL] = .failure(internalError)
+      throw internalError
     }
-    catalogToIndexMap[catalogURL] = catalogIndexResult
-    return try catalogIndexResult.get()
   }
 }
 
 /// Represents a potential error that the ``DocCCatalogIndexManager`` could encounter while indexing
 package enum DocCIndexError: LocalizedError {
-  case decodingFailure(Error)
-  case internalError(LocalizedError)
+  case internalError(any Error)
   case unexpectedlyNilRenderReferenceStore
 
   package var errorDescription: String? {
     switch self {
-    case .decodingFailure(let decodingError):
-      return "Failed to decode a received message: \(decodingError.localizedDescription)"
     case .internalError(let internalError):
       return "An internal error occurred: \(internalError.localizedDescription)"
     case .unexpectedlyNilRenderReferenceStore:
@@ -89,14 +75,13 @@ package enum DocCIndexError: LocalizedError {
 
 package struct DocCCatalogIndex: Sendable {
   private let assetReferenceToDataAsset: [String: DataAsset]
-  private let fuzzyAssetReferenceToDataAsset: [String: DataAsset]
   private let documentationExtensionToSourceURL: [DocCSymbolLink: URL]
   let articlePathToSourceURLAndReference: [String: (URL, TopicRenderReference)]
   let tutorialPathToSourceURLAndReference: [String: (URL, TopicRenderReference)]
   let tutorialOverviewPathToSourceURLAndReference: [String: (URL, TopicRenderReference)]
 
   func asset(for assetReference: AssetReference) -> DataAsset? {
-    assetReferenceToDataAsset[assetReference.assetName] ?? fuzzyAssetReferenceToDataAsset[assetReference.assetName]
+    assetReferenceToDataAsset[assetReference.assetName]
   }
 
   package func documentationExtension(for symbolLink: DocCSymbolLink) -> URL? {
@@ -105,21 +90,15 @@ package struct DocCCatalogIndex: Sendable {
 
   init(from renderReferenceStore: RenderReferenceStore) {
     // Assets
-    var assetReferenceToDataAsset = [String: DataAsset]()
-    var fuzzyAssetReferenceToDataAsset = [String: DataAsset]()
+    var assetReferenceToDataAsset: [String: DataAsset] = [:]
     for (reference, asset) in renderReferenceStore.assets {
       var asset = asset
       asset.variants = asset.variants.compactMapValues { $0.withScheme("doc-asset") }
       assetReferenceToDataAsset[reference.assetName] = asset
-      if let indexOfExtensionDelimiter = reference.assetName.lastIndex(of: ".") {
-        let assetNameWithoutExtension = reference.assetName.prefix(upTo: indexOfExtensionDelimiter)
-        fuzzyAssetReferenceToDataAsset[String(assetNameWithoutExtension)] = asset
-      }
     }
     self.assetReferenceToDataAsset = assetReferenceToDataAsset
-    self.fuzzyAssetReferenceToDataAsset = fuzzyAssetReferenceToDataAsset
     // Markdown and Tutorial content
-    var documentationExtensionToSourceURL = [DocCSymbolLink: URL]()
+    var documentationExtensionToSourceURL: [DocCSymbolLink: URL] = [:]
     var articlePathToSourceURLAndReference = [String: (URL, TopicRenderReference)]()
     var tutorialPathToSourceURLAndReference = [String: (URL, TopicRenderReference)]()
     var tutorialOverviewPathToSourceURLAndReference = [String: (URL, TopicRenderReference)]()
@@ -129,27 +108,23 @@ package struct DocCCatalogIndex: Sendable {
       else {
         continue
       }
+      let lastPathComponent = renderReferenceKey.url.lastPathComponent
 
-      if topicContentValue.isDocumentationExtensionContent {
-        guard
+      switch topicRenderReference.kind {
+      case .article:
+        articlePathToSourceURLAndReference[lastPathComponent] = (topicContentSource, topicRenderReference)
+      case .tutorial:
+        tutorialPathToSourceURLAndReference[lastPathComponent] = (topicContentSource, topicRenderReference)
+      case .overview:
+        tutorialOverviewPathToSourceURLAndReference[lastPathComponent] = (topicContentSource, topicRenderReference)
+      default:
+        guard topicContentValue.isDocumentationExtensionContent,
           let absoluteSymbolLink = AbsoluteSymbolLink(string: topicContentValue.renderReference.identifier.identifier)
         else {
           continue
         }
         let doccSymbolLink = DocCSymbolLink(absoluteSymbolLink: absoluteSymbolLink)
         documentationExtensionToSourceURL[doccSymbolLink] = topicContentValue.source
-      } else if topicRenderReference.kind == .article {
-        articlePathToSourceURLAndReference[renderReferenceKey.url.lastPathComponent] = (
-          topicContentSource, topicRenderReference
-        )
-      } else if topicRenderReference.kind == .tutorial {
-        tutorialPathToSourceURLAndReference[renderReferenceKey.url.lastPathComponent] = (
-          topicContentSource, topicRenderReference
-        )
-      } else if topicRenderReference.kind == .overview {
-        tutorialOverviewPathToSourceURLAndReference[renderReferenceKey.url.lastPathComponent] = (
-          topicContentSource, topicRenderReference
-        )
       }
     }
     self.documentationExtensionToSourceURL = documentationExtensionToSourceURL

Sources/DocCDocumentation/DocCDocumentationManager.swift

@@ -3,6 +3,7 @@ package import BuildSystemIntegration
 package import Foundation
 package import IndexStoreDB
 package import LanguageServerProtocol
+import SKLogging
 package import SemanticIndex
 import SwiftDocC
 
@@ -27,7 +28,9 @@ package struct DocCDocumentationManager: Sendable {
 
   package func filesDidChange(_ events: [FileEvent]) async {
     for event in events {
-      guard let catalogURL = await buildSystemManager.doccCatalog(for: event.uri) else {
+      guard let target = await buildSystemManager.canonicalTarget(for: event.uri),
+        let catalogURL = await buildSystemManager.doccCatalog(for: target)
+      else {
         continue
       }
       await catalogIndexManager.invalidate(catalogURL)
@@ -42,27 +45,38 @@ package struct DocCDocumentationManager: Sendable {
     DocCSymbolLink(string: string)
   }
 
+  private func parentSymbol(of symbol: SymbolOccurrence, in index: CheckedIndex) -> SymbolOccurrence? {
+    let allParentRelations = symbol.relations
+      .filter { $0.roles.contains(.childOf) }
+      .sorted()
+    if allParentRelations.count > 1 {
+      logger.debug("Symbol \(symbol.symbol.usr) has multiple parent symbols")
+    }
+    guard let parentRelation = allParentRelations.first else {
+      return nil
+    }
+    if parentRelation.symbol.kind == .extension {
+      let allSymbolOccurrences = index.occurrences(relatedToUSR: parentRelation.symbol.usr, roles: .extendedBy)
+        .sorted()
+      if allSymbolOccurrences.count > 1 {
+        logger.debug("Extension \(parentRelation.symbol.usr) extends multiple symbols")
+      }
+      return allSymbolOccurrences.first
+    }
+    return index.primaryDefinitionOrDeclarationOccurrence(ofUSR: parentRelation.symbol.usr)
+  }
+
   package func symbolLink(forUSR usr: String, in index: CheckedIndex) -> DocCSymbolLink? {
     guard let topLevelSymbolOccurrence = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: usr) else {
       return nil
     }
     let module = topLevelSymbolOccurrence.location.moduleName
     var components = [topLevelSymbolOccurrence.symbol.name]
-    // Find any child symbols
-    var symbolOccurrence: SymbolOccurrence? = topLevelSymbolOccurrence
-    while let currentSymbolOccurrence = symbolOccurrence, components.count > 0 {
-      let parentRelation = currentSymbolOccurrence.relations.first { $0.roles.contains(.childOf) }
-      guard let parentRelation else {
-        break
-      }
-      if parentRelation.symbol.kind == .extension {
-        symbolOccurrence = index.occurrences(relatedToUSR: parentRelation.symbol.usr, roles: .extendedBy).first
-      } else {
-        symbolOccurrence = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: parentRelation.symbol.usr)
-      }
-      if let symbolOccurrence {
-        components.insert(symbolOccurrence.symbol.name, at: 0)
-      }
+    // Find any parent symbols
+    var symbolOccurrence: SymbolOccurrence = topLevelSymbolOccurrence
+    while let parentSymbolOccurrence = parentSymbol(of: symbolOccurrence, in: index) {
+      components.insert(parentSymbolOccurrence.symbol.name, at: 0)
+      symbolOccurrence = parentSymbolOccurrence
     }
     return DocCSymbolLink(string: module)?.appending(components: components)
   }
@@ -81,39 +95,47 @@ package struct DocCDocumentationManager: Sendable {
     }
     // Do a lookup to find the top level symbol
     let topLevelSymbolName = components.removeLast().name
-    var topLevelSymbolOccurrences = [SymbolOccurrence]()
+    var topLevelSymbolOccurrences: [SymbolOccurrence] = []
     index.forEachCanonicalSymbolOccurrence(byName: topLevelSymbolName) { symbolOccurrence in
       guard symbolOccurrence.location.moduleName == symbolLink.moduleName else {
-        return true
+        return true  // continue
       }
       topLevelSymbolOccurrences.append(symbolOccurrence)
-      return true
+      return true  // continue
     }
-    guard let topLevelSymbolOccurrence = topLevelSymbolOccurrences.first else {
-      return nil
-    }
-    // Find any child symbols
-    var symbolOccurrence: SymbolOccurrence? = topLevelSymbolOccurrence
-    while let currentSymbolOccurrence = symbolOccurrence, components.count > 0 {
-      let nextComponent = components.removeLast()
-      let parentRelation = currentSymbolOccurrence.relations.first {
-        $0.roles.contains(.childOf) && $0.symbol.name == nextComponent.name
-      }
-      guard let parentRelation else {
-        break
+    // Search each potential symbol's parents to find an exact match
+    let symbolOccurences = topLevelSymbolOccurrences.filter { topLevelSymbolOccurrence in
+      var components = components
+      var symbolOccurrence = topLevelSymbolOccurrence
+      while let parentSymbolOccurrence = parentSymbol(of: symbolOccurrence, in: index), !components.isEmpty {
+        let nextComponent = components.removeLast()
+        guard parentSymbolOccurrence.symbol.name == nextComponent.name else {
+          return false
+        }
+        symbolOccurrence = parentSymbolOccurrence
       }
-      if parentRelation.symbol.kind == .extension {
-        symbolOccurrence = index.occurrences(relatedToUSR: parentRelation.symbol.usr, roles: .extendedBy).first
-      } else {
-        symbolOccurrence = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: parentRelation.symbol.usr)
+      guard components.isEmpty else {
+        return false
       }
+      return true
+    }.sorted()
+    if symbolOccurences.count > 1 {
+      logger.debug("Multiple symbols found for DocC symbol link '\(symbolLink.absoluteString)'")
     }
-    guard symbolOccurrence != nil else {
-      return nil
-    }
-    return topLevelSymbolOccurrence
+    return symbolOccurences.first
   }
 
+  /// Generates the SwiftDocC RenderNode for a given symbol, tutorial, or markdown file.
+  ///
+  /// - Parameters:
+  ///   - symbolUSR: The USR of the symbol to render
+  ///   - symbolGraph: The symbol graph that includes the given symbol USR
+  ///   - markupFile: The markdown article or symbol extension to render
+  ///   - tutorialFile: The tutorial file to render
+  ///   - moduleName: The name of the Swift module that will be rendered
+  ///   - catalogURL: The URL pointing to the docc catalog that this symbol, tutorial, or markdown file is a part of
+  /// - Throws: A ResponseError if something went wrong
+  /// - Returns: The DoccDocumentationResponse containing the RenderNode if successful
   package func renderDocCDocumentation(
     symbolUSR: String? = nil,
     symbolGraph: String? = nil,
@@ -132,15 +154,15 @@ package struct DocCDocumentationManager: Sendable {
         overridingDocumentationComments[symbolUSR] = overrideDocComments
       }
     }
-    var symbolGraphs = [Data]()
+    var symbolGraphs: [Data] = []
     if let symbolGraphData = symbolGraph?.data(using: .utf8) {
       symbolGraphs.append(symbolGraphData)
     }
-    var markupFiles = [Data]()
+    var markupFiles: [Data] = []
     if let markupFile = markupFile?.data(using: .utf8) {
       markupFiles.append(markupFile)
     }
-    var tutorialFiles = [Data]()
+    var tutorialFiles: [Data] = []
     if let tutorialFile = tutorialFile?.data(using: .utf8) {
       tutorialFiles.append(tutorialFile)
     }