Skip to content

Deprecate two-step documentation context creation #1059

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 33 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
b9a3b8a
Deprecate `DocumentationConverter
d-ronnqvist Oct 15, 2024
6eaf217
Update DataProvider protocol to a be general purpose
d-ronnqvist Oct 15, 2024
0dd80c7
Update DiagnosticConsoleWriter to use DataProvider protocol
d-ronnqvist Oct 15, 2024
4caf0b6
Update LinkResolver to use DataProvider protocol
d-ronnqvist Oct 15, 2024
6e5cb19
Update one test helper to use InputProvider to discover inputs
d-ronnqvist Oct 15, 2024
476070a
Update tests to load inputs using test helpers
d-ronnqvist Oct 15, 2024
73465e7
Avoid loading same bundle twice in same test
d-ronnqvist Oct 15, 2024
ffc41a9
Avoid creating unused workspace and context in tests
d-ronnqvist Oct 15, 2024
e277b24
Use InMemoryDataProvider instead of custom type in test
d-ronnqvist Oct 15, 2024
01e02e1
Avoid force try in one test
d-ronnqvist Oct 15, 2024
0ac31ea
Move additional global checks to context configuration
d-ronnqvist Oct 15, 2024
1c8252a
Avoid copying the test bundle when the catalog content isn't modified
d-ronnqvist Oct 15, 2024
e2932f8
Update ConvertActionIndexerTests to use InputProvider to discover inputs
d-ronnqvist Oct 15, 2024
9a01653
Avoid using `_legacyDataProvider` in tests
d-ronnqvist Oct 15, 2024
4b827aa
Update tests to pass link dependency files instead of using `configur…
d-ronnqvist Oct 15, 2024
600e623
Update final test helper to use `InputProvider` to discover inputs
d-ronnqvist Oct 15, 2024
7e19a29
Use test file system for empty test bundle
d-ronnqvist Oct 15, 2024
dce5adf
Use test helper for setting feature flags
d-ronnqvist Oct 15, 2024
59d6a19
Deprecate `DocumentationWorkspace`
d-ronnqvist Oct 15, 2024
27d0685
Update BundleDiscoveryTests to use test helpers for input discovery
d-ronnqvist Oct 15, 2024
33eafe1
Remove unused data provider parameter from convert action initializer
d-ronnqvist Oct 15, 2024
373246f
Stop conforming test file system to workspace data provider
d-ronnqvist Oct 15, 2024
9f39d67
Deprecate `GeneratedDataProvider`
d-ronnqvist Oct 15, 2024
d372098
Deprecate `PrebuiltLocalFileSystemDataProvider`
d-ronnqvist Oct 15, 2024
7b32c5d
Remove unused SymbolGraphLoader initializer
d-ronnqvist Oct 15, 2024
bdaf112
Indicate in deprecated tests when they can be removed
d-ronnqvist Oct 15, 2024
9a4f86a
Remove out-of-date deprecation warning for already removed code
d-ronnqvist Oct 15, 2024
5f5c960
Deprecate `DocumentationWorkspaceDataProvider`
d-ronnqvist Oct 15, 2024
9c2d754
Deprecate `DocumentationContextDataProvider`
d-ronnqvist Oct 15, 2024
d16f146
Deprecate additional multi-bundle-related properties
d-ronnqvist Oct 15, 2024
bc9a2b5
Merge branch 'main' into deprecate-workspace
d-ronnqvist Oct 24, 2024
0d28c07
Add comments to deprecated tests explaining that they aren't skipped
d-ronnqvist Oct 25, 2024
84b0dc0
Update comment about why unexpected errors and turned into diagnostic…
d-ronnqvist Oct 25, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ extension ConvertService {
),
InMemoryDataProvider(
files: files,
fallbackFileManager: FileManager.default
fallback: FileManager.default
)
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ public struct ConvertService: DocumentationService {
}

let bundle: DocumentationBundle
let dataProvider: DocumentationBundleDataProvider
let dataProvider: DataProvider

