From 7fad5a65059509502b7c41de9f79c3fe90d67792 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Wed, 5 Apr 2023 17:30:33 -0700 Subject: [PATCH 1/2] Fix crash when exported import protocol is aliased with same name rdar://107623749 --- .../Link Resolution/PathHierarchy.swift | 35 +- .../Infrastructure/PathHierarchyTests.swift | 33 ++ ...ementationsWithExportedImport.symbols.json | 387 ++++++++++++++++++ 3 files changed, 453 insertions(+), 2 deletions(-) create mode 100644 Tests/SwiftDocCTests/Test Bundles/DefaultImplementationsWithExportedImport.docc/DefaultImplementationsWithExportedImport.symbols.json diff --git a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift index e488c2b25d..bcd9fb132f 100644 --- a/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift +++ b/Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy.swift @@ -135,6 +135,26 @@ struct PathHierarchy { } } + for relationship in graph.relationships where relationship.kind == .defaultImplementationOf { + guard let sourceNode = nodes[relationship.source] else { + continue + } + + let targetNodes = nodes[relationship.target].map { [$0] } ?? allNodes[relationship.target] ?? [] + guard !targetNodes.isEmpty else { + continue + } + + for requirementTarget in targetNodes { + assert( + requirementTarget.parent != nil, + "The 'defaultImplementationOf' symbol should be a 'memberOf' a known protocol symbol but didn't have a parent relationship in the hierarchy." + ) + requirementTarget.parent?.add(symbolChild: sourceNode) + } + topLevelCandidates.removeValue(forKey: relationship.source) + } + // The hierarchy doesn't contain any non-symbol nodes yet. It's OK to unwrap the `symbol` property. for topLevelNode in topLevelCandidates.values where topLevelNode.symbol!.pathComponents.count == 1 { moduleNode.add(symbolChild: topLevelNode) @@ -155,6 +175,10 @@ struct PathHierarchy { components = components.dropFirst() } for component in components { + assert( + parent.children[components.first!] == nil, + "Shouldn't create a new sparse node when symbol node already exist. This is an indication that a symbol is missing a relationship." + ) let component = Self.parse(pathComponent: component[...]) let nodeWithoutSymbol = Node(name: component.name) parent.add(child: nodeWithoutSymbol, kind: component.kind, hash: component.hash) @@ -164,6 +188,10 @@ 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())" + ) allNodes.removeAll() // build the lookup list by traversing the hierarchy and adding identifiers to each node @@ -199,12 +227,15 @@ struct PathHierarchy { self.tutorialContainer = newNode(bundleName) self.tutorialOverviewContainer = newNode("tutorials") - assert(lookup.allSatisfy({ $0.key == $0.value.identifier})) + assert( + lookup.allSatisfy({ $0.key == $0.value.identifier }), + "Every node lookup should match a node with that identifier." + ) self.modules = roots self.lookup = lookup - assert(topLevelSymbols().allSatisfy({ lookup[$0] != nil})) + assert(topLevelSymbols().allSatisfy({ lookup[$0] != nil })) } /// Adds an article to the path hierarchy. diff --git a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift index 7a621f2aab..be6c3bb5d4 100644 --- a/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift +++ b/Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift @@ -674,6 +674,39 @@ class PathHierarchyTests: XCTestCase { try assertFindsPath("/MixedFramework-module-9r7pl/myTopLevelVariable-var-520ez", in: tree, asSymbolID: "s:14MixedFramework18myTopLevelVariableSbvp") } + func testDefaultImplementationWithCollidingTargetSymbol() throws { + try XCTSkipUnless(LinkResolutionMigrationConfiguration.shouldUseHierarchyBasedLinkResolver) + + // ---- Inner + // public protocol Something { + // func doSomething() + // } + // public extension Something { + // func doSomething() {} + // } + // + // ---- Outer + // @_exported import Inner + // public typealias Something = Inner.Something + let (_, context) = try testBundleAndContext(named: "DefaultImplementationsWithExportedImport") + let tree = try XCTUnwrap(context.hierarchyBasedLinkResolver?.pathHierarchy) + + // The @_export imported protocol can be found + try assertFindsPath("/DefaultImplementationsWithExportedImport/Something-protocol", in: tree, asSymbolID: "s:5Inner9SomethingP") + // The wrapping type alias can be found + try assertFindsPath("/DefaultImplementationsWithExportedImport/Something-typealias", in: tree, asSymbolID: "s:40DefaultImplementationsWithExportedImport9Somethinga") + + // The protocol requirement and the default implementation both exist at the @_export imported Something protocol. + try assertPathRaisesErrorMessage("DefaultImplementationsWithExportedImport/Something-protocol/doSomething()", in: tree, context: context, expectedErrorMessage: """ + 'doSomething()' is ambiguous at '/DefaultImplementationsWithExportedImport/Something' + """) { error in + XCTAssertEqual(error.solutions, [ + .init(summary: "Insert '8skxc' for\n'func doSomething()'", replacements: [("-8skxc", 73, 73)]), + .init(summary: "Insert 'scj9' for\n'func doSomething()'", replacements: [("-scj9", 73, 73)]), + ]) + } + } + func testDisambiguatedPaths() throws { try XCTSkipUnless(LinkResolutionMigrationConfiguration.shouldUseHierarchyBasedLinkResolver) let (_, context) = try testBundleAndContext(named: "MixedLanguageFrameworkWithLanguageRefinements") diff --git a/Tests/SwiftDocCTests/Test Bundles/DefaultImplementationsWithExportedImport.docc/DefaultImplementationsWithExportedImport.symbols.json b/Tests/SwiftDocCTests/Test Bundles/DefaultImplementationsWithExportedImport.docc/DefaultImplementationsWithExportedImport.symbols.json new file mode 100644 index 0000000000..39e92b45b4 --- /dev/null +++ b/Tests/SwiftDocCTests/Test Bundles/DefaultImplementationsWithExportedImport.docc/DefaultImplementationsWithExportedImport.symbols.json @@ -0,0 +1,387 @@ +{ + "metadata": { + "formatVersion": { + "major": 0, + "minor": 6, + "patch": 0 + }, + "generator": "Apple Swift version 5.9 (swiftlang-5.9.0.106.53 clang-1500.0.13.6)" + }, + "module": { + "name": "DefaultImplementationsWithExportedImport", + "platform": { + "architecture": "arm64", + "vendor": "apple", + "operatingSystem": { + "name": "macosx", + "minimumVersion": { + "major": 13, + "minor": 4 + } + } + } + }, + "symbols": [ + { + "kind": { + "identifier": "swift.method", + "displayName": "Instance Method" + }, + "identifier": { + "precise": "s:5Inner9SomethingPAAE02doB0yyF", + "interfaceLanguage": "swift" + }, + "pathComponents": [ + "Something", + "doSomething()" + ], + "names": { + "title": "doSomething()", + "subHeading": [ + { + "kind": "keyword", + "spelling": "func" + }, + { + "kind": "text", + "spelling": " " + }, + { + "kind": "identifier", + "spelling": "doSomething" + }, + { + "kind": "text", + "spelling": "()" + } + ] + }, + "docComment": { + "uri": "file:///Users/username/path/to/DefaultImplementationsWithExportedImport/Inner/Something.swift", + "module": "Inner", + "lines": [ + { + "range": { + "start": { + "line": 16, + "character": 8 + }, + "end": { + "line": 16, + "character": 60 + } + }, + "text": "A default implementation of the protocol requirement" + } + ] + }, + "functionSignature": { + "returns": [ + { + "kind": "text", + "spelling": "()" + } + ] + }, + "swiftExtension": { + "extendedModule": "Inner", + "typeKind": "swift.protocol" + }, + "declarationFragments": [ + { + "kind": "keyword", + "spelling": "func" + }, + { + "kind": "text", + "spelling": " " + }, + { + "kind": "identifier", + "spelling": "doSomething" + }, + { + "kind": "text", + "spelling": "()" + } + ], + "accessLevel": "public", + "location": { + "uri": "file:///Users/username/path/to/DefaultImplementationsWithExportedImport/Inner/Something.swift", + "position": { + "line": 17, + "character": 9 + } + } + }, + { + "kind": { + "identifier": "swift.method", + "displayName": "Instance Method" + }, + "identifier": { + "precise": "s:5Inner9SomethingP02doB0yyF", + "interfaceLanguage": "swift" + }, + "pathComponents": [ + "Something", + "doSomething()" + ], + "names": { + "title": "doSomething()", + "subHeading": [ + { + "kind": "keyword", + "spelling": "func" + }, + { + "kind": "text", + "spelling": " " + }, + { + "kind": "identifier", + "spelling": "doSomething" + }, + { + "kind": "text", + "spelling": "()" + } + ] + }, + "docComment": { + "uri": "file:///Users/username/path/to/DefaultImplementationsWithExportedImport/Inner/Something.swift", + "module": "Inner", + "lines": [ + { + "range": { + "start": { + "line": 11, + "character": 8 + }, + "end": { + "line": 11, + "character": 33 + } + }, + "text": "Some protocol requirement" + } + ] + }, + "functionSignature": { + "returns": [ + { + "kind": "text", + "spelling": "()" + } + ] + }, + "declarationFragments": [ + { + "kind": "keyword", + "spelling": "func" + }, + { + "kind": "text", + "spelling": " " + }, + { + "kind": "identifier", + "spelling": "doSomething" + }, + { + "kind": "text", + "spelling": "()" + } + ], + "accessLevel": "public", + "location": { + "uri": "file:///Users/username/path/to/DefaultImplementationsWithExportedImport/Inner/Something.swift", + "position": { + "line": 12, + "character": 9 + } + } + }, + { + "kind": { + "identifier": "swift.protocol", + "displayName": "Protocol" + }, + "identifier": { + "precise": "s:5Inner9SomethingP", + "interfaceLanguage": "swift" + }, + "pathComponents": [ + "Something" + ], + "names": { + "title": "Something", + "navigator": [ + { + "kind": "identifier", + "spelling": "Something" + } + ], + "subHeading": [ + { + "kind": "keyword", + "spelling": "protocol" + }, + { + "kind": "text", + "spelling": " " + }, + { + "kind": "identifier", + "spelling": "Something" + } + ] + }, + "docComment": { + "uri": "file:///Users/username/path/to/DefaultImplementationsWithExportedImport/Inner/Something.swift", + "module": "Inner", + "lines": [ + { + "range": { + "start": { + "line": 9, + "character": 4 + }, + "end": { + "line": 9, + "character": 17 + } + }, + "text": "Some protocol" + } + ] + }, + "declarationFragments": [ + { + "kind": "keyword", + "spelling": "protocol" + }, + { + "kind": "text", + "spelling": " " + }, + { + "kind": "identifier", + "spelling": "Something" + } + ], + "accessLevel": "public", + "location": { + "uri": "file:///Users/username/path/to/DefaultImplementationsWithExportedImport/Inner/Something.swift", + "position": { + "line": 10, + "character": 16 + } + } + }, + { + "kind": { + "identifier": "swift.typealias", + "displayName": "Type Alias" + }, + "identifier": { + "precise": "s:40DefaultImplementationsWithExportedImport9Somethinga", + "interfaceLanguage": "swift" + }, + "pathComponents": [ + "Something" + ], + "names": { + "title": "Something", + "navigator": [ + { + "kind": "identifier", + "spelling": "Something" + } + ], + "subHeading": [ + { + "kind": "keyword", + "spelling": "typealias" + }, + { + "kind": "text", + "spelling": " " + }, + { + "kind": "identifier", + "spelling": "Something" + } + ] + }, + "docComment": { + "uri": "file:///Users/username/path/to/DefaultImplementationsWithExportedImport/DefaultImplementationsWithExportedImport/Wrapper.swift", + "module": "DefaultImplementationsWithExportedImport", + "lines": [ + { + "range": { + "start": { + "line": 11, + "character": 4 + }, + "end": { + "line": 11, + "character": 50 + } + }, + "text": "An alias of Something to the wrapped Something" + } + ] + }, + "declarationFragments": [ + { + "kind": "keyword", + "spelling": "typealias" + }, + { + "kind": "text", + "spelling": " " + }, + { + "kind": "identifier", + "spelling": "Something" + }, + { + "kind": "text", + "spelling": " = Inner" + }, + { + "kind": "text", + "spelling": "." + }, + { + "kind": "typeIdentifier", + "spelling": "Something", + "preciseIdentifier": "s:5Inner9SomethingP" + } + ], + "accessLevel": "public", + "location": { + "uri": "file:///Users/username/path/to/DefaultImplementationsWithExportedImport/DefaultImplementationsWithExportedImport/Wrapper.swift", + "position": { + "line": 12, + "character": 17 + } + } + } + ], + "relationships": [ + { + "kind": "requirementOf", + "source": "s:5Inner9SomethingP02doB0yyF", + "target": "s:5Inner9SomethingP", + "targetFallback": "Inner.Something" + }, + { + "kind": "defaultImplementationOf", + "source": "s:5Inner9SomethingPAAE02doB0yyF", + "target": "s:5Inner9SomethingP02doB0yyF", + "targetFallback": "Inner.Something.doSomething()" + } + ] +} From 4d6879a7935b07535c51a93c190c812c32f36db8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6nnqvist?= Date: Wed, 5 Apr 2023 17:30:53 -0700 Subject: [PATCH 2/2] Remove incorrect memberOf relationships in handcrafted test symbol data --- .../Test Bundles/TestBundle.docc/sidekit.symbols.json | 5 ----- .../Test Bundles/TestBundle.docc/sidekit.symbols.json | 5 ----- 2 files changed, 10 deletions(-) diff --git a/Tests/SwiftDocCTests/Test Bundles/TestBundle.docc/sidekit.symbols.json b/Tests/SwiftDocCTests/Test Bundles/TestBundle.docc/sidekit.symbols.json index c9c872cb0c..e356acfaf3 100644 --- a/Tests/SwiftDocCTests/Test Bundles/TestBundle.docc/sidekit.symbols.json +++ b/Tests/SwiftDocCTests/Test Bundles/TestBundle.docc/sidekit.symbols.json @@ -687,11 +687,6 @@ "target" : "s:5SideKit0A5SideProtocolP", "kind" : "requirementOf" }, - { - "source" : "s:5MyKit0A5MyProtocol0Afunc()DefaultImp", - "target" : "s:5SideKit0A5SideProtocolP", - "kind" : "memberOf" - }, { "source" : "s:5MyKit0A5MyProtocol0Afunc()DefaultImp", "target" : "s:5MyKit0A5MyProtocol0Afunc()", diff --git a/Tests/SwiftDocCUtilitiesTests/Test Bundles/TestBundle.docc/sidekit.symbols.json b/Tests/SwiftDocCUtilitiesTests/Test Bundles/TestBundle.docc/sidekit.symbols.json index 3ab6bb180d..ed323fff99 100644 --- a/Tests/SwiftDocCUtilitiesTests/Test Bundles/TestBundle.docc/sidekit.symbols.json +++ b/Tests/SwiftDocCUtilitiesTests/Test Bundles/TestBundle.docc/sidekit.symbols.json @@ -630,11 +630,6 @@ "target" : "s:5SideKit0A5SideProtocolP", "kind" : "requirementOf" }, - { - "source" : "s:5MyKit0A5MyProtocol0Afunc()DefaultImp", - "target" : "s:5SideKit0A5SideProtocolP", - "kind" : "memberOf" - }, { "source" : "s:5MyKit0A5MyProtocol0Afunc()DefaultImp", "target" : "s:5MyKit0A5MyProtocol0Afunc()",