Skip to content

Commit f0a3e95

Browse files
authored
Populate abbreviated declaration fragments in external render nodes (#1255)
* Clarify doc comment for `OutOfProcessReferenceResolver.declarationFragments` These declaration fragments are directly used to populate `TopicRenderReference.fragmentsVariants` [1], which is described as "The abbreviated declaration of the symbol to display in links" [2]. This change clarifies that the declaration fragments are expected to be the abbreviated declaration fragments. [1]: https://github.com/swiftlang/swift-docc/blob/65aaf926ec079ddbd40f29540d4180a70af99e5e/Sources/SwiftDocC/Infrastructure/External%20Data/OutOfProcessReferenceResolver.swift#L169 [2]: https://github.com/swiftlang/swift-docc/blob/65aaf926ec079ddbd40f29540d4180a70af99e5e/Sources/SwiftDocC/Model/Rendering/References/TopicRenderReference.swift#L50-L53 * Add full (symbol) name to LinkDestinationSummary The only use-case for storing the full declaration fragments in `LinkDestinationSummary` rather than the shortened declaration fragments intended for display, is to derive the full name of the symbol in diagnostics [1] [2]. We have a number of tests [3] which verify that the diagnostic emitted when a symbol is referenced locally is the same as when the symbol is referenced externally. However, we want to stop storing the full declaration fragments in favour of the shortened ones because: - the full fragments are never displayed when referenced externally, as they are too long - the full fragments take up additionally storage space, and encoding/decoding time, to only be used to derive the full name of the symbol This commit updates the logic such that, we pre-derive the full name of the symbol from its declaration fragments, and then store that as part of `LinkDestinationSummary`, so that in a subsequent commit we can stop storing the full declaration fragments and store only the shortened ones instead. The diagnostic logic has also been updated to use the new field rather than the full declaration fragments. [1]: https://github.com/swiftlang/swift-docc/blob/1b4a1850dd2785a8ebabded139ae0af3551bb029/Sources/SwiftDocC/Infrastructure/Link%20Resolution/ExternalPathHierarchyResolver.swift#L61-L63 [2]: https://github.com/swiftlang/swift-docc/blob/1b4a1850dd2785a8ebabded139ae0af3551bb029/Sources/SwiftDocC/Infrastructure/Link%20Resolution/PathHierarchy%2BError.swift#L115-L117 [3]: https://github.com/swiftlang/swift-docc/blob/9413c599817abc9dd4f8feeed57a998b19a05a6f/Tests/SwiftDocCTests/Infrastructure/ExternalPathHierarchyResolverTests.swift#L907-L918 * Use the abbreviated declaration fragments in LinkDestinationSummary LinkDestinationSummary contains a summary of an element that you can link to from outside the documentation bundle. [1] This information is meant to be used by a server to provide information to an out-of-process resolver to resolve links to external entities, so that the partner implementation of `LinkDestinationSummary` is `OutOfProcessReferenceResolver.ResolvedInformation` [2]. However, currently `OutOfProcessReferenceResolver.ResolvedInformation. declarationFragments` is expecting the abbreviated declaration fragments, but we are storing the full fragments instead. [3] Instead, we should be storing the abbreviated declaration fragments, which are stored as the `subHeading` of the symbol. [4] This subheading is further processed during the render node transformation phase [5], and stored as `renderNode.metadata.fragmentsVariants`. This commit modifies `LinkDestinationSummary` such that its declaration fragments are the abbreviated declaration fragments from `renderNode.metadata.fragmentsVariants` rather than the full declaration fragments. The final result will be that declaration fragments for external links will behave the same as local links when referencing them in the Topics section. They will both now use the abbreviated declaration fragments. [1]: https://github.com/swiftlang/swift-docc/blob/65aaf926ec079ddbd40f29540d4180a70af99e5e/Sources/SwiftDocC/LinkTargets/LinkDestinationSummary.swift#L66 [2]: https://github.com/swiftlang/swift-docc/blob/65aaf926ec079ddbd40f29540d4180a70af99e5e/Sources/SwiftDocC/Infrastructure/External%20Data/OutOfProcessReferenceResolver.swift#L558-L562 [3]: https://github.com/swiftlang/swift-docc/blob/65aaf926ec079ddbd40f29540d4180a70af99e5e/Sources/SwiftDocC/LinkTargets/LinkDestinationSummary.swift#L445 [4]: https://github.com/swiftlang/swift-docc-symbolkit/blob/ebe89c7da4cf03ded04cd708f3399087c6f2dad7/Sources/SymbolKit/SymbolGraph/Symbol/Names.swift#L28-L31 * Populate declaration fragments in navigator metadata for external links Now that the declaration fragments will be the abbreviated declaration fragments from `LinkDestinationSummary`, we can propagate those to the navigator metadata for them to be used to inform the title of the navigator item [1]. Fixes rdar://152652751. [1]: https://github.com/swiftlang/swift-docc/blob/65aaf926ec079ddbd40f29540d4180a70af99e5e/Sources/SwiftDocC/Indexing/Navigator/RenderNode%2BNavigatorIndex.swift#L140 * Add navigator title to LinkDestinationSummary The navigator title, which contains abbreviated fragments of a symbol's declaration for display in navigation [1], was missing from LinkDestinationSummary. The navigator title is used here [2], but the information wasn't being populated anywhere [3]. This commit captures the navigator title information from the render metadata, and stores it as part of `LinkDestinationSummary`. While the original information comes from the symbol graph [4], the `DocumentationContentRenderer` has some special logic to sanitise the declaration fragments, particularly for Swift [5]. In order to ensure that we have parity between internal and external navigator titles, used the render metadata rather than the original information from the symbol graph. This mirrors what we're already doing with the shortened declaration fragments, which also are processed with the same logic. Finally, `LinkDestinationSummary.makeTopicRenderReference()` has been updated to populate the navigator title variants, such that after this commit navigator titles will be correctly populated for external links in the navigator. Fixes rdar://156488184. [1]: https://github.com/swiftlang/swift-docc/blob/257c62d1ef11efc023667fda89c0cd4419a8afa3/Sources/SwiftDocC/Model/Rendering/References/TopicRenderReference.swift#L60-L63 [2]: https://github.com/swiftlang/swift-docc/blob/257c62d1ef11efc023667fda89c0cd4419a8afa3/Sources/SwiftDocC/Indexing/Navigator/RenderNode%2BNavigatorIndex.swift#L153 [3]: https://github.com/swiftlang/swift-docc/blob/257c62d1ef11efc023667fda89c0cd4419a8afa3/Sources/SwiftDocC/Infrastructure/Link%20Resolution/ExternalPathHierarchyResolver.swift#L225 [4]: https://github.com/swiftlang/swift-docc-symbolkit/blob/3fc0c6880f712ce38137d0752f24b568fce1522e/Sources/SymbolKit/SymbolGraph/Symbol/Names.swift#L23-L26 [5]: https://github.com/swiftlang/swift-docc/blob/257c62d1ef11efc023667fda89c0cd4419a8afa3/Sources/SwiftDocC/Model/Rendering/DocumentationContentRenderer.swift#L87-L105 * Add navigator title tests for external links in the navigator Now that the navigator title is supported in external references, we can add some tests which verify this logic is working as expected. Added some tests which verify that the navigator title is used as expected for both Swift and Objective C symbols. * Rename `fullName` to `plainTextDeclaration` in LinkDestinationSummary `plainTextDeclaration` is clearer and makes it harder to confuse it with `title`. It is also closer to how the value is derived. Also updates the JSON schema in `LinkableEntities.json` to add the new `plainTextDeclaration` property to the `LinkDestinationSummary` and `LinkDestinationSummaryVariant` schemas. * Rename to `subheadingDeclarationFragments` for clarity `declarationFragments` was not descriptive enough about the contents of the property, so renamed to `subheadingDeclarationFragments` and deprecated `declarationFragments` for backwards compatibility. The JSON encoding key is kept the same (`fragments`) for backwards compatibility. * Rename `navigatorTitle` to `navigatorDeclarationFragments` Renamed the property in `LinkDestinationSummary` which stores the declaration fragments for the navigator to `navigatorDeclarationFragments`, to follow the same naming convention as `subheadingDeclarationFragments` which fulfils a very similar purpose. The documentation comments mirror each other, with one explaining that it's intended for use in topic groups, and the other that it's intended for use in navigators. * Rename helper function for readability Clarifies the function name so that it clearly states that only top level nodes are returned, as well as improving the grammar at the call site. * Fix minor issue in comment
1 parent 163fd03 commit f0a3e95

File tree

14 files changed

+448
-150
lines changed

14 files changed

+448
-150
lines changed

Sources/SwiftDocC/Infrastructure/External Data/OutOfProcessReferenceResolver+DeprecatedCommunication.swift

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,8 @@ extension OutOfProcessReferenceResolver {
162162
/// Information about the platforms and their versions where the resolved node is available, if any.
163163
public let platforms: [PlatformAvailability]?
164164
/// Information about the resolved declaration fragments, if any.
165+
///
166+
/// This is expected to be an abbreviated declaration for the symbol.
165167
public let declarationFragments: DeclarationFragments?
166168

167169
// We use the real types here because they're Codable and don't have public member-wise initializers.
@@ -205,7 +207,7 @@ extension OutOfProcessReferenceResolver {
205207
/// - language: The resolved language.
206208
/// - availableLanguages: The languages where the resolved node is available.
207209
/// - platforms: The platforms and their versions where the resolved node is available, if any.
208-
/// - declarationFragments: The resolved declaration fragments, if any.
210+
/// - declarationFragments: The resolved declaration fragments, if any. This is expected to be an abbreviated declaration for the symbol.
209211
/// - topicImages: Images that are used to represent the summarized element.
210212
/// - references: References used in the content of the summarized element.
211213
/// - variants: The variants of content for this resolver information.
@@ -259,6 +261,8 @@ extension OutOfProcessReferenceResolver {
259261
public let language: VariantValue<SourceLanguage>
260262
/// The declaration fragments of the variant or `nil` if the declaration is the same as the resolved information.
261263
///
264+
/// This is expected to be an abbreviated declaration for the symbol.
265+
///
262266
/// If the resolver information has a declaration but the variant doesn't, this property will be `Optional.some(nil)`.
263267
public let declarationFragments: VariantValue<DeclarationFragments?>
264268

@@ -271,7 +275,7 @@ extension OutOfProcessReferenceResolver {
271275
/// - title: The resolved title
272276
/// - abstract: The resolved (plain text) abstract.
273277
/// - language: The resolved language.
274-
/// - declarationFragments: The resolved declaration fragments, if any.
278+
/// - declarationFragments: The resolved declaration fragments, if any. This is expected to be an abbreviated declaration for the symbol.
275279
public init(
276280
traits: [RenderNode.Variant.Trait],
277281
kind: VariantValue<DocumentationNode.Kind> = nil,

Sources/SwiftDocC/Infrastructure/Link Resolution/ExternalPathHierarchyResolver.swift

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,13 @@ final class ExternalPathHierarchyResolver {
5858
return collidingNode.name
5959
}
6060
if let symbolID = collidingNode.symbol?.identifier {
61-
if symbolID.interfaceLanguage == summary.language.id, let fragments = summary.declarationFragments {
62-
return fragments.plainTextDeclaration()
61+
if symbolID.interfaceLanguage == summary.language.id, let plainTextDeclaration = summary.plainTextDeclaration {
62+
return plainTextDeclaration
6363
}
6464
if let variant = summary.variants.first(where: { $0.traits.contains(.interfaceLanguage(symbolID.interfaceLanguage)) }),
65-
let fragments = variant.declarationFragments ?? summary.declarationFragments
65+
let plainTextDeclaration = variant.plainTextDeclaration ?? summary.plainTextDeclaration
6666
{
67-
return fragments.plainTextDeclaration()
67+
return plainTextDeclaration
6868
}
6969
}
7070
return summary.title
@@ -153,12 +153,6 @@ final class ExternalPathHierarchyResolver {
153153
}
154154
}
155155

156-
private extension Sequence<DeclarationRenderSection.Token> {
157-
func plainTextDeclaration() -> String {
158-
return self.map(\.text).joined().split(whereSeparator: { $0.isWhitespace || $0.isNewline }).joined(separator: " ")
159-
}
160-
}
161-
162156
// MARK: ExternalEntity
163157

164158
extension LinkDestinationSummary {
@@ -177,7 +171,8 @@ extension LinkDestinationSummary {
177171

178172
var titleVariants = VariantCollection(defaultValue: title)
179173
var abstractVariants = VariantCollection(defaultValue: abstract ?? [])
180-
var fragmentVariants = VariantCollection(defaultValue: declarationFragments)
174+
var fragmentVariants = VariantCollection(defaultValue: subheadingDeclarationFragments)
175+
var navigatorTitleVariants = VariantCollection(defaultValue: navigatorDeclarationFragments)
181176

182177
for variant in variants {
183178
let traits = variant.traits
@@ -187,9 +182,12 @@ extension LinkDestinationSummary {
187182
if let abstract = variant.abstract {
188183
abstractVariants.variants.append(.init(traits: traits, patch: [.replace(value: abstract ?? [])]))
189184
}
190-
if let fragment = variant.declarationFragments {
185+
if let fragment = variant.subheadingDeclarationFragments {
191186
fragmentVariants.variants.append(.init(traits: traits, patch: [.replace(value: fragment)]))
192187
}
188+
if let navigatorTitle = variant.navigatorDeclarationFragments {
189+
navigatorTitleVariants.variants.append(.init(traits: traits, patch: [.replace(value: navigatorTitle)]))
190+
}
193191
}
194192

195193
return TopicRenderReference(
@@ -201,7 +199,7 @@ extension LinkDestinationSummary {
201199
required: false,
202200
role: role,
203201
fragmentsVariants: fragmentVariants,
204-
navigatorTitleVariants: .init(defaultValue: nil),
202+
navigatorTitleVariants: navigatorTitleVariants,
205203
estimatedTime: nil,
206204
conformance: nil,
207205
isBeta: isBeta,

Sources/SwiftDocC/Infrastructure/Link Resolution/LinkResolver+NavigatorIndex.swift

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,13 @@ package struct ExternalRenderNode {
6969
topicRenderReference.navigatorTitleVariants
7070
}
7171

72+
/// The variants of the abbreviated declaration of the symbol to display in links and fall-back to in navigation.
73+
///
74+
/// This value is `nil` if the referenced page is not a symbol.
75+
var fragmentsVariants: VariantCollection<[DeclarationRenderSection.Token]?> {
76+
topicRenderReference.fragmentsVariants
77+
}
78+
7279
/// Author provided images that represent this page.
7380
var images: [TopicImage] {
7481
entity.topicImages ?? []
@@ -129,7 +136,8 @@ struct NavigatorExternalRenderNode: NavigatorIndexableRenderNodeRepresentation {
129136
role: renderNode.role,
130137
symbolKind: renderNode.symbolKind?.renderingIdentifier,
131138
images: renderNode.images,
132-
isBeta: renderNode.isBeta
139+
isBeta: renderNode.isBeta,
140+
fragments: renderNode.fragmentsVariants.value(for: traits)
133141
)
134142
}
135143
}
@@ -143,19 +151,16 @@ struct ExternalRenderNodeMetadataRepresentation: NavigatorIndexableRenderMetadat
143151
var symbolKind: String?
144152
var images: [TopicImage]
145153
var isBeta: Bool
154+
var fragments: [DeclarationRenderSection.Token]?
146155

147156
// Values that we have insufficient information to derive.
148157
// These are needed to conform to the navigator indexable metadata protocol.
149158
//
150-
// The fragments that we get as part of the external link are the full declaration fragments.
151-
// These are too verbose for the navigator, so instead of using them, we rely on the title, navigator title and symbol kind instead.
152-
//
153159
// The role heading is used to identify Property Lists.
154160
// The value being missing is used for computing the final navigator title.
155161
//
156162
// The platforms are used for generating the availability index,
157163
// but doesn't affect how the node is rendered in the sidebar.
158-
var fragments: [DeclarationRenderSection.Token]? = nil
159164
var roleHeading: String? = nil
160165
var platforms: [AvailabilityRenderItem]? = nil
161166
}

0 commit comments

Comments
 (0)