let inputProvider = DocumentationContext.InputsProvider()
if let bundleLocation = request.bundleLocation,
Expand Down Expand Up @@ -267,7 +267,7 @@ public struct ConvertService: DocumentationService {
.compactMap { (value, isDocumentationExtensionContent) -> (ResolvedTopicReference, RenderReferenceStore.TopicContent)? in
let (topicReference, article) = value

guard let bundle = context.bundle(identifier: topicReference.bundleIdentifier) else { return nil }
guard let bundle = context.bundle, bundle.identifier == topicReference.bundleIdentifier else { return nil }
let renderer = DocumentationContentRenderer(documentationContext: context, bundle: bundle)

let documentationNodeKind: DocumentationNode.Kind = isDocumentationExtensionContent ? .unknownSymbol : .article
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,5 +94,15 @@ extension DocumentationContext {
/// Controls whether the context stores the set of references that are manually curated.
package var shouldStoreManuallyCuratedReferences: Bool = false
}

// MARK: Topic analysis

/// Configuration related to topic analysis.
var topicAnalysisConfiguration = TopicAnalysisConfiguration()

/// A collection of configuration related to topic analysis.
struct TopicAnalysisConfiguration {
var additionalChecks: [DocumentationContext.ReferenceCheck] = []
}
}
}
8 changes: 6 additions & 2 deletions Sources/SwiftDocC/Infrastructure/ConvertActionConverter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,12 @@ package enum ConvertActionConverter {

// An inner function to gather problems for errors encountered during the conversion.
//
// These problems only represent unexpected thrown errors and aren't particularly user-facing but because
// `DocumentationConverter.convert(outputConsumer:)` emits them as diagnostics we do the same here.
// These problems only represent unexpected thrown errors and aren't particularly user-facing.
// For now we emit them as diagnostics because `DocumentationConverter.convert(outputConsumer:)` (which this replaced) used to do that.
//
// FIXME: In the future we could simplify this control flow by not catching these errors and turning them into diagnostics.
// Since both error-level diagnostics and thrown errors fail the documentation build,
// the only practical different this would have is that we stop on the first unexpected error instead of processing all pages and gathering all unexpected errors.
func recordProblem(from error: Swift.Error, in problems: inout [Problem], withIdentifier identifier: String) {
let problem = Problem(diagnostic: Diagnostic(
severity: .error,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,23 +33,23 @@ public final class DiagnosticConsoleWriter: DiagnosticFormattingConsumer {
baseURL: URL? = nil,
highlight: Bool? = nil
) {
self.init(stream, formattingOptions: options, baseURL: baseURL, highlight: highlight, fileManager: FileManager.default)
self.init(stream, formattingOptions: options, baseURL: baseURL, highlight: highlight, dataProvider: FileManager.default)
}

package init(
_ stream: TextOutputStream = LogHandle.standardError,
formattingOptions options: DiagnosticFormattingOptions = [],
baseURL: URL? = nil,
highlight: Bool? = nil,
fileManager: FileManagerProtocol = FileManager.default
dataProvider: DataProvider = FileManager.default
) {
outputStream = stream
formattingOptions = options
diagnosticFormatter = Self.makeDiagnosticFormatter(
options,
baseURL: baseURL,
highlight: highlight ?? TerminalHelper.isConnectedToTerminal,
fileManager: fileManager
dataProvider: dataProvider
)
}

Expand Down Expand Up @@ -79,12 +79,12 @@ public final class DiagnosticConsoleWriter: DiagnosticFormattingConsumer {
_ options: DiagnosticFormattingOptions,
baseURL: URL?,
highlight: Bool,
fileManager: FileManagerProtocol
dataProvider: DataProvider
) -> DiagnosticConsoleFormatter {
if options.contains(.formatConsoleOutputForTools) {
return IDEDiagnosticConsoleFormatter(options: options)
} else {
return DefaultDiagnosticConsoleFormatter(baseUrl: baseURL, highlight: highlight, options: options, fileManager: fileManager)
return DefaultDiagnosticConsoleFormatter(baseUrl: baseURL, highlight: highlight, options: options, dataProvider: dataProvider)
}
}
}
Expand All @@ -103,15 +103,15 @@ extension DiagnosticConsoleWriter {
formattedDescription(for: problem, options: options, fileManager: FileManager.default)
}
package static func formattedDescription(for problem: Problem, options: DiagnosticFormattingOptions = [], fileManager: FileManagerProtocol = FileManager.default) -> String {
let diagnosticFormatter = makeDiagnosticFormatter(options, baseURL: nil, highlight: TerminalHelper.isConnectedToTerminal, fileManager: fileManager)
let diagnosticFormatter = makeDiagnosticFormatter(options, baseURL: nil, highlight: TerminalHelper.isConnectedToTerminal, dataProvider: fileManager)
return diagnosticFormatter.formattedDescription(for: problem)
}

public static func formattedDescription(for diagnostic: Diagnostic, options: DiagnosticFormattingOptions = []) -> String {
formattedDescription(for: diagnostic, options: options, fileManager: FileManager.default)
}
package static func formattedDescription(for diagnostic: Diagnostic, options: DiagnosticFormattingOptions = [], fileManager: FileManagerProtocol) -> String {
let diagnosticFormatter = makeDiagnosticFormatter(options, baseURL: nil, highlight: TerminalHelper.isConnectedToTerminal, fileManager: fileManager)
let diagnosticFormatter = makeDiagnosticFormatter(options, baseURL: nil, highlight: TerminalHelper.isConnectedToTerminal, dataProvider: fileManager)
return diagnosticFormatter.formattedDescription(for: diagnostic)
}
}
Expand Down Expand Up @@ -220,7 +220,7 @@ final class DefaultDiagnosticConsoleFormatter: DiagnosticConsoleFormatter {
private let baseUrl: URL?
private let highlight: Bool
private var sourceLines: [URL: [String]] = [:]
private var fileManager: FileManagerProtocol
private var dataProvider: DataProvider

/// The number of additional lines from the source file that should be displayed both before and after the diagnostic source line.
private static let contextSize = 2
Expand All @@ -229,12 +229,12 @@ final class DefaultDiagnosticConsoleFormatter: DiagnosticConsoleFormatter {
baseUrl: URL?,
highlight: Bool,
options: DiagnosticFormattingOptions,
fileManager: FileManagerProtocol
dataProvider: DataProvider
) {
self.baseUrl = baseUrl
self.highlight = highlight
self.options = options
self.fileManager = fileManager
self.dataProvider = dataProvider
}

func formattedDescription(for problems: some Sequence<Problem>) -> String {
Expand Down Expand Up @@ -507,7 +507,7 @@ extension DefaultDiagnosticConsoleFormatter {
}

// TODO: Add support for also getting the source lines from the symbol graph files.
guard let data = fileManager.contents(atPath: url.path),
guard let data = try? dataProvider.contents(of: url),
let content = String(data: data, encoding: .utf8)
else {
return []
Expand Down
48 changes: 36 additions & 12 deletions Sources/SwiftDocC/Infrastructure/DocumentationContext.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import Markdown
import SymbolKit

/// A type that provides information about documentation bundles and their content.
@available(*, deprecated, message: "Pass the context its inputs at initialization instead. This deprecated API will be removed after 6.2 is released")
public protocol DocumentationContextDataProvider {
/// An object to notify when bundles are added or removed.
var delegate: DocumentationContextDataProviderDelegate? { get set }
Expand All @@ -31,6 +32,7 @@ public protocol DocumentationContextDataProvider {
}

/// An object that responds to changes in available documentation bundles for a specific provider.
@available(*, deprecated, message: "Pass the context its inputs at initialization instead. This deprecated API will be removed after 6.2 is released")
public protocol DocumentationContextDataProviderDelegate: AnyObject {

/// Called when the `dataProvider` has added a new documentation bundle to its list of `bundles`.
Expand Down Expand Up @@ -79,7 +81,7 @@ public typealias BundleIdentifier = String
/// - ``children(of:kind:)``
/// - ``parents(of:)``
///
public class DocumentationContext: DocumentationContextDataProviderDelegate {
public class DocumentationContext {

/// An error that's encountered while interacting with a ``SwiftDocC/DocumentationContext``.
public enum ContextError: DescribedError {
Expand Down Expand Up @@ -111,16 +113,18 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
}

/// A class that resolves documentation links by orchestrating calls to other link resolver implementations.
public var linkResolver = LinkResolver()
public var linkResolver: LinkResolver

private enum _Provider {
@available(*, deprecated, message: "Use 'DataProvider' instead. This deprecated API will be removed after 6.2 is released")
case legacy(DocumentationContextDataProvider)
case new(DocumentationBundleDataProvider)
case new(DataProvider)
}
private var dataProvider: _Provider

/// The provider of documentation bundles for this context.
var _legacyDataProvider: DocumentationContextDataProvider! {
@available(*, deprecated, message: "Use 'DataProvider' instead. This deprecated API will be removed after 6.2 is released")
private var _legacyDataProvider: DocumentationContextDataProvider! {
get {
switch dataProvider {
case .legacy(let legacyDataProvider):
Expand All @@ -144,6 +148,7 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
}
}

/// The documentation bundle that is registered with the context.
var bundle: DocumentationBundle?

/// A collection of configuration for this context.
Expand Down Expand Up @@ -285,6 +290,7 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
/// - diagnosticEngine: The pre-configured engine that will collect problems encountered during compilation.
/// - configuration: A collection of configuration for the created context.
/// - Throws: If an error is encountered while registering a documentation bundle.
@available(*, deprecated, message: "Pass the context its inputs at initialization instead. This deprecated API will be removed after 6.2 is released")
public init(
dataProvider: DocumentationContextDataProvider,
diagnosticEngine: DiagnosticEngine = .init(),
Expand All @@ -293,6 +299,7 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
self.dataProvider = .legacy(dataProvider)
self.diagnosticEngine = diagnosticEngine
self._configuration = configuration
self.linkResolver = LinkResolver(dataProvider: FileManager.default)

_legacyDataProvider.delegate = self

Expand All @@ -311,14 +318,15 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
/// - Throws: If an error is encountered while registering a documentation bundle.
package init(
bundle: DocumentationBundle,
dataProvider: DocumentationBundleDataProvider,
dataProvider: DataProvider,
diagnosticEngine: DiagnosticEngine = .init(),
configuration: Configuration = .init()
) throws {
self.bundle = bundle
self.dataProvider = .new(dataProvider)
self.diagnosticEngine = diagnosticEngine
self._configuration = configuration
self.linkResolver = LinkResolver(dataProvider: dataProvider)

ResolvedTopicReference.enableReferenceCaching(for: bundle.identifier)
try register(bundle)
Expand All @@ -329,6 +337,7 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
/// - Parameters:
/// - dataProvider: The provider that added this bundle.
/// - bundle: The bundle that was added.
@available(*, deprecated, message: "Pass the context its inputs at initialization instead. This deprecated API will be removed after 6.2 is released")
public func dataProvider(_ dataProvider: DocumentationContextDataProvider, didAddBundle bundle: DocumentationBundle) throws {
try benchmark(wrap: Benchmark.Duration(id: "bundle-registration")) {
// Enable reference caching for this documentation bundle.
Expand All @@ -343,6 +352,7 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
/// - Parameters:
/// - dataProvider: The provider that removed this bundle.
/// - bundle: The bundle that was removed.
@available(*, deprecated, message: "Pass the context its inputs at initialization instead. This deprecated API will be removed after 6.2 is released")
public func dataProvider(_ dataProvider: DocumentationContextDataProvider, didRemoveBundle bundle: DocumentationBundle) throws {
linkResolver.localResolver?.unregisterBundle(identifier: bundle.identifier)

Expand All @@ -354,7 +364,20 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
}

/// The documentation bundles that are currently registered with the context.
@available(*, deprecated, message: "Use 'bundle' instead. This deprecated API will be removed after 6.2 is released")
public var registeredBundles: some Collection<DocumentationBundle> {
_registeredBundles
}

/// Returns the `DocumentationBundle` with the given `identifier` if it's registered with the context, otherwise `nil`.
@available(*, deprecated, message: "Use 'bundle' instead. This deprecated API will be removed after 6.2 is released")
public func bundle(identifier: String) -> DocumentationBundle? {
_bundle(identifier: identifier)
}

// Remove these when removing `registeredBundles` and `bundle(identifier:)`.
// These exist so that internal code that need to be compatible with legacy data providers can access the bundles without deprecation warnings.
var _registeredBundles: [DocumentationBundle] {
switch dataProvider {
case .legacy(let legacyDataProvider):
Array(legacyDataProvider.bundles.values)
Expand All @@ -363,8 +386,7 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
}
}

/// Returns the `DocumentationBundle` with the given `identifier` if it's registered with the context, otherwise `nil`.
public func bundle(identifier: String) -> DocumentationBundle? {
func _bundle(identifier: String) -> DocumentationBundle? {
switch dataProvider {
case .legacy(let legacyDataProvider):
return legacyDataProvider.bundles[identifier]
Expand Down Expand Up @@ -2551,14 +2573,13 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {

/// A closure type getting the information about a reference in a context and returns any possible problems with it.
public typealias ReferenceCheck = (DocumentationContext, ResolvedTopicReference) -> [Problem]

private var checks: [ReferenceCheck] = []

/// Adds new checks to be run during the global topic analysis; after a bundle has been fully registered and its topic graph has been fully built.
///
/// - Parameter newChecks: The new checks to add.
@available(*, deprecated, message: "Use 'TopicAnalysisConfiguration.additionalChecks' instead. This deprecated API will be removed after 6.2 is released")
public func addGlobalChecks(_ newChecks: [ReferenceCheck]) {
checks.append(contentsOf: newChecks)
configuration.topicAnalysisConfiguration.additionalChecks.append(contentsOf: newChecks)
}

/// Crawls the hierarchy of the given list of nodes, adding relationships in the topic graph for all resolvable task group references.
Expand Down Expand Up @@ -2624,7 +2645,7 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
func topicGraphGlobalAnalysis() {
// Run any checks added to the context.
let problems = knownIdentifiers.flatMap { reference in
return checks.flatMap { check in
return configuration.topicAnalysisConfiguration.additionalChecks.flatMap { check in
return check(self, reference)
}
}
Expand Down Expand Up @@ -2672,7 +2693,7 @@ public class DocumentationContext: DocumentationContextDataProviderDelegate {
- Throws: ``ContextError/notFound(_:)` if a resource with the given was not found.
*/
public func resource(with identifier: ResourceReference, trait: DataTraitCollection = .init()) throws -> Data {
guard let bundle = bundle(identifier: identifier.bundleIdentifier),
guard let bundle,
let assetManager = assetManagers[identifier.bundleIdentifier],
let asset = assetManager.allData(named: identifier.path) else {
throw ContextError.notFound(identifier.url)
Expand Down Expand Up @@ -3055,3 +3076,6 @@ extension DataAsset {
}
}
}

@available(*, deprecated, message: "This deprecated API will be removed after 6.2 is released")
extension DocumentationContext: DocumentationContextDataProviderDelegate {}
2 changes: 2 additions & 0 deletions Sources/SwiftDocC/Infrastructure/DocumentationConverter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import Foundation
/// ## See Also
///
/// - ``DocumentationConverter``
@available(*, deprecated, message: "This deprecated API will be removed after 6.2 is released")
public protocol DocumentationConverterProtocol {
/// Converts documentation, outputting products using the given output consumer.
/// - Parameter outputConsumer: The output consumer for content produced during conversion.
Expand All @@ -37,6 +38,7 @@ public protocol DocumentationConverterProtocol {
///
/// You can also configure the documentation converter to emit extra metadata such as linkable entities and indexing records
/// information.
@available(*, deprecated, message: "This deprecated API will be removed after 6.2 is released")
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed a comment near ConvertActionConverter.convert.recordProblem mentions DocumentationConverter - we might want to remove or update that:

These problems only represent unexpected thrown errors and aren't particularly user-facing but because
DocumentationConverter.convert(outputConsumer:) emits them as diagnostics we do the same here.

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 rephrased this comment in 84b0dc0 to add a future action to change it while still mentioning that this current behavior came from DocumentationConverter in case the reader wonders why the code does this today.

public struct DocumentationConverter: DocumentationConverterProtocol {
let rootURL: URL?
let emitDigest: Bool
Expand Down
Loading