Merge branch 'main' into jed/doxygen-discussion-note

Author: j-f1

Sources/SwiftDocC/Infrastructure/DocumentationContext.swift

@@ -1261,9 +1261,9 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
                     diagnosticEngine.emit(Problem(diagnostic: diagnostic))
                     continue
                 }
-                guard let url = ValidatedURL(parsingExact: destination) else {
+                guard let url = ValidatedURL(parsingAuthoredLink: destination) else {
                     let diagnostic = Diagnostic(source: documentationExtension.source, severity: .warning, range: link.range, identifier: "org.swift.docc.invalidLinkDestination", summary: """
-                        \(destination.singleQuoted) is
+                        \(destination.singleQuoted) is not a valid RFC 3986 URL.
                         """, explanation: nil, notes: [])
                     diagnosticEngine.emit(Problem(diagnostic: diagnostic))
                     continue

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

@@ -106,7 +106,7 @@ final class PathHierarchyBasedLinkResolver {
     }
 
     private func addTutorial(reference: ResolvedTopicReference, source: URL, landmarks: [Landmark]) {
-        let tutorialID = pathHierarchy.addTutorial(name: urlReadablePath(source.deletingPathExtension().lastPathComponent))
+        let tutorialID = pathHierarchy.addTutorial(name: linkName(filename: source.deletingPathExtension().lastPathComponent))
         resolvedReferenceMap[tutorialID] = reference
 
         for landmark in landmarks {
@@ -119,7 +119,7 @@ final class PathHierarchyBasedLinkResolver {
     func addTechnology(_ technology: DocumentationContext.SemanticResult<Technology>) {
         let reference = technology.topicGraphNode.reference
 
-        let technologyID = pathHierarchy.addTutorialOverview(name: urlReadablePath(technology.source.deletingPathExtension().lastPathComponent))
+        let technologyID = pathHierarchy.addTutorialOverview(name: linkName(filename: technology.source.deletingPathExtension().lastPathComponent))
         resolvedReferenceMap[technologyID] = reference
 
         var anonymousVolumeID: ResolvedIdentifier?
@@ -149,21 +149,20 @@ final class PathHierarchyBasedLinkResolver {
 
     /// Adds a technology root article and its headings to the path hierarchy.
     func addRootArticle(_ article: DocumentationContext.SemanticResult<Article>, anchorSections: [AnchorSection]) {
-        let articleID = pathHierarchy.addTechnologyRoot(name: article.source.deletingPathExtension().lastPathComponent)
+        let linkName = linkName(filename: article.source.deletingPathExtension().lastPathComponent)
+        let articleID = pathHierarchy.addTechnologyRoot(name: linkName)
         resolvedReferenceMap[articleID] = article.topicGraphNode.reference
         addAnchors(anchorSections, to: articleID)
     }
 
     /// Adds an article and its headings to the path hierarchy.
     func addArticle(_ article: DocumentationContext.SemanticResult<Article>, anchorSections: [AnchorSection]) {
-        let articleID = pathHierarchy.addArticle(name: article.source.deletingPathExtension().lastPathComponent)
-        resolvedReferenceMap[articleID] = article.topicGraphNode.reference
-        addAnchors(anchorSections, to: articleID)
+        addArticle(filename: article.source.deletingPathExtension().lastPathComponent, reference: article.topicGraphNode.reference, anchorSections: anchorSections)
     }
 
     /// Adds an article and its headings to the path hierarchy.
     func addArticle(filename: String, reference: ResolvedTopicReference, anchorSections: [AnchorSection]) {
-        let articleID = pathHierarchy.addArticle(name: filename)
+        let articleID = pathHierarchy.addArticle(name: linkName(filename: filename))
         resolvedReferenceMap[articleID] = reference
         addAnchors(anchorSections, to: articleID)
     }
@@ -186,7 +185,7 @@ final class PathHierarchyBasedLinkResolver {
     /// Adds a task group on a given page to the documentation hierarchy.
     func addTaskGroup(named name: String, reference: ResolvedTopicReference, to parent: ResolvedTopicReference) {
         let parentID = resolvedReferenceMap[parent]!
-        let taskGroupID = pathHierarchy.addNonSymbolChild(parent: parentID, name: urlReadablePath(name), kind: "taskGroup")
+        let taskGroupID = pathHierarchy.addNonSymbolChild(parent: parentID, name: urlReadableFragment(name), kind: "taskGroup")
         resolvedReferenceMap[taskGroupID] = reference
     }
 
@@ -272,3 +271,19 @@ final class PathHierarchyBasedLinkResolver {
         return result
     }
 }
+
+/// Creates a more writable version of an articles file name for use in documentation links.
+///
+/// Compared to `urlReadablePath(_:)` this preserves letters in other written languages.
+private func linkName<S: StringProtocol>(filename: S) -> String {
+    // It would be a nice enhancement to also remove punctuation from the filename to allow an article in a file named "One, two, & three!"
+    // to be referenced with a link as `"One-two-three"` instead of `"One,-two-&-three!"` (rdar://120722917)
+    return filename
+        // Replace continuous whitespace and dashes
+        .components(separatedBy: whitespaceAndDashes)
+        .filter({ !$0.isEmpty })
+        .joined(separator: "-")
+}
+
+private let whitespaceAndDashes = CharacterSet.whitespaces
+    .union(CharacterSet(charactersIn: "-–—")) // hyphen, en dash, em dash

Sources/SwiftDocC/Model/Identifier.swift

@@ -141,10 +141,14 @@ extension TopicReferenceResolutionErrorInfo {
 /// > its data.
 public struct ResolvedTopicReference: Hashable, Codable, Equatable, CustomStringConvertible {
     typealias ReferenceBundleIdentifier = String
-    typealias ReferenceKey = String
+    private struct ReferenceKey: Hashable {
+        var path: String
+        var fragment: String?
+        var sourceLanguages: Set<SourceLanguage>
+    }
 
     /// A synchronized reference cache to store resolved references.
-    static var sharedPool = Synchronized([ReferenceBundleIdentifier: [ReferenceKey: ResolvedTopicReference]]())
+    private static var sharedPool = Synchronized([ReferenceBundleIdentifier: [ReferenceKey: ResolvedTopicReference]]())
 
     /// Clears cached references belonging to the bundle with the given identifier.
     /// - Parameter bundleIdentifier: The identifier of the bundle to which the method should clear belonging references.
@@ -219,11 +223,7 @@ public struct ResolvedTopicReference: Hashable, Codable, Equatable, CustomString
     private init(bundleIdentifier: String, urlReadablePath: String, urlReadableFragment: String? = nil, sourceLanguages: Set<SourceLanguage>) {
         precondition(!sourceLanguages.isEmpty, "ResolvedTopicReference.sourceLanguages cannot be empty")
         // Check for a cached instance of the reference
-        let key = Self.cacheKey(
-            urlReadablePath: urlReadablePath,
-            urlReadableFragment: urlReadableFragment,
-            sourceLanguages: sourceLanguages
-        )
+        let key = ReferenceKey(path: urlReadablePath, fragment: urlReadableFragment, sourceLanguages: sourceLanguages)
         let cached = Self.sharedPool.sync { $0[bundleIdentifier]?[key] }
         if let resolved = cached {
             self = resolved
@@ -244,20 +244,6 @@ public struct ResolvedTopicReference: Hashable, Codable, Equatable, CustomString
         }
     }
 
-    private static func cacheKey(
-        urlReadablePath path: String,
-        urlReadableFragment fragment: String?,
-        sourceLanguages: Set<SourceLanguage>
-    ) -> String {
-        let sourceLanguagesString = sourceLanguages.map(\.id).sorted().joined(separator: "-")
-        
-        if let fragment = fragment {
-            return "\(path):\(fragment):\(sourceLanguagesString)"
-        } else {
-            return "\(path):\(sourceLanguagesString)"
-        }
-    }
-    
     /// The topic URL as you would write in a link.
     public var url: URL {
         return _storage.url
@@ -495,22 +481,10 @@ public struct ResolvedTopicReference: Hashable, Codable, Equatable, CustomString
             self.absoluteString = self.url.absoluteString
         }
     }
-}
-
-typealias ResolvedTopicReferenceCacheKey = String
-
-extension ResolvedTopicReference {
-    /// Returns a unique cache ID for a pair of unresolved and parent references.
-    static func cacheIdentifier(_ reference: UnresolvedTopicReference, fromSymbolLink: Bool, in parent: ResolvedTopicReference?) -> ResolvedTopicReferenceCacheKey {
-        let isSymbolLink = fromSymbolLink ? ":symbol" : ""
-        if let parent = parent {
-            // Create a cache id in the parent context
-            return "\(reference.topicURL.absoluteString):\(parent.bundleIdentifier):\(parent.path):\(parent.sourceLanguage.id)\(isSymbolLink)"
-        } else {
-            // A cache ID for an external reference
-            assert(reference.topicURL.components.host != nil)
-            return reference.topicURL.absoluteString.appending(isSymbolLink)
-        }
+    
+    // For testing the caching
+    static func _numberOfCachedReferences(bundleID: ReferenceBundleIdentifier) -> Int? {
+        return Self.sharedPool.sync { $0[bundleID]?.count }
     }
 }
 
@@ -643,6 +617,7 @@ func urlReadablePath<S: StringProtocol>(_ path: S) -> String {
 }
 
 private extension CharacterSet {
+    // For fragments
     static let fragmentCharactersToRemove = CharacterSet.punctuationCharacters // Remove punctuation from fragments
         .union(CharacterSet(charactersIn: "`"))       // Also consider back-ticks as punctuation. They are used as quotes around symbols or other code.
         .subtracting(CharacterSet(charactersIn: "-")) // Don't remove hyphens. They are used as a whitespace replacement.
@@ -669,3 +644,4 @@ func urlReadableFragment<S: StringProtocol>(_ fragment: S) -> String {
 
     return fragment
 }
+

Sources/SwiftDocC/Model/Rendering/DocumentationContentRenderer.swift

@@ -494,12 +494,7 @@ public class DocumentationContentRenderer {
 
                 // For external links, verify they've resolved successfully and return `nil` otherwise.
                 if linkHost != reference.bundleIdentifier {
-                    let externalReference = ResolvedTopicReference(
-                        bundleIdentifier: linkHost,
-                        path: destination.path,
-                        sourceLanguages: node.availableSourceLanguages
-                    )
-                    if documentationContext.externallyResolvedSymbols.contains(externalReference) {
+                    if let url = ValidatedURL(destination), case .success(let externalReference) = documentationContext.externallyResolvedLinks[url] {
                         return externalReference
                     }
                     return nil

Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift

@@ -598,7 +598,8 @@ public struct RenderNodeTranslator: SemanticVisitor {
 
     public mutating func visitArticle(_ article: Article) -> RenderTree? {
         var node = RenderNode(identifier: identifier, kind: .article)
-        var contentCompiler = RenderContentCompiler(context: context, bundle: bundle, identifier: identifier)
+        // Contains symbol references declared in the Topics section.
+        var topicSectionContentCompiler = RenderContentCompiler(context: context, bundle: bundle, identifier: identifier)
 
         node.metadata.title = article.title!.plainText
 
@@ -674,7 +675,7 @@ public struct RenderNodeTranslator: SemanticVisitor {
                         allowExternalLinks: false,
                         allowedTraits: allowedTraits,
                         availableTraits: documentationNode.availableVariantTraits,
-                        contentCompiler: &contentCompiler
+                        contentCompiler: &topicSectionContentCompiler
                     )
                 )
             }
@@ -685,7 +686,7 @@ public struct RenderNodeTranslator: SemanticVisitor {
                 sections.append(
                     contentsOf: renderAutomaticTaskGroupsSection(
                         article.automaticTaskGroups.filter { $0.renderPositionPreference == .top },
-                        contentCompiler: &contentCompiler
+                        contentCompiler: &topicSectionContentCompiler
                     )
                 )
             }
@@ -714,7 +715,7 @@ public struct RenderNodeTranslator: SemanticVisitor {
                 }
 
                 // Collect all child topic references.
-                contentCompiler.collectedTopicReferences.append(contentsOf: groups.flatMap(\.references))
+                topicSectionContentCompiler.collectedTopicReferences.append(contentsOf: groups.flatMap(\.references))
                 // Add the final groups to the node.
                 sections.append(contentsOf: groups.map(TaskGroupRenderSection.init(taskGroup:)))
             }
@@ -725,7 +726,7 @@ public struct RenderNodeTranslator: SemanticVisitor {
                 sections.append(
                     contentsOf: renderAutomaticTaskGroupsSection(
                         article.automaticTaskGroups.filter { $0.renderPositionPreference == .bottom },
-                        contentCompiler: &contentCompiler
+                        contentCompiler: &topicSectionContentCompiler
                     )
                 )
             }
@@ -736,11 +737,30 @@ public struct RenderNodeTranslator: SemanticVisitor {
         node.topicSectionsStyle = topicsSectionStyle(for: documentationNode)
 
         if shouldCreateAutomaticRoleHeading(for: documentationNode) {
-            if node.topicSections.isEmpty {
-                // Set an eyebrow for articles
+            
+            let role = DocumentationContentRenderer.roleForArticle(article, nodeKind: documentationNode.kind)
+            node.metadata.role = role.rawValue
+            
+            switch role {
+            case .article:
+                // If there are no links to other nodes from the article,
+                // set the eyebrow for articles.
                 node.metadata.roleHeading = "Article"
+            case .collectionGroup:
+                // If the article links to other nodes, set the eyebrow for
+                // API Collections if any linked node is a symbol.
+                //
+                // If none of the linked nodes are symbols (it's a plain collection),
+                // don't display anything as the eyebrow title.
+                let curatesSymbols = topicSectionContentCompiler.collectedTopicReferences.contains { topicReference in
+                    context.topicGraph.nodeWithReference(topicReference)?.kind.isSymbol ?? false
+                }
+                if curatesSymbols {
+                    node.metadata.roleHeading = "API Collection"
+                }
+            default:
+                break
             }
-            node.metadata.role = DocumentationContentRenderer.roleForArticle(article, nodeKind: documentationNode.kind).rawValue
         }
 
         if let pageImages = documentationNode.metadata?.pageImages {
@@ -780,7 +800,7 @@ public struct RenderNodeTranslator: SemanticVisitor {
                         allowExternalLinks: true,
                         allowedTraits: allowedTraits,
                         availableTraits: documentationNode.availableVariantTraits,
-                        contentCompiler: &contentCompiler
+                        contentCompiler: &topicSectionContentCompiler
                     )
                 )
             }
@@ -794,7 +814,7 @@ public struct RenderNodeTranslator: SemanticVisitor {
                 renderContext: renderContext,
                 renderer: contentRenderer
             ) {
-                contentCompiler.collectedTopicReferences.append(contentsOf: seeAlso.references)
+                topicSectionContentCompiler.collectedTopicReferences.append(contentsOf: seeAlso.references)
                 seeAlsoSections.append(TaskGroupRenderSection(taskGroup: seeAlso))
             }
 
@@ -845,7 +865,7 @@ public struct RenderNodeTranslator: SemanticVisitor {
             node.metadata.roleHeading = titleHeading.heading
         } 
 
-        collectedTopicReferences.append(contentsOf: contentCompiler.collectedTopicReferences)
+        collectedTopicReferences.append(contentsOf: topicSectionContentCompiler.collectedTopicReferences)
         node.references = createTopicRenderReferences()
 
         addReferences(imageReferences, to: &node)
@@ -854,7 +874,7 @@ public struct RenderNodeTranslator: SemanticVisitor {
         addReferences(downloadReferences, to: &node)
         // See Also can contain external links, we need to separately transfer
         // link references from the content compiler
-        addReferences(contentCompiler.linkReferences, to: &node)
+        addReferences(topicSectionContentCompiler.linkReferences, to: &node)
 
         return node
     }

Tests/SwiftDocCTests/Converter/RenderNodeCodableTests.swift

@@ -146,21 +146,14 @@ class RenderNodeCodableTests: XCTestCase {
             subdirectory: "Test Resources"
         )!
 
-        let uniqueBundleIdentifier = #function
+        let bundleID = #function
 
-        let renderNodeWithUniqueBundleID = try String(
-            contentsOf: exampleRenderNodeJSON
-        )
-        .replacingOccurrences(
-            of: "org.swift.docc.example",
-            with: uniqueBundleIdentifier
-        )
+        let renderNodeWithUniqueBundleID = try String(contentsOf: exampleRenderNodeJSON)
+        .replacingOccurrences(of: "org.swift.docc.example", with: bundleID)
 
         _ = try JSONDecoder().decode(RenderNode.self, from: Data(renderNodeWithUniqueBundleID.utf8))
 
-        ResolvedTopicReference.sharedPool.sync { sharedPool in
-            XCTAssertNil(sharedPool[uniqueBundleIdentifier])
-        }
+        XCTAssertNil(ResolvedTopicReference._numberOfCachedReferences(bundleID: bundleID))
     }
 
     func testDecodeRenderNodeWithoutTopicSectionStyle() throws {

Tests/SwiftDocCTests/Indexing/NavigatorIndexTests.swift

@@ -260,9 +260,7 @@ Root
             atomically: true
         )
 
-        ResolvedTopicReference.sharedPool.sync { sharedPool in
-            XCTAssertNil(sharedPool[uniqueTestBundleIdentifier])
-        }
+        XCTAssertNil(ResolvedTopicReference._numberOfCachedReferences(bundleID: uniqueTestBundleIdentifier))
     }