Skip to content

Add successfully resolved external references to reference index #582

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

Merged
Merged
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
3 changes: 3 additions & 0 deletions Sources/SwiftDocC/Infrastructure/DocumentationContext.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2442,6 +2442,9 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
for reference in knownIdentifiers {
referenceIndex[reference.absoluteString] = reference
}
for case .success(let reference) in externallyResolvedLinks.values {
referenceIndex[reference.absoluteString] = reference
}
for reference in nodeAnchorSections.keys {
referenceIndex[reference.absoluteString] = reference
}
Expand Down
6 changes: 6 additions & 0 deletions Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,16 @@ struct RenderContentCompiler: MarkupVisitor {

mutating func resolveTopicReference(_ destination: String) -> ResolvedTopicReference? {
if let cached = context.referenceIndex[destination] {
if let node = context.topicGraph.nodeWithReference(cached), !context.topicGraph.isLinkable(node.reference) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the change here needed? Was it just missing before?

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 thought that it was needed because the code below does it but now that I think about it I also think that the link resolution would have already done this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, only MarkupReferenceResolver filters references that are linkable so I guess it's possible for the context to have resolved and saved a reference that isn't linkable.

So in order for the cached and non-cached code paths to behave the same, the cached code path should also check that the reference is linkable.

return nil
}
collectedTopicReferences.append(cached)
return cached
}

// FIXME: Links from this build already exist in the reference index and don't need to be resolved again.
// https://github.com/apple/swift-docc/issues/581

guard let validatedURL = ValidatedURL(parsingAuthoredLink: destination) else {
return nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import XCTest
@testable import SwiftDocC
import Markdown
import SymbolKit
import SwiftDocCTestUtilities

class ExternalReferenceResolverTests: XCTestCase {
class TestExternalReferenceResolver: ExternalReferenceResolver, FallbackReferenceResolver {
Expand Down Expand Up @@ -399,6 +400,66 @@ Document @1:1-1:35
])
}

func testExternalReferenceWithDifferentResolvedPath() throws {
let externalResolver = TestExternalReferenceResolver()
externalResolver.bundleIdentifier = "com.test.external"
// Return a different path for this resolved reference
externalResolver.expectedReferencePath = "/path/to/externally-resolved-symbol"
externalResolver.resolvedEntityTitle = "ClassName"
externalResolver.resolvedEntityKind = .class

let tempFolder = try createTempFolder(content: [
Folder(name: "SingleArticleWithExternalLink.docc", content: [
TextFile(name: "article.md", utf8Content: """
# Article with external link

@Metadata {
@TechnologyRoot
}

Link to an external page: <doc://com.test.external/path/to/external/symbol>
""")
])
])

let workspace = DocumentationWorkspace()
let context = try DocumentationContext(dataProvider: workspace)
context.externalReferenceResolvers = [externalResolver.bundleIdentifier: externalResolver]
let dataProvider = try LocalFileSystemDataProvider(rootURL: tempFolder)
try workspace.registerProvider(dataProvider)
let bundle = try XCTUnwrap(workspace.bundles.first?.value)

let converter = DocumentationNodeConverter(bundle: bundle, context: context)
let node = try context.entity(with: ResolvedTopicReference(bundleIdentifier: bundle.identifier, path: "/documentation/article", sourceLanguage: .swift))

guard let fileURL = context.documentURL(for: node.reference) else {
XCTFail("Unable to find the file for \(node.reference.path)")
return
}

let renderNode = try converter.convert(node, at: fileURL)

XCTAssertEqual(externalResolver.resolvedExternalPaths, ["/path/to/external/symbol"], "The authored link was resolved")

// Verify that the article contains the external reference
guard let symbolRenderReference = renderNode.references["doc://com.test.external/path/to/externally-resolved-symbol"] as? TopicRenderReference else {
XCTFail("The external reference should be resolved and included among the article's references.")
return
}

XCTAssertEqual(symbolRenderReference.identifier.identifier, "doc://com.test.external/path/to/externally-resolved-symbol")
XCTAssertEqual(symbolRenderReference.title, "ClassName")
XCTAssertEqual(symbolRenderReference.url, "/example/path/to/externally-resolved-symbol") // External references in topic groups use relative URLs
XCTAssertEqual(symbolRenderReference.kind, .symbol)

// Verify that the rendered abstract contains the resolved link
if case RenderInlineContent.reference(identifier: let identifier, isActive: true, overridingTitle: _, overridingTitleInlineContent: _)? = renderNode.abstract?.last {
XCTAssertEqual(identifier.identifier, "doc://com.test.external/path/to/externally-resolved-symbol")
} else {
XCTFail("Unexpected abstract content: \(renderNode.abstract ?? [])")
}
}

func testSampleCodeReferenceHasSampleCodeRole() throws {
let externalResolver = TestExternalReferenceResolver()
externalResolver.bundleIdentifier = "com.test.external"
Expand Down