From 0e0cc28820ab12b520023f515c179c8a9cf27fe0 Mon Sep 17 00:00:00 2001 From: Hironori Ichimiya Date: Tue, 26 Mar 2024 21:30:35 +0900 Subject: [PATCH 1/8] Fix a doc anchor is not encoded correctly --- .../Infrastructure/NodeURLGenerator.swift | 2 +- .../Rendering/RenderContentCompiler.swift | 6 ++++- .../NodeURLGeneratorTests.swift | 13 +++++++++++ .../Model/RenderContentMetadataTests.swift | 22 +++++++++++++++++++ 4 files changed, 41 insertions(+), 2 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/NodeURLGenerator.swift b/Sources/SwiftDocC/Infrastructure/NodeURLGenerator.swift index 71efab8a1a..f279e5b765 100644 --- a/Sources/SwiftDocC/Infrastructure/NodeURLGenerator.swift +++ b/Sources/SwiftDocC/Infrastructure/NodeURLGenerator.swift @@ -171,7 +171,7 @@ public struct NodeURLGenerator { ) } else { let url = baseURL.appendingPathComponent(safePath, isDirectory: false) - return url.withFragment(reference.url.fragment) + return url.withFragment(reference.url.fragment?.removingPercentEncoding) } } diff --git a/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift b/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift index 1a26f01c3a..34a985c8b6 100644 --- a/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift +++ b/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift @@ -46,7 +46,11 @@ struct RenderContentCompiler: MarkupVisitor { } mutating func visitHeading(_ heading: Heading) -> [RenderContent] { - return [RenderBlockContent.heading(.init(level: heading.level, text: heading.plainText, anchor: urlReadableFragment(heading.plainText)))] + let fragment = urlReadableFragment(heading.plainText) + var components = URLComponents() + components.fragment = fragment + let anchor = components.url?.fragment ?? fragment + return [RenderBlockContent.heading(.init(level: heading.level, text: heading.plainText, anchor: anchor))] } mutating func visitListItem(_ listItem: ListItem) -> [RenderContent] { diff --git a/Tests/SwiftDocCTests/Infrastructure/NodeURLGeneratorTests.swift b/Tests/SwiftDocCTests/Infrastructure/NodeURLGeneratorTests.swift index a3b3c6aca1..ecfe47cc73 100644 --- a/Tests/SwiftDocCTests/Infrastructure/NodeURLGeneratorTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/NodeURLGeneratorTests.swift @@ -77,6 +77,19 @@ class NodeURLGeneratorTests: XCTestCase { XCTAssertEqual(generator.urlForReference(classIdentifier).absoluteString, "file:///path/to/bundle/_testbundle-ctlj/products/documentation.builtbundle/com.example.testbundle/data/folder/_privateclass/_privatesubclass") } + func testsafeURLWithEncodableFragment() throws { + // This is a realist DerivedData folder. + let baseURL = URL(string: "file:///path/to/bundle/_testbundle-ctlj/products/documentation.builtbundle/com.example.testbundle/data/")! + let generator = NodeURLGenerator(baseURL: baseURL) + + let basicIdentifier = ResolvedTopicReference(bundleIdentifier: "com.example.testbundle", + path: "/folder/class/symbol", + fragment: "テスト", // Non-ASCII; E3 83 86 E3 82 B9 E3 83 88 in UTF-8 + sourceLanguage: .swift) + + XCTAssertEqual(generator.urlForReference(basicIdentifier).absoluteString, "file:///path/to/bundle/_testbundle-ctlj/products/documentation.builtbundle/com.example.testbundle/data/folder/class/symbol#%E3%83%86%E3%82%B9%E3%83%88") + } + var inputLongPaths = [ // Paths within the limits "/path/to/symbol.json", diff --git a/Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift b/Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift index 07d79cac60..c52d33a020 100644 --- a/Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift +++ b/Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift @@ -297,4 +297,26 @@ class RenderContentMetadataTests: XCTestCase { default: XCTFail("Unexpected element") } } + + func testHeadingAnchorShouldBeEncoded() throws { + let (bundle, context) = try testBundleAndContext(named: "TestBundle") + var renderContentCompiler = RenderContentCompiler(context: context, bundle: bundle, identifier: ResolvedTopicReference(bundleIdentifier: bundle.identifier, path: "/path", fragment: nil, sourceLanguage: .swift)) + + let source = """ + ## テスト + + Note that テスト consists of Non-ASCII characters of E3 83 86 E3 82 B9 E3 83 88 in UTF-8. + """ + let document = Document(parsing: source) + + let result = try XCTUnwrap(renderContentCompiler.visit(document.child(at: 0)!)) + let element = try XCTUnwrap(result.first as? RenderBlockContent) + switch element { + case .heading(let heading): + XCTAssertEqual(heading.level, 2) + XCTAssertEqual(heading.text, "テスト") + XCTAssertEqual(heading.anchor, "%E3%83%86%E3%82%B9%E3%83%88") + default: XCTFail("Unexpected element") + } + } } From ea31679a2694da0320109d2064f4d53ca6942f96 Mon Sep 17 00:00:00 2001 From: Hironori Ichimiya Date: Sat, 27 Apr 2024 12:53:25 +0900 Subject: [PATCH 2/8] Add a test for rendering heading anchors --- .../Rendering/HeadingAnchorTests.swift | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 Tests/SwiftDocCTests/Rendering/HeadingAnchorTests.swift diff --git a/Tests/SwiftDocCTests/Rendering/HeadingAnchorTests.swift b/Tests/SwiftDocCTests/Rendering/HeadingAnchorTests.swift new file mode 100644 index 0000000000..f19550c241 --- /dev/null +++ b/Tests/SwiftDocCTests/Rendering/HeadingAnchorTests.swift @@ -0,0 +1,59 @@ +/* + This source file is part of the Swift.org open source project + + Copyright (c) 2021-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 + See https://swift.org/CONTRIBUTORS.txt for Swift project authors +*/ + +import Foundation +import XCTest +@testable import SwiftDocC +@_spi(FileManagerProtocol) import SwiftDocCTestUtilities + +class HeadingAnchorTests: XCTestCase { + func testEncodeHeadingAnchor() throws { + let catalogURL = try createTempFolder(content: [ + Folder(name: "unit-test.docc", content: [ + TextFile(name: "Root.md", utf8Content: """ + # My root page + + This single article defines two headings and links to them + + ### テスト + - + + ### Some heading + - + """), + ]) + ]) + let (_, bundle, context) = try loadBundle(from: catalogURL) + + let reference = try XCTUnwrap(context.soleRootModuleReference) + let node = try context.entity(with: reference) + let renderContext = RenderContext(documentationContext: context, bundle: bundle) + let converter = DocumentationContextConverter(bundle: bundle, context: context, renderContext: renderContext) + let renderNode = try XCTUnwrap(converter.renderNode(for: node, at: nil)) + + // Check heading anchors are encoded + let contentSection = try XCTUnwrap(renderNode.primaryContentSections.first as? ContentRenderSection) + let headings: [RenderBlockContent.Heading] = contentSection.content.compactMap { + if case .heading(let heading) = $0 { + return heading + } else { + return nil + } + } + XCTAssertEqual(headings[0].anchor, "%E3%83%86%E3%82%B9%E3%83%88") + XCTAssertEqual(headings[1].anchor, "Some-heading") + + // Check links to them + let testTopic0 = try XCTUnwrap(renderNode.references["doc://unit-test/documentation/Root#%E3%83%86%E3%82%B9%E3%83%88"] as? TopicRenderReference) + XCTAssertEqual(testTopic0.url, "/documentation/root#%E3%83%86%E3%82%B9%E3%83%88") + let testTopic1 = try XCTUnwrap(renderNode.references["doc://unit-test/documentation/Root#Some-heading"] as? TopicRenderReference) + XCTAssertEqual(testTopic1.url, "/documentation/root#Some-heading") + } +} From 0c6d03af251ee7cea2e019e9f3a852e3caa93390 Mon Sep 17 00:00:00 2001 From: Hironori Ichimiya Date: Sat, 27 Apr 2024 12:56:41 +0900 Subject: [PATCH 3/8] Pass stored unencoded fragment --- Sources/SwiftDocC/Infrastructure/NodeURLGenerator.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/SwiftDocC/Infrastructure/NodeURLGenerator.swift b/Sources/SwiftDocC/Infrastructure/NodeURLGenerator.swift index f279e5b765..577bcd0dfb 100644 --- a/Sources/SwiftDocC/Infrastructure/NodeURLGenerator.swift +++ b/Sources/SwiftDocC/Infrastructure/NodeURLGenerator.swift @@ -171,7 +171,7 @@ public struct NodeURLGenerator { ) } else { let url = baseURL.appendingPathComponent(safePath, isDirectory: false) - return url.withFragment(reference.url.fragment?.removingPercentEncoding) + return url.withFragment(reference.fragment) } } From 1a0da0eccd6b8efd926f6f1ccd447d53c3170bda Mon Sep 17 00:00:00 2001 From: Hironori Ichimiya Date: Wed, 1 May 2024 08:19:46 +0900 Subject: [PATCH 4/8] Remove the unsuitable test case Also apply suggestions from code review. --- .../Model/Rendering/RenderContentCompiler.swift | 6 +----- .../Infrastructure/NodeURLGeneratorTests.swift | 13 ------------- .../Model/RenderContentMetadataTests.swift | 4 +--- 3 files changed, 2 insertions(+), 21 deletions(-) diff --git a/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift b/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift index 34a985c8b6..201aafc912 100644 --- a/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift +++ b/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift @@ -46,11 +46,7 @@ struct RenderContentCompiler: MarkupVisitor { } mutating func visitHeading(_ heading: Heading) -> [RenderContent] { - let fragment = urlReadableFragment(heading.plainText) - var components = URLComponents() - components.fragment = fragment - let anchor = components.url?.fragment ?? fragment - return [RenderBlockContent.heading(.init(level: heading.level, text: heading.plainText, anchor: anchor))] + return [RenderBlockContent.heading(.init(level: heading.level, text: heading.plainText, anchor: urlReadableFragment(heading.plainText).addingPercentEncoding(withAllowedCharacters: .urlFragmentAllowed)))] } mutating func visitListItem(_ listItem: ListItem) -> [RenderContent] { diff --git a/Tests/SwiftDocCTests/Infrastructure/NodeURLGeneratorTests.swift b/Tests/SwiftDocCTests/Infrastructure/NodeURLGeneratorTests.swift index ecfe47cc73..a3b3c6aca1 100644 --- a/Tests/SwiftDocCTests/Infrastructure/NodeURLGeneratorTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/NodeURLGeneratorTests.swift @@ -77,19 +77,6 @@ class NodeURLGeneratorTests: XCTestCase { XCTAssertEqual(generator.urlForReference(classIdentifier).absoluteString, "file:///path/to/bundle/_testbundle-ctlj/products/documentation.builtbundle/com.example.testbundle/data/folder/_privateclass/_privatesubclass") } - func testsafeURLWithEncodableFragment() throws { - // This is a realist DerivedData folder. - let baseURL = URL(string: "file:///path/to/bundle/_testbundle-ctlj/products/documentation.builtbundle/com.example.testbundle/data/")! - let generator = NodeURLGenerator(baseURL: baseURL) - - let basicIdentifier = ResolvedTopicReference(bundleIdentifier: "com.example.testbundle", - path: "/folder/class/symbol", - fragment: "テスト", // Non-ASCII; E3 83 86 E3 82 B9 E3 83 88 in UTF-8 - sourceLanguage: .swift) - - XCTAssertEqual(generator.urlForReference(basicIdentifier).absoluteString, "file:///path/to/bundle/_testbundle-ctlj/products/documentation.builtbundle/com.example.testbundle/data/folder/class/symbol#%E3%83%86%E3%82%B9%E3%83%88") - } - var inputLongPaths = [ // Paths within the limits "/path/to/symbol.json", diff --git a/Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift b/Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift index c52d33a020..6a46ba7b86 100644 --- a/Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift +++ b/Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift @@ -304,8 +304,6 @@ class RenderContentMetadataTests: XCTestCase { let source = """ ## テスト - - Note that テスト consists of Non-ASCII characters of E3 83 86 E3 82 B9 E3 83 88 in UTF-8. """ let document = Document(parsing: source) @@ -315,7 +313,7 @@ class RenderContentMetadataTests: XCTestCase { case .heading(let heading): XCTAssertEqual(heading.level, 2) XCTAssertEqual(heading.text, "テスト") - XCTAssertEqual(heading.anchor, "%E3%83%86%E3%82%B9%E3%83%88") + XCTAssertEqual(heading.anchor, "%E3%83%86%E3%82%B9%E3%83%88", "The UTF-8 representation of テスト is E3 83 86 E3 82 B9 E3 83 88") default: XCTFail("Unexpected element") } } From a8a36fcdd0f97210c8708f88466200f6d31b42e9 Mon Sep 17 00:00:00 2001 From: Hironori Ichimiya Date: Mon, 6 May 2024 18:23:54 +0900 Subject: [PATCH 5/8] Apply suggestions from code review --- Tests/SwiftDocCTests/Rendering/HeadingAnchorTests.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/SwiftDocCTests/Rendering/HeadingAnchorTests.swift b/Tests/SwiftDocCTests/Rendering/HeadingAnchorTests.swift index f19550c241..589ed10f60 100644 --- a/Tests/SwiftDocCTests/Rendering/HeadingAnchorTests.swift +++ b/Tests/SwiftDocCTests/Rendering/HeadingAnchorTests.swift @@ -1,7 +1,7 @@ /* This source file is part of the Swift.org open source project - Copyright (c) 2021-2024 Apple Inc. and the Swift project authors + Copyright (c) 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 @@ -11,7 +11,7 @@ import Foundation import XCTest @testable import SwiftDocC -@_spi(FileManagerProtocol) import SwiftDocCTestUtilities +import SwiftDocCTestUtilities class HeadingAnchorTests: XCTestCase { func testEncodeHeadingAnchor() throws { From 4e3f8dad43e2b9a3440713bcc8cd680f08de4ba7 Mon Sep 17 00:00:00 2001 From: Hironori Ichimiya Date: Wed, 8 May 2024 19:18:29 +0900 Subject: [PATCH 6/8] Treat a link destination as the "authored link" --- Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift b/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift index 201aafc912..d7b768064b 100644 --- a/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift +++ b/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift @@ -131,7 +131,7 @@ struct RenderContentCompiler: MarkupVisitor { mutating func visitLink(_ link: Link) -> [RenderContent] { let destination = link.destination ?? "" // Before attempting to resolve the link, we confirm that is has a ResolvedTopicReference urlScheme - guard ResolvedTopicReference.urlHasResolvedTopicScheme(URL(string: destination)) else { + guard ResolvedTopicReference.urlHasResolvedTopicScheme(ValidatedURL(parsingAuthoredLink: destination)?.url) else { // This is an external URL which needs a ``LinkRenderReference``. let linkTitleInlineContent = link.children.reduce(into: [], { result, child in result.append(contentsOf: visit(child))}) as! [RenderInlineContent] let plainTextLinkTitle = linkTitleInlineContent.plainText From a7c6a43dcf02ecf645b5e8c342b5a32deea8252f Mon Sep 17 00:00:00 2001 From: Hironori Ichimiya Date: Thu, 9 May 2024 07:55:14 +0900 Subject: [PATCH 7/8] Revert "Treat a link destination as the "authored link"" This reverts commit 4e3f8dad43e2b9a3440713bcc8cd680f08de4ba7. --- Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift b/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift index d7b768064b..201aafc912 100644 --- a/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift +++ b/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift @@ -131,7 +131,7 @@ struct RenderContentCompiler: MarkupVisitor { mutating func visitLink(_ link: Link) -> [RenderContent] { let destination = link.destination ?? "" // Before attempting to resolve the link, we confirm that is has a ResolvedTopicReference urlScheme - guard ResolvedTopicReference.urlHasResolvedTopicScheme(ValidatedURL(parsingAuthoredLink: destination)?.url) else { + guard ResolvedTopicReference.urlHasResolvedTopicScheme(URL(string: destination)) else { // This is an external URL which needs a ``LinkRenderReference``. let linkTitleInlineContent = link.children.reduce(into: [], { result, child in result.append(contentsOf: visit(child))}) as! [RenderInlineContent] let plainTextLinkTitle = linkTitleInlineContent.plainText From 9b7cadf320febab8818f646e9f601a3237ef1657 Mon Sep 17 00:00:00 2001 From: Hironori Ichimiya Date: Thu, 9 May 2024 07:58:10 +0900 Subject: [PATCH 8/8] Mark the test article as a technology root --- Tests/SwiftDocCTests/Rendering/HeadingAnchorTests.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Tests/SwiftDocCTests/Rendering/HeadingAnchorTests.swift b/Tests/SwiftDocCTests/Rendering/HeadingAnchorTests.swift index 589ed10f60..7aeed16ef5 100644 --- a/Tests/SwiftDocCTests/Rendering/HeadingAnchorTests.swift +++ b/Tests/SwiftDocCTests/Rendering/HeadingAnchorTests.swift @@ -22,6 +22,10 @@ class HeadingAnchorTests: XCTestCase { This single article defines two headings and links to them + @Metadata { + @TechnologyRoot + } + ### テスト -