From ea39ee848528d54ab2fe464725397207d2372020 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Fri, 7 Nov 2025 11:30:23 +0100 Subject: [PATCH 1/2] Improve parsing of links with type disambiguation that include generics rdar://160232871 --- .../PathHierarchy+TypeSignature.swift | 41 +++++++-- .../Infrastructure/PathHierarchyTests.swift | 85 +++++++++++++++++++ 2 files changed, 118 insertions(+), 8 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+TypeSignature.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+TypeSignature.swift index 06d33cdf38..41bb59cac4 100644 --- a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+TypeSignature.swift +++ b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+TypeSignature.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 @@ -555,6 +555,15 @@ private struct StringScanner: ~Copyable { return remaining[.. Bool) -> Substring? { + guard let beforeIndex = remaining.firstIndex(where: predicate) else { + return nil + } + let index = remaining.index(after: beforeIndex) + defer { remaining = remaining[index...] } + return remaining[.. 0 { - if $0 == ")" { - depth -= 1 - } + else if $0 == ")" { + depth -= 1 + return depth == 0 // stop only if we've reached a balanced number of parenthesis + } + return false // keep scanning + } + + return scan(past: predicate) + } + + mutating func scanValue() -> Substring? { + // The value may contain any number of nested generics. Keep track of the open and close angle brackets while scanning. + var depth = 0 + let predicate: (Character) -> Bool = { + if $0 == "<" { + depth += 1 + return false // keep scanning + } + else if $0 == ">" { + depth -= 1 return false // keep scanning } - return $0 == "," || $0 == ")" + return depth == 0 && ($0 == "," || $0 == ")") } return scan(until: predicate) } diff --git a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift index c7c54f5495..57627d5086 100644 --- a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift @@ -2037,6 +2037,71 @@ class PathHierarchyTests: XCTestCase { try assertFindsPath("doSomething()-9kd0v", in: tree, asSymbolID: "some-function-id-AnyObject") } + func testParameterDisambiguationWithKeyPathType() async throws { + // Create two overloads with different key path parameter types + let parameterTypes: [SymbolGraph.Symbol.DeclarationFragments.Fragment] = [ + // Swift.Int + .init(kind: .typeIdentifier, spelling: "Int", preciseIdentifier: "s:Si"), + // Swift.Bool + .init(kind: .typeIdentifier, spelling: "Bool", preciseIdentifier: "s:Sb"), + ] + + 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)-KeyPath", kind: .func, pathComponents: ["doSomething(keyPath:)"], signature: .init( + parameters: [ + // "keyPath: KeyPath" or "keyPath: KeyPath" + .init(name: "keyPath", externalName: nil, declarationFragments: [ + .init(kind: .identifier, spelling: "keyPath", preciseIdentifier: nil), + .init(kind: .text, spelling: ": ", preciseIdentifier: nil), + .init(kind: .typeIdentifier, spelling: "KeyPath", preciseIdentifier: "s:s7KeyPathC"), + .init(kind: .text, spelling: "<", preciseIdentifier: nil), + .init(kind: .typeIdentifier, spelling: "String", preciseIdentifier: "s:SS"), + .init(kind: .text, spelling: ", ", preciseIdentifier: nil), + parameterTypeFragment, + .init(kind: .text, spelling: ">", preciseIdentifier: nil) + ], children: []) + ], + returns: [ + .init(kind: .text, spelling: "()", preciseIdentifier: nil) // 'Void' in text representation + ] + )) + })), + ]) + let (_, context) = try await 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-Int-KeyPath"], "/ModuleName/doSomething(keyPath:)-(KeyPath)") + XCTAssertEqual(paths["some-function-id-Bool-KeyPath"], "/ModuleName/doSomething(keyPath:)-(KeyPath)") + + try assertPathCollision("doSomething(keyPath:)", in: tree, collisions: [ + ("some-function-id-Int-KeyPath", "-(KeyPath)"), + ("some-function-id-Bool-KeyPath", "-(KeyPath)"), + ]) + + try assertPathRaisesErrorMessage("doSomething(keyPath:)", in: tree, context: context, expectedErrorMessage: "'doSomething(keyPath:)' 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 '-(KeyPath)' for \n'doSomething(keyPath:)'" , replacements: [("-(KeyPath)", 21, 21)])) + XCTAssertEqual(error.solutions.dropFirst(1).first, .init(summary: "Insert '-(KeyPath)' for \n'doSomething(keyPath:)'" /* the test symbols don't have full declarations */, replacements: [("-(KeyPath)", 21, 21)])) + } + + assertParsedPathComponents("doSomething(keyPath:)-(KeyPath)", [("doSomething(keyPath:)", .typeSignature(parameterTypes: ["KeyPath"], returnTypes: nil))]) + try assertFindsPath("doSomething(keyPath:)-(KeyPath)", in: tree, asSymbolID: "some-function-id-Int-KeyPath") + try assertFindsPath("doSomething(keyPath:)-(KeyPath)->()", in: tree, asSymbolID: "some-function-id-Int-KeyPath") + try assertFindsPath("doSomething(keyPath:)-2zg7h", in: tree, asSymbolID: "some-function-id-Int-KeyPath") + + assertParsedPathComponents("doSomething(keyPath:)-(KeyPath)", [("doSomething(keyPath:)", .typeSignature(parameterTypes: ["KeyPath"], returnTypes: nil))]) + try assertFindsPath("doSomething(keyPath:)-(KeyPath)", in: tree, asSymbolID: "some-function-id-Bool-KeyPath") + try assertFindsPath("doSomething(keyPath:)-(KeyPath)->()", in: tree, asSymbolID: "some-function-id-Bool-KeyPath") + try assertFindsPath("doSomething(keyPath:)-2frrn", in: tree, asSymbolID: "some-function-id-Bool-KeyPath") + } + func testOverloadGroupSymbolsResolveLinksWithoutHash() async throws { enableFeatureFlag(\.isExperimentalOverloadedSymbolPresentationEnabled) @@ -4404,6 +4469,26 @@ class PathHierarchyTests: XCTestCase { } assertParsedPathComponents("operator[]-(std::string&)->std::string&", [("operator[]", .typeSignature(parameterTypes: ["std::string&"], returnTypes: ["std::string&"]))]) + + // Nested generic types + assertParsedPathComponents("functionName-(KeyPath)", [("functionName", .typeSignature(parameterTypes: ["KeyPath"], returnTypes: nil))]) + assertParsedPathComponents("functionName->KeyPath", [("functionName", .typeSignature(parameterTypes: nil, returnTypes: ["KeyPath"]))]) + + assertParsedPathComponents("functionName-(KeyPath,Dictionary)", [("functionName", .typeSignature(parameterTypes: ["KeyPath", "Dictionary"], returnTypes: nil))]) + assertParsedPathComponents("functionName->(KeyPath,Dictionary)", [("functionName", .typeSignature(parameterTypes: nil, returnTypes: ["KeyPath", "Dictionary"]))]) + + assertParsedPathComponents("functionName-(KeyPath>)", [("functionName", .typeSignature(parameterTypes: ["KeyPath>"], returnTypes: nil))]) + assertParsedPathComponents("functionName->KeyPath>", [("functionName", .typeSignature(parameterTypes: nil, returnTypes: ["KeyPath>"]))]) + + assertParsedPathComponents("functionName-(KeyPath,Dictionary>)", [("functionName", .typeSignature(parameterTypes: ["KeyPath,Dictionary>"], returnTypes: nil))]) + assertParsedPathComponents("functionName->KeyPath,Dictionary>", [("functionName", .typeSignature(parameterTypes: nil, returnTypes: ["KeyPath,Dictionary>"]))]) + + // Nested generics and tuple types + assertParsedPathComponents( "functionName-(A,(D,H<(I,J),(K,L)>),M,R),S>)", [("functionName", .typeSignature(parameterTypes: ["A", "(D,H<(I,J),(K,L)>)", "M,R),S>"], returnTypes: nil))]) + assertParsedPathComponents("functionName->(A,(D,H<(I,J),(K,L)>),M,R),S>)", [("functionName", .typeSignature(parameterTypes: nil, returnTypes: ["A", "(D,H<(I,J),(K,L)>)", "M,R),S>"]))]) + // With special characters + assertParsedPathComponents( "functionName-(Å<𝔹,©>,(Δ<∃,⨍,𝄞>,ℌ<(𝓲,ⅉ),(🄺,ƛ)>),𝔐<𝚗,(Ω<π,Ⓠ>,℟),𝔖>)", [("functionName", .typeSignature(parameterTypes: ["Å<𝔹,©>", "(Δ<∃,⨍,𝄞>,ℌ<(𝓲,ⅉ),(🄺,ƛ)>)", "𝔐<𝚗,(Ω<π,Ⓠ>,℟),𝔖>"], returnTypes: nil))]) + assertParsedPathComponents("functionName->(Å<𝔹,©>,(Δ<∃,⨍,𝄞>,ℌ<(𝓲,ⅉ),(🄺,ƛ)>),𝔐<𝚗,(Ω<π,Ⓠ>,℟),𝔖>)", [("functionName", .typeSignature(parameterTypes: nil, returnTypes: ["Å<𝔹,©>", "(Δ<∃,⨍,𝄞>,ℌ<(𝓲,ⅉ),(🄺,ƛ)>)", "𝔐<𝚗,(Ω<π,Ⓠ>,℟),𝔖>"]))]) } func testResolveExternalLinkFromTechnologyRoot() async throws { From 6a9718b98074ca4f3795dcddd6ba51ce441c16e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Wed, 12 Nov 2025 14:50:57 +0100 Subject: [PATCH 2/2] Add documentation for the private type signature string scanner API --- .../PathHierarchy+TypeSignature.swift | 131 ++++++++++++++++-- 1 file changed, 122 insertions(+), 9 deletions(-) diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+TypeSignature.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+TypeSignature.swift index 41bb59cac4..1644116036 100644 --- a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+TypeSignature.swift +++ b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+TypeSignature.swift @@ -522,6 +522,20 @@ extension PathHierarchy.PathParser { // MARK: Scanning a substring +/// A file-private, low-level string scanner type that's only designed for parsing type signature based disambiguation suffixes in authored links. +/// +/// ## Correct usage +/// +/// The higher level methods like ``scanReturnTypes()``, ``scanArguments()``, ``scanTuple()``, or ``scanValue()`` makes assumptions about the scanners content and current state. +/// For example: +/// - ``scanReturnTypes()`` knows that return types are specified after any parameter types and requires that the caller has already scanned the parameter types and advanced past the `"->"` separator. +/// It's the caller's (`parseTypeSignatureDisambiguation(pathComponent:)` above) responsibility to do these things correctly. +/// Similarly, it's the caller's responsibility to advance past the `"-"` prefix verify that the scanner points to an open parenthesis character (`(`) that before calling ``scanArguments()`` to scan the parameter types. +/// Failing to do either of these things will result in unexpected parsed disambiguation that DocC will fail to find a match for. +/// - Both ``scanArguments()``, or ``scanTuple()`` expects that the disambiguation portion of the authored link has a balanced number of open and closer parenthesis (`(` and `)`). +/// If the authored link contains unbalanced parenthesis then disambiguation isn't valid and the scanner will return a parsed value that DocC will fail to find a match for. +/// - ``scanValue()`` expects that the disambiguation portion of the authored link has a balanced number of open and closer angle brackets (`<` and `>`). +/// If the authored link contains unbalanced angle brackets then disambiguation isn't valid and the scanner will return a parsed value that DocC will fail to find a match for. private struct StringScanner: ~Copyable { private var remaining: Substring @@ -529,25 +543,45 @@ private struct StringScanner: ~Copyable { remaining = original } - func peek() -> Character? { + /// Returns the next character _without_ advancing the scanner + private func peek() -> Character? { remaining.first } - mutating func take() -> Character { + /// Advances the scanner and returns the scanned character. + private mutating func take() -> Character { remaining.removeFirst() } + /// Advances the scanner by `count` elements and returns the scanned substring. mutating func take(_ count: Int) -> Substring { defer { remaining = remaining.dropFirst(count) } return remaining.prefix(count) } - mutating func takeAll() -> Substring { + /// Advances the scanner to the end and returns the scanned substring. + private mutating func takeAll() -> Substring { defer { remaining.removeAll() } return remaining } - mutating func scan(until predicate: (Character) -> Bool) -> Substring? { + /// Advances the scanner up to the first character that satisfies the given `predicate` and returns the scanned substring. + /// + /// If the scanner doesn't contain any characters that satisfy the given `predicate`, then this method returns `nil` _without_ advancing the scanner. + /// + /// For example, consider a scanner that has already advanced 4 characters into the string `"One,Two,Three"` + /// ``` + /// One,Two,Three + /// ^ + /// ``` + /// Calling `scanner.scan(until: \.isNumber)` returns `nil` without advancing the scanner because none of the (remaining) characters is a number. + /// + /// Calling `scanner.scan(until: { $0 == "," })` advances the scanner by 3 additional characters, returning the scanned `"Two"` substring. + /// ``` + /// One,Two,Three + /// ^ + /// ``` + private mutating func scan(until predicate: (Character) -> Bool) -> Substring? { guard let index = remaining.firstIndex(where: predicate) else { return nil } @@ -555,7 +589,23 @@ private struct StringScanner: ~Copyable { return remaining[.. Bool) -> Substring? { + /// Advances the scanner up to and past the first character that satisfies the given `predicate` and returns the scanned substring. + /// + /// If the scanner doesn't contain any characters that satisfy the given `predicate`, then this method returns `nil` _without_ advancing the scanner. + /// + /// For example, consider a scanner that has already advanced 4 characters into the string `"One,Two,Three"` + /// ``` + /// One,Two,Three + /// ^ + /// ``` + /// Calling `scanner.scan(until: \.isNumber)` returns `nil` without advancing the scanner because none of the (remaining) characters is a number. + /// + /// Calling `scanner.scan(until: { $0 == "," })` advances the scanner by 4 additional characters, returning the scanned `"Two,"` substring. + /// ``` + /// One,Two,Three + /// ^ + /// ``` + private mutating func scan(past predicate: (Character) -> Bool) -> Substring? { guard let beforeIndex = remaining.firstIndex(where: predicate) else { return nil } @@ -564,16 +614,29 @@ private struct StringScanner: ~Copyable { return remaining[.. Bool { remaining.hasPrefix(prefix) } // MARK: Parsing argument types by scanning + /// Scans the remainder of the scanner's contents as the individual elements of a tuple return type, + /// or as a single return type if the scanners current position isn't an open parenthesis (`(`) + /// + /// For example, consider a scanner that has already advanced 8 characters into the string `"-(One)->(Two,Three)"` + /// ``` + /// -(One)->(Two, Three) + /// ^ + /// ``` + /// Because the scanner's current position is an open parenthesis (`(`), the scanner advances all the way to the end and returns `["Two", "Three"]` representing two elements in the tuple return value. + /// + /// - Note: The scanner expects that the caller has already scanned any parameter types and advanced past the `"->"` separator. mutating func scanReturnTypes() -> [Substring] { if peek() == "(" { _ = take() // the leading parenthesis @@ -582,7 +645,20 @@ private struct StringScanner: ~Copyable { return [takeAll()] } } - + + /// Scans the list of individual parameter type names as if the scanner's current position was 1 past the open parenthesis (`(`) or a tuple. + /// + /// For example, consider a scanner that has already advanced 2 characters into the string `"-(One,(A,B))->(Two)"` + /// ``` + /// -(One,(A,B))->(Two) + /// ^ + /// ``` + /// The scanner parses two parameter return types---`"One"` and `"(A,B)"`---before the parenthesis balance out, advancing its position to one after the arguments list's closing parenthesis (`)`). + /// ``` + /// -(One,(A,B))->(Two) + /// ^ + /// ``` + /// - Note: The scanner expects that the caller has already advanced past the open parenthesis (`(`) that begins the list of parameter types. mutating func scanArguments() -> [Substring] { guard peek() != ")" else { _ = take() // drop the ")" @@ -600,7 +676,19 @@ private struct StringScanner: ~Copyable { return arguments } - mutating func scanArgument() -> Substring? { + /// Scans a single type name, representing either a scalar value (such as `One`) or a nested tuple (such as `(A,B)`). + /// + /// For example, consider a scanner that has already advanced 6 characters into the string `"-(One,(A,B))->(Two)"` + /// ``` + /// -(One,(A,B))->(Two) + /// ^ + /// ``` + /// Because the value starts with an opening parenthesis (`(`), the scanner advances until the parenthesis balance out, returning `"(A,B)"`. + /// ``` + /// -(One,(A,B))->(Two) + /// ^ + /// ``` + private mutating func scanArgument() -> Substring? { guard peek() == "(" else { // If the argument doesn't start with "(" it can't be neither a tuple nor a closure type. // In this case, scan until the next argument (",") or the end of the arguments (")") @@ -631,7 +719,20 @@ private struct StringScanner: ~Copyable { return argumentString + returnValue } - mutating func scanTuple() -> Substring? { + /// Scans a nested tuple as a single substring. + /// + /// For example, consider a scanner that has already advanced 6 character into the string `"-(One,(A,B))->(Two)"` + /// ``` + /// -(One,(A,B))->(Two) + /// ^ + /// ``` + /// Because the value starts with an opening parenthesis (`(`), the scanner advances until the parenthesis balance out, returning `"(A,B)"`. + /// ``` + /// -(One,(A,B))->(Two) + /// ^ + /// ``` + /// - Note: The scanner expects that the caller has already advanced to the open parenthesis (`(`) that's the start of the nested tuple. + private mutating func scanTuple() -> Substring? { assert(peek() == "(", "The caller should have checked that this is a tuple") // The tuple may contain any number of nested tuples. Keep track of the open and close parenthesis while scanning. @@ -651,7 +752,19 @@ private struct StringScanner: ~Copyable { return scan(past: predicate) } - mutating func scanValue() -> Substring? { + /// Scans a single type name. + /// + /// For example, consider a scanner that has already advanced 2 character into the string `"-(One,Two)"` + /// ``` + /// -(One,Two) + /// ^ + /// ``` + /// Because the value contains generics (``), the scanner advances until the angle brackets balance out, returning `"One"`. + /// ``` + /// -(One,Two) + /// ^ + /// ``` + private mutating func scanValue() -> Substring? { // The value may contain any number of nested generics. Keep track of the open and close angle brackets while scanning. var depth = 0 let predicate: (Character) -> Bool = {