From 5d4c87df1388164cf1ab3c330ecfc83f99c638e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Tue, 4 Apr 2023 10:14:31 -0700 Subject: [PATCH 1/5] Fast path to look up resolved references by their absolute string rdar://85531439 --- .../Infrastructure/DocumentationContext.swift | 11 +++++++++++ .../Infrastructure/DocumentationCurator.swift | 2 +- .../Model/Rendering/RenderContentCompiler.swift | 7 ++++++- .../Model/Rendering/RenderNodeTranslator.swift | 3 +-- .../SwiftDocC/Semantics/MarkupReferenceResolver.swift | 2 +- 5 files changed, 20 insertions(+), 5 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift index cfb8637b81..5ea4be6190 100644 --- a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift +++ b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift @@ -1003,6 +1003,9 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate { /// This allows convenient access to other symbol's documentation nodes while building relationships between symbols. private(set) var symbolIndex = [String: DocumentationNode]() + /// A lookup of resolved references based on the reference's absolute string. + private(set) var referenceIndex = [String: ResolvedTopicReference]() + private func nodeWithInitializedContent(reference: ResolvedTopicReference, matches: [DocumentationContext.SemanticResult
]?) -> DocumentationNode { precondition(documentationCache.keys.contains(reference)) @@ -2311,6 +2314,14 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate { topicGraphGlobalAnalysis() preResolveModuleNames() + + referenceIndex.reserveCapacity(knownIdentifiers.count + nodeAnchorSections.count) + for reference in knownIdentifiers { + referenceIndex[reference.absoluteString] = reference + } + for reference in nodeAnchorSections.keys { + referenceIndex[reference.absoluteString] = reference + } } /// Given a list of topics that have been automatically curated, checks if a topic has been additionally manually curated diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationCurator.swift b/Sources/SwiftDocC/Infrastructure/DocumentationCurator.swift index 9aaae8ba2a..74f51a8c36 100644 --- a/Sources/SwiftDocC/Infrastructure/DocumentationCurator.swift +++ b/Sources/SwiftDocC/Infrastructure/DocumentationCurator.swift @@ -38,7 +38,7 @@ struct DocumentationCurator { } // Optimization for absolute links. - if let cached = context.documentationCacheBasedLinkResolver.referenceFor(absoluteSymbolPath: destination, parent: resolved) { + if let cached = context.referenceIndex[destination] { return cached } diff --git a/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift b/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift index ed7e28c023..1718b348a8 100644 --- a/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift +++ b/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift @@ -192,6 +192,11 @@ struct RenderContentCompiler: MarkupVisitor { } mutating func resolveTopicReference(_ destination: String) -> ResolvedTopicReference? { + if let cached = context.referenceIndex[destination] { + collectedTopicReferences.append(cached) + return cached + } + guard let validatedURL = ValidatedURL(parsingAuthoredLink: destination) else { return nil } @@ -215,7 +220,7 @@ struct RenderContentCompiler: MarkupVisitor { } func resolveSymbolReference(destination: String) -> ResolvedTopicReference? { - if let cached = context.documentationCacheBasedLinkResolver.referenceFor(absoluteSymbolPath: destination, parent: identifier) { + if let cached = context.referenceIndex[destination] { return cached } diff --git a/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift b/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift index 18960359d2..948d3aab7c 100644 --- a/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift +++ b/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift @@ -534,8 +534,7 @@ public struct RenderNodeTranslator: SemanticVisitor { let action: RenderInlineContent // We expect, at this point of the rendering, this API to be called with valid URLs, otherwise crash. - let unresolved = UnresolvedTopicReference(topicURL: ValidatedURL(link)!) - if case let .success(resolved) = context.resolve(.unresolved(unresolved), in: bundle.rootReference) { + if let resolved = context.referenceIndex[link.absoluteString] { action = RenderInlineContent.reference(identifier: RenderReferenceIdentifier(resolved.absoluteString), isActive: true, overridingTitle: overridingTitle, diff --git a/Sources/SwiftDocC/Semantics/MarkupReferenceResolver.swift b/Sources/SwiftDocC/Semantics/MarkupReferenceResolver.swift index 4a070b992b..dad2d0902d 100644 --- a/Sources/SwiftDocC/Semantics/MarkupReferenceResolver.swift +++ b/Sources/SwiftDocC/Semantics/MarkupReferenceResolver.swift @@ -118,7 +118,7 @@ struct MarkupReferenceResolver: MarkupRewriter { } mutating func resolveAbsoluteSymbolLink(unresolvedDestination: String, elementRange range: SourceRange?) -> ResolvedTopicReference? { - if let cached = context.documentationCacheBasedLinkResolver.referenceFor(absoluteSymbolPath: unresolvedDestination, parent: rootReference) { + if let cached = context.referenceIndex[unresolvedDestination] { guard context.topicGraph.isLinkable(cached) == true else { problems.append(disabledLinkDestinationProblem(reference: cached, source: source, range: range, severity: .warning)) return nil From 99d22eb14dfdff3a1dddac1aa64b6a21fff58620 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Tue, 4 Apr 2023 15:05:07 -0700 Subject: [PATCH 2/5] Fix bug where links with special characters used the link as title rdar://85531439 --- Sources/SwiftDocC/Model/Identifier.swift | 4 +- .../Rendering/RenderContentCompiler.swift | 20 ++- .../Rendering/RenderNodeTranslator.swift | 2 +- .../Semantics/MarkupReferenceResolver.swift | 5 + .../DocumentationContextTests.swift | 154 ++++++++++++++++++ 5 files changed, 179 insertions(+), 6 deletions(-) diff --git a/Sources/SwiftDocC/Model/Identifier.swift b/Sources/SwiftDocC/Model/Identifier.swift index 94c839d5ec..36154c9f1e 100644 --- a/Sources/SwiftDocC/Model/Identifier.swift +++ b/Sources/SwiftDocC/Model/Identifier.swift @@ -623,7 +623,7 @@ public struct ResourceReference: Hashable { /// If this step is not performed, the disallowed characters are instead percent escape encoded instead which is less readable. /// For example, a path like `"hello world/example project"` is converted to `"hello-world/example-project"` /// instead of `"hello%20world/example%20project"`. -func urlReadablePath(_ path: String) -> String { +func urlReadablePath(_ path: S) -> String { return path.components(separatedBy: .urlPathNotAllowed).joined(separator: "-") } @@ -639,7 +639,7 @@ private extension CharacterSet { /// /// If this step is not performed, the disallowed characters are instead percent escape encoded, which is less readable. /// For example, a fragment like `"#hello world"` is converted to `"#hello-world"` instead of `"#hello%20world"`. -func urlReadableFragment(_ fragment: String) -> String { +func urlReadableFragment(_ fragment: S) -> String { var fragment = fragment // Trim leading/trailing whitespace .trimmingCharacters(in: .whitespaces) diff --git a/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift b/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift index 1718b348a8..500b02b04d 100644 --- a/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift +++ b/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift @@ -175,9 +175,23 @@ struct RenderContentCompiler: MarkupVisitor { useOverriding = false } else if let overridingTitle = overridingTitle, overridingTitle.hasPrefix(ResolvedTopicReference.urlScheme + ":"), - destination.hasPrefix(ResolvedTopicReference.urlScheme + "://"), - destination.hasSuffix(overridingTitle.dropFirst((ResolvedTopicReference.urlScheme + ":").count)) { // If the link is a transformed doc link, we don't use overriding info - useOverriding = false + destination.hasPrefix(ResolvedTopicReference.urlScheme + "://") + { + // The overriding title looks like a documentation link. Escape it like a resolved reference string to compare it with the destination. + let withoutScheme = overridingTitle.dropFirst((ResolvedTopicReference.urlScheme + ":").count) + if destination.hasSuffix(withoutScheme) { + useOverriding = false + } else { + let escapedTitle: String + if let fragmentIndex = withoutScheme.firstIndex(of: "#") { + let escapedFragment = withoutScheme[fragmentIndex...].dropFirst().addingPercentEncoding(withAllowedCharacters: .urlFragmentAllowed) ?? "" + escapedTitle = "\(urlReadablePath(withoutScheme[.. + - + - + + Now test the same links in topic curation. + + ## Topics + + - ``MyClass/myFunc🙂()`` + - + - + - + """), + ]) + let bundleURL = try testBundle.write(inside: createTemporaryDirectory()) + let (_, bundle, context) = try loadBundle(from: bundleURL) + + let problems = context.problems + XCTAssertEqual(problems.count, 0, "Unexpected problems: \(problems.map(\.diagnostic.summary).sorted())") + + let moduleReference = ResolvedTopicReference(bundleIdentifier: bundle.identifier, path: "/documentation/MyKit", sourceLanguage: .swift) + let entity = try context.entity(with: moduleReference) + + let moduleSymbol = try XCTUnwrap(entity.semantic as? Symbol) + let topicSection = try XCTUnwrap(moduleSymbol.topics?.taskGroups.first) + + // Verify that all the links in the topic section resolved + XCTAssertEqual(topicSection.links.map(\.destination), [ + "doc://special-characters/documentation/MyKit/MyClass/myFunc_()", + "doc://special-characters/documentation/special-characters/article-with-emoji-in-heading#Hello-%F0%9F%8C%8D", + "doc://special-characters/documentation/special-characters/article-with---in-filename", + "doc://special-characters/documentation/special-characters/article-with---in-filename#Hello-world", + ]) + + // Verify that all resolved link exist in the context. + for reference in topicSection.links { + XCTAssertNotNil(reference.destination) + XCTAssert(context.knownPages.contains(where: { $0.absoluteString == reference.destination }) + || context.nodeAnchorSections.keys.contains(where: { $0.absoluteString == reference.destination }) + ) + } + + var translator = RenderNodeTranslator(context: context, bundle: bundle, identifier: moduleReference, source: nil) + let renderNode = translator.visit(moduleSymbol) as! RenderNode + + // Verify that the resolved links rendered as links + XCTAssertEqual(renderNode.topicSections.first?.identifiers.count, 4) + XCTAssertEqual(renderNode.topicSections.first?.identifiers, [ + "doc://special-characters/documentation/MyKit/MyClass/myFunc_()", + "doc://special-characters/documentation/special-characters/article-with-emoji-in-heading#Hello-%F0%9F%8C%8D", + "doc://special-characters/documentation/special-characters/article-with---in-filename", + "doc://special-characters/documentation/special-characters/article-with---in-filename#Hello-world", + ]) + + + let contentSection = try XCTUnwrap(renderNode.primaryContentSections.first as? ContentRenderSection) + let lists = contentSection.content.compactMap({ content in + if case let .unorderedList(list) = content { + return list + } else { + return nil + } + }) + + XCTAssertEqual(lists.count, 1) + let list = try XCTUnwrap(lists.first) + XCTAssertEqual(list.items.count, 4, "Unexpected list items: \(list.items.map(\.content))") + + func withContentAsReference(_ listItem: RenderBlockContent.ListItem?, verify: (RenderReferenceIdentifier, Bool, String?, [RenderInlineContent]?) -> Void) { + guard let listItem = listItem else { + XCTFail("Missing list item") + return + } + if case let .paragraph(paragraph) = listItem.content.first, + case let .reference(identifier, isActive, overridingTitle, overridingTitleInlineContent) = paragraph.inlineContent.first { + verify(identifier, isActive, overridingTitle, overridingTitleInlineContent) + } else { + XCTFail("Unexpected list item kind: \(listItem.content)") + } + } + + // First + withContentAsReference(list.items.first) { identifier, isActive, overridingTitle, overridingTitleInlineContent in + XCTAssertEqual(identifier.identifier, "doc://special-characters/documentation/MyKit/MyClass/myFunc_()") + XCTAssertEqual(isActive, true) + XCTAssertEqual(overridingTitle, nil) + XCTAssertEqual(overridingTitleInlineContent, nil) + } + withContentAsReference(list.items.dropFirst().first) { identifier, isActive, overridingTitle, overridingTitleInlineContent in + XCTAssertEqual(identifier.identifier, "doc://special-characters/documentation/special-characters/article-with-emoji-in-heading#Hello-%F0%9F%8C%8D") + XCTAssertEqual(isActive, true) + XCTAssertEqual(overridingTitle, nil) + XCTAssertEqual(overridingTitleInlineContent, nil) + } + withContentAsReference(list.items.dropFirst(2).first) { identifier, isActive, overridingTitle, overridingTitleInlineContent in + XCTAssertEqual(identifier.identifier, "doc://special-characters/documentation/special-characters/article-with---in-filename") + XCTAssertEqual(isActive, true) + XCTAssertEqual(overridingTitle, nil) + XCTAssertEqual(overridingTitleInlineContent, nil) + } + withContentAsReference(list.items.dropFirst(3).first) { identifier, isActive, overridingTitle, overridingTitleInlineContent in + XCTAssertEqual(identifier.identifier, "doc://special-characters/documentation/special-characters/article-with---in-filename#Hello-world") + XCTAssertEqual(isActive, true) + XCTAssertEqual(overridingTitle, nil) + XCTAssertEqual(overridingTitleInlineContent, nil) + } + + // Verify that the topic render references have titles with special characters when the original content contained special characters + XCTAssertEqual( + (renderNode.references["doc://special-characters/documentation/MyKit/MyClass/myFunc_()"] as? TopicRenderReference)?.title, + "myFunc🙂()" + ) + XCTAssertEqual( + (renderNode.references["doc://special-characters/documentation/special-characters/article-with-emoji-in-heading#Hello-%F0%9F%8C%8D"] as? TopicRenderReference)?.title, + "Hello 🌍" + ) + XCTAssertEqual( + (renderNode.references["doc://special-characters/documentation/special-characters/article-with---in-filename"] as? TopicRenderReference)?.title, + "Article with 😃 emoji in file name" + ) + XCTAssertEqual( + (renderNode.references["doc://special-characters/documentation/special-characters/article-with---in-filename#Hello-world"] as? TopicRenderReference)?.title, + "Hello world" + ) + } + func testNonOverloadCollisionFromExtension() throws { // Add some symbol collisions to graph let (_, _, context) = try testBundleAndContext(copying: "TestBundle", excludingPaths: ["mykit-iOS.symbols.json"]) { root in From 6f40724f9015b5e33006c16dcf6cb943b27f907a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Wed, 5 Apr 2023 10:03:36 -0700 Subject: [PATCH 3/5] Skip new test when running old link resolver implementation --- .../DocumentationContext/DocumentationContextTests.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift index cc5d1b2d9e..2c39b01b2b 100644 --- a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift @@ -1668,6 +1668,8 @@ let expected = """ } func testSpecialCharactersInLinks() throws { + try XCTSkipUnless(LinkResolutionMigrationConfiguration.shouldUseHierarchyBasedLinkResolver) + let originalSymbolGraph = Bundle.module.url(forResource: "TestBundle", withExtension: "docc", subdirectory: "Test Bundles")!.appendingPathComponent("mykit-iOS.symbols.json") let testBundle = Folder(name: "special-characters.docc", content: [ From 81d7e51db73546c437a927182e7335891dbb7b2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Wed, 5 Apr 2023 10:18:52 -0700 Subject: [PATCH 4/5] Add more type annotations in test to accommodate other compiler versions --- .../DocumentationContext/DocumentationContextTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift index 2c39b01b2b..f22a50e3c4 100644 --- a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift @@ -1753,7 +1753,7 @@ let expected = """ let contentSection = try XCTUnwrap(renderNode.primaryContentSections.first as? ContentRenderSection) - let lists = contentSection.content.compactMap({ content in + let lists: [RenderBlockContent.UnorderedList] = contentSection.content.compactMap({ (content: RenderBlockContent) -> RenderBlockContent.UnorderedList? in if case let .unorderedList(list) = content { return list } else { From c20c52e8ce9699807475a0f4b0b2b8f660e995b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Thu, 6 Apr 2023 13:52:06 -0700 Subject: [PATCH 5/5] Expand the fallback parsing of authored documentation links --- Sources/SwiftDocC/Utility/ValidatedURL.swift | 53 ++++++++++++++++++-- 1 file changed, 49 insertions(+), 4 deletions(-) diff --git a/Sources/SwiftDocC/Utility/ValidatedURL.swift b/Sources/SwiftDocC/Utility/ValidatedURL.swift index 883e2befd4..58ee986b55 100644 --- a/Sources/SwiftDocC/Utility/ValidatedURL.swift +++ b/Sources/SwiftDocC/Utility/ValidatedURL.swift @@ -62,12 +62,57 @@ public struct ValidatedURL: Hashable, Equatable { return } - // If the string doesn't contain a fragment and the string couldn't be parsed with `ValidatedURL(parsing:)` above, then consider it invalid. - guard let fragmentSeparatorIndex = string.firstIndex(of: "#"), var components = URLComponents(string: String(string[..