From 545d95180ab57a575c8321c15e045974e8b2ac1e Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Thu, 10 Apr 2025 14:22:26 -0700 Subject: [PATCH 01/22] Change package dependency to `release/6.2` (#1196) --- Package.resolved | 12 ++++++------ Package.swift | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Package.resolved b/Package.resolved index 3e7b804f6e..f5f7a9eeda 100644 --- a/Package.resolved +++ b/Package.resolved @@ -68,8 +68,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/swiftlang/swift-docc-symbolkit.git", "state" : { - "branch" : "main", - "revision" : "96bce1cfad4f4d7e265c1eb46729ebf8a7695f4b" + "branch" : "release/6.2", + "revision" : "467084cd380d352abcd128b27927ecdc8cb5bae8" } }, { @@ -77,8 +77,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/swiftlang/swift-lmdb.git", "state" : { - "branch" : "main", - "revision" : "c42582487fe84f72a4d417dd2d8493757bd4d072" + "branch" : "release/6.2", + "revision" : "1ad9a2d80b6fcde498c2242f509bd1be7d667ff8" } }, { @@ -86,8 +86,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/swiftlang/swift-markdown.git", "state" : { - "branch" : "main", - "revision" : "d21714073e0d16ba78eebdf36724863afc36871d" + "branch" : "release/6.2", + "revision" : "e62a44fd1f2764ba8807db3b6f257627449bbb8c" } }, { diff --git a/Package.swift b/Package.swift index 2381b5dfeb..5c5ac49c76 100644 --- a/Package.swift +++ b/Package.swift @@ -134,10 +134,10 @@ if ProcessInfo.processInfo.environment["SWIFTCI_USE_LOCAL_DEPS"] == nil { // Building standalone, so fetch all dependencies remotely. package.dependencies += [ .package(url: "https://github.com/apple/swift-nio.git", from: "2.53.0"), - .package(url: "https://github.com/swiftlang/swift-markdown.git", branch: "main"), - .package(url: "https://github.com/swiftlang/swift-lmdb.git", branch: "main"), + .package(url: "https://github.com/swiftlang/swift-markdown.git", branch: "release/6.2"), + .package(url: "https://github.com/swiftlang/swift-lmdb.git", branch: "release/6.2"), .package(url: "https://github.com/apple/swift-argument-parser.git", from: "1.2.2"), - .package(url: "https://github.com/swiftlang/swift-docc-symbolkit.git", branch: "main"), + .package(url: "https://github.com/swiftlang/swift-docc-symbolkit.git", branch: "release/6.2"), .package(url: "https://github.com/apple/swift-crypto.git", from: "3.0.0"), .package(url: "https://github.com/swiftlang/swift-docc-plugin.git", from: "1.2.0"), ] From ac9b204f7fa6ea4840de47b71fa80242912858f2 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Tue, 15 Apr 2025 10:57:14 -0600 Subject: [PATCH 02/22] update swift-markdown and swift-cmark for 6.2 (#1199) --- Package.resolved | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Package.resolved b/Package.resolved index f5f7a9eeda..21a53c4f86 100644 --- a/Package.resolved +++ b/Package.resolved @@ -3,7 +3,7 @@ { "identity" : "swift-argument-parser", "kind" : "remoteSourceControl", - "location" : "https://github.com/apple/swift-argument-parser.git", + "location" : "https://github.com/apple/swift-argument-parser", "state" : { "revision" : "0fbc8848e389af3bb55c182bc19ca9d5dc2f255b", "version" : "1.4.0" @@ -32,8 +32,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/swiftlang/swift-cmark.git", "state" : { - "branch" : "gfm", - "revision" : "2c47322cb32cbed555f13bf5cbfaa488cc30a785" + "branch" : "release/6.2", + "revision" : "b97d09472e847a416629f026eceae0e2afcfad65" } }, { @@ -87,7 +87,7 @@ "location" : "https://github.com/swiftlang/swift-markdown.git", "state" : { "branch" : "release/6.2", - "revision" : "e62a44fd1f2764ba8807db3b6f257627449bbb8c" + "revision" : "bc668ada42187e6c18eac420d66050a76d4ed764" } }, { From 879158477e1433b6b99b9fdb9a6416682f9324ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Wed, 23 Apr 2025 15:25:58 +0200 Subject: [PATCH 03/22] Fix crash when a nested symbol is extended and the a local symbol shadow extended module name (#1202) (#1206) * Use `inContextOf` relationships to form hierarchy * Fixed inversed filters in debug assertion messages * Avoid computing disambiguation during path hierarchy initialization rdar://149838723 --- .../PathHierarchy+DisambiguatedPaths.swift | 2 + .../Link Resolution/PathHierarchy+Find.swift | 21 ++++-- .../Link Resolution/PathHierarchy.swift | 12 ++-- .../Infrastructure/PathHierarchyTests.swift | 72 +++++++++++++++++++ 4 files changed, 94 insertions(+), 13 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+DisambiguatedPaths.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+DisambiguatedPaths.swift index 545e5f50ff..b5cb8b4492 100644 --- a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+DisambiguatedPaths.swift +++ b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+DisambiguatedPaths.swift @@ -193,6 +193,8 @@ extension PathHierarchy.DisambiguationContainer { includeLanguage: Bool = false, allowAdvancedDisambiguation: Bool = true ) -> [(value: PathHierarchy.Node, disambiguation: Disambiguation)] { + assert(elements.allSatisfy({ $0.node.identifier != nil }), "All nodes should have been assigned an identifier before their disambiguation can be computed.") + var collisions = _disambiguatedValues(for: elements, includeLanguage: includeLanguage, allowAdvancedDisambiguation: allowAdvancedDisambiguation) // If all but one of the collisions are disfavored, remove the disambiguation for the only favored element. diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Find.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Find.swift index c83d49db6c..0ead6bbe16 100644 --- a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Find.swift +++ b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Find.swift @@ -447,19 +447,26 @@ extension PathHierarchy.DisambiguationContainer { case lookupCollision([(node: PathHierarchy.Node, disambiguation: String)]) } - /// Attempts to find a value in the disambiguation tree based on partial disambiguation information. + /// Attempts to find the only element in the disambiguation container without using any disambiguation information. + /// + /// - Returns: The only element in the container or `nil` if the container has more than one element. + func singleMatch() -> PathHierarchy.Node? { + if storage.count <= 1 { + return storage.first?.node + } else { + return storage.singleMatch({ !$0.node.isDisfavoredInLinkCollisions })?.node + } + } + + /// Attempts to find a value in the disambiguation container based on partial disambiguation information. /// /// There are 3 possible results: /// - No match is found; indicated by a `nil` return value. /// - Exactly one match is found; indicated by a non-nil return value. /// - More than one match is found; indicated by a raised error listing the matches and their missing disambiguation. func find(_ disambiguation: PathHierarchy.PathComponent.Disambiguation?) throws -> PathHierarchy.Node? { - if disambiguation == nil { - if storage.count <= 1 { - return storage.first?.node - } else if let favoredMatch = storage.singleMatch({ !$0.node.isDisfavoredInLinkCollisions }) { - return favoredMatch.node - } + if disambiguation == nil, let match = singleMatch() { + return match } switch disambiguation { diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift index 7341f1b785..3dbb6ddc0e 100644 --- a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift +++ b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift @@ -232,7 +232,7 @@ struct PathHierarchy { return original } }(node.symbol!)[...].dropLast() - while !components.isEmpty, let child = try? parent.children[components.first!]?.find(nil) { + while !components.isEmpty, let child = parent.children[components.first!]?.singleMatch() { parent = child components = components.dropFirst() } @@ -311,7 +311,7 @@ struct PathHierarchy { assert( allNodes.allSatisfy({ $0.value[0].parent != nil || roots[$0.key] != nil }), """ Every node should either have a parent node or be a root node. \ - This wasn't true for \(allNodes.filter({ $0.value[0].parent != nil || roots[$0.key] != nil }).map(\.key).sorted()) + This wasn't true for \(allNodes.filter({ $0.value[0].parent == nil && roots[$0.key] == nil }).map(\.key).sorted()) """ ) @@ -320,7 +320,7 @@ struct PathHierarchy { Array(sequence(first: node, next: \.parent)).last!.symbol!.kind.identifier == .module }) }), """ Every node should reach a root node by following its parents up. \ - This wasn't true for \(allNodes.filter({ $0.value.allSatisfy({ Array(sequence(first: $0, next: \.parent)).last!.symbol!.kind.identifier == .module }) }).map(\.key).sorted()) + This wasn't true for \(allNodes.filter({ $0.value.allSatisfy({ Array(sequence(first: $0, next: \.parent)).last!.symbol!.kind.identifier != .module }) }).map(\.key).sorted()) """ ) @@ -366,7 +366,7 @@ struct PathHierarchy { assert( lookup.allSatisfy({ $0.value.parent != nil || roots[$0.value.name] != nil }), """ Every node should either have a parent node or be a root node. \ - This wasn't true for \(allNodes.filter({ $0.value[0].parent != nil || roots[$0.key] != nil }).map(\.key).sorted()) + This wasn't true for \(allNodes.filter({ $0.value[0].parent == nil && roots[$0.key] == nil }).map(\.key).sorted()) """ ) @@ -389,7 +389,7 @@ struct PathHierarchy { assert( lookup.values.allSatisfy({ $0.parent?.identifier == nil || lookup[$0.parent!.identifier] != nil }), """ Every node's findable parent should exist in the lookup. \ - This wasn't true for \(lookup.values.filter({ $0.parent?.identifier == nil || lookup[$0.parent!.identifier] != nil }).map(\.symbol!.identifier.precise).sorted()) + This wasn't true for \(lookup.values.filter({ $0.parent?.identifier != nil && lookup[$0.parent!.identifier] == nil }).map(\.symbol!.identifier.precise).sorted()) """ ) @@ -799,7 +799,7 @@ private extension SymbolGraph.Relationship.Kind { /// Whether or not this relationship kind forms a hierarchical relationship between the source and the target. var formsHierarchy: Bool { switch self { - case .memberOf, .optionalMemberOf, .requirementOf, .optionalRequirementOf, .extensionTo, .declaredIn: + case .memberOf, .optionalMemberOf, .requirementOf, .optionalRequirementOf, .extensionTo, .inContextOf, .declaredIn: return true default: return false diff --git a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift index 220babb16b..8c0609848f 100644 --- a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift @@ -2072,6 +2072,78 @@ class PathHierarchyTests: XCTestCase { try assertFindsPath("Inner/InnerClass/something()", in: tree, asSymbolID: "s:5Inner0A5ClassC5OuterE9somethingyyF") } + func testExtensionSymbolsWithSameNameAsExtendedModule() throws { + // ---- ExtendedModule + // public struct SomeStruct { + // public struct SomeNestedStruct {} + // } + // + // ---- ModuleName + // public import ExtendedModule + // + // // Shadow the ExtendedModule module with a local type + // public enum ExtendedModule {} + // + // // Extend the nested type from the extended module + // public extension SomeStruct.SomeNestedStruct { + // func doSomething() {} + // } + + let extensionMixin = SymbolGraph.Symbol.Swift.Extension(extendedModule: "ExtendedModule", typeKind: .struct, constraints: []) + + let extensionSymbolID = "s:e:s:14ExtendedModule10SomeStructV0c6NestedD0V0B4NameE11doSomethingyyF" + let extendedMethodSymbolID = "s:14ExtendedModule10SomeStructV0c6NestedD0V0B4NameE11doSomethingyyF" + + let catalog = Folder(name: "CatalogName.docc", content: [ + JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph( + moduleName: "ModuleName", + symbols: [ + makeSymbol(id: "s:10ModuleName08ExtendedA0O", kind: .enum, pathComponents: ["ExtendedModule"]) + ]) + ), + + JSONFile(name: "ModuleName@ExtendedModule.symbols.json", content: makeSymbolGraph( + moduleName: "ModuleName", + symbols: [ + // The 'SomeNestedStruct' extension + makeSymbol(id: extensionSymbolID, kind: .extension, pathComponents: ["SomeStruct", "SomeNestedStruct"], otherMixins: [extensionMixin]), + // The 'doSomething()' method added in the extension + makeSymbol(id: extendedMethodSymbolID, kind: .method, pathComponents: ["SomeStruct", "SomeNestedStruct", "doSomething()"], otherMixins: [extensionMixin]), + ], + relationships: [ + // 'doSomething()' is a member of the extension + .init(source: extendedMethodSymbolID, target: extensionSymbolID, kind: .memberOf, targetFallback: "ExtendedModule.SomeStruct.SomeNestedStruct"), + // The extension extends the external 'SomeNestedStruct' symbol + .init(source: extensionSymbolID, target: "s:14ExtendedModule10SomeStructV0c6NestedD0V", kind: .extensionTo, targetFallback: "ExtendedModule.SomeStruct.SomeNestedStruct"), + ]) + ), + ]) + + let (_, context) = try loadBundle(catalog: catalog) + let tree = context.linkResolver.localResolver.pathHierarchy + + let paths = tree.caseInsensitiveDisambiguatedPaths() + XCTAssertEqual(paths[extendedMethodSymbolID], "/ModuleName/ExtendedModule/SomeStruct/SomeNestedStruct/doSomething()") + + try assertPathCollision("ModuleName/ExtendedModule", in: tree, collisions: [ + ("s:m:s:e:\(extensionSymbolID)", "-module.extension"), + ("s:10ModuleName08ExtendedA0O", "-enum"), + ]) + // If the first path component is ambiguous, it should have the same error as if that was a later path component. + try assertPathCollision("ExtendedModule", in: tree, collisions: [ + ("s:m:s:e:\(extensionSymbolID)", "-module.extension"), + ("s:10ModuleName08ExtendedA0O", "-enum"), + ]) + + try assertFindsPath("ExtendedModule-enum", in: tree, asSymbolID: "s:10ModuleName08ExtendedA0O") + try assertFindsPath("ExtendedModule-module.extension", in: tree, asSymbolID: "s:m:s:e:\(extensionSymbolID)") + + // The "Inner" struct doesn't have "InnerStruct" or "InnerClass" descendants so the path is not ambiguous. + try assertFindsPath("ExtendedModule/SomeStruct", in: tree, asSymbolID: "s:e:\(extensionSymbolID)") + try assertFindsPath("ExtendedModule/SomeStruct/SomeNestedStruct", in: tree, asSymbolID: extensionSymbolID) + try assertFindsPath("ExtendedModule/SomeStruct/SomeNestedStruct/doSomething()", in: tree, asSymbolID: extendedMethodSymbolID) + } + func testContinuesSearchingIfNonSymbolMatchesSymbolLink() throws { let exampleDocumentation = Folder(name: "CatalogName.docc", content: [ JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(moduleName: "ModuleName", symbols: [ From 687bc8fb21ca41f495ce45547a2efa44c0624c38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Wed, 23 Apr 2025 15:43:08 +0200 Subject: [PATCH 04/22] Support `Any` Swift keyword in type signature disambiguation (#1201) (#1205) rdar://148522712 --- .../PathHierarchy+TypeSignature.swift | 3 + .../Infrastructure/PathHierarchyTests.swift | 129 +++++++++++++++++- 2 files changed, 127 insertions(+), 5 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+TypeSignature.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+TypeSignature.swift index dd1ed9218b..52af84c174 100644 --- a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+TypeSignature.swift +++ b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+TypeSignature.swift @@ -129,6 +129,9 @@ extension PathHierarchy { // Accumulate all of the identifier tokens' spelling. accumulated.append(contentsOf: fragment.spelling.utf8) + case .keyword where fragment.spelling == "Any": + accumulated.append(contentsOf: fragment.spelling.utf8) + case .text: // In Swift, we're only want some `text` tokens characters in the type disambiguation. // For example: "[", "?", "<", "...", ",", "(", "->" etc. contribute to the type spellings like // `[Name]`, `Name?`, "Name", "Name...", "()", "(Name, Name)", "(Name)->Name" and more. diff --git a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift index 8c0609848f..08145d8a89 100644 --- a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift @@ -1408,6 +1408,19 @@ class PathHierarchyTests: XCTestCase { .init(kind: .text, spelling: ">", preciseIdentifier: nil), ])) + // Any + XCTAssertEqual("Any", functionSignatureParameterTypeName([ + .init(kind: .keyword, spelling: "Any", preciseIdentifier: nil), + ])) + + // Array + XCTAssertEqual("[Any]", functionSignatureParameterTypeName([ + .init(kind: .typeIdentifier, spelling: "Array", preciseIdentifier: "s:Sa"), + .init(kind: .text, spelling: "<", preciseIdentifier: nil), + .init(kind: .keyword, spelling: "Any", preciseIdentifier: nil), + .init(kind: .text, spelling: ">", preciseIdentifier: nil), + ])) + // some Sequence XCTAssertEqual("Sequence", functionSignatureParameterTypeName([ .init(kind: .keyword, spelling: "some", preciseIdentifier: nil), @@ -1773,7 +1786,7 @@ class PathHierarchyTests: XCTestCase { .init(name: "someName", externalName: nil, declarationFragments: [ .init(kind: .identifier, spelling: "someName", preciseIdentifier: nil), .init(kind: .text, spelling: ": ((", preciseIdentifier: nil), - .init(kind: .typeIdentifier, spelling: "Int", preciseIdentifier: "s:Si"), + .init(kind: .keyword, spelling: "Any", preciseIdentifier: nil), .init(kind: .text, spelling: ", ", preciseIdentifier: nil), .init(kind: .typeIdentifier, spelling: "String", preciseIdentifier: "s:SS"), .init(kind: .text, spelling: "), ", preciseIdentifier: nil), @@ -1789,10 +1802,10 @@ class PathHierarchyTests: XCTestCase { .init(kind: .text, spelling: "?)", preciseIdentifier: nil), ]) ) - XCTAssertEqual(tupleArgument?.parameterTypeNames, ["((Int,String),Date)"]) + XCTAssertEqual(tupleArgument?.parameterTypeNames, ["((Any,String),Date)"]) XCTAssertEqual(tupleArgument?.returnTypeNames, ["[Int]", "String?"]) - // func doSomething() -> ((Double, Double) -> Double, [Int: (Int, Int)], (Bool, Bool), String?) + // func doSomething() -> ((Double, Double) -> Double, [Int: (Int, Int)], (Bool, Any), String?) let bigTupleReturnType = functionSignatureTypeNames(.init( parameters: [], returns: [ @@ -1811,7 +1824,7 @@ class PathHierarchyTests: XCTestCase { .init(kind: .text, spelling: ")], (", preciseIdentifier: nil), .init(kind: .typeIdentifier, spelling: "Bool", preciseIdentifier: "s:Si"), .init(kind: .text, spelling: ", ", preciseIdentifier: nil), - .init(kind: .typeIdentifier, spelling: "Bool", preciseIdentifier: "s:Si"), + .init(kind: .keyword, spelling: "Any", preciseIdentifier: nil), .init(kind: .text, spelling: "), ", preciseIdentifier: nil), .init(kind: .typeIdentifier, spelling: "Optional", preciseIdentifier: "s:Sq"), .init(kind: .text, spelling: "<", preciseIdentifier: nil), @@ -1820,7 +1833,7 @@ class PathHierarchyTests: XCTestCase { ]) ) XCTAssertEqual(bigTupleReturnType?.parameterTypeNames, []) - XCTAssertEqual(bigTupleReturnType?.returnTypeNames, ["(Double,Double)->Double", "[Int:(Int,Int)]", "(Bool,Bool)", "String?"]) + XCTAssertEqual(bigTupleReturnType?.returnTypeNames, ["(Double,Double)->Double", "[Int:(Int,Int)]", "(Bool,Any)", "String?"]) // func doSomething(with someName: [Int?: String??]) let dictionaryWithOptionalsArgument = functionSignatureTypeNames(.init( @@ -1918,6 +1931,112 @@ class PathHierarchyTests: XCTestCase { } } + func testParameterDisambiguationWithAnyType() throws { + // Create two overloads with different parameter types + let parameterTypes: [SymbolGraph.Symbol.DeclarationFragments.Fragment] = [ + // Any (swift) + .init(kind: .keyword, spelling: "Any", preciseIdentifier: nil), + // AnyObject (swift) + .init(kind: .typeIdentifier, spelling: "AnyObject", preciseIdentifier: "s:s9AnyObjecta"), + ] + + let catalog = Folder(name: "CatalogName.docc", content: [ + JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(moduleName: "ModuleName", symbols: parameterTypes.map { parameterTypeFragment in + makeSymbol(id: "some-function-id-\(parameterTypeFragment.spelling)", kind: .func, pathComponents: ["doSomething(with:)"], signature: .init( + parameters: [ + .init(name: "something", externalName: "with", declarationFragments: [ + .init(kind: .identifier, spelling: "something", preciseIdentifier: nil), + .init(kind: .text, spelling: ": ", preciseIdentifier: nil), + parameterTypeFragment + ], children: []) + ], + returns: [ + .init(kind: .text, spelling: "()", preciseIdentifier: nil) // 'Void' in text representation + ] + )) + })), + ]) + let (_, context) = try loadBundle(catalog: catalog) + let tree = context.linkResolver.localResolver.pathHierarchy + + XCTAssert(context.problems.isEmpty, "Unexpected problems \(context.problems.map(\.diagnostic.summary))") + + let paths = tree.caseInsensitiveDisambiguatedPaths() + + XCTAssertEqual(paths["some-function-id-Any"], "/ModuleName/doSomething(with:)-(Any)") + XCTAssertEqual(paths["some-function-id-AnyObject"], "/ModuleName/doSomething(with:)-(AnyObject)") + + try assertPathCollision("doSomething(with:)", in: tree, collisions: [ + ("some-function-id-Any", "-(Any)"), + ("some-function-id-AnyObject", "-(AnyObject)"), + ]) + + try assertPathRaisesErrorMessage("doSomething(with:)", in: tree, context: context, expectedErrorMessage: "'doSomething(with:)' is ambiguous at '/ModuleName'") { error in + XCTAssertEqual(error.solutions.count, 2) + + // These test symbols don't have full declarations. A real solution would display enough information to distinguish these. + XCTAssertEqual(error.solutions.dropFirst(0).first, .init(summary: "Insert '-(Any)' for \n'doSomething(with:)'" , replacements: [("-(Any)", 18, 18)])) + XCTAssertEqual(error.solutions.dropFirst(1).first, .init(summary: "Insert '-(AnyObject)' for \n'doSomething(with:)'" /* the test symbols don't have full declarations */, replacements: [("-(AnyObject)", 18, 18)])) + } + + try assertFindsPath("doSomething(with:)-(Any)", in: tree, asSymbolID: "some-function-id-Any") + try assertFindsPath("doSomething(with:)-(Any)->()", in: tree, asSymbolID: "some-function-id-Any") + try assertFindsPath("doSomething(with:)-5gdco", in: tree, asSymbolID: "some-function-id-Any") + + try assertFindsPath("doSomething(with:)-(AnyObject)", in: tree, asSymbolID: "some-function-id-AnyObject") + try assertFindsPath("doSomething(with:)-(AnyObject)->()", in: tree, asSymbolID: "some-function-id-AnyObject") + try assertFindsPath("doSomething(with:)-9kd0v", in: tree, asSymbolID: "some-function-id-AnyObject") + } + + func testReturnDisambiguationWithAnyType() throws { + // Create two overloads with different return types + let returnTypes: [SymbolGraph.Symbol.DeclarationFragments.Fragment] = [ + // Any (swift) + .init(kind: .keyword, spelling: "Any", preciseIdentifier: nil), + // AnyObject (swift) + .init(kind: .typeIdentifier, spelling: "AnyObject", preciseIdentifier: "s:s9AnyObjecta"), + ] + + let catalog = Folder(name: "CatalogName.docc", content: [ + JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(moduleName: "ModuleName", symbols: returnTypes.map { parameterTypeFragment in + makeSymbol(id: "some-function-id-\(parameterTypeFragment.spelling)", kind: .func, pathComponents: ["doSomething()"], signature: .init( + parameters: [], + returns: [parameterTypeFragment] + )) + })), + ]) + let (_, context) = try loadBundle(catalog: catalog) + let tree = context.linkResolver.localResolver.pathHierarchy + + XCTAssert(context.problems.isEmpty, "Unexpected problems \(context.problems.map(\.diagnostic.summary))") + + let paths = tree.caseInsensitiveDisambiguatedPaths() + + XCTAssertEqual(paths["some-function-id-Any"], "/ModuleName/doSomething()->Any") + XCTAssertEqual(paths["some-function-id-AnyObject"], "/ModuleName/doSomething()->AnyObject") + + try assertPathCollision("doSomething()", in: tree, collisions: [ + ("some-function-id-Any", "->Any"), + ("some-function-id-AnyObject", "->AnyObject"), + ]) + + try assertPathRaisesErrorMessage("doSomething()", in: tree, context: context, expectedErrorMessage: "'doSomething()' is ambiguous at '/ModuleName'") { error in + XCTAssertEqual(error.solutions.count, 2) + + // These test symbols don't have full declarations. A real solution would display enough information to distinguish these. + XCTAssertEqual(error.solutions.dropFirst(0).first, .init(summary: "Insert '->Any' for \n'doSomething()'" , replacements: [("->Any", 13, 13)])) + XCTAssertEqual(error.solutions.dropFirst(1).first, .init(summary: "Insert '->AnyObject' for \n'doSomething()'" /* the test symbols don't have full declarations */, replacements: [("->AnyObject", 13, 13)])) + } + + try assertFindsPath("doSomething()->Any", in: tree, asSymbolID: "some-function-id-Any") + try assertFindsPath("doSomething()-()->Any", in: tree, asSymbolID: "some-function-id-Any") + try assertFindsPath("doSomething()-5gdco", in: tree, asSymbolID: "some-function-id-Any") + + try assertFindsPath("doSomething()->AnyObject", in: tree, asSymbolID: "some-function-id-AnyObject") + try assertFindsPath("doSomething()-()->AnyObject", in: tree, asSymbolID: "some-function-id-AnyObject") + try assertFindsPath("doSomething()-9kd0v", in: tree, asSymbolID: "some-function-id-AnyObject") + } + func testOverloadGroupSymbolsResolveLinksWithoutHash() throws { enableFeatureFlag(\.isExperimentalOverloadedSymbolPresentationEnabled) From 68bb59dc65d06a0fd07d99570a83db59cd6f40d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Wed, 23 Apr 2025 15:49:35 +0200 Subject: [PATCH 05/22] Use the identifier from the lookup's key instead of the node (#1200) (#1204) rdar://148504975 --- .../Link Resolution/PathHierarchy+Serialization.swift | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Serialization.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Serialization.swift index 44dccd4814..d1cfe15985 100644 --- a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Serialization.swift +++ b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Serialization.swift @@ -37,9 +37,11 @@ extension PathHierarchy.FileRepresentation { } let nodes = [Node](unsafeUninitializedCapacity: lookup.count) { buffer, initializedCount in - for node in lookup.values { + for (identifier, node) in lookup { + assert(identifier == node.identifier, "Every node lookup should match a node with that identifier.") + buffer.initializeElement( - at: identifierMap[node.identifier]!, + at: identifierMap[identifier]!, to: Node( name: node.name, rawSpecialBehavior: node.specialBehaviors.rawValue, From 10aaa0a93b889248a71f2497f5a1801a56a79cc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Wed, 23 Apr 2025 17:07:24 +0200 Subject: [PATCH 06/22] =?UTF-8?q?Implemented=20alphabetic=20sorting=20by?= =?UTF-8?q?=20titles=20when=20automatically=20curating=20=E2=80=A6=20(#119?= =?UTF-8?q?5)=20(#1207)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Implemented alphabetic sorting by titles when automatically curating documentation + added testing and inline comments on such implementation * Transitioned to implict parameters within sorting closure, as well as additional test on case-insensitive sorting with its respective comments Co-authored-by: Ricardo Mendez Cavalieri <110005362+ramcav@users.noreply.github.com> rdar://148534834 --- .../Infrastructure/DocumentationContext.swift | 10 +- .../AutomaticCurationTests.swift | 106 ++++++++++++++++++ 2 files changed, 114 insertions(+), 2 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift index ff47ec35ee..f0b11089b1 100644 --- a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift +++ b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift @@ -2126,7 +2126,7 @@ public class DocumentationContext { /// - otherArticles: Non-root articles to curate. /// - rootNode: The node that will serve as the source of any topic graph edges created by this method. /// - Throws: If looking up a `DocumentationNode` for the root module reference fails. - /// - Returns: An array of resolved references to the articles that were automatically curated. + /// - Returns: An array of resolved references to the articles that were automatically curated, sorted by their titles. private func autoCurateArticles(_ otherArticles: DocumentationContext.Articles, startingFrom rootNode: TopicGraph.Node) throws -> [ResolvedTopicReference] { let articlesToAutoCurate = otherArticles.filter { article in let reference = article.topicGraphNode.reference @@ -2140,8 +2140,14 @@ public class DocumentationContext { topicGraph.addEdge(from: rootNode, to: article.topicGraphNode) uncuratedArticles.removeValue(forKey: article.topicGraphNode.reference) } + + // Sort the articles by their titles to ensure a deterministic order + let sortedArticles = articlesToAutoCurate.sorted { + $0.topicGraphNode.title.lowercased() < $1.topicGraphNode.title.lowercased() + } + + let articleReferences = sortedArticles.map(\.topicGraphNode.reference) - let articleReferences = articlesToAutoCurate.map(\.topicGraphNode.reference) let automaticTaskGroup = AutomaticTaskGroupSection( title: "Articles", references: articleReferences, diff --git a/Tests/SwiftDocCTests/Infrastructure/AutomaticCurationTests.swift b/Tests/SwiftDocCTests/Infrastructure/AutomaticCurationTests.swift index cc8cdc6be1..1d5761ee54 100644 --- a/Tests/SwiftDocCTests/Infrastructure/AutomaticCurationTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/AutomaticCurationTests.swift @@ -1265,4 +1265,110 @@ class AutomaticCurationTests: XCTestCase { XCTAssertFalse(renderNode.topicSections.first?.generated ?? false) } } + + func testAutomaticallyCuratedArticlesAreSortedByTitle() throws { + // Test bundle with articles where file names and titles are in different orders + let catalog = Folder(name: "TestBundle.docc", content: [ + JSONFile(name: "TestModule.symbols.json", content: makeSymbolGraph(moduleName: "TestModule")), + + TextFile(name: "C-Article.md", utf8Content: """ + # A Article + """), + + TextFile(name: "B-Article.md", utf8Content: """ + # B Article + """), + + TextFile(name: "A-Article.md", utf8Content: """ + # C Article + """), + ]) + + // Load the bundle + let (_, context) = try loadBundle(catalog: catalog) + XCTAssert(context.problems.isEmpty, "Unexpected problems: \(context.problems.map(\.diagnostic.summary))") + + // Get the module and its automatic curation groups + let moduleReference = try XCTUnwrap(context.soleRootModuleReference) + let moduleNode = try XCTUnwrap(context.entity(with: moduleReference)) + let symbol = try XCTUnwrap(moduleNode.semantic as? Symbol) + let articlesGroup = try XCTUnwrap( + symbol.automaticTaskGroups.first(where: { $0.title == "Articles" }), + "Expected 'Articles' automatic task group" + ) + + // Get the titles of the articles in the order they appear in the automatic curation + let titles = articlesGroup.references.compactMap { + context.topicGraph.nodes[$0]?.title + } + + // Verify we have 3 articles in title order (A, B, C)—file order does not matter + XCTAssertEqual(titles, ["A Article", "B Article", "C Article"], + "Articles should be sorted by title, not by file name") + } + + // autoCuratedArticles are sorted by title in a case-insensitive manner + // this test verifies that the sorting is correct even when the file names have different cases + func testAutomaticallyCuratedArticlesAreSortedByTitleDifferentCases() throws { + + // In the catalog, the articles are named with the same letter, different cases, + // and other articles are added as well + let catalog = Folder(name: "TestBundle.docc", content: [ + JSONFile(name: "TestModule.symbols.json", content: makeSymbolGraph(moduleName: "TestModule")), + + TextFile(name: "C-article.md", utf8Content: """ + # C Article + """), + + TextFile(name: "c-article.md", utf8Content: """ + # c Article2 + """), + + TextFile(name: "A-article.md", utf8Content: """ + # A Article + """), + + TextFile(name: "a-article.md", utf8Content: """ + # a Article2 + """), + + TextFile(name: "B-article.md", utf8Content: """ + # B Article + """), + + TextFile(name: "b-article.md", utf8Content: """ + # b Article2 + """), + + TextFile(name: "k-article.md", utf8Content: """ + # k Article + """), + + + TextFile(name: "random-article.md", utf8Content: """ + # Z Article + """), + ]) + + // Load the bundle + let (_, context) = try loadBundle(catalog: catalog) + XCTAssert(context.problems.isEmpty, "Unexpected problems: \(context.problems.map(\.diagnostic.summary))") + + // Get the module and its automatic curation groups + let moduleReference = try XCTUnwrap(context.soleRootModuleReference) + let moduleNode = try XCTUnwrap(context.entity(with: moduleReference)) + let symbol = try XCTUnwrap(moduleNode.semantic as? Symbol) + let articlesGroup = try XCTUnwrap( + symbol.automaticTaskGroups.first(where: { $0.title == "Articles" }), + "Expected 'Articles' automatic task group" + ) + + let titles = articlesGroup.references.compactMap { + context.topicGraph.nodes[$0]?.title + } + + // Verify that the articles are sorted by title, not by file name + XCTAssertEqual(titles, ["A Article", "a Article2", "B Article", "b Article2", "C Article", "c Article2", "k Article", "Z Article"], + "Articles should be sorted by title, not by file name") + } } From 5ba3de2c2acf76b382e3a78de8aa3216a0d6d4ad Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Tue, 29 Apr 2025 15:12:15 -0600 Subject: [PATCH 07/22] clone path hierarchy nodes when their parent has a language-specific counterpart (#1203) (#1209) rdar://144862231 --- .../Link Resolution/PathHierarchy.swift | 64 ++++++++++++++- .../Infrastructure/PathHierarchyTests.swift | 80 ++++++++++++++++++- 2 files changed, 141 insertions(+), 3 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift index 3dbb6ddc0e..c72e73add6 100644 --- a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift +++ b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift @@ -148,7 +148,51 @@ struct PathHierarchy { // would require that we redundantly create multiple nodes for the same symbol in many common cases and then merge them. To avoid doing that, we instead check // the source symbol's path components to find the correct target symbol by matching its name. if let targetNode = nodes[relationship.target], targetNode.name == expectedContainerName { - targetNode.add(symbolChild: sourceNode) + if sourceNode.parent == nil { + targetNode.add(symbolChild: sourceNode) + } else if sourceNode.parent !== targetNode { + // If the node we have for the child has an existing parent that doesn't + // match the parent from this symbol graph, we need to clone the child to + // ensure that the hierarchy remains consistent. + let clonedSourceNode = Node( + cloning: sourceNode, + symbol: graph.symbols[relationship.source], + children: [:], + languages: [language!] + ) + + // The original node no longer represents this symbol graph's language, + // so remove that data from there. + sourceNode.languages.remove(language!) + + // Make sure that the clone's children can all line up with symbols from this symbol graph. + for (childName, children) in sourceNode.children { + for child in children.storage { + guard let childSymbol = child.node.symbol else { + // We shouldn't come across any non-symbol nodes here, + // but assume they can work as child of both variants. + clonedSourceNode.add(child: child.node, kind: child.kind, hash: child.hash) + continue + } + if nodes[childSymbol.identifier.precise] === child.node { + clonedSourceNode.add(symbolChild: child.node) + } + } + } + + // Track the cloned node in the lists of nodes. + nodes[relationship.source] = clonedSourceNode + if let existingNodes = allNodes[relationship.source] { + clonedSourceNode.counterpart = existingNodes.first + for other in existingNodes { + other.counterpart = clonedSourceNode + } + } + allNodes[relationship.source, default: []].append(clonedSourceNode) + + // Finally, add the cloned node as a child of its parent. + targetNode.add(symbolChild: clonedSourceNode) + } topLevelCandidates.removeValue(forKey: relationship.source) } else if var targetNodes = allNodes[relationship.target] { // If the source was added in an extension symbol graph file, then its target won't be found in the same symbol graph file (in `nodes`). @@ -532,7 +576,21 @@ extension PathHierarchy { self.children = [:] self.specialBehaviors = [] } - + + /// Initializes a node with a new identifier but the data from an existing node. + fileprivate init( + cloning source: Node, + symbol: SymbolGraph.Symbol?? = nil, + children: [String: DisambiguationContainer]? = nil, + languages: Set? = nil + ) { + self.symbol = symbol ?? source.symbol + self.name = source.name + self.children = children ?? source.children + self.specialBehaviors = source.specialBehaviors + self.languages = languages ?? source.languages + } + /// Adds a descendant to this node, providing disambiguation information from the node's symbol. fileprivate func add(symbolChild: Node) { precondition(symbolChild.symbol != nil) @@ -558,6 +616,8 @@ extension PathHierarchy { ) return } + + assert(child.parent == nil, "Nodes that already have a parent should not be added to a different parent.") // If the name was passed explicitly, then the node could have spaces in its name child.parent = self children[child.name, default: .init()].add(child, kind: kind, hash: hash, parameterTypes: parameterTypes, returnTypes: returnTypes) diff --git a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift index 08145d8a89..469f5718bb 100644 --- a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift @@ -2820,7 +2820,85 @@ class PathHierarchyTests: XCTestCase { XCTAssertEqual(paths[containerID], "/ModuleName/ContainerName") XCTAssertEqual(paths[memberID], "/ModuleName/ContainerName/memberName") // The Swift spelling is preferred } - + + func testLanguageRepresentationsWithDifferentParentKinds() throws { + enableFeatureFlag(\.isExperimentalLinkHierarchySerializationEnabled) + + let containerID = "some-container-symbol-id" + let memberID = "some-member-symbol-id" + + let catalog = Folder(name: "unit-test.docc", content: [ + Folder(name: "clang", content: [ + JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph( + moduleName: "ModuleName", + symbols: [ + makeSymbol(id: containerID, language: .objectiveC, kind: .union, pathComponents: ["ContainerName"]), + makeSymbol(id: memberID, language: .objectiveC, kind: .property, pathComponents: ["ContainerName", "MemberName"]), + ], + relationships: [ + .init(source: memberID, target: containerID, kind: .memberOf, targetFallback: nil) + ] + )), + ]), + + Folder(name: "swift", content: [ + JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph( + moduleName: "ModuleName", + symbols: [ + makeSymbol(id: containerID, kind: .struct, pathComponents: ["ContainerName"]), + makeSymbol(id: memberID, kind: .property, pathComponents: ["ContainerName", "MemberName"]), + ], + relationships: [ + .init(source: memberID, target: containerID, kind: .memberOf, targetFallback: nil) + ] + )), + ]) + ]) + + let (_, context) = try loadBundle(catalog: catalog) + let tree = context.linkResolver.localResolver.pathHierarchy + + let resolvedSwiftContainerID = try tree.find(path: "/ModuleName/ContainerName-struct", onlyFindSymbols: true) + let resolvedSwiftContainer = try XCTUnwrap(tree.lookup[resolvedSwiftContainerID]) + XCTAssertEqual(resolvedSwiftContainer.name, "ContainerName") + XCTAssertEqual(resolvedSwiftContainer.symbol?.identifier.precise, containerID) + XCTAssertEqual(resolvedSwiftContainer.symbol?.kind.identifier, .struct) + XCTAssertEqual(resolvedSwiftContainer.languages, [.swift]) + + let resolvedObjcContainerID = try tree.find(path: "/ModuleName/ContainerName-union", onlyFindSymbols: true) + let resolvedObjcContainer = try XCTUnwrap(tree.lookup[resolvedObjcContainerID]) + XCTAssertEqual(resolvedObjcContainer.name, "ContainerName") + XCTAssertEqual(resolvedObjcContainer.symbol?.identifier.precise, containerID) + XCTAssertEqual(resolvedObjcContainer.symbol?.kind.identifier, .union) + XCTAssertEqual(resolvedObjcContainer.languages, [.objectiveC]) + + let resolvedContainerID = try tree.find(path: "/ModuleName/ContainerName", onlyFindSymbols: true) + XCTAssertEqual(resolvedContainerID, resolvedSwiftContainerID) + + let resolvedSwiftMemberID = try tree.find(path: "/ModuleName/ContainerName-struct/MemberName", onlyFindSymbols: true) + let resolvedSwiftMember = try XCTUnwrap(tree.lookup[resolvedSwiftMemberID]) + XCTAssertEqual(resolvedSwiftMember.name, "MemberName") + XCTAssertEqual(resolvedSwiftMember.parent?.identifier, resolvedSwiftContainerID) + XCTAssertEqual(resolvedSwiftMember.symbol?.identifier.precise, memberID) + XCTAssertEqual(resolvedSwiftMember.symbol?.kind.identifier, .property) + XCTAssertEqual(resolvedSwiftMember.languages, [.swift]) + + let resolvedObjcMemberID = try tree.find(path: "/ModuleName/ContainerName-union/MemberName", onlyFindSymbols: true) + let resolvedObjcMember = try XCTUnwrap(tree.lookup[resolvedObjcMemberID]) + XCTAssertEqual(resolvedObjcMember.name, "MemberName") + XCTAssertEqual(resolvedObjcMember.parent?.identifier, resolvedObjcContainerID) + XCTAssertEqual(resolvedObjcMember.symbol?.identifier.precise, memberID) + XCTAssertEqual(resolvedObjcMember.symbol?.kind.identifier, .property) + XCTAssertEqual(resolvedObjcMember.languages, [.objectiveC]) + + let resolvedMemberID = try tree.find(path: "/ModuleName/ContainerName/MemberName", onlyFindSymbols: true) + XCTAssertEqual(resolvedMemberID, resolvedSwiftMemberID) + + let paths = tree.caseInsensitiveDisambiguatedPaths() + XCTAssertEqual(paths[containerID], "/ModuleName/ContainerName") + XCTAssertEqual(paths[memberID], "/ModuleName/ContainerName/MemberName") + } + func testMixedLanguageSymbolAndItsExtendingModuleWithDifferentContainerNames() throws { let containerID = "some-container-symbol-id" let memberID = "some-member-symbol-id" From 03bf4d8bd11ec22f1cbb2fb298259dac84a7ea0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sof=C3=ADa=20Rodr=C3=ADguez?= Date: Fri, 2 May 2025 16:52:00 +0100 Subject: [PATCH 08/22] Use availability item platform name to compute default availability information. (#1208) (#1211) Use availability item platform name to compute default availability information. This change makes a small tweak to the default availability logic so it uses the symbol availability item platform name instead of the symbol graph platform name to calculate the corresponding default introduced version from the default availability. rdar://146774862 --- .../Symbol Graph/SymbolGraphLoader.swift | 7 ++-- .../Rendering/SymbolAvailabilityTests.swift | 35 +++++++++++++++++-- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/Symbol Graph/SymbolGraphLoader.swift b/Sources/SwiftDocC/Infrastructure/Symbol Graph/SymbolGraphLoader.swift index 920fa88397..50baa2529c 100644 --- a/Sources/SwiftDocC/Infrastructure/Symbol Graph/SymbolGraphLoader.swift +++ b/Sources/SwiftDocC/Infrastructure/Symbol Graph/SymbolGraphLoader.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) 2021-2025 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 @@ -317,9 +317,10 @@ struct SymbolGraphLoader { // Fill introduced versions when missing. availability.availability = availability.availability.map { - $0.fillingMissingIntroducedVersion( + let availabilityPlatformName = $0.domain.map { PlatformName(operatingSystemName: $0.rawValue) } ?? platformName + return $0.fillingMissingIntroducedVersion( from: defaultAvailabilityVersionByPlatform, - fallbackPlatform: DefaultAvailability.fallbackPlatforms[platformName]?.rawValue + fallbackPlatform: DefaultAvailability.fallbackPlatforms[availabilityPlatformName]?.rawValue ) } // Add the module availability information to each of the symbols availability mixin. diff --git a/Tests/SwiftDocCTests/Rendering/SymbolAvailabilityTests.swift b/Tests/SwiftDocCTests/Rendering/SymbolAvailabilityTests.swift index 643e9ba2f2..a0c0b129e9 100644 --- a/Tests/SwiftDocCTests/Rendering/SymbolAvailabilityTests.swift +++ b/Tests/SwiftDocCTests/Rendering/SymbolAvailabilityTests.swift @@ -1,7 +1,7 @@ /* This source file is part of the Swift.org open source project - Copyright (c) 2024 Apple Inc. and the Swift project authors + Copyright (c) 2024-2025 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 @@ -45,6 +45,7 @@ class SymbolAvailabilityTests: XCTestCase { private func renderNodeAvailability( defaultAvailability: [DefaultAvailability.ModuleAvailability] = [], symbolGraphOperatingSystemPlatformName: String, + symbolGraphEnvironmentName: String? = nil, symbols: [SymbolGraph.Symbol], symbolName: String ) throws -> [AvailabilityRenderItem] { @@ -56,7 +57,7 @@ class SymbolAvailabilityTests: XCTestCase { ]), JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph( moduleName: "ModuleName", - platform: SymbolGraph.Platform(architecture: nil, vendor: nil, operatingSystem: SymbolGraph.OperatingSystem(name: symbolGraphOperatingSystemPlatformName), environment: nil), + platform: SymbolGraph.Platform(architecture: nil, vendor: nil, operatingSystem: SymbolGraph.OperatingSystem(name: symbolGraphOperatingSystemPlatformName), environment: symbolGraphEnvironmentName), symbols: symbols, relationships: [] )), @@ -71,7 +72,7 @@ class SymbolAvailabilityTests: XCTestCase { func testSymbolGraphSymbolWithoutDeprecatedVersionAndIntroducedVersion() throws { - let availability = try renderNodeAvailability( + var availability = try renderNodeAvailability( defaultAvailability: [], symbolGraphOperatingSystemPlatformName: "ios", symbols: [ @@ -91,6 +92,34 @@ class SymbolAvailabilityTests: XCTestCase { "iPadOS - 1.2.3", "Mac Catalyst - 1.2.3", ]) + + availability = try renderNodeAvailability( + defaultAvailability: [ + DefaultAvailability.ModuleAvailability(platformName: PlatformName(operatingSystemName: "iOS"), platformVersion: "1.2.3") + ], + symbolGraphOperatingSystemPlatformName: "ios", + symbolGraphEnvironmentName: "macabi", + symbols: [ + makeSymbol( + id: "platform-1-symbol", + kind: .class, + pathComponents: ["SymbolName"], + availability: [ + makeAvailabilityItem(domainName: "iOS", deprecated: SymbolGraph.SemanticVersion(string: "1.2.3")), + makeAvailabilityItem(domainName: "visionOS", deprecated: SymbolGraph.SemanticVersion(string: "1.0.0")) + ] + ) + ], + symbolName: "SymbolName" + ) + + XCTAssertEqual(availability.map { "\($0.name ?? "") \($0.introduced ?? "") - \($0.deprecated ?? "")" }, [ + // The default availability for iOS shouldnt be copied to visionOS. + "iOS 1.2.3 - 1.2.3", + "iPadOS 1.2.3 - ", + "Mac Catalyst 1.2.3 - 1.2.3", + "visionOS - 1.0", + ]) } func testSymbolGraphSymbolWithObsoleteVersion() throws { From bc58ec268f2054c7646ad97625defee7b7807ce6 Mon Sep 17 00:00:00 2001 From: Pat Shaughnessy Date: Fri, 2 May 2025 07:38:49 -1000 Subject: [PATCH 09/22] [6.2] Strip Leading Whitespace From Symbol Graph Doc Comments (#1210) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Strip Leading Whitespace From Symbol Graph Doc Comments (#1190) * Strip the minimum leading whitespace from all doc comments loaded from symbol graphs. This worksaround a Clang compiler issue with parsing comments like this that are missing the leading asterisk: /** Foo's brief description. Foo's discussion. */ ...as opposed to this which Clang does parse properly: /** * Foo's brief description. * * Foo's discussion. */ "Minumum leading whitespace" is defined as: - Find the doc comment line with least amount of leading whitespace. Ignore blank lines during this search. - Remove that number of whitespace chars from all the lines (including blank lines). * Update Sources/SwiftDocC/Model/DocumentationNode.swift Refactor a loop to use `contains(where:)` Co-authored-by: David Rönnqvist * [String].linesWithoutLeadingWhitespace returns an array of substrings, not strings. * [String].linesWithoutLeadingWhitespace checks for an empty collection before continuing to parse all the lines * Tweak comments --------- Co-authored-by: David Rönnqvist * Improve style of doc comment in DocumentationNode.swift. Co-authored-by: Pete Lawrence --------- Co-authored-by: David Rönnqvist Co-authored-by: Pete Lawrence --- .../SwiftDocC/Model/DocumentationNode.swift | 44 ++++- .../Semantics/SymbolTests.swift | 152 ++++++++++++++++++ 2 files changed, 195 insertions(+), 1 deletion(-) diff --git a/Sources/SwiftDocC/Model/DocumentationNode.swift b/Sources/SwiftDocC/Model/DocumentationNode.swift index eb09820a29..a9e3133225 100644 --- a/Sources/SwiftDocC/Model/DocumentationNode.swift +++ b/Sources/SwiftDocC/Model/DocumentationNode.swift @@ -523,7 +523,10 @@ public struct DocumentationNode { DocumentationChunk(source: .documentationExtension, markup: documentationExtensionMarkup) ] } else if let symbol = documentedSymbol, let docComment = symbol.docComment { - let docCommentString = docComment.lines.map { $0.text }.joined(separator: "\n") + let docCommentString = docComment.lines + .map(\.text) + .linesWithoutLeadingWhitespace() + .joined(separator: "\n") let docCommentLocation: SymbolGraph.Symbol.Location? = { if let uri = docComment.uri, let position = docComment.lines.first?.range?.start { @@ -849,3 +852,42 @@ private extension BlockDirective { directivesSupportedInDocumentationComments.contains(name) } } + +extension [String] { + + /// Strips the minimum leading whitespace from all the strings in the array. + /// + /// The method does the following: + /// - Find the line with least amount of leading whitespace. Ignore blank lines during this search. + /// - Remove that number of whitespace chars from all the lines (including blank lines). + /// - Returns: An array of substrings of the original lines with the minimum leading whitespace removed. + func linesWithoutLeadingWhitespace() -> [Substring] { + + // Optimization for the common case: If any of the lines does not start + // with whitespace, or if there are no lines, then return the original lines + // as substrings. + if isEmpty || contains(where: { $0.first?.isWhitespace == false }) { + return self.map{ .init($0) } + } + + /// - Count the leading whitespace characters in the given string. + /// - Returns: The count of leading whitespace characters, if the string is not blank, + /// or `nil` if the string is empty or blank (contains only whitespace) + func leadingWhitespaceCount(_ line: String) -> Int? { + let count = line.prefix(while: \.isWhitespace).count + guard count < line.count else { return nil } + return count + } + + // Find the minimum count of leading whitespace. If there are no + // leading whitespace counts (if all the lines were blank) then return + // the original lines as substrings. + guard let minimumWhitespaceCount = self.compactMap(leadingWhitespaceCount).min() else { + return self.map{ .init($0) } + } + + // Drop the leading whitespace from all the lines and return the + // modified lines as substrings of the original lines. + return self.map { $0.dropFirst(minimumWhitespaceCount) } + } +} diff --git a/Tests/SwiftDocCTests/Semantics/SymbolTests.swift b/Tests/SwiftDocCTests/Semantics/SymbolTests.swift index 371e608488..5b59db8a3a 100644 --- a/Tests/SwiftDocCTests/Semantics/SymbolTests.swift +++ b/Tests/SwiftDocCTests/Semantics/SymbolTests.swift @@ -1352,6 +1352,158 @@ class SymbolTests: XCTestCase { XCTAssert(problems.isEmpty) } + // MARK: - Leading Whitespace in Doc Comments + + func testWithoutLeadingWhitespace() { + let lines = [ + "One", + "Two Words", + "With Trailing Whitespace " + ] + let linesWithoutLeadingWhitespace: [Substring] = [ + "One", + "Two Words", + "With Trailing Whitespace " + ] + XCTAssertEqual(lines.linesWithoutLeadingWhitespace(), linesWithoutLeadingWhitespace) + } + + func testWithLeadingWhitespace() { + let lines = [ + " One", + " Two Words", + " With Trailing Whitespace " + ] + let linesWithoutLeadingWhitespace: [Substring] = [ + "One", + "Two Words", + "With Trailing Whitespace " + ] + XCTAssertEqual(lines.linesWithoutLeadingWhitespace(), linesWithoutLeadingWhitespace) + } + + func testWithIncreasingLeadingWhitespace() { + let lines = [ + " One", + " Two Words", + " With Trailing Whitespace " + ] + let linesWithoutLeadingWhitespace: [Substring] = [ + "One", + " Two Words", + " With Trailing Whitespace " + ] + XCTAssertEqual(lines.linesWithoutLeadingWhitespace(), linesWithoutLeadingWhitespace) + } + + func testWithDecreasingLeadingWhitespace() { + let lines = [ + " One", + " Two Words", + " With Trailing Whitespace " + ] + let linesWithoutLeadingWhitespace: [Substring] = [ + " One", + " Two Words", + "With Trailing Whitespace " + ] + XCTAssertEqual(lines.linesWithoutLeadingWhitespace(), linesWithoutLeadingWhitespace) + } + + func testWithoutLeadingWhitespaceBlankLines() { + let lines = [ + " One", + " ", + " Two Words", + " ", + " With Trailing Whitespace " + ] + let linesWithoutLeadingWhitespace: [Substring] = [ + "One", + " ", + "Two Words", + "", + "With Trailing Whitespace " + ] + + XCTAssertEqual(lines.linesWithoutLeadingWhitespace(), linesWithoutLeadingWhitespace) + } + + func testWithoutLeadingWhitespaceEmptyLines() { + let lines = [ + " One", + "", + " Two Words", + "", + " With Trailing Whitespace " + ] + let linesWithoutLeadingWhitespace: [Substring] = [ + "One", + "", + "Two Words", + "", + "With Trailing Whitespace " + ] + + XCTAssertEqual(lines.linesWithoutLeadingWhitespace(), linesWithoutLeadingWhitespace) + } + + func testWithoutLeadingWhitespaceAllEmpty() { + let lines = [ + "", + "", + ] + let linesWithoutLeadingWhitespace: [Substring] = [ + "", + "", + ] + + XCTAssertEqual(lines.linesWithoutLeadingWhitespace(), linesWithoutLeadingWhitespace) + } + + func testWithoutLeadingWhitespaceAllBlank() { + let lines = [ + " ", + " ", + ] + let linesWithoutLeadingWhitespace: [Substring] = [ + " ", + " ", + ] + + XCTAssertEqual(lines.linesWithoutLeadingWhitespace(), linesWithoutLeadingWhitespace) + } + + func testWithoutLeadingWhitespaceEmpty() { + let lines = [String]() + let linesWithoutLeadingWhitespace = [Substring]() + + XCTAssertEqual(lines.linesWithoutLeadingWhitespace(), linesWithoutLeadingWhitespace) + } + + func testLeadingWhitespaceInDocComment() throws { + let (semanticWithLeadingWhitespace, problems) = try makeDocumentationNodeSymbol( + docComment: """ + This is an abstract. + + This is a multi-paragraph overview. + + It continues here. + """, + articleContent: nil + ) + XCTAssert(problems.isEmpty) + XCTAssertEqual(semanticWithLeadingWhitespace.abstract?.format(), "This is an abstract.") + let lines = semanticWithLeadingWhitespace.discussion?.content.map{ $0.format() } ?? [] + let expectedDiscussion = """ + This is a multi-paragraph overview. + + It continues here. + """ + XCTAssertEqual(lines.joined(), expectedDiscussion) + } + + // MARK: - Helpers func makeDocumentationNodeForSymbol( From 22f3d61e7b702d62a944b8bd4f89c00fd790e0ee Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Fri, 9 May 2025 15:52:22 -0600 Subject: [PATCH 10/22] use found disambiguation to calculate diagnostic range if present (#1216) rdar://150207195 --- .../Link Resolution/PathHierarchy+Error.swift | 20 ++- .../Semantics/ReferenceResolver.swift | 6 + .../DocumentationContextTests.swift | 160 +++++++++++++++++- 3 files changed, 182 insertions(+), 4 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Error.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Error.swift index 9f85fc1a2c..67851a2126 100644 --- a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Error.swift +++ b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Error.swift @@ -223,13 +223,27 @@ extension PathHierarchy.Error { case .lookupCollision(partialResult: let partialResult, remaining: let remaining, collisions: let collisions): let nextPathComponent = remaining.first! - let (pathPrefix, _, solutions) = makeCollisionSolutions(from: collisions, nextPathComponent: nextPathComponent, partialResultPrefix: partialResult.pathPrefix) - + let (pathPrefix, foundDisambiguation, solutions) = makeCollisionSolutions( + from: collisions, + nextPathComponent: nextPathComponent, + partialResultPrefix: partialResult.pathPrefix) + + let rangeAdjustment: SourceRange + if !foundDisambiguation.isEmpty { + rangeAdjustment = .makeRelativeRange( + startColumn: pathPrefix.count - foundDisambiguation.count, + length: foundDisambiguation.count) + } else { + rangeAdjustment = .makeRelativeRange( + startColumn: pathPrefix.count - nextPathComponent.full.count, + length: nextPathComponent.full.count) + } + return TopicReferenceResolutionErrorInfo(""" \(nextPathComponent.full.singleQuoted) is ambiguous at \(partialResult.node.pathWithoutDisambiguation().singleQuoted) """, solutions: solutions, - rangeAdjustment: .makeRelativeRange(startColumn: pathPrefix.count - nextPathComponent.full.count, length: nextPathComponent.full.count) + rangeAdjustment: rangeAdjustment ) } } diff --git a/Sources/SwiftDocC/Semantics/ReferenceResolver.swift b/Sources/SwiftDocC/Semantics/ReferenceResolver.swift index d3498fb041..9703463ad0 100644 --- a/Sources/SwiftDocC/Semantics/ReferenceResolver.swift +++ b/Sources/SwiftDocC/Semantics/ReferenceResolver.swift @@ -44,6 +44,12 @@ func unresolvedReferenceProblem(source: URL?, range: SourceRange?, severity: Dia let diagnosticRange: SourceRange? if var rangeAdjustment = errorInfo.rangeAdjustment, let referenceSourceRange { rangeAdjustment.offsetWithRange(referenceSourceRange) + assert(rangeAdjustment.lowerBound.column >= 0, """ + Unresolved topic reference range adjustment created range with negative column. + Source: \(source?.absoluteString ?? "nil") + Range: \(rangeAdjustment.lowerBound.description):\(rangeAdjustment.upperBound.description) + Summary: \(errorInfo.message) + """) diagnosticRange = rangeAdjustment } else { diagnosticRange = referenceSourceRange diff --git a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift index 148b37fa91..b9e67fcb4c 100644 --- a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift @@ -5266,7 +5266,165 @@ let expected = """ } } } - + + func testContextDiagnosesInsufficientDisambiguationWithCorrectRange() throws { + // This test deliberately does not turn on the overloads feature + // to ensure the symbol link below does not accidentally resolve correctly. + for symbolKindID in SymbolGraph.Symbol.KindIdentifier.allCases where !symbolKindID.isOverloadableKind { + // Generate a 4 symbols with the same name for every non overloadable symbol kind + let symbols: [SymbolGraph.Symbol] = [ + makeSymbol(id: "first-\(symbolKindID.identifier)-id", kind: symbolKindID, pathComponents: ["SymbolName"]), + makeSymbol(id: "second-\(symbolKindID.identifier)-id", kind: symbolKindID, pathComponents: ["SymbolName"]), + makeSymbol(id: "third-\(symbolKindID.identifier)-id", kind: symbolKindID, pathComponents: ["SymbolName"]), + makeSymbol(id: "fourth-\(symbolKindID.identifier)-id", kind: symbolKindID, pathComponents: ["SymbolName"]), + ] + + let catalog = + Folder(name: "unit-test.docc", content: [ + JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph( + moduleName: "ModuleName", + symbols: symbols + )), + + TextFile(name: "ModuleName.md", utf8Content: """ + # ``ModuleName`` + + This is a test file for ModuleName. + + ## Topics + + - ``SymbolName-\(symbolKindID.identifier)`` + """) + ]) + + let (_, context) = try loadBundle(catalog: catalog) + + let problems = context.problems.sorted(by: \.diagnostic.summary) + XCTAssertEqual(problems.count, 1) + + let problem = try XCTUnwrap(problems.first) + + XCTAssertEqual(problem.diagnostic.summary, "'SymbolName-\(symbolKindID.identifier)' is ambiguous at '/ModuleName'") + + XCTAssertEqual(problem.possibleSolutions.count, 4) + + for solution in problem.possibleSolutions { + XCTAssertEqual(solution.replacements.count, 1) + let replacement = try XCTUnwrap(solution.replacements.first) + + XCTAssertEqual(replacement.range.lowerBound, .init(line: 7, column: 15, source: nil)) + XCTAssertEqual( + replacement.range.upperBound, + .init(line: 7, column: 16 + symbolKindID.identifier.count, source: nil) + ) + } + } + } + + func testContextDiagnosesIncorrectDisambiguationWithCorrectRange() throws { + // This test deliberately does not turn on the overloads feature + // to ensure the symbol link below does not accidentally resolve correctly. + for symbolKindID in SymbolGraph.Symbol.KindIdentifier.allCases where !symbolKindID.isOverloadableKind { + // Generate a 4 symbols with the same name for every non overloadable symbol kind + let symbols: [SymbolGraph.Symbol] = [ + makeSymbol(id: "first-\(symbolKindID.identifier)-id", kind: symbolKindID, pathComponents: ["SymbolName"]), + makeSymbol(id: "second-\(symbolKindID.identifier)-id", kind: symbolKindID, pathComponents: ["SymbolName"]), + makeSymbol(id: "third-\(symbolKindID.identifier)-id", kind: symbolKindID, pathComponents: ["SymbolName"]), + makeSymbol(id: "fourth-\(symbolKindID.identifier)-id", kind: symbolKindID, pathComponents: ["SymbolName"]), + ] + + let catalog = + Folder(name: "unit-test.docc", content: [ + JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph( + moduleName: "ModuleName", + symbols: symbols + )), + + TextFile(name: "ModuleName.md", utf8Content: """ + # ``ModuleName`` + + This is a test file for ModuleName. + + ## Topics + + - ``SymbolName-abc123`` + """) + ]) + + let (_, context) = try loadBundle(catalog: catalog) + + let problems = context.problems.sorted(by: \.diagnostic.summary) + XCTAssertEqual(problems.count, 1) + + let problem = try XCTUnwrap(problems.first) + + XCTAssertEqual(problem.diagnostic.summary, "'abc123' isn't a disambiguation for 'SymbolName' at '/ModuleName'") + + XCTAssertEqual(problem.possibleSolutions.count, 4) + + for solution in problem.possibleSolutions { + XCTAssertEqual(solution.replacements.count, 1) + let replacement = try XCTUnwrap(solution.replacements.first) + + // "Replace '-abc123' with '-(hash)'" where 'abc123' is at 7:15-7:22 + XCTAssertEqual(replacement.range.lowerBound, .init(line: 7, column: 15, source: nil)) + XCTAssertEqual(replacement.range.upperBound, .init(line: 7, column: 22, source: nil)) + } + } + } + + func testContextDiagnosesIncorrectSymbolNameWithCorrectRange() throws { + // This test deliberately does not turn on the overloads feature + // to ensure the symbol link below does not accidentally resolve correctly. + for symbolKindID in SymbolGraph.Symbol.KindIdentifier.allCases where !symbolKindID.isOverloadableKind { + // Generate a 4 symbols with the same name for every non overloadable symbol kind + let symbols: [SymbolGraph.Symbol] = [ + makeSymbol(id: "first-\(symbolKindID.identifier)-id", kind: symbolKindID, pathComponents: ["SymbolName"]), + makeSymbol(id: "second-\(symbolKindID.identifier)-id", kind: symbolKindID, pathComponents: ["SymbolName"]), + makeSymbol(id: "third-\(symbolKindID.identifier)-id", kind: symbolKindID, pathComponents: ["SymbolName"]), + makeSymbol(id: "fourth-\(symbolKindID.identifier)-id", kind: symbolKindID, pathComponents: ["SymbolName"]), + ] + + let catalog = + Folder(name: "unit-test.docc", content: [ + JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph( + moduleName: "ModuleName", + symbols: symbols + )), + + TextFile(name: "ModuleName.md", utf8Content: """ + # ``ModuleName`` + + This is a test file for ModuleName. + + ## Topics + + - ``Symbol`` + """) + ]) + + let (_, context) = try loadBundle(catalog: catalog) + + let problems = context.problems.sorted(by: \.diagnostic.summary) + XCTAssertEqual(problems.count, 1) + + let problem = try XCTUnwrap(problems.first) + + XCTAssertEqual(problem.diagnostic.summary, "'Symbol' doesn't exist at '/ModuleName'") + + XCTAssertEqual(problem.possibleSolutions.count, 1) + let solution = try XCTUnwrap(problem.possibleSolutions.first) + + XCTAssertEqual(solution.summary, "Replace 'Symbol' with 'SymbolName'") + + XCTAssertEqual(solution.replacements.count, 1) + let replacement = try XCTUnwrap(solution.replacements.first) + + XCTAssertEqual(replacement.range.lowerBound, .init(line: 7, column: 5, source: nil)) + XCTAssertEqual(replacement.range.upperBound, .init(line: 7, column: 11, source: nil)) + } + } + func testResolveExternalLinkFromTechnologyRoot() throws { enableFeatureFlag(\.isExperimentalLinkHierarchySerializationEnabled) From 5065f71080f7321066171f35dd71cb8f47a4b2b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sof=C3=ADa=20Rodr=C3=ADguez?= Date: Thu, 15 May 2025 17:59:55 +0100 Subject: [PATCH 11/22] Prevent accessing unicode scalar element with incorrect index. (#1214) (#1219) This change prevents a potential DocC crash by making sure that it does not access an array element using an index that is outside of the bonds of the array. `.index(after:` can crash in some cases if the passed index belongs to the last element of the collection. with this change we ensure that the passed index is not the last index. rdar://150760264 --- Sources/SwiftDocC/Utility/ValidatedURL.swift | 26 ++++++---- .../Utility/ValidatedURLTests.swift | 48 ++++++++++++------- 2 files changed, 48 insertions(+), 26 deletions(-) diff --git a/Sources/SwiftDocC/Utility/ValidatedURL.swift b/Sources/SwiftDocC/Utility/ValidatedURL.swift index e9e96a2389..772911a308 100644 --- a/Sources/SwiftDocC/Utility/ValidatedURL.swift +++ b/Sources/SwiftDocC/Utility/ValidatedURL.swift @@ -179,23 +179,29 @@ private extension StringProtocol { func addingPercentEncodingIfNeeded(withAllowedCharacters allowedCharacters: CharacterSet) -> String? { var needsPercentEncoding: Bool { for (index, character) in unicodeScalars.indexed() where !allowedCharacters.contains(character) { + // Check if the character "%" represents a percent encoded URL. + // Any other disallowed character is an indication that this substring needs percent encoding. if character == "%" { // % isn't allowed in a URL fragment but it is also the escape character for percent encoding. - let firstFollowingIndex = unicodeScalars.index(after: index) - let secondFollowingIndex = unicodeScalars.index(after: firstFollowingIndex) - - guard secondFollowingIndex < unicodeScalars.endIndex else { + guard self.distance(from: index, to: self.endIndex) >= 2 else { // There's not two characters after the "%". This "%" can't represent a percent encoded character. return true } - // If either of the two following characters aren't hex digits, the "%" doesn't represent a - return !Character(unicodeScalars[firstFollowingIndex]).isHexDigit - || !Character(unicodeScalars[secondFollowingIndex]).isHexDigit + let firstFollowingIndex = self.index(after: index) + let secondFollowingIndex = self.index(after: firstFollowingIndex) - } else { - // Any other disallowed character is an indication that this substring needs percent encoding. - return true + // Check if the next two characthers represent a percent encoded + // URL. + // If either of the two following characters aren't hex digits, + // the "%" doesn't represent a percent encoded character. + if Character(unicodeScalars[firstFollowingIndex]).isHexDigit, + Character(unicodeScalars[secondFollowingIndex]).isHexDigit + { + // Later characters in the string might require percentage encoding. + continue + } } + return true } return false } diff --git a/Tests/SwiftDocCTests/Utility/ValidatedURLTests.swift b/Tests/SwiftDocCTests/Utility/ValidatedURLTests.swift index 48b098adfb..2264f9cdd7 100644 --- a/Tests/SwiftDocCTests/Utility/ValidatedURLTests.swift +++ b/Tests/SwiftDocCTests/Utility/ValidatedURLTests.swift @@ -81,6 +81,14 @@ class ValidatedURLTests: XCTestCase { } func testQueryIsPartOfPathForAuthoredLinks() throws { + + func validate(linkText: String, expectedPath: String, expectedFragment: String? = nil,file: StaticString = #filePath, line: UInt = #line) throws { + let validated = try XCTUnwrap(ValidatedURL(parsingAuthoredLink: linkText), "Failed to parse \(linkText.singleQuoted) as authored link") + XCTAssertNil(validated.components.queryItems, "Authored documentation links don't include query items", file: file, line: line) + XCTAssertEqual(validated.components.path, expectedPath, file: file, line: line) + XCTAssertEqual(validated.components.fragment, expectedFragment, file: file, line: line) + } + // Test return type disambiguation for linkText in [ "SymbolName/memberName()->Int?", @@ -91,14 +99,8 @@ class ValidatedURLTests: XCTestCase { ? "/SymbolName/memberName()->Int?" : "SymbolName/memberName()->Int?" - let validated = try XCTUnwrap(ValidatedURL(parsingAuthoredLink: linkText), "Failed to parse \(linkText.singleQuoted) as authored link") - XCTAssertNil(validated.components.queryItems, "Authored documentation links don't include query items") - XCTAssertEqual(validated.components.path, expectedPath) - - let validatedWithHeading = try XCTUnwrap(ValidatedURL(parsingAuthoredLink: linkText + "#Heading-Name"), "Failed to parse '\(linkText)#Heading-Name' as authored link") - XCTAssertNil(validatedWithHeading.components.queryItems, "Authored documentation links don't include query items") - XCTAssertEqual(validatedWithHeading.components.path, expectedPath) - XCTAssertEqual(validatedWithHeading.components.fragment, "Heading-Name") + try validate(linkText: linkText, expectedPath: expectedPath) + try validate(linkText: linkText + "#Heading-Name", expectedPath: expectedPath, expectedFragment: "Heading-Name") } // Test parameter type disambiguation @@ -111,15 +113,29 @@ class ValidatedURLTests: XCTestCase { ? "/SymbolName/memberName(with:and:)-(Int?,_)" : "SymbolName/memberName(with:and:)-(Int?,_)" - let validated = try XCTUnwrap(ValidatedURL(parsingAuthoredLink: linkText), "Failed to parse \(linkText.singleQuoted) as authored link") - XCTAssertNil(validated.components.queryItems, "Authored documentation links don't include query items") - XCTAssertEqual(validated.components.path, expectedPath) - - let validatedWithHeading = try XCTUnwrap(ValidatedURL(parsingAuthoredLink: linkText + "#Heading-Name"), "Failed to parse '\(linkText)#Heading-Name' as authored link") - XCTAssertNil(validatedWithHeading.components.queryItems, "Authored documentation links don't include query items") - XCTAssertEqual(validatedWithHeading.components.path, expectedPath) - XCTAssertEqual(validatedWithHeading.components.fragment, "Heading-Name") + try validate(linkText: linkText, expectedPath: expectedPath) + try validate(linkText: linkText + "#Heading-Name", expectedPath: expectedPath, expectedFragment: "Heading-Name") } + + // Test parameter with percent encoding + var linkText = "doc://com.example.test/docc=Whats%20New&version=DocC&Title=[Update]" + var expectedPath = "/docc=Whats%20New&version=DocC&Title=[Update]" + try validate(linkText: linkText, expectedPath: expectedPath) + + // Test parameter with percent encoding at the end of the URL + linkText = "doc://com.example.test/docc=Whats%20New&version=DocC&Title=[Update]%20" + expectedPath = "/docc=Whats%20New&version=DocC&Title=[Update]%20" + try validate(linkText: linkText, expectedPath: expectedPath) + + // Test parameter without percent encoding + linkText = "doc://com.example.test/docc=WhatsNew&version=DocC&Title=[Update]" + expectedPath = "/docc=WhatsNew&version=DocC&Title=[Update]" + try validate(linkText: linkText, expectedPath: expectedPath) + + // Test parameter with special characters + linkText = "doc://com.example.test/テスト" + expectedPath = "/テスト" + try validate(linkText: linkText, expectedPath: expectedPath) } func testEscapedFragment() throws { From add78e5eb91d6dae525233c9d757a85f2b34d16f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Tue, 20 May 2025 18:55:31 +0200 Subject: [PATCH 12/22] Fix crash caused by repeatedly cloning the same path hierarchy node (#1220) (#1226) * Don't repeatedly clone nodes that have already been cloned rdar://150706871 * Fix unrelated code warning about unused local variable * Fix unrelated list of symbols in other assertion message * Avoid repetition in debug assertions * Only create sparse nodes after processing all symbol graph files * Try to find existing nodes before creating sparse nodes rdar://148247074 * Remove trailing commas for compatibility before Swift 6.1 * Remove unintentional print statement * Try to repair symbol graphs with deep hierarchies but no actual memberOf relationships * Add additional test assertion about counterpart references --- .../Link Resolution/PathHierarchy.swift | 135 ++++++++---- .../Infrastructure/PathHierarchyTests.swift | 206 +++++++++++++++++- 2 files changed, 288 insertions(+), 53 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift index c72e73add6..47edbc00be 100644 --- a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift +++ b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift @@ -1,7 +1,7 @@ /* This source file is part of the Swift.org open source project - Copyright (c) 2022-2024 Apple Inc. and the Swift project authors + Copyright (c) 2022-2025 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 @@ -74,7 +74,11 @@ struct PathHierarchy { .sorted(by: { lhs, rhs in return !lhs.url.lastPathComponent.contains("@") }) - + + // To try to handle certain invalid symbol graph files gracefully, we track symbols that don't have a place in the hierarchy so that we can look for a place for those symbols. + // Because this is a last resort, we only want to do this processing after all the symbol graphs have already been processed. + var symbolNodesOutsideOfHierarchyByModule: [String: [Node]] = [:] + for (url, graph, language) in symbolGraphs { let moduleName = graph.module.name let moduleNode: Node @@ -115,7 +119,10 @@ struct PathHierarchy { || ($0.symbol!.pathComponents == symbol.pathComponents && $0.symbol!.kind.identifier == symbol.kind.identifier) }) { nodes[id] = existingNode - existingNode.languages.insert(language!) // If we have symbols in this graph we have a language as well + if existingNode.counterpart?.languages.contains(language!) != true { + // Unless this symbol is already split into language counterparts, add the languages to this node. + existingNode.languages.insert(language!) + } } else { assert(!symbol.pathComponents.isEmpty, "A symbol should have at least its own name in its path components.") @@ -150,7 +157,7 @@ struct PathHierarchy { if let targetNode = nodes[relationship.target], targetNode.name == expectedContainerName { if sourceNode.parent == nil { targetNode.add(symbolChild: sourceNode) - } else if sourceNode.parent !== targetNode { + } else if sourceNode.parent !== targetNode && sourceNode.counterpart?.parent !== targetNode { // If the node we have for the child has an existing parent that doesn't // match the parent from this symbol graph, we need to clone the child to // ensure that the hierarchy remains consistent. @@ -166,7 +173,7 @@ struct PathHierarchy { sourceNode.languages.remove(language!) // Make sure that the clone's children can all line up with symbols from this symbol graph. - for (childName, children) in sourceNode.children { + for children in sourceNode.children.values { for child in children.storage { guard let childSymbol = child.node.symbol else { // We shouldn't come across any non-symbol nodes here, @@ -198,8 +205,13 @@ struct PathHierarchy { // If the source was added in an extension symbol graph file, then its target won't be found in the same symbol graph file (in `nodes`). // We may have encountered multiple language representations of the target symbol. Try to find the best matching representation of the target to add the source to. - // Remove any targets that don't match the source symbol's path components (see comment above for more details). - targetNodes.removeAll(where: { $0.name != expectedContainerName }) + // Remove any targets that don't match the source symbol's path components (see comment above for more details) and languages (see comments below). + targetNodes.removeAll(where: { $0.name != expectedContainerName || $0.languages.isDisjoint(with: sourceNode.languages) }) + guard !targetNodes.isEmpty else { + // If none of the symbol graphs contain a matching node it's likely a bug in the tool that generated the symbol graph. + // If this happens we leave the source node in `topLevelCandidates` to try and let a later fallback code path recover from the symbol graph issue. + continue + } // Prefer the symbol that matches the relationship's language. if let targetNode = targetNodes.first(where: { $0.symbol!.identifier.interfaceLanguage == language?.id }) { @@ -208,7 +220,7 @@ struct PathHierarchy { // It's not clear which target to add the source to, so we add it to all of them. // This will likely hit a _debug_ assertion (later in this initializer) about inconsistent traversal through the hierarchy, // but in release builds DocC will "repair" the inconsistent hierarchy. - for targetNode in targetNodes { + for targetNode in targetNodes where !sourceNode.languages.isDisjoint(with: targetNode.languages) { targetNode.add(symbolChild: sourceNode) } } @@ -256,14 +268,17 @@ struct PathHierarchy { moduleNode.add(symbolChild: topLevelNode) } - assert( - topLevelCandidates.values.filter({ $0.symbol!.pathComponents.count > 1 }).allSatisfy({ $0.parent == nil }), """ - Top-level candidates shouldn't already exist in the hierarchy. \ - This wasn't true for \(topLevelCandidates.filter({ $0.value.symbol!.pathComponents.count > 1 && $0.value.parent != nil }).map(\.key).sorted()) - """ - ) + assertAllNodes(in: topLevelCandidates.values.filter { $0.symbol!.pathComponents.count > 1 }, satisfy: { $0.parent == nil }, + "Top-level candidates shouldn't already exist in the hierarchy.") for node in topLevelCandidates.values where node.symbol!.pathComponents.count > 1 && node.parent == nil { + symbolNodesOutsideOfHierarchyByModule[moduleNode.symbol!.identifier.precise, default: []].append(node) + } + } + + for (moduleID, nodes) in symbolNodesOutsideOfHierarchyByModule { + let moduleNode = roots[moduleID]! + for node in nodes where node.parent == nil { var parent = moduleNode var components = { (symbol: SymbolGraph.Symbol) -> [String] in let original = symbol.pathComponents @@ -281,10 +296,31 @@ struct PathHierarchy { components = components.dropFirst() } for component in components { + // FIXME: + // This code path is both expected (when `knownDisambiguatedPathComponents` is non-nil) and unexpected (when the symbol graph is missing data or contains extra relationships). + // It would be good to restructure this code to better distinguish what's supported behavior and what's a best-effort attempt at gracefully handle invalid symbol graphs. + if let existing = parent.children[component] { + // This code tries to repair incomplete symbol graph files by guessing that the symbol with the most overlapping languages is the intended container. + // Valid symbol graph files we should never end up here. + var bestLanguageMatch: (node: Node, count: Int)? + for element in existing.storage { + let numberOfMatchingLanguages = node.languages.intersection(element.node.languages).count + if (bestLanguageMatch?.count ?? .min) < numberOfMatchingLanguages { + bestLanguageMatch = (node: element.node, count: numberOfMatchingLanguages) + } + } + if let bestLanguageMatch { + // If there's a real symbol that matches this node's languages, use that node instead of creating a placeholder node + parent = bestLanguageMatch.node + continue + } + } + assert( - parent.children[components.first!] == nil, + parent.children[component] == nil, "Shouldn't create a new sparse node when symbol node already exist. This is an indication that a symbol is missing a relationship." ) + guard knownDisambiguatedPathComponents != nil else { // If the path hierarchy wasn't passed any "known disambiguated path components" then the sparse/placeholder nodes won't contain any disambiguation. let nodeWithoutSymbol = Node(name: component) @@ -352,21 +388,11 @@ struct PathHierarchy { } } - assert( - allNodes.allSatisfy({ $0.value[0].parent != nil || roots[$0.key] != nil }), """ - Every node should either have a parent node or be a root node. \ - This wasn't true for \(allNodes.filter({ $0.value[0].parent == nil && roots[$0.key] == nil }).map(\.key).sorted()) - """ - ) + assertAllNodes(in: allNodes, satisfy: { $0.parent != nil || roots[$0.symbol!.identifier.precise] != nil }, + "Every node should either have a parent node or be a root node.") - assert( - allNodes.values.allSatisfy({ nodesWithSameUSR in nodesWithSameUSR.allSatisfy({ node in - Array(sequence(first: node, next: \.parent)).last!.symbol!.kind.identifier == .module }) - }), """ - Every node should reach a root node by following its parents up. \ - This wasn't true for \(allNodes.filter({ $0.value.allSatisfy({ Array(sequence(first: $0, next: \.parent)).last!.symbol!.kind.identifier != .module }) }).map(\.key).sorted()) - """ - ) + assertAllNodes(in: allNodes, satisfy: { Array(sequence(first: $0, next: \.parent)).last!.symbol!.kind.identifier == .module }, + "Every node should reach a root node by following its parents up.") allNodes.removeAll() @@ -407,12 +433,11 @@ struct PathHierarchy { descend(module) } - assert( - lookup.allSatisfy({ $0.value.parent != nil || roots[$0.value.name] != nil }), """ - Every node should either have a parent node or be a root node. \ - This wasn't true for \(allNodes.filter({ $0.value[0].parent == nil && roots[$0.key] == nil }).map(\.key).sorted()) - """ - ) + assertAllNodes(in: lookup.values, satisfy: { $0.parent != nil || roots[$0.name] != nil }, + "Every node should either have a parent node or be a root node.") + + assertAllNodes(in: lookup.values, satisfy: { $0.counterpart == nil || lookup[$0.counterpart!.identifier] != nil }, + "Every counterpart node should exist in the hierarchy.") func newNode(_ name: String) -> Node { let id = ResolvedIdentifier() @@ -430,12 +455,8 @@ struct PathHierarchy { "Every node lookup should match a node with that identifier." ) - assert( - lookup.values.allSatisfy({ $0.parent?.identifier == nil || lookup[$0.parent!.identifier] != nil }), """ - Every node's findable parent should exist in the lookup. \ - This wasn't true for \(lookup.values.filter({ $0.parent?.identifier != nil && lookup[$0.parent!.identifier] == nil }).map(\.symbol!.identifier.precise).sorted()) - """ - ) + assertAllNodes(in: lookup.values, satisfy: { $0.parent?.identifier == nil || lookup[$0.parent!.identifier] != nil }, + "Every node's findable parent should exist in the lookup.") self.modules = Array(roots.values) self.lookup = lookup @@ -884,3 +905,35 @@ extension LinkCompletionTools { return (node, id) } } + +// MARK: Assertion + +private func assertAllNodes( + in collection: @autoclosure () -> some Sequence, + satisfy condition: (PathHierarchy.Node) -> Bool, + _ message: @autoclosure () -> String, + file: StaticString = #file, + line: UInt = #line +) { + assert( + collection().allSatisfy(condition), + "\(message()) This wasn't true for \(collection().filter { !condition($0) }.map(\.symbol!.identifier.precise).sorted())", + file: file, + line: line + ) +} + +private func assertAllNodes( + in collectionsByStringKey: @autoclosure () -> [String: some Collection], + satisfy condition: (PathHierarchy.Node) -> Bool, + _ message: @autoclosure () -> String, + file: StaticString = #file, + line: UInt = #line +) { + assert( + collectionsByStringKey().values.allSatisfy { $0.allSatisfy(condition) }, + "\(message()) This wasn't true for \(collectionsByStringKey().filter { $0.value.contains(where: { !condition($0)}) }.map(\.key).sorted())", + file: file, + line: line + ) +} diff --git a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift index 469f5718bb..6228a4c592 100644 --- a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift @@ -2827,32 +2827,38 @@ class PathHierarchyTests: XCTestCase { let containerID = "some-container-symbol-id" let memberID = "some-member-symbol-id" + // Repeat the same symbols in both languages for many platforms. + let platforms = (1...10).map { + let name = "Platform\($0)" + return (name: name, availability: [makeAvailabilityItem(domainName: name)]) + } + let catalog = Folder(name: "unit-test.docc", content: [ - Folder(name: "clang", content: [ - JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph( + Folder(name: "clang", content: platforms.map { platform in + JSONFile(name: "ModuleName-\(platform.name).symbols.json", content: makeSymbolGraph( moduleName: "ModuleName", symbols: [ - makeSymbol(id: containerID, language: .objectiveC, kind: .union, pathComponents: ["ContainerName"]), - makeSymbol(id: memberID, language: .objectiveC, kind: .property, pathComponents: ["ContainerName", "MemberName"]), + makeSymbol(id: containerID, language: .objectiveC, kind: .union, pathComponents: ["ContainerName"], availability: platform.availability), + makeSymbol(id: memberID, language: .objectiveC, kind: .property, pathComponents: ["ContainerName", "MemberName"], availability: platform.availability), ], relationships: [ .init(source: memberID, target: containerID, kind: .memberOf, targetFallback: nil) ] - )), - ]), + )) + }), - Folder(name: "swift", content: [ - JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph( + Folder(name: "swift", content: platforms.map { platform in + JSONFile(name: "ModuleName-\(platform.name).symbols.json", content: makeSymbolGraph( moduleName: "ModuleName", symbols: [ - makeSymbol(id: containerID, kind: .struct, pathComponents: ["ContainerName"]), - makeSymbol(id: memberID, kind: .property, pathComponents: ["ContainerName", "MemberName"]), + makeSymbol(id: containerID, kind: .struct, pathComponents: ["ContainerName"], availability: platform.availability), + makeSymbol(id: memberID, kind: .property, pathComponents: ["ContainerName", "MemberName"], availability: platform.availability), ], relationships: [ .init(source: memberID, target: containerID, kind: .memberOf, targetFallback: nil) ] - )), - ]) + )) + }) ]) let (_, context) = try loadBundle(catalog: catalog) @@ -3146,6 +3152,182 @@ class PathHierarchyTests: XCTestCase { try assertFindsPath("/MainModule/TopLevelProtocol/InnerStruct/extensionMember(_:)", in: tree, asSymbolID: "extensionMember2") } + func testMissingRequiredMemberOfSymbolGraphRelationshipInOneLanguageAcrossManyPlatforms() throws { + // We make a best-effort attempt to create a valid path hierarchy, even if the symbol graph inputs are not valid. + + // If the symbol graph files define container and member symbols without the required memberOf relationships we still try to match them up. + + let containerID = "some-container-symbol-id" + let memberID = "some-member-symbol-id" + + // Repeat the same symbols in both languages for many platforms. + let platforms = (1...10).map { + let name = "Platform\($0)" + return (name: name, availability: [makeAvailabilityItem(domainName: name)]) + } + + let catalog = Folder(name: "unit-test.docc", content: [ + Folder(name: "swift", content: platforms.map { platform in + JSONFile(name: "ModuleName-\(platform.name).symbols.json", content: makeSymbolGraph( + moduleName: "ModuleName", + symbols: [ + makeSymbol(id: containerID, kind: .struct, pathComponents: ["ContainerName"], availability: platform.availability), + makeSymbol(id: memberID, kind: .property, pathComponents: ["ContainerName", "memberName"], availability: platform.availability), + ], + relationships: [/* the memberOf relationship is missing */] + )) + }) + ]) + + let (_, context) = try loadBundle(catalog: catalog) + let tree = context.linkResolver.localResolver.pathHierarchy + + let container = try tree.findNode(path: "/ModuleName/ContainerName-struct", onlyFindSymbols: true) + XCTAssertEqual(container.languages, [.swift]) + + let member = try tree.findNode(path: "/ModuleName/ContainerName/memberName", onlyFindSymbols: true) + XCTAssertEqual(member.languages, [.swift]) + + XCTAssertEqual(member.parent?.identifier, container.identifier) + + let paths = tree.caseInsensitiveDisambiguatedPaths() + XCTAssertEqual(paths[containerID], "/ModuleName/ContainerName") + XCTAssertEqual(paths[memberID], "/ModuleName/ContainerName/memberName") + + try assertFindsPath("/ModuleName/ContainerName/memberName", in: tree, asSymbolID: memberID) + try assertFindsPath("/ModuleName/ContainerName", in: tree, asSymbolID: containerID) + } + + func testInvalidSymbolGraphWithNoMemberOfRelationshipsDesptiteDeepHierarchyAcrossManyPlatforms() throws { + // We make a best-effort attempt to create a valid path hierarchy, even if the symbol graph inputs are not valid. + + // If the symbol graph files define a deep hierarchy, with the same symbol names but different symbol kinds across different, we try to match them up by language. + + // Repeat the same symbols in both languages for many platforms. + let platforms = (1...10).map { + let name = "Platform\($0)" + return (name: name, availability: [makeAvailabilityItem(domainName: name)]) + } + + let catalog = Folder(name: "unit-test.docc", content: [ + Folder(name: "clang", content: platforms.map { platform in + return JSONFile(name: "ModuleName-\(platform.name).symbols.json", content: makeSymbolGraph( + moduleName: "ModuleName", + symbols: [ + makeSymbol(id: "some-outer-container-id", language: .objectiveC, kind: .class, pathComponents: ["OuterContainerName"]), + makeSymbol(id: "some-middle-container-id", language: .objectiveC, kind: .class, pathComponents: ["OuterContainerName", "MiddleContainerName"]), + makeSymbol(id: "some-inner-container-id", language: .objectiveC, kind: .class, pathComponents: ["OuterContainerName", "MiddleContainerName", "InnerContainerName"]), + makeSymbol(id: "some-objc-specific-member-id", language: .objectiveC, kind: .property, pathComponents: ["OuterContainerName", "MiddleContainerName", "InnerContainerName", "objcSpecificMember"]), + ], + relationships: [/* all required memberOf relationships all missing */] + )) + }), + + Folder(name: "swift", content: platforms.map { platform in + return JSONFile(name: "ModuleName-\(platform.name).symbols.json", content: makeSymbolGraph( + moduleName: "ModuleName", + symbols: [ + makeSymbol(id: "some-outer-container-id", kind: .struct, pathComponents: ["OuterContainerName"]), + makeSymbol(id: "some-middle-container-id", kind: .struct, pathComponents: ["OuterContainerName", "MiddleContainerName"]), + makeSymbol(id: "some-inner-container-id", kind: .struct, pathComponents: ["OuterContainerName", "MiddleContainerName", "InnerContainerName"]), + makeSymbol(id: "some-swift-specific-member-id", kind: .method, pathComponents: ["OuterContainerName", "MiddleContainerName", "InnerContainerName", "swiftSpecificMember()"]), + ], + relationships: [/* all required memberOf relationships all missing */] + )) + }) + ]) + + let (_, context) = try loadBundle(catalog: catalog) + let tree = context.linkResolver.localResolver.pathHierarchy + + let swiftSpecificNode = try tree.findNode(path: "/ModuleName/OuterContainerName-struct/MiddleContainerName-struct/InnerContainerName-struct/swiftSpecificMember()", onlyFindSymbols: true, parent: nil) + XCTAssertEqual(swiftSpecificNode.symbol?.identifier.precise, "some-swift-specific-member-id") + // Trace up and check that each node is represented by a symbol + XCTAssertEqual(swiftSpecificNode.parent?.symbol?.identifier.precise, "some-inner-container-id") + XCTAssertEqual(swiftSpecificNode.parent?.parent?.symbol?.identifier.precise, "some-middle-container-id") + XCTAssertEqual(swiftSpecificNode.parent?.parent?.parent?.symbol?.identifier.precise, "some-outer-container-id") + + let objcSpecificNode = try tree.findNode(path: "/ModuleName/OuterContainerName-class/MiddleContainerName-class/InnerContainerName-class/objcSpecificMember", onlyFindSymbols: true, parent: nil) + XCTAssertEqual(objcSpecificNode.symbol?.identifier.precise, "some-objc-specific-member-id") + // Trace up and check that each node is represented by a symbol + XCTAssertEqual(objcSpecificNode.parent?.symbol?.identifier.precise, "some-inner-container-id") + XCTAssertEqual(objcSpecificNode.parent?.parent?.symbol?.identifier.precise, "some-middle-container-id") + XCTAssertEqual(objcSpecificNode.parent?.parent?.parent?.symbol?.identifier.precise, "some-outer-container-id") + + // Check that each language has different nodes + XCTAssertNotEqual(swiftSpecificNode.parent?.identifier, objcSpecificNode.parent?.identifier) + XCTAssertNotEqual(swiftSpecificNode.parent?.parent?.identifier, objcSpecificNode.parent?.parent?.identifier) + XCTAssertNotEqual(swiftSpecificNode.parent?.parent?.parent?.identifier, objcSpecificNode.parent?.parent?.parent?.identifier) + + // Check the each language representation maps to the other language representation + XCTAssertEqual(swiftSpecificNode.parent?.counterpart?.identifier, objcSpecificNode.parent?.identifier) + XCTAssertEqual(swiftSpecificNode.parent?.parent?.counterpart?.identifier, objcSpecificNode.parent?.parent?.identifier) + XCTAssertEqual(swiftSpecificNode.parent?.parent?.parent?.counterpart?.identifier, objcSpecificNode.parent?.parent?.parent?.identifier) + + // Check that neither path require disambiguation + let paths = tree.caseInsensitiveDisambiguatedPaths() + + XCTAssertEqual(paths["some-outer-container-id"], "/ModuleName/OuterContainerName") + XCTAssertEqual(paths["some-middle-container-id"], "/ModuleName/OuterContainerName/MiddleContainerName") + XCTAssertEqual(paths["some-inner-container-id"], "/ModuleName/OuterContainerName/MiddleContainerName/InnerContainerName") + XCTAssertEqual(paths["some-swift-specific-member-id"], "/ModuleName/OuterContainerName/MiddleContainerName/InnerContainerName/swiftSpecificMember()") + XCTAssertEqual(paths["some-objc-specific-member-id"], "/ModuleName/OuterContainerName/MiddleContainerName/InnerContainerName/objcSpecificMember") + + // Check that the hierarchy doesn't contain any sparse nodes + var remaining = tree.modules[...] + XCTAssertFalse(remaining.isEmpty) + + while let node = remaining.popFirst() { + XCTAssertNotNil(node.symbol, "Unexpected sparse node named '\(node.name)' in hierarchy") + + for container in node.children.values { + remaining.append(contentsOf: container.storage.map(\.node)) + } + } + } + + func testMissingReferencedContainerSymbolOnSomePlatforms() throws { + // We make a best-effort attempt to create a valid path hierarchy, even if the symbol graph inputs are not valid. + + // If some platforms are missing the local container symbol from a `memberOf` relationship, but other platforms with the same relationship define that symbol, + // we use the symbols from the platforms that define the symbol and the relationship. + // The symbol with a `memberOf` relationship to a missing local symbol is not valid but together there's sufficient information to handle it gracefully. + + // Define many platforms, some with the referenced local container symbol and some _without_ the referenced local container symbol. + let platforms = (1...10).map { + let name = "Platform\($0)" + return (name: name, availability: [makeAvailabilityItem(domainName: name)], withoutRequiredContainerSymbol: $0.isMultiple(of: 2)) + } + + let containerID = "some-container-id" + let memberID = "some-member-id" + + let catalog = Folder(name: "unit-test.docc", content: platforms.map { platform in + var symbols = [ + makeSymbol(id: containerID, kind: .struct, pathComponents: ["ContainerName"]), + makeSymbol(id: memberID, kind: .func, pathComponents: ["ContainerName", "memberName"]), + ] + if platform.withoutRequiredContainerSymbol { + // This is not valid because this symbol graph defines a `memberOf` relationship to this symbol in the same module. + symbols.remove(at: 0) + } + + return JSONFile(name: "ModuleName-\(platform.name).symbols.json", content: makeSymbolGraph( + moduleName: "ModuleName", + symbols: symbols, + relationships: [ + .init(source: memberID, target: containerID, kind: .memberOf, targetFallback: nil) + ] + )) + }) + + let (_, context) = try loadBundle(catalog: catalog) + let tree = context.linkResolver.localResolver.pathHierarchy + + try assertFindsPath("/ModuleName/ContainerName/memberName", in: tree, asSymbolID: memberID) + try assertFindsPath("/ModuleName/ContainerName", in: tree, asSymbolID: containerID) + } + func testLinksToCxxOperators() throws { let (_, context) = try testBundleAndContext(named: "CxxOperators") let tree = context.linkResolver.localResolver.pathHierarchy From ef7e33cfc23be0e0393273c4805ec57f6f160f21 Mon Sep 17 00:00:00 2001 From: Marcus Ortiz Date: Tue, 20 May 2025 15:59:20 -0700 Subject: [PATCH 13/22] Sort equal weighted "Mentioned In" articles alphabetically (#1221) (#1225) If articles have same # of mentions, sort them alphabetically. Resolves: rdar://150870055 --- .../Article/ArticleSymbolMentions.swift | 9 +++- .../ArticleSymbolMentionsTests.swift | 50 +++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/Sources/SwiftDocC/Semantics/Article/ArticleSymbolMentions.swift b/Sources/SwiftDocC/Semantics/Article/ArticleSymbolMentions.swift index bc461bc7ce..18d56ae5d6 100644 --- a/Sources/SwiftDocC/Semantics/Article/ArticleSymbolMentions.swift +++ b/Sources/SwiftDocC/Semantics/Article/ArticleSymbolMentions.swift @@ -32,7 +32,14 @@ struct ArticleSymbolMentions { // Mentions are sorted on demand based on the number of mentions. // This could change in the future. return mentions[symbol, default: [:]].sorted { - $0.value > $1.value + // If a pair of articles have the same number of mentions, sort + // them alphabetically. + if $0.value == $1.value { + return $0.key.description < $1.key.description + } + + // Otherwise, sort articles with more mentions first. + return $0.value > $1.value } .map { $0.key } } diff --git a/Tests/SwiftDocCTests/Semantics/ArticleSymbolMentionsTests.swift b/Tests/SwiftDocCTests/Semantics/ArticleSymbolMentionsTests.swift index 3cb9bb3f7d..14cd755e81 100644 --- a/Tests/SwiftDocCTests/Semantics/ArticleSymbolMentionsTests.swift +++ b/Tests/SwiftDocCTests/Semantics/ArticleSymbolMentionsTests.swift @@ -36,6 +36,56 @@ class ArticleSymbolMentionsTests: XCTestCase { XCTAssertEqual(gottenArticle, article) } + // Test the sorting of articles mentioning a given symbol + func testArticlesMentioningSorting() throws { + let bundleID: DocumentationBundle.Identifier = "org.swift.test" + let articles = ["a", "b", "c", "d", "e", "f"].map { letter in + ResolvedTopicReference( + bundleID: bundleID, + path: "/\(letter)", + sourceLanguage: .swift + ) + } + let symbol = ResolvedTopicReference( + bundleID: bundleID, + path: "/z", + sourceLanguage: .swift + ) + + var mentions = ArticleSymbolMentions() + XCTAssertTrue(mentions.articlesMentioning(symbol).isEmpty) + + // test that mentioning articles are sorted by weight + mentions.article(articles[0], didMention: symbol, weight: 10) + mentions.article(articles[1], didMention: symbol, weight: 42) + mentions.article(articles[2], didMention: symbol, weight: 1) + mentions.article(articles[3], didMention: symbol, weight: 14) + mentions.article(articles[4], didMention: symbol, weight: 2) + mentions.article(articles[5], didMention: symbol, weight: 6) + XCTAssertEqual(mentions.articlesMentioning(symbol), [ + articles[1], + articles[3], + articles[0], + articles[5], + articles[4], + articles[2], + ]) + + // test that mentioning articles w/ same weights are sorted alphabetically + // + // note: this test is done multiple times with a shuffled list to ensure + // that it isn't just passing by pure chance due to the unpredictable + // order of Swift dictionaries + for _ in 1...10 { + mentions = ArticleSymbolMentions() + XCTAssertTrue(mentions.articlesMentioning(symbol).isEmpty) + for article in articles.shuffled() { + mentions.article(article, didMention: symbol, weight: 1) + } + XCTAssertEqual(mentions.articlesMentioning(symbol), articles) + } + } + func testSymbolLinkCollectorEnabled() throws { let (bundle, context) = try createMentionedInTestBundle() From 5efe57c81a1ded160ebe85c4612bb31441ef10f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Tue, 3 Jun 2025 10:54:17 +0200 Subject: [PATCH 14/22] Fix a rarely occurring crash when processing certain symbols for many platforms (#1227) (#1229) * Fix a rare crash that sometimes (depending on file read order) happen when processing members of structs nested in unions for many platforms. rdar://151854292 * update test to correctly trigger underlying issue * recursively clone children and transplant those with mismatched languages * Simplify and correct deep cloning of path hierarchy nodes --------- Co-authored-by: Vera Mitchell --- .../Link Resolution/PathHierarchy.swift | 152 +++++++++++------- .../Infrastructure/PathHierarchyTests.swift | 70 ++++++++ 2 files changed, 164 insertions(+), 58 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift index 47edbc00be..be9144cc7e 100644 --- a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift +++ b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift @@ -112,12 +112,15 @@ struct PathHierarchy { var nodes: [String: Node] = [:] nodes.reserveCapacity(graph.symbols.count) for (id, symbol) in graph.symbols { - if let existingNode = allNodes[id]?.first(where: { - // If both identifiers are in the same language, they are the same symbol. - $0.symbol!.identifier.interfaceLanguage == symbol.identifier.interfaceLanguage - // If both have the same path components and kind, their differences don't matter for link resolution purposes. - || ($0.symbol!.pathComponents == symbol.pathComponents && $0.symbol!.kind.identifier == symbol.kind.identifier) - }) { + if let possibleNodes = allNodes[id], + let existingNode = possibleNodes.first(where: { + // If both identifiers are in the same language, they are the same symbol. + $0.symbol!.identifier.interfaceLanguage == symbol.identifier.interfaceLanguage + }) ?? possibleNodes.first(where: { + // Otherwise, if both have the same path components and kind, their differences don't matter for link resolution purposes. + $0.symbol!.pathComponents == symbol.pathComponents && $0.symbol!.kind.identifier == symbol.kind.identifier + }) + { nodes[id] = existingNode if existingNode.counterpart?.languages.contains(language!) != true { // Unless this symbol is already split into language counterparts, add the languages to this node. @@ -158,46 +161,17 @@ struct PathHierarchy { if sourceNode.parent == nil { targetNode.add(symbolChild: sourceNode) } else if sourceNode.parent !== targetNode && sourceNode.counterpart?.parent !== targetNode { - // If the node we have for the child has an existing parent that doesn't - // match the parent from this symbol graph, we need to clone the child to - // ensure that the hierarchy remains consistent. - let clonedSourceNode = Node( - cloning: sourceNode, - symbol: graph.symbols[relationship.source], - children: [:], - languages: [language!] - ) - - // The original node no longer represents this symbol graph's language, - // so remove that data from there. - sourceNode.languages.remove(language!) - - // Make sure that the clone's children can all line up with symbols from this symbol graph. - for children in sourceNode.children.values { - for child in children.storage { - guard let childSymbol = child.node.symbol else { - // We shouldn't come across any non-symbol nodes here, - // but assume they can work as child of both variants. - clonedSourceNode.add(child: child.node, kind: child.kind, hash: child.hash) - continue - } - if nodes[childSymbol.identifier.precise] === child.node { - clonedSourceNode.add(symbolChild: child.node) - } + // If the source node already exist in a different location in the hierarchy we need to split it into separate nodes for each language representation. + // This ensures that each node has a single parent, so that the hierarchy can be unambiguously walked upwards to expand the "scope" of a search. + let clonedSourceNode = sourceNode.deepClone( + separating: language!, + keeping: sourceNode.languages.subtracting([language!]), + symbolsByUSR: graph.symbols, + didCloneNode: { newNode, newSymbol in + nodes[newSymbol.identifier.precise] = newNode + allNodes[newSymbol.identifier.precise, default: []].append(newNode) } - } - - // Track the cloned node in the lists of nodes. - nodes[relationship.source] = clonedSourceNode - if let existingNodes = allNodes[relationship.source] { - clonedSourceNode.counterpart = existingNodes.first - for other in existingNodes { - other.counterpart = clonedSourceNode - } - } - allNodes[relationship.source, default: []].append(clonedSourceNode) - - // Finally, add the cloned node as a child of its parent. + ) targetNode.add(symbolChild: clonedSourceNode) } topLevelCandidates.removeValue(forKey: relationship.source) @@ -597,19 +571,81 @@ extension PathHierarchy { self.children = [:] self.specialBehaviors = [] } - - /// Initializes a node with a new identifier but the data from an existing node. - fileprivate init( - cloning source: Node, - symbol: SymbolGraph.Symbol?? = nil, - children: [String: DisambiguationContainer]? = nil, - languages: Set? = nil - ) { - self.symbol = symbol ?? source.symbol - self.name = source.name - self.children = children ?? source.children - self.specialBehaviors = source.specialBehaviors - self.languages = languages ?? source.languages + + fileprivate func deepClone( + separating separatedLanguage: SourceLanguage, + keeping otherLanguages: Set, + symbolsByUSR: borrowing [String: SymbolGraph.Symbol], + didCloneNode: (Node, SymbolGraph.Symbol) -> Void + ) -> Node { + assert(!otherLanguages.contains(separatedLanguage), "The caller should have already removed '\(separatedLanguage.id)' from '\(languages.sorted().map(\.id).joined(separator: ", "))'") + + let clone: Node + if let currentSymbol = symbol { + // If a representation of the symbol exist in the current local symbol graph, prefer that for more correct disambiguation information. + let symbol = symbolsByUSR[currentSymbol.identifier.precise] ?? currentSymbol + clone = Node(symbol: symbol, name: name) + didCloneNode(clone, symbol) + } else { + assertionFailure("Unexpectedly cloned a non-symbol node '\(name)' into separate language representations ('\(separatedLanguage.id)' vs '\(otherLanguages.sorted().map(\.id).joined(separator: ", "))').") + clone = Node(name: name) + } + // Update languages and counterparts + clone.languages = [separatedLanguage] + languages.remove(separatedLanguage) + assert(!languages.isEmpty, """ + Unexpectedly cloned '\(symbol?.identifier.precise ?? "non-symbol named \(name)")' for '\(separatedLanguage.id)' when it was already the only language it was available for. + """) + + clone.counterpart = self + self.counterpart = clone + + // Assign all the children to either the original, the clone, or both. + let originalChildren = children + children.removeAll(keepingCapacity: true) + + func addOrMove(_ node: Node, to containerNode: Node) { + if node.symbol != nil { + containerNode.add(symbolChild: node) + } else { + containerNode.add(child: node, kind: nil, hash: nil) + } + assert(!containerNode.languages.isDisjoint(with: node.languages), """ + Unexpectedly added a node to a container without any overlapping languages. + Child node languages: \(node.languages.sorted().map(\.id).joined(separator: ", ")) + Parent node languages: \(node.languages.sorted().map(\.id).joined(separator: ", ")) + """) + } + + for elements in originalChildren.values { + for element in elements.storage { + let node = element.node + node.parent = nil // Remove the association with the original container. This node will be added to either the original (again) or to the clone. + let nodeLanguages = node.languages + + switch (nodeLanguages.contains(separatedLanguage), !nodeLanguages.isDisjoint(with: languages)) { + case (true, false): + // This node only exist for the separated language, so it only belongs in the clone. No recursive copying needed. + addOrMove(node, to: clone) + + case (false, true): + // This node doesn't exist for the separated language, so it only belongs in the original. No recursive copying needed. + addOrMove(node, to: self) + + case (true, true): + // This node needs to have deep copies for both the original and the clone. + let innerClone = node.deepClone(separating: separatedLanguage, keeping: otherLanguages, symbolsByUSR: symbolsByUSR, didCloneNode: didCloneNode) + addOrMove(node, to: self) + addOrMove(innerClone, to: clone) + + case (false, false): + assertionFailure("Node \(node.name) (\(node.languages.sorted().map(\.id).joined(separator: ","))) doesn't belong in either '\(separatedLanguage.id)' or '\(otherLanguages.sorted().map(\.id).joined(separator: ", "))'.") + continue + } + } + } + + return clone } /// Adds a descendant to this node, providing disambiguation information from the node's symbol. diff --git a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift index 6228a4c592..47c61b9702 100644 --- a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift @@ -3328,6 +3328,76 @@ class PathHierarchyTests: XCTestCase { try assertFindsPath("/ModuleName/ContainerName", in: tree, asSymbolID: containerID) } + func testMissingMemberOfAnonymousStructInsideUnion() throws { + let outerContainerID = "some-outer-container-symbol-id" + let innerContainerID = "some-inner-container-symbol-id" + let memberID = "some-member-symbol-id" + + // Repeat the same symbols in both languages for many platforms. + let platforms = (1...10).map { + let name = "Platform\($0)" + return (name: name, availability: [makeAvailabilityItem(domainName: name)]) + } + + let catalog = Folder(name: "unit-test.docc", content: [ + // union Outer { + // struct { + // uint32_t member; + // } inner; + // }; + Folder(name: "clang", content: platforms.map { platform in + JSONFile(name: "ModuleName-\(platform.name).symbols.json", content: makeSymbolGraph( + moduleName: "ModuleName", + symbols: [ + makeSymbol(id: outerContainerID, language: .objectiveC, kind: .union, pathComponents: ["Outer"], availability: platform.availability), + makeSymbol(id: innerContainerID, language: .objectiveC, kind: .property, pathComponents: ["Outer", "inner"], availability: platform.availability), + makeSymbol(id: memberID, language: .objectiveC, kind: .property, pathComponents: ["Outer", "inner", "member"], availability: platform.availability), + ], + relationships: [ + .init(source: memberID, target: innerContainerID, kind: .memberOf, targetFallback: nil), + .init(source: innerContainerID, target: outerContainerID, kind: .memberOf, targetFallback: nil), + ] + )) + }), + + // struct Outer { + // struct __Unnamed_struct_inner { + // var member: UInt32 // <-- This symbol is missing due to rdar://152157610 + // } + // var inner: Outer.__Unnamed_struct_inner + // } + Folder(name: "swift", content: platforms.map { platform in + JSONFile(name: "ModuleName-\(platform.name).symbols.json", content: makeSymbolGraph( + moduleName: "ModuleName", + symbols: [ + makeSymbol(id: outerContainerID, language: .swift, kind: .struct, pathComponents: ["Outer"], availability: platform.availability), + makeSymbol(id: innerContainerID, language: .swift, kind: .property, pathComponents: ["Outer", "inner"], availability: platform.availability), + // The `member` property is missing due to rdar://152157610 + ], + relationships: [ + .init(source: innerContainerID, target: outerContainerID, kind: .memberOf, targetFallback: nil), + ] + )) + }) + ]) + + let (_, context) = try loadBundle(catalog: catalog) + let tree = context.linkResolver.localResolver.pathHierarchy + + let paths = tree.caseInsensitiveDisambiguatedPaths() + XCTAssertEqual(paths[outerContainerID], "/ModuleName/Outer") + XCTAssertEqual(paths[innerContainerID], "/ModuleName/Outer/inner") + XCTAssertEqual(paths[memberID], "/ModuleName/Outer/inner/member") + + try assertFindsPath("/ModuleName/Outer-union", in: tree, asSymbolID: outerContainerID) + try assertFindsPath("/ModuleName/Outer-union/inner", in: tree, asSymbolID: innerContainerID) + try assertFindsPath("/ModuleName/Outer-union/inner/member", in: tree, asSymbolID: memberID) + + try assertFindsPath("/ModuleName/Outer-struct", in: tree, asSymbolID: outerContainerID) + try assertFindsPath("/ModuleName/Outer-struct/inner", in: tree, asSymbolID: innerContainerID) + try assertPathNotFound("/ModuleName/Outer-struct/inner/member", in: tree) + } + func testLinksToCxxOperators() throws { let (_, context) = try testBundleAndContext(named: "CxxOperators") let tree = context.linkResolver.localResolver.pathHierarchy From d1345489615b267ee4ddb481b7870b165d902d3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Thu, 12 Jun 2025 18:09:39 +0200 Subject: [PATCH 15/22] Crawl the curation of auto-curated articles (#1236) (#1237) rdar://151731131 --- .../Infrastructure/DocumentationContext.swift | 11 ++- .../Topic Graph/TopicGraph.swift | 2 +- .../DocumentationCuratorTests.swift | 84 +++++++++++++++++++ 3 files changed, 90 insertions(+), 7 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift index f0b11089b1..224bd5f287 100644 --- a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift +++ b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift @@ -2428,18 +2428,17 @@ public class DocumentationContext { // Crawl the rest of the symbols that haven't been crawled so far in hierarchy pre-order. allCuratedReferences = try crawlSymbolCuration(in: automaticallyCurated.map(\.symbol), bundle: bundle, initial: allCuratedReferences) - - // Remove curation paths that have been created automatically above - // but we've found manual curation for in the second crawl pass. - removeUnneededAutomaticCuration(automaticallyCurated) // Automatically curate articles that haven't been manually curated // Article curation is only done automatically if there is only one root module if let rootNode = rootNodeForAutomaticCuration { let articleReferences = try autoCurateArticles(otherArticles, startingFrom: rootNode) - preResolveExternalLinks(references: articleReferences, localBundleID: bundle.id) - resolveLinks(curatedReferences: Set(articleReferences), bundle: bundle) + allCuratedReferences = try crawlSymbolCuration(in: articleReferences, bundle: bundle, initial: allCuratedReferences) } + + // Remove curation paths that have been created automatically above + // but we've found manual curation for in the second crawl pass. + removeUnneededAutomaticCuration(automaticallyCurated) // Remove any empty "Extended Symbol" pages whose children have been curated elsewhere. for module in rootModules { diff --git a/Sources/SwiftDocC/Infrastructure/Topic Graph/TopicGraph.swift b/Sources/SwiftDocC/Infrastructure/Topic Graph/TopicGraph.swift index d994f415b1..9bd9746983 100644 --- a/Sources/SwiftDocC/Infrastructure/Topic Graph/TopicGraph.swift +++ b/Sources/SwiftDocC/Infrastructure/Topic Graph/TopicGraph.swift @@ -330,7 +330,7 @@ struct TopicGraph { } var result = "" - result.append("\(decorator) \(node[keyPath: keyPath])\r\n") + result.append("\(decorator) \(node[keyPath: keyPath])\n") if let childEdges = edges[node.reference]?.sorted(by: { $0.path < $1.path }) { for (index, childRef) in childEdges.enumerated() { var decorator = decorator diff --git a/Tests/SwiftDocCTests/Infrastructure/DocumentationCuratorTests.swift b/Tests/SwiftDocCTests/Infrastructure/DocumentationCuratorTests.swift index fea832171e..f40e9ef03d 100644 --- a/Tests/SwiftDocCTests/Infrastructure/DocumentationCuratorTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/DocumentationCuratorTests.swift @@ -201,6 +201,90 @@ class DocumentationCuratorTests: XCTestCase { XCTAssertEqual(curationProblem.possibleSolutions.map(\.summary), ["Remove '- '"]) } + func testCurationInUncuratedAPICollection() throws { + // Everything should behave the same when an API Collection is automatically curated as when it is explicitly curated + for shouldCurateAPICollection in [true, false] { + let assertionMessageDescription = "when the API collection is \(shouldCurateAPICollection ? "explicitly curated" : "auto-curated as an article under the module")." + + let catalog = Folder(name: "unit-test.docc", content: [ + JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(moduleName: "ModuleName", symbols: [ + makeSymbol(id: "some-symbol-id", kind: .class, pathComponents: ["SomeClass"]) + ])), + + TextFile(name: "ModuleName.md", utf8Content: """ + # ``ModuleName`` + + \(shouldCurateAPICollection ? "## Topics\n\n### Explicit curation\n\n- " : "") + """), + + TextFile(name: "API-Collection.md", utf8Content: """ + # Some API collection + + Curate the only symbol + + ## Topics + + - ``SomeClass`` + - ``NotFound`` + """), + ]) + let (bundle, context) = try loadBundle(catalog: catalog) + XCTAssertEqual( + context.problems.map(\.diagnostic.summary), + [ + // There should only be a single problem about the unresolvable link in the API collection. + "'NotFound' doesn't exist at '/unit-test/API-Collection'" + ], + "Unexpected problems: \(context.problems.map(\.diagnostic.summary).joined(separator: "\n")) \(assertionMessageDescription)" + ) + + // Verify that the topic graph paths to the symbol (although not used for its breadcrumbs) doesn't have the automatic edge anymore. + let symbolReference = try XCTUnwrap(context.knownPages.first(where: { $0.lastPathComponent == "SomeClass" })) + XCTAssertEqual( + context.finitePaths(to: symbolReference).map { $0.map(\.path) }, + [ + // The automatic default `["/documentation/ModuleName"]` curation _shouldn't_ be here. + + // The authored curation in the uncurated API collection + ["/documentation/ModuleName", "/documentation/unit-test/API-Collection"], + ], + "Unexpected 'paths' to the symbol page \(assertionMessageDescription)" + ) + + // Verify that the symbol page shouldn't auto-curate in its canonical location. + let symbolTopicNode = try XCTUnwrap(context.topicGraph.nodeWithReference(symbolReference)) + XCTAssertFalse(symbolTopicNode.shouldAutoCurateInCanonicalLocation, "Symbol node is unexpectedly configured to auto-curate \(assertionMessageDescription)") + + // Verify that the topic graph doesn't have the automatic edge anymore. + XCTAssertEqual(context.dumpGraph(), """ + doc://unit-test/documentation/ModuleName + ╰ doc://unit-test/documentation/unit-test/API-Collection + ╰ doc://unit-test/documentation/ModuleName/SomeClass + + """, + "Unexpected topic graph \(assertionMessageDescription)" + ) + + // Verify that the rendered top-level page doesn't have an automatic "Classes" topic section anymore. + let converter = DocumentationNodeConverter(bundle: bundle, context: context) + let moduleReference = try XCTUnwrap(context.soleRootModuleReference) + let rootRenderNode = converter.convert(try context.entity(with: moduleReference)) + + XCTAssertEqual( + rootRenderNode.topicSections.map(\.title), + [shouldCurateAPICollection ? "Explicit curation" : "Articles"], + "Unexpected rendered topic sections on the module page \(assertionMessageDescription)" + ) + XCTAssertEqual( + rootRenderNode.topicSections.map(\.identifiers), + [ + ["doc://unit-test/documentation/unit-test/API-Collection"], + ], + "Unexpected rendered topic sections on the module page \(assertionMessageDescription)" + ) + } + } + func testModuleUnderTechnologyRoot() throws { let (_, bundle, context) = try testBundleAndContext(copying: "SourceLocations") { url in try """ From 886f9a1a602f048b8ff1e1e80b65991c3515939c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Wed, 18 Jun 2025 14:27:22 +0200 Subject: [PATCH 16/22] Don't omit "Type" suffix from disambiguation for Swift metatypes (#1230) (#1240) * Don't omit "Type" suffix from disambiguation for Swift metatype parameters rdar://152496046 * Remove trailing commas for compatibility before Swift 6.1 --- .../PathHierarchy+TypeSignature.swift | 5 +++ .../Infrastructure/PathHierarchyTests.swift | 36 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+TypeSignature.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+TypeSignature.swift index 52af84c174..4eb428bca4 100644 --- a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+TypeSignature.swift +++ b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+TypeSignature.swift @@ -136,6 +136,11 @@ extension PathHierarchy { // For example: "[", "?", "<", "...", ",", "(", "->" etc. contribute to the type spellings like // `[Name]`, `Name?`, "Name", "Name...", "()", "(Name, Name)", "(Name)->Name" and more. let utf8Spelling = fragment.spelling.utf8 + guard !utf8Spelling.elementsEqual(".Type".utf8) else { + // Once exception to that is "Name.Type" which is different from just "Name" (and we don't want a trailing ".") + accumulated.append(contentsOf: utf8Spelling) + continue + } for index in utf8Spelling.indices { let char = utf8Spelling[index] switch char { diff --git a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift index 47c61b9702..88eed6aa90 100644 --- a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift @@ -3772,6 +3772,42 @@ class PathHierarchyTests: XCTestCase { ]) } + // The second overload refers to the metatype of the parameter + do { + func makeSignature(first: DeclToken...) -> SymbolGraph.Symbol.FunctionSignature { + .init( + parameters: [.init(name: "first", externalName: "with", declarationFragments: makeFragments(first), children: []),], + returns: makeFragments([voidType]) + ) + } + + let someGenericTypeID = "some-generic-type-id" + let catalog = Folder(name: "unit-test.docc", content: [ + JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph( + moduleName: "ModuleName", + symbols: [ + makeSymbol(id: "function-overload-1", kind: .func, pathComponents: ["doSomething(with:)"], signature: makeSignature( + // GenericName + first: .typeIdentifier("GenericName", precise: someGenericTypeID) + )), + + makeSymbol(id: "function-overload-2", kind: .func, pathComponents: ["doSomething(with:)"], signature: makeSignature( + // GenericName.Type + first: .typeIdentifier("GenericName", precise: someGenericTypeID), ".Type" + )), + ] + )) + ]) + + let (_, context) = try loadBundle(catalog: catalog) + let tree = context.linkResolver.localResolver.pathHierarchy + + try assertPathCollision("ModuleName/doSomething(with:)", in: tree, collisions: [ + (symbolID: "function-overload-1", disambiguation: "-(GenericName)"), // GenericName + (symbolID: "function-overload-2", disambiguation: "-(GenericName.Type)"), // GenericName.Type + ]) + } + // Second overload requires combination of two non-unique types to disambiguate do { // String Set (Double)->Void From 13bb4bac3309d473e2e68c473e5c740035c4e1bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Wed, 18 Jun 2025 14:38:58 +0200 Subject: [PATCH 17/22] Omit whitespace from suggested link completion disambiguations (#1232) (#1241) rdar://152496072 --- .../LinkCompletionTools.swift | 10 +++++++-- .../LinkCompletionToolsTests.swift | 21 +++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/Sources/SwiftDocC/DocumentationService/Convert/Symbol Link Resolution/LinkCompletionTools.swift b/Sources/SwiftDocC/DocumentationService/Convert/Symbol Link Resolution/LinkCompletionTools.swift index 7ed5ac3d52..a747125410 100644 --- a/Sources/SwiftDocC/DocumentationService/Convert/Symbol Link Resolution/LinkCompletionTools.swift +++ b/Sources/SwiftDocC/DocumentationService/Convert/Symbol Link Resolution/LinkCompletionTools.swift @@ -131,8 +131,8 @@ public enum LinkCompletionTools { node, kind: symbol.kind, hash: symbol.symbolIDHash, - parameterTypes: symbol.parameterTypes, - returnTypes: symbol.returnTypes + parameterTypes: symbol.parameterTypes?.map { $0.withoutWhitespace() }, + returnTypes: symbol.returnTypes?.map { $0.withoutWhitespace() } ) } @@ -236,3 +236,9 @@ private extension PathHierarchy.PathComponent.Disambiguation { } } } + +private extension String { + func withoutWhitespace() -> String { + filter { !$0.isWhitespace } + } +} diff --git a/Tests/SwiftDocCTests/Infrastructure/Symbol Link Resolution/LinkCompletionToolsTests.swift b/Tests/SwiftDocCTests/Infrastructure/Symbol Link Resolution/LinkCompletionToolsTests.swift index 6cc34779d6..76d963527b 100644 --- a/Tests/SwiftDocCTests/Infrastructure/Symbol Link Resolution/LinkCompletionToolsTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/Symbol Link Resolution/LinkCompletionToolsTests.swift @@ -237,4 +237,25 @@ class LinkCompletionToolsTests: XCTestCase { "->_", // The only overload that returns something ]) } + + func testRemovesWhitespaceFromTypeSignatureDisambiguation() { + let overloads = [ + // The caller included whitespace in these closure type spellings but the DocC disambiguation won't include this whitespace. + (parameters: ["(Int) -> Int"], returns: []), // ((Int) -> Int) -> Void + (parameters: ["(Bool) -> ()"], returns: []), // ((Bool) -> () ) -> Void + ].map { + LinkCompletionTools.SymbolInformation( + kind: "func", + symbolIDHash: "\($0)".stableHashString, + parameterTypes: $0.parameters, + returnTypes: $0.returns + ) + } + + XCTAssertEqual(LinkCompletionTools.suggestedDisambiguation(forCollidingSymbols: overloads), [ + // Both parameters require the only parameter type as disambiguation. The suggested disambiguation shouldn't contain extra whitespace. + "-((Int)->Int)", + "-((Bool)->())", + ]) + } } From c0873cdbe42de789db0fc9ef15dd8383cac0cc60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Wed, 18 Jun 2025 17:06:06 +0200 Subject: [PATCH 18/22] Don't remove leading and trailing parentheses for closure types in type signature disambiguation (#1231) (#1243) * Fix issue where suggested link disambiguation removed leading and trailing parentheses for some closure types rdar://151311221 * Minor unrelated correction of whitespace in symbol declaration test data * Include more information in test failure messages about unknown disambiguation --- .../PathHierarchy+TypeSignature.swift | 42 ++++-- .../Infrastructure/PathHierarchyTests.swift | 123 +++++++++++++++++- 2 files changed, 152 insertions(+), 13 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+TypeSignature.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+TypeSignature.swift index 4eb428bca4..481615a90a 100644 --- a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+TypeSignature.swift +++ b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+TypeSignature.swift @@ -45,7 +45,7 @@ extension PathHierarchy { } let spelling = utf8TypeSpelling(for: fragments, isSwift: isSwift) - guard isSwift, spelling[...].isTuple() else { + guard isSwift, spelling[...].shapeOfSwiftTypeSpelling() == .tuple else { return [String(decoding: spelling, as: UTF8.self)] } @@ -194,14 +194,14 @@ extension PathHierarchy { } // Check if the type names are wrapped in redundant parenthesis and remove them - if accumulated.first == openParen, accumulated.last == closeParen, !accumulated[...].isTuple() { + if accumulated.first == openParen, accumulated.last == closeParen, accumulated[...].shapeOfSwiftTypeSpelling() == .scalar { // In case there are multiple // Use a temporary slice until all the layers of redundant parenthesis have been removed. var temp = accumulated[...] repeat { temp = temp.dropFirst().dropLast() - } while temp.first == openParen && temp.last == closeParen && !temp.isTuple() + } while temp.first == openParen && temp.last == closeParen && temp.shapeOfSwiftTypeSpelling() == .scalar // Adjust the markers so that they align with the expected characters let difference = (accumulated.count - temp.count) / 2 @@ -282,26 +282,48 @@ private let question = UTF8.CodeUnit(ascii: "?") private let colon = UTF8.CodeUnit(ascii: ":") private let hyphen = UTF8.CodeUnit(ascii: "-") +/// A guesstimate of the "shape" of a Swift type based on its spelling. +private enum ShapeOfSwiftTypeSpelling { + /// This type spelling looks like a scalar. + /// + /// For example `Name` or `(Name)`. + /// - Note: We treat `(Name)` as a non-tuple so that we can remove the redundant leading and trailing parenthesis. + case scalar + /// This type spelling looks like a tuple. + /// + /// For example `(First, Second)`. + case tuple + /// This type spelling looks like a closure. + /// + /// For example `(First)->Second` or `(First, Second)->()` or `()->()`. + case closure +} + private extension ContiguousArray.SubSequence { - /// Checks if the UTF-8 string looks like a tuple with comma separated values. + /// Checks if the UTF-8 string looks like a tuple, scalar, or closure. /// /// This is used to remove redundant parenthesis around expressions. - func isTuple() -> Bool { - guard first == openParen, last == closeParen else { return false } + func shapeOfSwiftTypeSpelling() -> ShapeOfSwiftTypeSpelling { + guard first == openParen, last == closeParen else { return .scalar } var depth = 0 - for char in self { - switch char { + for index in indices { + switch self[index] { case openParen: depth += 1 case closeParen: depth -= 1 case comma where depth == 1: - return true + // If we find "," in one level of parenthesis, we've found a tuple. + return .tuple + case closeAngle where depth == 0 && index > startIndex && self[index - 1] == hyphen: + // If we find "->" outside any parentheses, we've found a closure. + return .closure default: continue } } - return false + // If we traversed the entire type name without finding a tuple or a closure we treat the type name as a scalar. + return .scalar } } diff --git a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift index 88eed6aa90..4c9c1b00dd 100644 --- a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift @@ -3328,6 +3328,73 @@ class PathHierarchyTests: XCTestCase { try assertFindsPath("/ModuleName/ContainerName", in: tree, asSymbolID: containerID) } + func testMinimalTypeDisambiguationForClosureParameterWithVoidReturnType() throws { + // Create a `doSomething(with:and:)` function with a `String` parameter (same in every overload) and a `(TYPE)->()` closure parameter. + func makeSymbolOverload(closureParameterType: SymbolGraph.Symbol.DeclarationFragments.Fragment) -> SymbolGraph.Symbol { + makeSymbol( + id: "some-function-overload-\(closureParameterType.spelling.lowercased())", + kind: .method, + pathComponents: ["doSomething(with:and:)"], + signature: .init( + parameters: [ + .init(name: "first", externalName: "with", declarationFragments: [ + .init(kind: .externalParameter, spelling: "with", preciseIdentifier: nil), + .init(kind: .text, spelling: " ", preciseIdentifier: nil), + .init(kind: .internalParameter, spelling: "first", preciseIdentifier: nil), + .init(kind: .text, spelling: " ", preciseIdentifier: nil), + .init(kind: .typeIdentifier, spelling: "String", preciseIdentifier: "s:SS") + ], children: []), + + .init(name: "second", externalName: "and", declarationFragments: [ + .init(kind: .externalParameter, spelling: "and", preciseIdentifier: nil), + .init(kind: .text, spelling: " ", preciseIdentifier: nil), + .init(kind: .internalParameter, spelling: "second", preciseIdentifier: nil), + .init(kind: .text, spelling: " (", preciseIdentifier: nil), + closureParameterType, + .init(kind: .text, spelling: ") -> ()", preciseIdentifier: nil), + ], children: []) + ], + returns: [.init(kind: .typeIdentifier, spelling: "Void", preciseIdentifier: "s:s4Voida")] + ) + ) + } + + let catalog = Folder(name: "unit-test.docc", content: [ + JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph( + moduleName: "ModuleName", + symbols: [ + makeSymbolOverload(closureParameterType: .init(kind: .typeIdentifier, spelling: "Int", preciseIdentifier: "s:Si")), // (String, (Int)->()) -> Void + makeSymbolOverload(closureParameterType: .init(kind: .typeIdentifier, spelling: "Double", preciseIdentifier: "s:Sd")), // (String, (Double)->()) -> Void + makeSymbolOverload(closureParameterType: .init(kind: .typeIdentifier, spelling: "Float", preciseIdentifier: "s:Sf")), // (String, (Float)->()) -> Void + ], + relationships: [] + )) + ]) + + let (_, context) = try loadBundle(catalog: catalog) + let tree = context.linkResolver.localResolver.pathHierarchy + + let link = "/ModuleName/doSomething(with:and:)" + try assertPathRaisesErrorMessage(link, in: tree, context: context, expectedErrorMessage: "'doSomething(with:and:)' is ambiguous at '/ModuleName'") { errorInfo in + XCTAssertEqual(errorInfo.solutions.count, 3, "There should be one suggestion per overload") + for solution in errorInfo.solutions { + // Apply the suggested replacements for each solution and verify that _that_ link resolves to a single symbol. + var linkWithSuggestion = link + XCTAssertFalse(solution.replacements.isEmpty, "Diagnostics about ambiguous links should have some replacements for each solution.") + for (replacementText, start, end) in solution.replacements { + let range = linkWithSuggestion.index(linkWithSuggestion.startIndex, offsetBy: start) ..< linkWithSuggestion.index(linkWithSuggestion.startIndex, offsetBy: end) + linkWithSuggestion.replaceSubrange(range, with: replacementText) + } + + XCTAssertNotNil(try? tree.findSymbol(path: linkWithSuggestion), """ + Failed to resolve \(linkWithSuggestion) after applying replacements \(solution.replacements.map { "'\($0.0)'@\($0.start)-\($0.end)" }.joined(separator: ",")) to '\(link)'. + + The replacement that DocC suggests in its warnings should unambiguously refer to a single symbol match. + """) + } + } + } + func testMissingMemberOfAnonymousStructInsideUnion() throws { let outerContainerID = "some-outer-container-symbol-id" let innerContainerID = "some-inner-container-symbol-id" @@ -3665,7 +3732,7 @@ class PathHierarchyTests: XCTestCase { let voidType = DeclToken.typeIdentifier("Void", precise: "s:s4Voida") func makeParameter(_ name: String, decl: [DeclToken]) -> SymbolGraph.Symbol.FunctionSignature.FunctionParameter { - .init(name: name, externalName: nil, declarationFragments: makeFragments([.internalParameter(name), .text("")] + decl), children: []) + .init(name: name, externalName: nil, declarationFragments: makeFragments([.internalParameter(name), .text(" ")] + decl), children: []) } func makeSignature(first: DeclToken..., second: DeclToken..., third: DeclToken...) -> SymbolGraph.Symbol.FunctionSignature { @@ -3772,6 +3839,56 @@ class PathHierarchyTests: XCTestCase { ]) } + // Each overload has a unique closure parameter with a "()" literal closure return type + do { + func makeSignature(first: DeclToken..., second: DeclToken...) -> SymbolGraph.Symbol.FunctionSignature { + .init( + parameters: [ + .init(name: "first", externalName: nil, declarationFragments: makeFragments(first), children: []), + .init(name: "second", externalName: nil, declarationFragments: makeFragments(second), children: []) + ], + returns: makeFragments([voidType]) + ) + } + + // String (Int)->() + // String (Double)->() + // String (Float)->() + let catalog = Folder(name: "unit-test.docc", content: [ + JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph( + moduleName: "ModuleName", + symbols: [ + // String (Int)->Void + makeSymbol(id: "function-overload-1", kind: .func, pathComponents: ["doSomething(first:second:)"], signature: makeSignature( + first: stringType, // String + second: "(", intType, ") -> ()" // (Int)->() + )), + + // String (Double)->Void + makeSymbol(id: "function-overload-2", kind: .func, pathComponents: ["doSomething(first:second:)"], signature: makeSignature( + first: stringType, // String + second: "(", doubleType, ") -> ()" // (Double)->() + )), + + // String (Float)->Void + makeSymbol(id: "function-overload-3", kind: .func, pathComponents: ["doSomething(first:second:)"], signature: makeSignature( + first: stringType, // String + second: "(", floatType, ") -> ()" // (Double)->() + )), + ] + )) + ]) + + let (_, context) = try loadBundle(catalog: catalog) + let tree = context.linkResolver.localResolver.pathHierarchy + + try assertPathCollision("ModuleName/doSomething(first:second:)", in: tree, collisions: [ + (symbolID: "function-overload-1", disambiguation: "-(_,(Int)->())"), // _ (Int)->() + (symbolID: "function-overload-2", disambiguation: "-(_,(Double)->())"), // _ (Double)->() + (symbolID: "function-overload-3", disambiguation: "-(_,(Float)->())"), // _ (Float)->() + ]) + } + // The second overload refers to the metatype of the parameter do { func makeSignature(first: DeclToken...) -> SymbolGraph.Symbol.FunctionSignature { @@ -4350,8 +4467,8 @@ class PathHierarchyTests: XCTestCase { XCTFail("Symbol for \(path.singleQuoted) not found in tree", file: file, line: line) } catch PathHierarchy.Error.unknownName { XCTFail("Symbol for \(path.singleQuoted) not found in tree. Only part of path is found.", file: file, line: line) - } catch PathHierarchy.Error.unknownDisambiguation { - XCTFail("Symbol for \(path.singleQuoted) not found in tree. Unknown disambiguation.", file: file, line: line) + } catch PathHierarchy.Error.unknownDisambiguation(_, _, let candidates) { + XCTFail("Symbol for \(path.singleQuoted) not found in tree. Unknown disambiguation. Suggested disambiguations: \(candidates.map(\.disambiguation.singleQuoted).sorted().joined(separator: ", "))", file: file, line: line) } catch PathHierarchy.Error.lookupCollision(_, _, let collisions) { let symbols = collisions.map { $0.node.symbol! } XCTFail("Unexpected collision for \(path.singleQuoted); \(symbols.map { return "\($0.names.title) - \($0.kind.identifier.identifier) - \($0.identifier.precise.stableHashString)"})", file: file, line: line) From 599ccba2f95466898da7dc64d07b21a5a8b478ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Fri, 27 Jun 2025 14:11:27 +0200 Subject: [PATCH 19/22] Fix false positive curation for article links with extra path components (#1235) (#1245) * Fix false positive curation for article links with extra path components rdar://150874238 * Remove explicit `.init` in test file * Remove unused variable in test * Add missing path separator in curator fallback code path --- .../Infrastructure/DocumentationCurator.swift | 20 ++- .../DocumentationCuratorTests.swift | 115 ++++++++++++++++-- 2 files changed, 123 insertions(+), 12 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationCurator.swift b/Sources/SwiftDocC/Infrastructure/DocumentationCurator.swift index b310940a53..1519965672 100644 --- a/Sources/SwiftDocC/Infrastructure/DocumentationCurator.swift +++ b/Sources/SwiftDocC/Infrastructure/DocumentationCurator.swift @@ -90,9 +90,22 @@ struct DocumentationCurator { } // Try extracting an article from the cache - let articleFilename = unresolved.topicURL.components.path.components(separatedBy: "/").last! - let sourceArticlePath = NodeURLGenerator.Path.article(bundleName: bundle.displayName, articleName: articleFilename).stringValue - + let sourceArticlePath: String = { + let path = unresolved.topicURL.components.path.removingLeadingSlash + + // The article path can either be written as + // - "ArticleName" + // - "CatalogName/ArticleName" + // - "documentation/CatalogName/ArticleName" + switch path.components(separatedBy: "/").count { + case 0,1: + return NodeURLGenerator.Path.article(bundleName: bundle.displayName, articleName: path).stringValue + case 2: + return "\(NodeURLGenerator.Path.documentationFolder)/\(path)" + default: + return path.prependingLeadingSlash + } + }() let reference = ResolvedTopicReference( bundleID: resolved.bundleID, path: sourceArticlePath, @@ -115,6 +128,7 @@ struct DocumentationCurator { context.topicGraph.addNode(curatedNode) // Move the article from the article cache to the documentation + let articleFilename = reference.url.pathComponents.last! context.linkResolver.localResolver.addArticle(filename: articleFilename, reference: reference, anchorSections: documentationNode.anchorSections) context.documentationCache[reference] = documentationNode diff --git a/Tests/SwiftDocCTests/Infrastructure/DocumentationCuratorTests.swift b/Tests/SwiftDocCTests/Infrastructure/DocumentationCuratorTests.swift index f40e9ef03d..0149024be8 100644 --- a/Tests/SwiftDocCTests/Infrastructure/DocumentationCuratorTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/DocumentationCuratorTests.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) 2021-2025 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 @@ -30,7 +30,7 @@ class DocumentationCuratorTests: XCTestCase { func testCrawl() throws { let (bundle, context) = try testBundleAndContext(named: "LegacyBundle_DoNotUseInNewTests") - var crawler = DocumentationCurator.init(in: context, bundle: bundle) + var crawler = DocumentationCurator(in: context, bundle: bundle) let mykit = try context.entity(with: ResolvedTopicReference(bundleID: "org.swift.docc.example", path: "/documentation/MyKit", sourceLanguage: .swift)) var symbolsWithCustomCuration = [ResolvedTopicReference]() @@ -303,7 +303,7 @@ class DocumentationCuratorTests: XCTestCase { """.write(to: url.appendingPathComponent("Root.md"), atomically: true, encoding: .utf8) } - let crawler = DocumentationCurator.init(in: context, bundle: bundle) + let crawler = DocumentationCurator(in: context, bundle: bundle) XCTAssert(context.problems.isEmpty, "Expected no problems. Found: \(context.problems.map(\.diagnostic.summary))") guard let moduleNode = context.documentationCache["SourceLocations"], @@ -316,11 +316,109 @@ class DocumentationCuratorTests: XCTestCase { XCTAssertEqual(root.path, "/documentation/Root") XCTAssertEqual(crawler.problems.count, 0) - + } + + func testCuratorDoesNotRelateNodesWhenArticleLinksContainExtraPathComponents() throws { + let (bundle, context) = try loadBundle(catalog: + Folder(name: "CatalogName.docc", content: [ + TextFile(name: "Root.md", utf8Content: """ + # Root + + @Metadata { + @TechnologyRoot + } + + Add an API Collection of indirection to more easily detect the failed curation. + + ## Topics + - + """), + + TextFile(name: "API-Collection.md", utf8Content: """ + # Some API Collection + + Fail to curate all 4 articles because of extra incorrect path components. + + ## Topics + + ### No links will resolve in this section + + - + - + - + - + """), + + TextFile(name: "First.md", utf8Content: "# First"), + TextFile(name: "Second.md", utf8Content: "# Second"), + TextFile(name: "Third.md", utf8Content: "# Third"), + TextFile(name: "Forth.md", utf8Content: "# Forth"), + ]) + ) + let (linkResolutionProblems, otherProblems) = context.problems.categorize(where: { $0.diagnostic.identifier == "org.swift.docc.unresolvedTopicReference" }) + XCTAssert(otherProblems.isEmpty, "Unexpected problems: \(otherProblems.map(\.diagnostic.summary).sorted())") + + XCTAssertEqual( + linkResolutionProblems.map(\.diagnostic.source?.lastPathComponent), + ["API-Collection.md", "API-Collection.md", "API-Collection.md", "API-Collection.md"], + "Every unresolved link is in the API collection" + ) + XCTAssertEqual( + linkResolutionProblems.map({ $0.diagnostic.range?.lowerBound.line }), [9, 10, 11, 12], + "There should be one warning about an unresolved reference for each link in the API collection's top" + ) + + let rootReference = try XCTUnwrap(context.soleRootModuleReference) + + for articleName in ["First", "Second", "Third", "Forth"] { + let reference = try XCTUnwrap(context.documentationCache.allReferences.first(where: { $0.lastPathComponent == articleName })) + XCTAssertEqual( + context.topicGraph.nodeWithReference(reference)?.shouldAutoCurateInCanonicalLocation, true, + "Article '\(articleName)' isn't (successfully) manually curated and should therefore automatically curate." + ) + XCTAssertEqual( + context.topicGraph.reverseEdges[reference]?.map(\.path), [rootReference.path], + "Article '\(articleName)' should only have a reverse edge to the root page where it will be automatically curated." + ) + } + + let apiCollectionReference = try XCTUnwrap(context.documentationCache.allReferences.first(where: { $0.lastPathComponent == "API-Collection" })) + let apiCollectionSemantic = try XCTUnwrap(try context.entity(with: apiCollectionReference).semantic as? Article) + XCTAssertEqual(apiCollectionSemantic.topics?.taskGroups.count, 1, "The API Collection has one topic section") + let topicSection = try XCTUnwrap(apiCollectionSemantic.topics?.taskGroups.first) + XCTAssertEqual(topicSection.links.map(\.destination), [ + // All these links are the same as they were authored which means that they didn't resolve. + "doc:WrongModuleName/First", + "doc:documentation/WrongModuleName/Second", + "doc:documentation/CatalogName/ExtraPathComponent/Third", + "doc:CatalogName/ExtraPathComponent/Forth", + ]) + + let rootPage = try context.entity(with: rootReference) + let renderer = DocumentationNodeConverter(bundle: bundle, context: context) + let renderNode = renderer.convert(rootPage) + + XCTAssertEqual(renderNode.topicSections.map(\.title), [ + nil, // An unnamed topic section + "Articles", // The automatic topic section + ]) + XCTAssertEqual(renderNode.topicSections.map { $0.identifiers.sorted() }, [ + // The unnamed topic section curates the API collection + [ + "doc://CatalogName/documentation/CatalogName/API-Collection" + ], + // The automatic "Articles" section curates all 4 articles + [ + "doc://CatalogName/documentation/CatalogName/First", + "doc://CatalogName/documentation/CatalogName/Forth", + "doc://CatalogName/documentation/CatalogName/Second", + "doc://CatalogName/documentation/CatalogName/Third", + ], + ]) } func testModuleUnderAncestorOfTechnologyRoot() throws { - let (_, bundle, context) = try testBundleAndContext(copying: "SourceLocations") { url in + let (_, _, context) = try testBundleAndContext(copying: "SourceLocations") { url in try """ # Root with ancestor curating a module @@ -347,7 +445,6 @@ class DocumentationCuratorTests: XCTestCase { """.write(to: url.appendingPathComponent("Ancestor.md"), atomically: true, encoding: .utf8) } - let _ = DocumentationCurator.init(in: context, bundle: bundle) XCTAssert(context.problems.isEmpty, "Expected no problems. Found: \(context.problems.map(\.diagnostic.summary))") guard let moduleNode = context.documentationCache["SourceLocations"], @@ -364,7 +461,7 @@ class DocumentationCuratorTests: XCTestCase { func testSymbolLinkResolving() throws { let (bundle, context) = try testBundleAndContext(named: "LegacyBundle_DoNotUseInNewTests") - let crawler = DocumentationCurator.init(in: context, bundle: bundle) + let crawler = DocumentationCurator(in: context, bundle: bundle) // Resolve top-level symbol in module parent do { @@ -417,7 +514,7 @@ class DocumentationCuratorTests: XCTestCase { func testLinkResolving() throws { let (sourceRoot, bundle, context) = try testBundleAndContext(named: "LegacyBundle_DoNotUseInNewTests") - var crawler = DocumentationCurator.init(in: context, bundle: bundle) + var crawler = DocumentationCurator(in: context, bundle: bundle) // Resolve and curate an article in module root (absolute link) do { @@ -510,7 +607,7 @@ class DocumentationCuratorTests: XCTestCase { """.write(to: root.appendingPathComponent("documentation").appendingPathComponent("api-collection.md"), atomically: true, encoding: .utf8) } - var crawler = DocumentationCurator.init(in: context, bundle: bundle) + var crawler = DocumentationCurator(in: context, bundle: bundle) let reference = ResolvedTopicReference(bundleID: "org.swift.docc.example", path: "/documentation/SideKit", sourceLanguage: .swift) try crawler.crawlChildren(of: reference, prepareForCuration: {_ in }) { (_, _) in } From 558118debe39d878012b985ffd76f558ff6a0a35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Tue, 5 Aug 2025 13:38:28 +0200 Subject: [PATCH 20/22] Add support for abstract Headerdoc tag (#1271) * Adding support for abstract headerdoc tag so the methods or properties with abstract headerdoc tag retains documentation rdar://147920933 * changes after merging swift-markdown changes (updating Package.resolved file) Co-authored-by: binamaniar --- Package.resolved | 4 ++-- .../SwiftDocC/Model/Rendering/RenderContentCompiler.swift | 4 ++++ Tests/SwiftDocCTests/Semantics/DoxygenTests.swift | 6 ++++-- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/Package.resolved b/Package.resolved index 21a53c4f86..03aed29581 100644 --- a/Package.resolved +++ b/Package.resolved @@ -3,7 +3,7 @@ { "identity" : "swift-argument-parser", "kind" : "remoteSourceControl", - "location" : "https://github.com/apple/swift-argument-parser", + "location" : "https://github.com/apple/swift-argument-parser.git", "state" : { "revision" : "0fbc8848e389af3bb55c182bc19ca9d5dc2f255b", "version" : "1.4.0" @@ -87,7 +87,7 @@ "location" : "https://github.com/swiftlang/swift-markdown.git", "state" : { "branch" : "release/6.2", - "revision" : "bc668ada42187e6c18eac420d66050a76d4ed764" + "revision" : "3be4e1a09cef425602f11bdb4b4de252e4badd11" } }, { diff --git a/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift b/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift index a8c03b5c26..58fabccede 100644 --- a/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift +++ b/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift @@ -359,6 +359,10 @@ struct RenderContentCompiler: MarkupVisitor { return renderableDirective.render(blockDirective, with: &self) } + mutating func visitDoxygenAbstract(_ doxygenAbstract: DoxygenAbstract) -> [any RenderContent] { + doxygenAbstract.children.flatMap { self.visit($0)} + } + mutating func visitDoxygenDiscussion(_ doxygenDiscussion: DoxygenDiscussion) -> [any RenderContent] { doxygenDiscussion.children.flatMap { self.visit($0) } } diff --git a/Tests/SwiftDocCTests/Semantics/DoxygenTests.swift b/Tests/SwiftDocCTests/Semantics/DoxygenTests.swift index 664ce47513..99b7d85950 100644 --- a/Tests/SwiftDocCTests/Semantics/DoxygenTests.swift +++ b/Tests/SwiftDocCTests/Semantics/DoxygenTests.swift @@ -19,6 +19,7 @@ class DoxygenTests: XCTestCase { func testDoxygenDiscussionAndNote() throws { let documentationLines: [SymbolGraph.LineList.Line] = """ This is an abstract. + @abstract This is description with abstract. @discussion This is a discussion linking to ``AnotherClass`` and ``AnotherClass/prop``. @@ -96,6 +97,7 @@ class DoxygenTests: XCTestCase { XCTAssertEqual(symbol.abstract?.format(), "This is an abstract.") XCTAssertEqual(symbol.discussion?.content.map { $0.format() }, [ + #"\abstract This is description with abstract."#, #"\discussion This is a discussion linking to ``doc://unit-test/documentation/ModuleName/AnotherClass`` and ``doc://unit-test/documentation/ModuleName/AnotherClass/prop``."#, #"\note This is a note linking to ``doc://unit-test/documentation/ModuleName/Class3`` and ``Class3/prop2``."# ]) @@ -108,10 +110,10 @@ class DoxygenTests: XCTestCase { XCTAssertEqual(renderNode.primaryContentSections.count, 1) let overviewSection = try XCTUnwrap(renderNode.primaryContentSections.first as? ContentRenderSection) - XCTAssertEqual(overviewSection.content.count, 3) + XCTAssertEqual(overviewSection.content.count, 4) XCTAssertEqual(overviewSection.content, [ .heading(.init(level: 2, text: "Overview", anchor: "overview")), - + .paragraph(.init(inlineContent: [.text("This is description with abstract.")])), .paragraph(.init(inlineContent: [ .text("This is a discussion linking to "), .reference( From 5d0835536ae932cff4b2d38f5c37eac91c52a2c0 Mon Sep 17 00:00:00 2001 From: kcieplak Date: Fri, 26 Sep 2025 13:34:16 -0400 Subject: [PATCH 21/22] Update dependant packages to match 6.2.1 branch (#1304) Update the dependant packages to match 6.2.1, so parent dependencies like swift-sourcekit-lsp will not have version errors. --- Package.resolved | 6 +++--- Package.swift | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Package.resolved b/Package.resolved index 03aed29581..b396f624fd 100644 --- a/Package.resolved +++ b/Package.resolved @@ -68,7 +68,7 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/swiftlang/swift-docc-symbolkit.git", "state" : { - "branch" : "release/6.2", + "branch" : "release/6.2.1", "revision" : "467084cd380d352abcd128b27927ecdc8cb5bae8" } }, @@ -77,7 +77,7 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/swiftlang/swift-lmdb.git", "state" : { - "branch" : "release/6.2", + "branch" : "release/6.2.1", "revision" : "1ad9a2d80b6fcde498c2242f509bd1be7d667ff8" } }, @@ -86,7 +86,7 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/swiftlang/swift-markdown.git", "state" : { - "branch" : "release/6.2", + "branch" : "release/6.2.1", "revision" : "3be4e1a09cef425602f11bdb4b4de252e4badd11" } }, diff --git a/Package.swift b/Package.swift index 5c5ac49c76..bed4eb49b2 100644 --- a/Package.swift +++ b/Package.swift @@ -134,10 +134,10 @@ if ProcessInfo.processInfo.environment["SWIFTCI_USE_LOCAL_DEPS"] == nil { // Building standalone, so fetch all dependencies remotely. package.dependencies += [ .package(url: "https://github.com/apple/swift-nio.git", from: "2.53.0"), - .package(url: "https://github.com/swiftlang/swift-markdown.git", branch: "release/6.2"), - .package(url: "https://github.com/swiftlang/swift-lmdb.git", branch: "release/6.2"), + .package(url: "https://github.com/swiftlang/swift-markdown.git", branch: "release/6.2.1"), + .package(url: "https://github.com/swiftlang/swift-lmdb.git", branch: "release/6.2.1"), .package(url: "https://github.com/apple/swift-argument-parser.git", from: "1.2.2"), - .package(url: "https://github.com/swiftlang/swift-docc-symbolkit.git", branch: "release/6.2"), + .package(url: "https://github.com/swiftlang/swift-docc-symbolkit.git", branch: "release/6.2.1"), .package(url: "https://github.com/apple/swift-crypto.git", from: "3.0.0"), .package(url: "https://github.com/swiftlang/swift-docc-plugin.git", from: "1.2.0"), ] From 41233e564faab74972585f948a69eecb4414b4ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Mon, 29 Sep 2025 17:46:30 +0200 Subject: [PATCH 22/22] [6.2.1] Improve handling of snippet symbol graph files (#1302) (#1307) * Improve handling of snippet symbol graph files (#1302) * Separate snippets from other symbols rdar://147926589 rdar://161164434 #1280 * Rename tests to clarify what they're verifying * Improve tests around optional snippet prefix components * Add additional test about resolving and rendering snippets * Make it easier to verify the diagnostic log output in tests * Add additional test about snippet warnings Also, update the behavior when a snippet slice is misspelled to match what's described in the warning. * Move snippet diagnostic creation to resolver type * Add near-miss suggestions for snippet paths and slices * Only highlight the misspelled portion of snippet paths * Update user-facing documentation about optional snippet paths prefixes * Clarify that Snippet argument parsing problems are reported elsewhere * Fix typo in code comment * Update docs about earliest version with an optional snippet path prefix * Fix test that became flaky after recent snippets change (#1308) Depending on which symbol was _first_ in the dictionary of symbols, the entire symbol graph was categorized as either a snippet symbol graph or a regular symbol graph. However, real symbol graph files don't mix snippets and framework symbols like that. This fixes the test to don't mix snippets with real symbols. It also stops adding module nodes inside the symbol graph, because that also doesn't match real symbol graph files. --- .../Infrastructure/DocumentationContext.swift | 5 + .../Link Resolution/PathHierarchy+Error.swift | 4 +- .../Link Resolution/SnippetResolver.swift | 156 ++++++++++ .../Symbol Graph/SymbolGraphLoader.swift | 23 +- .../SwiftDocC/Model/DocumentationNode.swift | 2 +- .../Semantics/MarkupReferenceResolver.swift | 28 +- .../Semantics/Snippets/Snippet.swift | 77 +++-- .../NonInclusiveLanguageCheckerTests.swift | 2 +- .../DocumentationContextTests.swift | 14 +- .../Infrastructure/PathHierarchyTests.swift | 25 -- .../Infrastructure/SnippetResolverTests.swift | 272 ++++++++++++++++++ .../Reference/TabNavigatorTests.swift | 14 +- .../Semantics/SnippetTests.swift | 63 ++-- .../XCTestCase+LoadingTestData.swift | 15 +- 14 files changed, 590 insertions(+), 110 deletions(-) create mode 100644 Sources/SwiftDocC/Infrastructure/Link Resolution/SnippetResolver.swift create mode 100644 Tests/SwiftDocCTests/Infrastructure/SnippetResolverTests.swift diff --git a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift index 224bd5f287..b0e25d87e0 100644 --- a/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift +++ b/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift @@ -167,6 +167,9 @@ public class DocumentationContext { /// > Important: The topic graph has no awareness of source language specific edges. var topicGraph = TopicGraph() + /// Will be assigned during context initialization + var snippetResolver: SnippetResolver! + /// User-provided global options for this documentation conversion. var options: Options? @@ -2235,6 +2238,8 @@ public class DocumentationContext { knownDisambiguatedPathComponents: configuration.convertServiceConfiguration.knownDisambiguatedSymbolPathComponents )) } + + self.snippetResolver = SnippetResolver(symbolGraphLoader: symbolGraphLoader) } catch { // Pipe the error out of the dispatch queue. discoveryError.sync({ diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Error.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Error.swift index 67851a2126..e02a6f4c00 100644 --- a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Error.swift +++ b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Error.swift @@ -1,7 +1,7 @@ /* This source file is part of the Swift.org open source project - Copyright (c) 2023-2024 Apple Inc. and the Swift project authors + Copyright (c) 2023-2025 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 @@ -285,7 +285,7 @@ private extension PathHierarchy.Node { } } -private extension SourceRange { +extension SourceRange { static func makeRelativeRange(startColumn: Int, endColumn: Int) -> SourceRange { return SourceLocation(line: 0, column: startColumn, source: nil) ..< SourceLocation(line: 0, column: endColumn, source: nil) } diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/SnippetResolver.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/SnippetResolver.swift new file mode 100644 index 0000000000..4c98f7354d --- /dev/null +++ b/Sources/SwiftDocC/Infrastructure/Link Resolution/SnippetResolver.swift @@ -0,0 +1,156 @@ +/* + This source file is part of the Swift.org open source project + + Copyright (c) 2025 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 SymbolKit +import Markdown + +/// A type that resolves snippet paths. +final class SnippetResolver { + typealias SnippetMixin = SymbolKit.SymbolGraph.Symbol.Snippet + typealias Explanation = Markdown.Document + + /// Information about a resolved snippet + struct ResolvedSnippet { + fileprivate var path: String // For use in diagnostics + var mixin: SnippetMixin + var explanation: Explanation? + } + /// A snippet that has been resolved, either successfully or not. + enum SnippetResolutionResult { + case success(ResolvedSnippet) + case failure(TopicReferenceResolutionErrorInfo) + } + + private var snippets: [String: ResolvedSnippet] = [:] + + init(symbolGraphLoader: SymbolGraphLoader) { + var snippets: [String: ResolvedSnippet] = [:] + + for graph in symbolGraphLoader.snippetSymbolGraphs.values { + for symbol in graph.symbols.values { + guard let snippetMixin = symbol[mixin: SnippetMixin.self] else { continue } + + let path: String = if symbol.pathComponents.first == "Snippets" { + symbol.pathComponents.dropFirst().joined(separator: "/") + } else { + symbol.pathComponents.joined(separator: "/") + } + + snippets[path] = .init(path: path, mixin: snippetMixin, explanation: symbol.docComment.map { + Document(parsing: $0.lines.map(\.text).joined(separator: "\n"), options: .parseBlockDirectives) + }) + } + } + + self.snippets = snippets + } + + func resolveSnippet(path authoredPath: String) -> SnippetResolutionResult { + // Snippet paths are relative to the root of the Swift Package. + // The first two components are always the same (the package name followed by "Snippets"). + // The later components can either be subdirectories of the "Snippets" directory or the base name of a snippet '.swift' file (without the extension). + + // Drop the common package name + "Snippets" prefix (that's always the same), if the authored path includes it. + // This enables the author to omit this prefix (but include it for backwards compatibility with older DocC versions). + var components = authoredPath.split(separator: "/", omittingEmptySubsequences: true) + + // It's possible that the package name is "Snippets", resulting in two identical components. Skip until the last of those two. + if let snippetsPrefixIndex = components.prefix(2).lastIndex(of: "Snippets"), + // Don't search for an empty string if the snippet happens to be named "Snippets" + let relativePathStart = components.index(snippetsPrefixIndex, offsetBy: 1, limitedBy: components.endIndex - 1) + { + components.removeFirst(relativePathStart) + } + + let path = components.joined(separator: "/") + if let found = snippets[path] { + return .success(found) + } else { + let replacementRange = SourceRange.makeRelativeRange(startColumn: authoredPath.utf8.count - path.utf8.count, length: path.utf8.count) + + let nearMisses = NearMiss.bestMatches(for: snippets.keys, against: path) + let solutions = nearMisses.map { candidate in + Solution(summary: "\(Self.replacementOperationDescription(from: path, to: candidate))", replacements: [ + Replacement(range: replacementRange, replacement: candidate) + ]) + } + + return .failure(.init("Snippet named '\(path)' couldn't be found", solutions: solutions, rangeAdjustment: replacementRange)) + } + } + + func validate(slice: String, for resolvedSnippet: ResolvedSnippet) -> TopicReferenceResolutionErrorInfo? { + guard resolvedSnippet.mixin.slices[slice] == nil else { + return nil + } + let replacementRange = SourceRange.makeRelativeRange(startColumn: 0, length: slice.utf8.count) + + let nearMisses = NearMiss.bestMatches(for: resolvedSnippet.mixin.slices.keys, against: slice) + let solutions = nearMisses.map { candidate in + Solution(summary: "\(Self.replacementOperationDescription(from: slice, to: candidate))", replacements: [ + Replacement(range: replacementRange, replacement: candidate) + ]) + } + + return .init("Slice named '\(slice)' doesn't exist in snippet '\(resolvedSnippet.path)'", solutions: solutions) + } +} + +// MARK: Diagnostics + +extension SnippetResolver { + static func unknownSnippetSliceProblem(source: URL?, range: SourceRange?, errorInfo: TopicReferenceResolutionErrorInfo) -> Problem { + _problem(source: source, range: range, errorInfo: errorInfo, id: "org.swift.docc.unknownSnippetPath") + } + + static func unresolvedSnippetPathProblem(source: URL?, range: SourceRange?, errorInfo: TopicReferenceResolutionErrorInfo) -> Problem { + _problem(source: source, range: range, errorInfo: errorInfo, id: "org.swift.docc.unresolvedSnippetPath") + } + + private static func _problem(source: URL?, range: SourceRange?, errorInfo: TopicReferenceResolutionErrorInfo, id: String) -> Problem { + var solutions: [Solution] = [] + var notes: [DiagnosticNote] = [] + if let range { + if let note = errorInfo.note, let source { + notes.append(DiagnosticNote(source: source, range: range, message: note)) + } + + solutions.append(contentsOf: errorInfo.solutions(referenceSourceRange: range)) + } + + let diagnosticRange: SourceRange? + if var rangeAdjustment = errorInfo.rangeAdjustment, let range { + rangeAdjustment.offsetWithRange(range) + assert(rangeAdjustment.lowerBound.column >= 0, """ + Unresolved snippet reference range adjustment created range with negative column. + Source: \(source?.absoluteString ?? "nil") + Range: \(rangeAdjustment.lowerBound.description):\(rangeAdjustment.upperBound.description) + Summary: \(errorInfo.message) + """) + diagnosticRange = rangeAdjustment + } else { + diagnosticRange = range + } + + let diagnostic = Diagnostic(source: source, severity: .warning, range: diagnosticRange, identifier: id, summary: errorInfo.message, notes: notes) + return Problem(diagnostic: diagnostic, possibleSolutions: solutions) + } + + private static func replacementOperationDescription(from: some StringProtocol, to: some StringProtocol) -> String { + if from.isEmpty { + return "Insert \(to.singleQuoted)" + } + if to.isEmpty { + return "Remove \(from.singleQuoted)" + } + return "Replace \(from.singleQuoted) with \(to.singleQuoted)" + } +} diff --git a/Sources/SwiftDocC/Infrastructure/Symbol Graph/SymbolGraphLoader.swift b/Sources/SwiftDocC/Infrastructure/Symbol Graph/SymbolGraphLoader.swift index 50baa2529c..b422c614e6 100644 --- a/Sources/SwiftDocC/Infrastructure/Symbol Graph/SymbolGraphLoader.swift +++ b/Sources/SwiftDocC/Infrastructure/Symbol Graph/SymbolGraphLoader.swift @@ -17,6 +17,7 @@ import SymbolKit /// which makes detecting symbol collisions and overloads easier. struct SymbolGraphLoader { private(set) var symbolGraphs: [URL: SymbolKit.SymbolGraph] = [:] + private(set) var snippetSymbolGraphs: [URL: SymbolKit.SymbolGraph] = [:] private(set) var unifiedGraphs: [String: SymbolKit.UnifiedSymbolGraph] = [:] private(set) var graphLocations: [String: [SymbolKit.GraphCollector.GraphKind]] = [:] // FIXME: After 6.2, when we no longer have `DocumentationContextDataProvider` we can simply this code to not use a closure to read data. @@ -58,7 +59,7 @@ struct SymbolGraphLoader { let loadingLock = Lock() - var loadedGraphs = [URL: (usesExtensionSymbolFormat: Bool?, graph: SymbolKit.SymbolGraph)]() + var loadedGraphs = [URL: (usesExtensionSymbolFormat: Bool?, isSnippetGraph: Bool, graph: SymbolKit.SymbolGraph)]() var loadError: (any Error)? let loadGraphAtURL: (URL) -> Void = { [dataLoader, bundle] symbolGraphURL in @@ -99,9 +100,13 @@ struct SymbolGraphLoader { usesExtensionSymbolFormat = symbolGraph.symbols.isEmpty ? nil : containsExtensionSymbols } + // If the graph doesn't have any symbols we treat it as a regular, but empty, graph. + // v + let isSnippetGraph = symbolGraph.symbols.values.first?.kind.identifier.isSnippetKind == true + // Store the decoded graph in `loadedGraphs` loadingLock.sync { - loadedGraphs[symbolGraphURL] = (usesExtensionSymbolFormat, symbolGraph) + loadedGraphs[symbolGraphURL] = (usesExtensionSymbolFormat, isSnippetGraph, symbolGraph) } } catch { // If the symbol graph was invalid, store the error @@ -141,8 +146,9 @@ struct SymbolGraphLoader { let mergeSignpostHandle = signposter.beginInterval("Build unified symbol graph", id: signposter.makeSignpostID()) let graphLoader = GraphCollector(extensionGraphAssociationStrategy: usingExtensionSymbolFormat ? .extendingGraph : .extendedGraph) - // feed the loaded graphs into the `graphLoader` - for (url, (_, graph)) in loadedGraphs { + + // feed the loaded non-snippet graphs into the `graphLoader` + for (url, (_, isSnippets, graph)) in loadedGraphs where !isSnippets { graphLoader.mergeSymbolGraph(graph, at: url) } @@ -152,7 +158,8 @@ struct SymbolGraphLoader { throw loadError } - self.symbolGraphs = loadedGraphs.mapValues(\.graph) + self.symbolGraphs = loadedGraphs.compactMapValues({ _, isSnippets, graph in isSnippets ? nil : graph }) + self.snippetSymbolGraphs = loadedGraphs.compactMapValues({ _, isSnippets, graph in isSnippets ? graph : nil }) (self.unifiedGraphs, self.graphLocations) = graphLoader.finishLoading( createOverloadGroups: FeatureFlags.current.isExperimentalOverloadedSymbolPresentationEnabled ) @@ -546,3 +553,9 @@ private extension SymbolGraph.Symbol.Availability.AvailabilityItem { domain?.rawValue.lowercased() == platform.rawValue.lowercased() } } + +extension SymbolGraph.Symbol.KindIdentifier { + var isSnippetKind: Bool { + self == .snippet || self == .snippetGroup + } +} diff --git a/Sources/SwiftDocC/Model/DocumentationNode.swift b/Sources/SwiftDocC/Model/DocumentationNode.swift index a9e3133225..0035be6c3d 100644 --- a/Sources/SwiftDocC/Model/DocumentationNode.swift +++ b/Sources/SwiftDocC/Model/DocumentationNode.swift @@ -853,7 +853,7 @@ private extension BlockDirective { } } -extension [String] { +extension Collection { /// Strips the minimum leading whitespace from all the strings in the array. /// diff --git a/Sources/SwiftDocC/Semantics/MarkupReferenceResolver.swift b/Sources/SwiftDocC/Semantics/MarkupReferenceResolver.swift index 8f34a30d46..00b57e5899 100644 --- a/Sources/SwiftDocC/Semantics/MarkupReferenceResolver.swift +++ b/Sources/SwiftDocC/Semantics/MarkupReferenceResolver.swift @@ -21,11 +21,6 @@ private func disabledLinkDestinationProblem(reference: ResolvedTopicReference, r return Problem(diagnostic: Diagnostic(source: range?.source, severity: severity, range: range, identifier: "org.swift.docc.disabledLinkDestination", summary: "The topic \(reference.path.singleQuoted) cannot be linked to."), possibleSolutions: []) } -private func unknownSnippetSliceProblem(snippetPath: String, slice: String, range: SourceRange?) -> Problem { - let diagnostic = Diagnostic(source: range?.source, severity: .warning, range: range, identifier: "org.swift.docc.unknownSnippetSlice", summary: "Snippet slice \(slice.singleQuoted) does not exist in snippet \(snippetPath.singleQuoted); this directive will be ignored") - return Problem(diagnostic: diagnostic, possibleSolutions: []) -} - private func removedLinkDestinationProblem(reference: ResolvedTopicReference, range: SourceRange?, severity: DiagnosticSeverity) -> Problem { var solutions = [Solution]() if let range, reference.pathComponents.count > 3 { @@ -171,24 +166,21 @@ struct MarkupReferenceResolver: MarkupRewriter { let source = blockDirective.range?.source switch blockDirective.name { case Snippet.directiveName: - var problems = [Problem]() - guard let snippet = Snippet(from: blockDirective, source: source, for: bundle, problems: &problems) else { + var ignoredParsingProblems = [Problem]() // Any argument parsing problems have already been reported elsewhere + guard let snippet = Snippet(from: blockDirective, source: source, for: bundle, problems: &ignoredParsingProblems) else { return blockDirective } - if let resolved = resolveAbsoluteSymbolLink(unresolvedDestination: snippet.path, elementRange: blockDirective.range) { - var argumentText = "path: \"\(resolved.absoluteString)\"" + switch context.snippetResolver.resolveSnippet(path: snippet.path) { + case .success(let resolvedSnippet): if let requestedSlice = snippet.slice, - let snippetMixin = try? context.entity(with: resolved).symbol? - .mixins[SymbolGraph.Symbol.Snippet.mixinKey] as? SymbolGraph.Symbol.Snippet { - guard snippetMixin.slices[requestedSlice] != nil else { - problems.append(unknownSnippetSliceProblem(snippetPath: snippet.path, slice: requestedSlice, range: blockDirective.nameRange)) - return blockDirective - } - argumentText.append(", slice: \"\(requestedSlice)\"") + let errorInfo = context.snippetResolver.validate(slice: requestedSlice, for: resolvedSnippet) + { + problems.append(SnippetResolver.unknownSnippetSliceProblem(source: source, range: blockDirective.arguments()["slice"]?.valueRange, errorInfo: errorInfo)) } - return BlockDirective(name: Snippet.directiveName, argumentText: argumentText, children: []) - } else { + return blockDirective + case .failure(let errorInfo): + problems.append(SnippetResolver.unresolvedSnippetPathProblem(source: source, range: blockDirective.arguments()["path"]?.valueRange, errorInfo: errorInfo)) return blockDirective } case ImageMedia.directiveName: diff --git a/Sources/SwiftDocC/Semantics/Snippets/Snippet.swift b/Sources/SwiftDocC/Semantics/Snippets/Snippet.swift index a2188a3c82..6984ff5a62 100644 --- a/Sources/SwiftDocC/Semantics/Snippets/Snippet.swift +++ b/Sources/SwiftDocC/Semantics/Snippets/Snippet.swift @@ -12,16 +12,42 @@ import Foundation public import Markdown import SymbolKit +/// Embeds a code example from the project's code snippets. +/// +/// Use a `Snippet` directive to embed a code example from the project's "Snippets" directory on the page. +/// The `path` argument is the relative path from the package's top-level "Snippets" directory to your snippet file without the `.swift` extension. +/// +/// ```markdown +/// @Snippet(path: "example-snippet", slice: "setup") +/// ``` +/// +/// If you prefer, you can specify the relative path from the package's _root_ directory (by including a "Snippets/" prefix). +/// You can also include the package name---as defined in `Package.swift`---before the "Snippets/" prefix. +/// Neither of these leading path components are necessary because all your snippet code files are always located in your package's "Snippets" directory. +/// +/// > Earlier Versions: +/// > Before Swift-DocC 6.2.1, the `@Snippet` path needed to include both the package name component and the "Snippets" component: +/// > +/// > ```markdown +/// > @Snippet(path: "my-package/Snippets/example-snippet") +/// > ``` +/// +/// You can define named slices of your snippet by annotating the snippet file with `// snippet.` and `// snippet.end` lines. +/// A named slice automatically ends at the start of the next named slice, without an explicit `snippet.end` annotation. +/// +/// If the referenced snippet includes annotated slices, you can limit the embedded code example to a certain line range by specifying a `slice` name. +/// By default, the embedded code example includes the full snippet. For more information, see . public final class Snippet: Semantic, AutomaticDirectiveConvertible { public static let introducedVersion = "5.6" public let originalMarkup: BlockDirective - /// The path components of a symbol link that would be used to resolve a reference to a snippet, - /// only occurring as a block directive argument. + /// The relative path from your package's top-level "Snippets" directory to the snippet file that you want to embed in the page, without the `.swift` file extension. @DirectiveArgumentWrapped public var path: String - /// An optional named range to limit the lines shown. + /// The name of a snippet slice to limit the embedded code example to a certain line range. + /// + /// By default, the embedded code example includes the full snippet. @DirectiveArgumentWrapped public var slice: String? = nil @@ -50,30 +76,33 @@ public final class Snippet: Semantic, AutomaticDirectiveConvertible { extension Snippet: RenderableDirectiveConvertible { func render(with contentCompiler: inout RenderContentCompiler) -> [any RenderContent] { - guard let snippet = Snippet(from: originalMarkup, for: contentCompiler.bundle) else { + guard case .success(let resolvedSnippet) = contentCompiler.context.snippetResolver.resolveSnippet(path: path) else { + return [] + } + let mixin = resolvedSnippet.mixin + + if let slice { + guard let sliceRange = mixin.slices[slice] else { + // The warning says that unrecognized snippet slices will ignore the entire snippet. return [] } + // Render only this slice without the explanatory content. + let lines = mixin.lines + // Trim the lines + .dropFirst(sliceRange.startIndex).prefix(sliceRange.endIndex - sliceRange.startIndex) + // Trim the whitespace + .linesWithoutLeadingWhitespace() + // Make dedicated copies of each line because the RenderBlockContent.codeListing requires it. + .map { String($0) } - guard let snippetReference = contentCompiler.resolveSymbolReference(destination: snippet.path), - let snippetEntity = try? contentCompiler.context.entity(with: snippetReference), - let snippetSymbol = snippetEntity.symbol, - let snippetMixin = snippetSymbol.mixins[SymbolGraph.Symbol.Snippet.mixinKey] as? SymbolGraph.Symbol.Snippet else { - return [] - } + return [RenderBlockContent.codeListing(.init(syntax: mixin.language, code: lines, metadata: nil))] + } else { + // Render the full snippet and its explanatory content. + let fullCode = RenderBlockContent.codeListing(.init(syntax: mixin.language, code: mixin.lines, metadata: nil)) - if let requestedSlice = snippet.slice, - let requestedLineRange = snippetMixin.slices[requestedSlice] { - // Render only the slice. - let lineRange = requestedLineRange.lowerBound.. ModuleName.md:7:\(16 + prefixLength)-7:\(20 + prefixLength) + 5 | ## Overview + 6 | + 7 + @Snippet(path: \(pathPrefix)\u{001B}[1;32mFrst\u{001B}[0;0m) + | \(String(repeating: " ", count: prefixLength)) ╰─\u{001B}[1;39msuggestion: Replace 'Frst' with 'First'\u{001B}[0;0m + 8 | + 9 | @Snippet(path: \(pathPrefix)First, slice: commt) + + \u{001B}[1;33mwarning: Slice named 'commt' doesn't exist in snippet 'First'\u{001B}[0;0m + --> ModuleName.md:9:\(30 + prefixLength)-9:\(35 + prefixLength) + 7 | @Snippet(path: \(pathPrefix)Frst) + 8 | + 9 + @Snippet(path: \(pathPrefix)First, slice: \u{001B}[1;32mcommt\u{001B}[0;0m) + | \(String(repeating: " ", count: prefixLength)) ╰─\u{001B}[1;39msuggestion: Replace 'commt' with 'comment'\u{001B}[0;0m + + """) + + // Because the snippet links failed to resolve, their content shouldn't render on the page. + XCTAssertTrue(snippetRenderBlocks.isEmpty, "There's no more content after the snippets") + } + } + + private func makeSnippetContext( + snippets: [SymbolGraph.Symbol], + rootContent: String, + file: StaticString = #filePath, + line: UInt = #line + ) throws -> ([Problem], logOutput: String, some Collection) { + let catalog = Folder(name: "Something.docc", content: [ + JSONFile(name: "something-snippets.symbols.json", content: makeSymbolGraph(moduleName: "Snippets", symbols: snippets)), + // Include a "real" module that's separate from the snippet symbol graph. + JSONFile(name: "ModuleName.symbols.json", content: makeSymbolGraph(moduleName: "ModuleName")), + + TextFile(name: "ModuleName.md", utf8Content: """ + # ``ModuleName`` + + Always include an abstract here before the custom markup + + ## Overview + + \(rootContent) + """) + ]) + // We make the "Overview" heading explicit above so that the rendered page will always have a `primaryContentSections`. + // This makes it easier for the test to then + + let logStore = LogHandle.LogStorage() + let (bundle, context) = try loadBundle(catalog: catalog, logOutput: LogHandle.memory(logStore)) + + XCTAssertEqual(context.knownIdentifiers.count, 1, "The snippets don't have their own identifiers", file: file, line: line) + + let reference = try XCTUnwrap(context.soleRootModuleReference, file: file, line: line) + let moduleNode = try context.entity(with: reference) + let renderNode = DocumentationNodeConverter(bundle: bundle, context: context).convert(moduleNode) + + let renderBlocks = try XCTUnwrap(renderNode.primaryContentSections.first as? ContentRenderSection, file: file, line: line).content + + if case .heading(let heading) = renderBlocks.first { + XCTAssertEqual(heading.level, 2, file: file, line: line) + XCTAssertEqual(heading.text, "Overview", file: file, line: line) + } else { + XCTFail("The rendered page is missing the 'Overview' heading. Something unexpected is happening with the page content.", file: file, line: line) + } + + return (context.problems.sorted(by: \.diagnostic.range!.lowerBound.line), logStore.text, renderBlocks.dropFirst()) + } + + private func makeSnippet( + pathComponents: [String], + explanation: String?, + code: String, + slices: [String: Range] = [:] + ) -> SymbolGraph.Symbol { + makeSymbol( + id: "$snippet__module-name.\(pathComponents.map { $0.lowercased() }.joined(separator: "."))", + kind: .snippet, + pathComponents: pathComponents, + docComment: explanation, + otherMixins: [ + SymbolGraph.Symbol.Snippet( + language: SourceLanguage.swift.id, + lines: code.components(separatedBy: "\n"), + slices: slices + ) + ] + ) + } +} diff --git a/Tests/SwiftDocCTests/Semantics/Reference/TabNavigatorTests.swift b/Tests/SwiftDocCTests/Semantics/Reference/TabNavigatorTests.swift index 73b8e928bd..3df578b9ed 100644 --- a/Tests/SwiftDocCTests/Semantics/Reference/TabNavigatorTests.swift +++ b/Tests/SwiftDocCTests/Semantics/Reference/TabNavigatorTests.swift @@ -160,15 +160,11 @@ class TabNavigatorTests: XCTestCase { XCTAssertNotNil(tabNavigator) - // UnresolvedTopicReference warning expected since the reference to the snippet "Snippets/Snippets/MySnippet" - // should fail to resolve here and then nothing would be added to the content. - XCTAssertEqual( - problems, - ["23: warning – org.swift.docc.unresolvedTopicReference"] - ) + // One warning is expected. This empty context has no snippets so the "Snippets/Snippets/MySnippet" path should fail to resolve. + XCTAssertEqual(problems, [ + "23: warning – org.swift.docc.unresolvedSnippetPath" + ]) - - XCTAssertEqual(renderBlockContent.count, 1) XCTAssertEqual( renderBlockContent.first, @@ -202,6 +198,8 @@ class TabNavigatorTests: XCTestCase { "Hey there.", .small(RenderBlockContent.Small(inlineContent: [.text("Hey but small.")])), + + // Because the the "Snippets/Snippets/MySnippet" snippet failed to resolve, we're not including any snippet content here. ] ), ] diff --git a/Tests/SwiftDocCTests/Semantics/SnippetTests.swift b/Tests/SwiftDocCTests/Semantics/SnippetTests.swift index cc5750d3fd..af5f678d47 100644 --- a/Tests/SwiftDocCTests/Semantics/SnippetTests.swift +++ b/Tests/SwiftDocCTests/Semantics/SnippetTests.swift @@ -15,8 +15,8 @@ import XCTest import Markdown class SnippetTests: XCTestCase { - func testNoPath() throws { - let (bundle, _) = try testBundleAndContext(named: "Snippets") + func testWarningAboutMissingPathPath() throws { + let (bundle, _) = try testBundleAndContext() let source = """ @Snippet() """ @@ -29,8 +29,8 @@ class SnippetTests: XCTestCase { XCTAssertEqual("org.swift.docc.HasArgument.path", problems[0].diagnostic.identifier) } - func testHasInnerContent() throws { - let (bundle, _) = try testBundleAndContext(named: "Snippets") + func testWarningAboutInnerContent() throws { + let (bundle, _) = try testBundleAndContext() let source = """ @Snippet(path: "path/to/snippet") { This content shouldn't be here. @@ -45,8 +45,8 @@ class SnippetTests: XCTestCase { XCTAssertEqual("org.swift.docc.Snippet.NoInnerContentAllowed", problems[0].diagnostic.identifier) } - func testLinkResolves() throws { - let (bundle, _) = try testBundleAndContext(named: "Snippets") + func testParsesPath() throws { + let (bundle, _) = try testBundleAndContext() let source = """ @Snippet(path: "Test/Snippets/MySnippet") """ @@ -58,23 +58,50 @@ class SnippetTests: XCTestCase { XCTAssertNotNil(snippet) XCTAssertTrue(problems.isEmpty) } + func testLinkResolvesWithoutOptionalPrefix() throws { + let (bundle, context) = try testBundleAndContext(named: "Snippets") + + for snippetPath in [ + "/Test/Snippets/MySnippet", + "Test/Snippets/MySnippet", + "Snippets/MySnippet", + "MySnippet", + ] { + let source = """ + @Snippet(path: "\(snippetPath)") + """ + let document = Document(parsing: source, options: .parseBlockDirectives) + var resolver = MarkupReferenceResolver(context: context, bundle: bundle, rootReference: try XCTUnwrap(context.soleRootModuleReference)) + _ = resolver.visit(document) + XCTAssertTrue(resolver.problems.isEmpty, "Unexpected problems: \(resolver.problems.map(\.diagnostic.summary))") + } + } - func testUnresolvedSnippetPathDiagnostic() throws { + func testWarningAboutUnresolvedSnippetPath() throws { let (bundle, context) = try testBundleAndContext(named: "Snippets") - let source = """ - @Snippet(path: "Test/Snippets/DoesntExist") - """ - let document = Document(parsing: source, options: .parseBlockDirectives) - var resolver = MarkupReferenceResolver(context: context, bundle: bundle, rootReference: context.rootModules[0]) - _ = resolver.visit(document) - XCTAssertEqual(1, resolver.problems.count) - resolver.problems.first.map { - XCTAssertEqual("org.swift.docc.unresolvedTopicReference", $0.diagnostic.identifier) + + for snippetPath in [ + "/Test/Snippets/DoesNotExist", + "Test/Snippets/DoesNotExist", + "Snippets/DoesNotExist", + "DoesNotExist", + ] { + let source = """ + @Snippet(path: "\(snippetPath)") + """ + let document = Document(parsing: source, options: .parseBlockDirectives) + var resolver = MarkupReferenceResolver(context: context, bundle: bundle, rootReference: try XCTUnwrap(context.soleRootModuleReference)) + _ = resolver.visit(document) + XCTAssertEqual(1, resolver.problems.count) + let problem = try XCTUnwrap(resolver.problems.first) + XCTAssertEqual(problem.diagnostic.identifier, "org.swift.docc.unresolvedSnippetPath") + XCTAssertEqual(problem.diagnostic.summary, "Snippet named 'DoesNotExist' couldn't be found") + XCTAssertEqual(problem.possibleSolutions.count, 0) } } - func testSliceResolves() throws { - let (bundle, _) = try testBundleAndContext(named: "Snippets") + func testParsesSlice() throws { + let (bundle, _) = try testBundleAndContext() let source = """ @Snippet(path: "Test/Snippets/MySnippet", slice: "foo") """ diff --git a/Tests/SwiftDocCTests/XCTestCase+LoadingTestData.swift b/Tests/SwiftDocCTests/XCTestCase+LoadingTestData.swift index 6937a6c6e5..a59afc8e89 100644 --- a/Tests/SwiftDocCTests/XCTestCase+LoadingTestData.swift +++ b/Tests/SwiftDocCTests/XCTestCase+LoadingTestData.swift @@ -43,21 +43,30 @@ extension XCTestCase { /// - Parameters: /// - catalog: The directory structure of the documentation catalog /// - otherFileSystemDirectories: Any other directories in the test file system. - /// - diagnosticEngine: The diagnostic engine for the created context. + /// - diagnosticFilterLevel: The minimum severity for diagnostics to emit. + /// - logOutput: An output stream to capture log output from creating the context. /// - configuration: Configuration for the created context. /// - Returns: The loaded documentation bundle and context for the given catalog input. func loadBundle( catalog: Folder, otherFileSystemDirectories: [Folder] = [], - diagnosticEngine: DiagnosticEngine = .init(), + diagnosticFilterLevel: DiagnosticSeverity = .warning, + logOutput: some TextOutputStream = LogHandle.none, configuration: DocumentationContext.Configuration = .init() ) throws -> (DocumentationBundle, DocumentationContext) { let fileSystem = try TestFileSystem(folders: [catalog] + otherFileSystemDirectories) + let catalogURL = URL(fileURLWithPath: "/\(catalog.name)") + + let diagnosticEngine = DiagnosticEngine(filterLevel: diagnosticFilterLevel) + diagnosticEngine.add(DiagnosticConsoleWriter(logOutput, formattingOptions: [], baseURL: catalogURL, highlight: true, dataProvider: fileSystem)) let (bundle, dataProvider) = try DocumentationContext.InputsProvider(fileManager: fileSystem) - .inputsAndDataProvider(startingPoint: URL(fileURLWithPath: "/\(catalog.name)"), options: .init()) + .inputsAndDataProvider(startingPoint: catalogURL, options: .init()) let context = try DocumentationContext(bundle: bundle, dataProvider: dataProvider, diagnosticEngine: diagnosticEngine, configuration: configuration) + + diagnosticEngine.flush() // Write to the logOutput + return (bundle, context) }