load cross-import overlay module data, even for extensions (#332)

QuietMisdreavus · web-flow · commit 57d09b306980 · 2022-07-11T08:10:13.000-06:00
rdar://94736726
diff --git a/Package.resolved b/Package.resolved
diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift
@@ -986,7 +986,7 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
     
     /// Creates a topic graph node and a documentation node for the given symbol.
     private func preparedSymbolData(_ symbol: UnifiedSymbolGraph.Symbol, reference: ResolvedTopicReference, module: SymbolGraph.Module, moduleReference: ResolvedTopicReference, bundle: DocumentationBundle, fileURL symbolGraphURL: URL?) -> AddSymbolResultWithProblems {
-        let documentation = DocumentationNode(reference: reference, unifiedSymbol: symbol, platformName: module.platform.name, moduleReference: moduleReference, bystanderModules: module.bystanders)
+        let documentation = DocumentationNode(reference: reference, unifiedSymbol: symbol, moduleData: module, moduleReference: moduleReference)
         let source: TopicGraph.Node.ContentLocation // TODO: use a list of URLs for the files in a unified graph
         if let symbolGraphURL = symbolGraphURL {
             source = .file(url: symbolGraphURL)
diff --git a/Sources/SwiftDocC/Infrastructure/Symbol Graph/SymbolGraphLoader.swift b/Sources/SwiftDocC/Infrastructure/Symbol Graph/SymbolGraphLoader.swift
@@ -139,7 +139,7 @@ struct SymbolGraphLoader {
         var symbolGraph = symbolGraphs[url]!
         let (moduleName, isMainSymbolGraph) = Self.moduleNameFor(symbolGraph, at: url)
         
-        if !isMainSymbolGraph {
+        if !isMainSymbolGraph && symbolGraph.module.bystanders == nil {
             // If this is an extending another module, change the module name to match the exteneded module.
             // This makes the symbols in this graph have a path that starts with the extended module's name.
             symbolGraph.module.name = moduleName
@@ -250,8 +250,12 @@ struct SymbolGraphLoader {
         let isMainSymbolGraph = !url.lastPathComponent.contains("@")
         
         let moduleName: String
-        if isMainSymbolGraph {
+        if isMainSymbolGraph || symbolGraph.module.bystanders != nil {
             // For main symbol graphs, get the module name from the symbol graph's data
+
+            // When bystander modules are present, the symbol graph is a cross-import overlay, and
+            // we need to preserve the original module name to properly render it. It is still
+            // kept with the extension symbols, due to the merging behavior of UnifiedSymbolGraph.
             moduleName = symbolGraph.module.name
         } else {
             // For extension symbol graphs, derive the extended module's name from the file name.
diff --git a/Sources/SwiftDocC/Model/DocumentationNode.swift b/Sources/SwiftDocC/Model/DocumentationNode.swift
@@ -159,7 +159,7 @@ public struct DocumentationNode {
     ///   - symbol: The symbol to create a documentation node for.
     ///   - platformName: The name of the platforms for which the node is available.
     ///   - moduleName: The name of the module that the symbol belongs to.
-    init(reference: ResolvedTopicReference, unifiedSymbol: UnifiedSymbolGraph.Symbol, platformName: String?, moduleReference: ResolvedTopicReference, bystanderModules: [String]? = nil) {
+    init(reference: ResolvedTopicReference, unifiedSymbol: UnifiedSymbolGraph.Symbol, moduleData: SymbolGraph.Module, moduleReference: ResolvedTopicReference) {
         self.reference = reference
         
         guard let defaultSymbol = unifiedSymbol.defaultSymbol else {
@@ -175,6 +175,8 @@ public struct DocumentationNode {
         self.markup = Document()
         self.docChunks = []
         self.tags = (returns: [], throws: [], parameters: [])
+
+        let platformName = moduleData.platform.name
         
         let symbolAvailabilityVariants = DocumentationDataVariants(
             symbolData: unifiedSymbol.mixins,
@@ -259,7 +261,7 @@ public struct DocumentationNode {
             returnsSectionVariants: .empty,
             parametersSectionVariants: .empty,
             redirectsVariants: .empty,
-            bystanderModuleNames: bystanderModules
+            crossImportOverlayModule: moduleData.bystanders.map({ (moduleData.name, $0) })
         )
 
         try! semanticSymbol.mergeDeclarations(unifiedSymbol: unifiedSymbol)
@@ -468,9 +470,9 @@ public struct DocumentationNode {
     }
 
     @available(*, deprecated, message: "Use init(reference:symbol:platformName:moduleReference:article:engine:bystanderModules:) instead")
-    public init(reference: ResolvedTopicReference, symbol: SymbolGraph.Symbol, platformName: String?, moduleName: String, article: Article?, engine: DiagnosticEngine, bystanderModules: [String]? = nil) {
+    public init(reference: ResolvedTopicReference, symbol: SymbolGraph.Symbol, platformName: String?, moduleName: String, article: Article?, engine: DiagnosticEngine) {
         let assumedModuleReference = ResolvedTopicReference(bundleIdentifier: reference.bundleIdentifier, path: reference.pathComponents.prefix(2).joined(separator: "/"), sourceLanguage: reference.sourceLanguage)
-        self.init(reference: reference, symbol: symbol, platformName: platformName, moduleReference: assumedModuleReference, article: article, engine: engine, bystanderModules: bystanderModules)
+        self.init(reference: reference, symbol: symbol, platformName: platformName, moduleReference: assumedModuleReference, article: article, engine: engine)
     }
     
     /// Initializes a documentation node to represent a symbol from a symbol graph.
@@ -483,7 +485,7 @@ public struct DocumentationNode {
     ///   - article: The documentation extension content for this symbol.
     ///   - engine:The engine that collects any problems encountered during initialization.
     ///   - bystanderModules: An optional list of cross-import module names.
-    public init(reference: ResolvedTopicReference, symbol: SymbolGraph.Symbol, platformName: String?, moduleReference: ResolvedTopicReference, article: Article?, engine: DiagnosticEngine, bystanderModules: [String]? = nil) {
+    public init(reference: ResolvedTopicReference, symbol: SymbolGraph.Symbol, platformName: String?, moduleReference: ResolvedTopicReference, article: Article?, engine: DiagnosticEngine) {
         self.reference = reference
         
         guard reference.sourceLanguage == .swift else {
@@ -559,8 +561,7 @@ public struct DocumentationNode {
             seeAlsoVariants: .init(swiftVariant: markupModel.seeAlsoSection),
             returnsSectionVariants: .init(swiftVariant: markupModel.discussionTags.flatMap({ $0.returns.isEmpty ? nil : ReturnsSection(content: $0.returns[0].contents) })),
             parametersSectionVariants: .init(swiftVariant: markupModel.discussionTags.flatMap({ $0.parameters.isEmpty ? nil : ParametersSection(parameters: $0.parameters) })),
-            redirectsVariants: .init(swiftVariant: article?.redirects),
-            bystanderModuleNames: bystanderModules
+            redirectsVariants: .init(swiftVariant: article?.redirects)
         )
         
         updateAnchorSections()
diff --git a/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift b/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift
@@ -1057,10 +1057,13 @@ public struct RenderNodeTranslator: SemanticVisitor {
         }
         
         let moduleName = context.moduleName(forModuleReference: symbol.moduleReference)
-        
-        node.metadata.modulesVariants = VariantCollection(defaultValue:
-            [RenderMetadata.Module(name: moduleName.displayName, relatedModules: symbol.bystanderModuleNames)]
-        )
+
+        if let crossImportOverlayModule = symbol.crossImportOverlayModule {
+            node.metadata.modulesVariants = VariantCollection(defaultValue: [RenderMetadata.Module(name: crossImportOverlayModule.declaringModule, relatedModules: crossImportOverlayModule.bystanderModules)])
+        } else {
+            node.metadata.modulesVariants = VariantCollection(defaultValue: [RenderMetadata.Module(name: moduleName.displayName, relatedModules: nil)]
+            )
+        }
         
         node.metadata.extendedModuleVariants = VariantCollection<String?>(defaultValue: symbol.extendedModule)
         
diff --git a/Sources/SwiftDocC/Semantics/ReferenceResolver.swift b/Sources/SwiftDocC/Semantics/ReferenceResolver.swift
@@ -423,7 +423,7 @@ struct ReferenceResolver: SemanticVisitor {
             returnsSectionVariants: newReturnsVariants,
             parametersSectionVariants: newParametersVariants,
             redirectsVariants: symbol.redirectsVariants,
-            bystanderModuleNames: symbol.bystanderModuleNames,
+            crossImportOverlayModule: symbol.crossImportOverlayModule,
             originVariants: symbol.originVariants,
             automaticTaskGroupsVariants: symbol.automaticTaskGroupsVariants
         )
diff --git a/Sources/SwiftDocC/Semantics/Symbol/Symbol.swift b/Sources/SwiftDocC/Semantics/Symbol/Symbol.swift
@@ -113,9 +113,15 @@ public final class Symbol: Semantic, Abstracted, Redirected, AutomaticTaskGroups
     
     /// The name of the module extension in which the symbol is defined, if applicable.
     internal(set) public var extendedModule: String?
+
+    /// The names of any "bystander" modules required for this symbol, if it came from a cross-import overlay.
+    @available(*, deprecated, message: "Use crossImportOverlayModule instead")
+    public var bystanderModuleNames: [String]? {
+        self.crossImportOverlayModule?.bystanderModules
+    }
     
     /// Optional cross-import module names of the symbol.
-    internal(set) public var bystanderModuleNames: [String]?
+    internal(set) public var crossImportOverlayModule: (declaringModule: String, bystanderModules: [String])?
     
     /// Whether the symbol is required in its context, in each language variant the symbol is available in.
     public var isRequiredVariants: DocumentationDataVariants<Bool>
@@ -227,7 +233,7 @@ public final class Symbol: Semantic, Abstracted, Redirected, AutomaticTaskGroups
         returnsSectionVariants: DocumentationDataVariants<ReturnsSection>,
         parametersSectionVariants: DocumentationDataVariants<ParametersSection>,
         redirectsVariants: DocumentationDataVariants<[Redirect]>,
-        bystanderModuleNames: [String]? = nil,
+        crossImportOverlayModule: (declaringModule: String, bystanderModules: [String])? = nil,
         originVariants: DocumentationDataVariants<SymbolGraph.Relationship.SourceOrigin> = .init(),
         automaticTaskGroupsVariants: DocumentationDataVariants<[AutomaticTaskGroupSection]> = .init(defaultVariantValue: [])
     ) {
@@ -238,7 +244,7 @@ public final class Symbol: Semantic, Abstracted, Redirected, AutomaticTaskGroups
         self.roleHeadingVariants = roleHeadingVariants
         self.platformNameVariants = platformNameVariants
         self.moduleReference = moduleReference
-        self.bystanderModuleNames = bystanderModuleNames
+        self.crossImportOverlayModule = crossImportOverlayModule
         self.isRequiredVariants = requiredVariants
         self.externalIDVariants = externalIDVariants
         self.accessLevelVariants = accessLevelVariants
diff --git a/Tests/SwiftDocCTests/Rendering/RenderMetadataTests.swift b/Tests/SwiftDocCTests/Rendering/RenderMetadataTests.swift
@@ -107,14 +107,41 @@ class RenderMetadataTests: XCTestCase {
         let symbol = try XCTUnwrap(entity.semantic as? Symbol)
         
         // Verify it contains the bystanders data
-        XCTAssertEqual(symbol.bystanderModuleNames, ["Foundation"])
+        XCTAssertEqual(symbol.crossImportOverlayModule?.bystanderModules, ["Foundation"])
         
         // Verify the rendered metadata contains the bystanders
         let converter = DocumentationNodeConverter(bundle: bundle, context: context)
         let renderNode = try converter.convert(entity, at: nil)
         XCTAssertEqual(renderNode.metadata.modules?.first?.name, "MyKit")
         XCTAssertEqual(renderNode.metadata.modules?.first?.relatedModules, ["Foundation"])
     }
+
+    /// Test that when a bystanders symbol graph is loaded that extends a different module, that
+    /// those symbols correctly report the modules when rendered.
+    func testRendersBystanderExtensionsFromSymbolGraph() throws {
+        let (_, bundle, context) = try testBundleAndContext(copying: "TestBundle", externalResolvers: [:]) { url in
+            let baseSymbolGraphURL = Bundle.module.url(
+                forResource: "BaseKit.symbols", withExtension: "json", subdirectory: "Test Resources")!
+            try FileManager.default.copyItem(at: baseSymbolGraphURL, to: url.appendingPathComponent("BaseKit.symbols.json"))
+            let overlaySymbolGraphURL = Bundle.module.url(
+                forResource: "_OverlayTest_BaseKit@BaseKit.symbols", withExtension: "json", subdirectory: "Test Resources")!
+            try FileManager.default.copyItem(at: overlaySymbolGraphURL, to: url.appendingPathComponent("_OverlayTest_BaseKit@BaseKit.symbols.json"))
+        }
+
+        // Verify the symbol from bystanders graph is present in the documentation context.
+        let reference = ResolvedTopicReference(bundleIdentifier: bundle.identifier, path: "/documentation/BaseKit/OtherStruct/someFunc()", sourceLanguage: .swift)
+        let entity = try XCTUnwrap(try? context.entity(with: reference))
+        let symbol = try XCTUnwrap(entity.semantic as? Symbol)
+
+        // Verify it contains the bystanders data
+        XCTAssertEqual(symbol.crossImportOverlayModule?.bystanderModules, ["BaseKit"])
+
+        // Verify the rendered metadata contains the bystanders
+        let converter = DocumentationNodeConverter(bundle: bundle, context: context)
+        let renderNode = try converter.convert(entity, at: nil)
+        XCTAssertEqual(renderNode.metadata.modules?.first?.name, "OverlayTest")
+        XCTAssertEqual(renderNode.metadata.modules?.first?.relatedModules, ["BaseKit"])
+    }
     
     func testEmitsTitleVariantsDuringEncoding() throws {
         var metadata = RenderMetadata()
diff --git a/Tests/SwiftDocCTests/Rendering/RenderNodeTranslatorSymbolVariantsTests.swift b/Tests/SwiftDocCTests/Rendering/RenderNodeTranslatorSymbolVariantsTests.swift
@@ -56,7 +56,35 @@ class RenderNodeTranslatorSymbolVariantsTests: XCTestCase {
                 context.preResolveModuleNames()
             },
             configureSymbol: { symbol in
-                symbol.bystanderModuleNames = ["Custom Bystander Title"]
+                symbol.crossImportOverlayModule = ("Custom Module Title", ["Custom Bystander Title"])
+            },
+            assertOriginalRenderNode: { renderNode in
+                try assertModule(
+                    renderNode.metadata.modules,
+                    expectedName: "Custom Module Title",
+                    expectedRelatedModules: ["Custom Bystander Title"]
+                )
+            },
+            assertAfterApplyingVariant: { renderNode in
+                try assertModule(
+                    renderNode.metadata.modules,
+                    expectedName: "Custom Module Title",
+                    expectedRelatedModules: ["Custom Bystander Title"]
+                )
+            }
+        )
+    }
+
+    /// Make sure that when a symbol has `crossImportOverlayModule` information, that module name is used instead of its `moduleReference`.
+    func testMultipleModulesWithDifferentBystanderModule() throws {
+        try assertMultiVariantSymbol(
+            configureContext: { context, resolvedTopicReference in
+                let moduleReference = ResolvedTopicReference(bundleIdentifier: resolvedTopicReference.bundleIdentifier, path: "/documentation/MyKit", sourceLanguage: .swift)
+                context.documentationCache[moduleReference]?.name = .conceptual(title: "Extended Module Title")
+                context.preResolveModuleNames()
+            },
+            configureSymbol: { symbol in
+                symbol.crossImportOverlayModule = ("Custom Module Title", ["Custom Bystander Title"])
             },
             assertOriginalRenderNode: { renderNode in
                 try assertModule(
diff --git a/Tests/SwiftDocCTests/Test Resources/BaseKit.symbols.json b/Tests/SwiftDocCTests/Test Resources/BaseKit.symbols.json
@@ -0,0 +1,86 @@
+{
+    "metadata": {
+        "formatVersion": {
+            "major": 0,
+            "minor": 5,
+            "patch": 3
+        },
+        "generator": "app/1.0"
+    },
+    "module": {
+        "name": "BaseKit",
+        "platform": {
+            "architecture": "x86_64",
+            "vendor": "apple",
+            "operatingSystem": {
+                "name": "macosx",
+                "minimumVersion": {
+                    "major": 12,
+                    "minor": 3,
+                    "patch": 0
+                }
+            }
+        }
+    },
+    "symbols": [
+        {
+            "kind": {
+                "identifier": "swift.struct",
+                "displayName": "Structure"
+            },
+            "identifier": {
+                "precise": "s:7BaseKit11OtherStructV",
+                "interfaceLanguage": "swift"
+            },
+            "pathComponents": [
+                "OtherStruct"
+            ],
+            "names": {
+                "title": "OtherStruct",
+                "navigator": [
+                    {
+                        "kind": "identifier",
+                        "spelling": "OtherStruct"
+                    }
+                ],
+                "subHeading": [
+                    {
+                        "kind": "keyword",
+                        "spelling": "struct"
+                    },
+                    {
+                        "kind": "text",
+                        "spelling": " "
+                    },
+                    {
+                        "kind": "identifier",
+                        "spelling": "OtherStruct"
+                    }
+                ]
+            },
+            "declarationFragments": [
+                {
+                    "kind": "keyword",
+                    "spelling": "struct"
+                },
+                {
+                    "kind": "text",
+                    "spelling": " "
+                },
+                {
+                    "kind": "identifier",
+                    "spelling": "OtherStruct"
+                }
+            ],
+            "accessLevel": "public",
+            "location": {
+                "uri": "file:///path/to/OverlayTest/BaseKit/BaseKit.swift",
+                "position": {
+                    "line": 9,
+                    "character": 14
+                }
+            }
+        }
+    ],
+    "relationships": []
+}
diff --git a/Tests/SwiftDocCTests/Test Resources/_OverlayTest_BaseKit@BaseKit.symbols.json b/Tests/SwiftDocCTests/Test Resources/_OverlayTest_BaseKit@BaseKit.symbols.json

Original file line number	Diff line number	Diff line change
`@@ -423,7 +423,7 @@ struct ReferenceResolver: SemanticVisitor {`
`423`	`423`	`returnsSectionVariants: newReturnsVariants,`
`424`	`424`	`parametersSectionVariants: newParametersVariants,`
`425`	`425`	`redirectsVariants: symbol.redirectsVariants,`
`426`		`- bystanderModuleNames: symbol.bystanderModuleNames,`
	`426`	`+ crossImportOverlayModule: symbol.crossImportOverlayModule,`
`427`	`427`	`originVariants: symbol.originVariants,`
`428`	`428`	`automaticTaskGroupsVariants: symbol.automaticTaskGroupsVariants`
`429`	`429`	`)`