Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extensions to External Types #335

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Package.resolved
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@
},
{
"package": "SymbolKit",
"repositoryURL": "https://github.com/apple/swift-docc-symbolkit",
"repositoryURL": "https://github.com/themomax/swift-docc-symbolkit",
"state": {
"branch": "main",
"revision": "da6cedd103e0e08a2bc7b14869ec37fba4db72d9",
"branch": "docc-extensions-to-external-types-base",
"revision": "0a67e26bb38d4f40c08bbf6a119affa3e02367ad",
"version": null
}
},
Expand Down
6 changes: 4 additions & 2 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,9 @@ let package = Package(
// Test utility library
.target(
name: "SwiftDocCTestUtilities",
dependencies: []),
dependencies: [
"SymbolKit"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly out of curiosity: what functionality of the utilities target is dealing with SymbolKit directly? My understanding is that the utilities target is mainly meant to cover the command-line interface and its related functionality but not any core logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I factored out makeSymbolGraph(moduleName:symbols:relationships:) into XCTestCase as it was used in more than one test file. See Sources/SwiftDocCTestUtilities/SymbolGraphCreation.swift

]),

// Command-line tool
.executableTarget(
Expand Down Expand Up @@ -124,7 +126,7 @@ if ProcessInfo.processInfo.environment["SWIFTCI_USE_LOCAL_DEPS"] == nil {
.package(name: "swift-markdown", url: "https://github.com/apple/swift-markdown.git", .branch("main")),
.package(name: "CLMDB", url: "https://github.com/apple/swift-lmdb.git", .branch("main")),
.package(url: "https://github.com/apple/swift-argument-parser", .upToNextMinor(from: "1.0.1")),
.package(name: "SymbolKit", url: "https://github.com/apple/swift-docc-symbolkit", .branch("main")),
.package(name: "SymbolKit", url: "https://github.com/themomax/swift-docc-symbolkit", .branch("docc-extensions-to-external-types-base")),
.package(url: "https://github.com/apple/swift-crypto.git", .upToNextMinor(from: "1.1.2")),
]

Expand Down
10 changes: 5 additions & 5 deletions Sources/SwiftDocC/Coverage/DocumentationCoverageOptions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -185,15 +185,15 @@ extension DocumentationCoverageOptions.KindFilterOptions {
/// Converts given ``DocumentationNode.Kind`` to corresponding `BitFlagRepresentation` if possible. Returns `nil` if the given Kind is not representable.
fileprivate init?(kind: DocumentationNode.Kind) {
switch kind {
case .module: // 1
case .module, .extendedModule: // 1
self = .module
case .class: // 2
case .class, .extendedClass: // 2
self = .class
case .structure: // 3
case .structure, .extendedStructure: // 3
self = .structure
case .enumeration: // 4
case .enumeration, .extendedEnumeration: // 4
self = .enumeration
case .protocol: // 5
case .protocol, .extendedProtocol: // 5
self = .protocol
case .typeAlias: // 6
self = .typeAlias
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public class FileSystemRenderNodeProvider: RenderNodeProvider {

extension RenderNode {
private static let typesThatShouldNotUseNavigatorTitle: Set<NavigatorIndex.PageType> = [
.framework, .class, .structure, .enumeration, .protocol, .typeAlias, .associatedType
.framework, .class, .structure, .enumeration, .protocol, .typeAlias, .associatedType, .extension
]

/// Returns a navigator title preferring the fragments inside the metadata, if applicable.
Expand Down
26 changes: 15 additions & 11 deletions Sources/SwiftDocC/Infrastructure/CoverageDataEntry.swift
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,11 @@ extension CoverageDataEntry {
.protocol,
.typeAlias,
.associatedType,
.typeDef:
.typeDef,
.extendedClass,
.extendedStructure,
.extendedEnumeration,
.extendedProtocol:
self = .types
case .localVariable,
.instanceProperty,
Expand All @@ -256,7 +260,7 @@ extension CoverageDataEntry {
.typeSubscript,
.instanceSubscript:
self = .members
case .function, .module, .globalVariable, .operator:
case .function, .module, .globalVariable, .operator, .extendedModule:
self = .globals
case let kind where SummaryCategory.allKnownNonSymbolKindNames.contains(kind.name):
self = .nonSymbol
Expand Down Expand Up @@ -297,46 +301,46 @@ extension CoverageDataEntry {
context: DocumentationContext
) throws {
switch documentationNode.kind {
case DocumentationNode.Kind.class:
case .class, .extendedClass:
self = try .class(
memberStats: KindSpecificData.extractChildStats(
documentationNode: documentationNode,
context: context))
case DocumentationNode.Kind.enumeration:
case .enumeration, .extendedEnumeration:
self = try .enumeration(
memberStats: KindSpecificData.extractChildStats(
documentationNode: documentationNode,
context: context))
case DocumentationNode.Kind.structure:
case .structure, .extendedStructure:
self = try .structure(
memberStats: KindSpecificData.extractChildStats(
documentationNode: documentationNode,
context: context))
case DocumentationNode.Kind.protocol:
self = try .enumeration(
case .protocol, .extendedProtocol:
self = try .protocol(
memberStats: KindSpecificData.extractChildStats(
documentationNode: documentationNode,
context: context))

case DocumentationNode.Kind.instanceMethod:
case .instanceMethod:
self = try .instanceMethod(
parameterStats: CoverageDataEntry.KindSpecificData.extractFunctionSignatureStats(
documentationNode: documentationNode,
context: context
, fieldName: "method parameters"))
case DocumentationNode.Kind.operator:
case .operator:
self = try .`operator`(
parameterStats: CoverageDataEntry.KindSpecificData.extractFunctionSignatureStats(
documentationNode: documentationNode,
context: context,
fieldName: "operator parameters"))
case DocumentationNode.Kind.function:
case .function:
self = try .`operator`(
parameterStats: CoverageDataEntry.KindSpecificData.extractFunctionSignatureStats(
documentationNode: documentationNode,
context: context,
fieldName: "function parameters"))
case DocumentationNode.Kind.initializer:
case .initializer:
self = try .`operator`(
parameterStats: CoverageDataEntry.KindSpecificData.extractFunctionSignatureStats(
documentationNode: documentationNode,
Expand Down
7 changes: 5 additions & 2 deletions Sources/SwiftDocC/Infrastructure/DocumentationContext.swift
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,9 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
public var externalMetadata = ExternalMetadata()


/// The decoder used in the `SymbolGraphLoader`
var decoder: JSONDecoder = JSONDecoder()

/// Initializes a documentation context with a given `dataProvider` and registers all the documentation bundles that it provides.
///
/// - Parameter dataProvider: The data provider to register bundles from.
Expand Down Expand Up @@ -1022,7 +1025,7 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
private func parentChildRelationship(from edge: SymbolGraph.Relationship) -> (ResolvedTopicReference, ResolvedTopicReference)? {
// Filter only parent <-> child edges
switch edge.kind {
case .memberOf, .requirementOf:
case .memberOf, .requirementOf, .declaredIn:
guard let parentRef = symbolIndex[edge.target]?.reference, let childRef = symbolIndex[edge.source]?.reference else {
return nil
}
Expand Down Expand Up @@ -1920,7 +1923,7 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
discoveryGroup.async(queue: discoveryQueue) { [unowned self] in
symbolGraphLoader = SymbolGraphLoader(bundle: bundle, dataProvider: self.dataProvider)
do {
try symbolGraphLoader.loadAll()
try symbolGraphLoader.loadAll(using: decoder)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly out of curiosity: why is a decoded passed as an argument? AFIACT the context's decoder is never modified.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be used (in tests) to decode symbol kinds not known to SymbolKit. See e.g. the changes in Tests/SwiftDocCTests/Infrastructure/AutomaticCurationTests.swift.

This registration process is part of swiftlang/swift-docc-symbolkit#39. For usage details please refer to the documentation here.

if LinkResolutionMigrationConfiguration.shouldSetUpHierarchyBasedLinkResolver {
let pathHierarchy = PathHierarchy(symbolGraphLoader: symbolGraphLoader, bundleName: urlReadablePath(bundle.displayName), knownDisambiguatedPathComponents: knownDisambiguatedSymbolPathComponents)
hierarchyBasedResolver = PathHierarchyBasedLinkResolver(pathHierarchy: pathHierarchy)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,16 @@ extension ExternalSymbolResolver {
symbolKind = .var
case .module:
symbolKind = .module
case .extendedModule:
symbolKind = .extendedModule
case .extendedStructure:
symbolKind = .extendedStructure
case .extendedClass:
symbolKind = .extendedClass
case .extendedEnumeration:
symbolKind = .extendedEnumeration
case .extendedProtocol:
symbolKind = .extendedProtocol

// There shouldn't be any reason for a symbol graph file to reference one of these kinds outside of the symbol graph itself.
// Return `.class` as the symbol kind (acting as "any symbol") so that the render reference gets a "symbol" role.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,15 +335,65 @@ final class DocumentationCacheBasedLinkResolver {
let moduleName: String
var languages: Set<SourceLanguage>
}
var pathCollisionInfo = [String: [PathCollisionInfo]]()
var pathCollisionInfo = [[String]: [PathCollisionInfo]]()
pathCollisionInfo.reserveCapacity(totalSymbolCount)

// Group symbols by path from all of the available symbol graphs
for (moduleName, symbolGraph) in unifiedGraphs {
let symbols = Array(symbolGraph.symbols.values)
let pathsAndLanguages: [[(String, SourceLanguage)]] = symbols.concurrentMap { referencesWithoutDisambiguationFor($0, moduleName: moduleName, bundle: bundle, context: context).map {
($0.path.lowercased(), $0.sourceLanguage)
} }

let referenceMap = symbols.concurrentMap { symbol in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to take some more time to review this change in detail. Like the comment at the top of this function says:

The implementation of this function is fairly tricky because in most cases it has to preserve past behavior.

Subtle changes, even bug fixes, can result in pages getting different paths or different disambiguation which results in broken links (since the documentation cache based link resolver requires the exact right paths and disambiguation) to resolve links.

(symbol, referencesWithoutDisambiguationFor(symbol, moduleName: moduleName, bundle: bundle, context: context))
}.reduce(into: [String: [SourceLanguage: ResolvedTopicReference]](), { result, next in
let (symbol, references) = next
for reference in references {
result[symbol.uniqueIdentifier, default: [:]][reference.sourceLanguage] = reference
}
})

let parentMap = symbolGraph.relationshipsByLanguage.reduce(into: [String: [SourceLanguage: String]](), { parentMap, next in
let (selector, relationships) = next
guard let language = SourceLanguage(knownLanguageIdentifier: selector.interfaceLanguage) else {
return
}

for relationship in relationships {
switch relationship.kind {
case .memberOf, .requirementOf, .declaredIn:
parentMap[relationship.source, default: [:]][language] = relationship.target
default:
break
}
}
})

let pathsAndLanguages: [[([String], SourceLanguage)]] = symbols.concurrentMap { symbol in
guard let references = referenceMap[symbol.uniqueIdentifier] else {
return []
}

return references.map { language, reference in
var prefixLength: Int
if let parentId = parentMap[symbol.uniqueIdentifier]?[language],
let parentReference = referenceMap[parentId]?[language] ?? referenceMap[parentId]?.values.first {
// This is a child of some other symbol
prefixLength = parentReference.pathComponents.count
} else {
// This is a top-level symbol or another symbol without parent (e.g. default implementation)
prefixLength = reference.pathComponents.count-1
}

// PathComponents can have prefixes which are not known locally. In that case,
// the "empty" segments will be cut out later on. We follow the same logic here, as otherwise
// some collisions would not be detected.
// E.g. consider an extension to an external nested type `SomeModule.SomeStruct.SomeStruct`. The
// parent of this extended type symbol is `SomeModule`, however, the path for the extended type symbol
// is `SomeModule/SomeStruct/SomeStruct`, later on, this will change to `SomeModule/SomeStruct`. Now, if
// we also extend `SomeModule.SomeStruct`, the paths for both extensions could collide. To recognize (and resolve)
// the collision here, we work with the same, shortened paths.
return ((reference.pathComponents[0..<prefixLength] + [reference.pathComponents.last!]).map{ $0.lowercased() }, reference.sourceLanguage)
}
}

for (symbol, symbolPathsAndLanguages) in zip(symbols, pathsAndLanguages) {
for (path, language) in symbolPathsAndLanguages {
Expand Down Expand Up @@ -495,10 +545,13 @@ final class DocumentationCacheBasedLinkResolver {
// therefore for the currently processed symbol to be a child of a re-written symbol it needs to have
// at least 3 components. It's a fair optimization to make since graphs will include a lot of root level symbols.
guard reference.pathComponents.count > 3,
// Fetch the symbol's parent
let parentReference = try symbolsURLHierarchy.parent(of: reference),
// If the parent path matches the current reference path, bail out
parentReference.pathComponents != reference.pathComponents.dropLast()
// Fetch the symbol's parent
let parentReference = try symbolsURLHierarchy.parent(of: reference),
// If the parent path matches the current reference path, bail out
parentReference.pathComponents != reference.pathComponents.dropLast(),
// If the parent is not from the same module (because we're dealing with a
// default implementation of an external protocol), bail out
parentReference.pathComponents[..<3] == reference.pathComponents[..<3]
else { return reference }

// Build an up to date reference path for the current node based on the parent path
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,17 @@ struct PathHierarchy {
parent = child
components = components.dropFirst()
}

// Symbols corresponding to nested types may appear outside of their original context, where
// their parent type may not be present. This happens e.g. for extensions to external nested
// types. In such cases, the nested type should be a direct child of whatever its parent is
// in this different context. Any other behavior would lead to truly empty pages.
var titlePrefix = node.symbol!.title.split(separator: ".").dropLast()
while !components.isEmpty && !titlePrefix.isEmpty && components.last! == titlePrefix.last! {
titlePrefix = titlePrefix.dropLast()
components = components.dropLast()
}

for component in components {
let component = Self.parse(pathComponent: component[...])
let nodeWithoutSymbol = Node(name: component.name)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
This source file is part of the Swift.org open source project

Copyright (c) 2021 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 SymbolKit

extension SymbolGraph.Symbol.AccessControl: Comparable {
private var level: Int? {
switch self {
case .private:
return 0
case .filePrivate:
return 1
case .internal:
return 2
case .public:
return 3
case .open:
return 4
default:
assertionFailure("Unknown AccessControl case was used in comparison.")
return nil
}
}

public static func < (lhs: SymbolGraph.Symbol.AccessControl, rhs: SymbolGraph.Symbol.AccessControl) -> Bool {
guard let lhs = lhs.level,
let rhs = rhs.level else {
return false
}

return lhs < rhs
}
}
Loading