Skip to content

Resolve all current code warnings except the one about NIO #1148

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.

Sign up for GitHub

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 6 commits into from
Feb 11, 2025
Merged

Resolve all current code warnings except the one about NIO #1148

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8da0e31
Revert "Revert "Resolve all current code warnings (#1143)" (#1145)"
d-ronnqvist Jan 23, 2025
cddb00e
Don't update to a new version of Swift NIO
d-ronnqvist Jan 23, 2025
9fbc5d1
Merge branch 'main' into fix-most-warnings-jan-2025
d-ronnqvist Jan 29, 2025
9d3b4ff
Merge branch 'main' into fix-most-warnings-jan-2025
d-ronnqvist Jan 30, 2025
fe89da2
Merge branch 'main' into fix-most-warnings-jan-2025
d-ronnqvist Feb 7, 2025
a5af78c
Merge branch 'main' into fix-most-warnings-jan-2025
d-ronnqvist Feb 11, 2025
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
10 changes: 7 additions & 3 deletions Sources/SwiftDocC/Indexing/Navigator/NavigatorIndex.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,11 @@ extension NavigatorIndex {

/// The data provider.
@available(*, deprecated, message: "This deprecated API will be removed after 6.2 is released")
public let renderNodeProvider: RenderNodeProvider?
public var renderNodeProvider: RenderNodeProvider? {
_renderNodeProvider as! RenderNodeProvider?
}
// This property only exist to be able to assign `nil` to `renderNodeProvider` in the new initializer without causing a deprecation warning.
private let _renderNodeProvider: Any?
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the comment here; maybe also explain the use of Any is required to avoid the warning as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For code that's scheduled to be removed I feel that this is fine. The general oddity of the declaration is explained by the comment above and anyone who tries to change the type will notice that doing so reintroduces the deprecation warning.


/// The documentation archive to build an index from.
public let archiveURL: URL?
Expand Down Expand Up @@ -575,7 +579,7 @@ extension NavigatorIndex {
/// - usePageTitle: Configure the builder to use the "page title" instead of the "navigator title" as the title for each entry.
public init(archiveURL: URL? = nil, outputURL: URL, bundleIdentifier: String, sortRootChildrenByName: Bool = false, groupByLanguage: Bool = false, writePathsOnDisk: Bool = true, usePageTitle: Bool = false) {
self.archiveURL = archiveURL
self.renderNodeProvider = nil
self._renderNodeProvider = nil
self.outputURL = outputURL
self.bundleIdentifier = bundleIdentifier
self.sortRootChildrenByName = sortRootChildrenByName
Expand All @@ -587,7 +591,7 @@ extension NavigatorIndex {
@available(*, deprecated, renamed: "init(archiveURL:outputURL:bundleIdentifier:sortRootChildrenByName:groupByLanguage:writePathsOnDisk:usePageTitle:)", message: "Use 'init(archiveURL:outputURL:bundleIdentifier:sortRootChildrenByName:groupByLanguage:writePathsOnDisk:usePageTitle:)' instead. This deprecated API will be removed after 6.2 is released")
@_disfavoredOverload
public init(renderNodeProvider: RenderNodeProvider? = nil, outputURL: URL, bundleIdentifier: String, sortRootChildrenByName: Bool = false, groupByLanguage: Bool = false, writePathsOnDisk: Bool = true, usePageTitle: Bool = false) {
self.renderNodeProvider = renderNodeProvider
self._renderNodeProvider = renderNodeProvider
self.archiveURL = nil
self.outputURL = outputURL
self.bundleIdentifier = bundleIdentifier
Expand Down
5 changes: 5 additions & 0 deletions Sources/SwiftDocC/Indexing/Navigator/NavigatorTree.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,12 @@ public class NavigatorTree {

func __read() {
let deadline = DispatchTime.now() + timeout
#if swift(>=5.10)
// Access to this local variable is synchronized using the DispatchQueue `queue`, passed as an argument.
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the explanation here.

nonisolated(unsafe) var processedNodes = [NavigatorTree.Node]()
#else
var processedNodes = [NavigatorTree.Node]()
#endif

while readingCursor.cursor < readingCursor.data.count {
let length = MemoryLayout<UInt32>.stride
Expand Down
16 changes: 7 additions & 9 deletions Sources/SwiftDocCTestUtilities/XCTestCase+TemporaryDirectory.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,8 @@ public extension XCTestCase {
///
/// - Parameters:
/// - pathComponents: The name of the temporary directory.
/// - fileManager: The file manager that will create the directory.
/// - Returns: The URL of the newly created directory.
func createTemporaryDirectory(named: String, fileManager: FileManager = .default) throws -> URL {
func createTemporaryDirectory(named: String) throws -> URL {
try createTemporaryDirectory(pathComponents: named)
}

Expand All @@ -48,9 +47,8 @@ public extension XCTestCase {
///
/// - Parameters:
/// - pathComponents: Additional path components to add to the temporary URL.
/// - fileManager: The file manager that will create the directory.
/// - Returns: The URL of the newly created directory.
func createTemporaryDirectory(pathComponents: String..., fileManager: FileManager = .default) throws -> URL {
func createTemporaryDirectory(pathComponents: String...) throws -> URL {
let bundleParentDir = Bundle(for: Self.self).bundleURL.deletingLastPathComponent()
let baseURL = bundleParentDir.appendingPathComponent(name.replacingWhitespaceAndPunctuation(with: "-"))

Expand All @@ -62,20 +60,20 @@ public extension XCTestCase {

addTeardownBlock {
do {
if fileManager.fileExists(atPath: baseURL.path) {
try fileManager.removeItem(at: baseURL)
if FileManager.default.fileExists(atPath: baseURL.path) {
try FileManager.default.removeItem(at: baseURL)
}
} catch {
XCTFail("Failed to remove temporary directory: '\(error)'")
}
}

if !fileManager.fileExists(atPath: bundleParentDir.path) {
if !FileManager.default.fileExists(atPath: bundleParentDir.path) {
// Create the base URL directory without intermediate directories so that an error is raised if the parent directory doesn't exist.
try fileManager.createDirectory(at: baseURL, withIntermediateDirectories: false, attributes: nil)
try FileManager.default.createDirectory(at: baseURL, withIntermediateDirectories: false, attributes: nil)
}

try fileManager.createDirectory(at: tempURL, withIntermediateDirectories: true, attributes: nil)
try FileManager.default.createDirectory(at: tempURL, withIntermediateDirectories: true, attributes: nil)

return tempURL
}
Expand Down
1 change: 1 addition & 0 deletions Sources/SwiftDocCUtilities/PreviewServer/PreviewHTTPHandler.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ final class PreviewHTTPHandler: ChannelInboundHandler {

// If we don't need to keep the connection alive, close `context` after flushing the response
if !self.keepAlive {
// When we can update to Swift NIO 2.78.0 we can call `assumeIsolated()` on the future result and avoid this 'capture of non-sendable type' warning.
promise.futureResult.whenComplete { _ in context.close(promise: nil) }
}

Expand Down
2 changes: 0 additions & 2 deletions Tests/SwiftDocCUtilitiesTests/PreviewServer/PreviewHTTPHandlerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,8 @@ class PreviewHTTPHandlerTests: XCTestCase {

let response = Response()

XCTAssertNoThrow(try channel.pipeline.addHandler(HTTPResponseEncoder()).wait())
XCTAssertNoThrow(try channel.pipeline.addHandler(response).wait())
XCTAssertNoThrow(try channel.pipeline.addHandler(channelHandler).wait())
XCTAssertNoThrow(try channel.pipeline.addHandler(HTTPServerPipelineHandler()).wait())

XCTAssertNoThrow(try channel.connect(to: SocketAddress(ipAddress: "127.0.0.1", port: 1)).wait())

Expand Down
2 changes: 0 additions & 2 deletions Tests/SwiftDocCUtilitiesTests/PreviewServer/ServerTestUtils.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,8 @@ func responseWithPipeline(request: HTTPRequestHead, handler factory: RequestHand

let response = Response()

XCTAssertNoThrow(try channel.pipeline.addHandler(HTTPResponseEncoder()).wait(), file: (file), line: line)
XCTAssertNoThrow(try channel.pipeline.addHandler(response).wait(), file: (file), line: line)
XCTAssertNoThrow(try channel.pipeline.addHandler(channelHandler).wait(), file: (file), line: line)
XCTAssertNoThrow(try channel.pipeline.addHandler(HTTPServerPipelineHandler()).wait(), file: (file), line: line)

XCTAssertNoThrow(try channel.connect(to: SocketAddress(ipAddress: "127.0.0.1", port: 1)).wait(), file: (file), line: line)

Expand Down
2 changes: 1 addition & 1 deletion bin/check-source
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ for language in swift-or-c bash md-or-tutorial html docker; do
declare -a matching_files
declare -a exceptions
declare -a reader
expections=( )
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated to this PR: it might be worth documenting the purpose and features of this script a bit. It seems useful and important, but some contributors might not be aware it exists or ever run it except as part of the bin/test.sh script.

matching_files=( -name '*' )
reader=head
case "$language" in
Expand Down Expand Up @@ -136,6 +135,7 @@ EOF
cd "$here/.."
find . \
\( \! -path './.build/*' -a \
\! -path './bin/benchmark/.build/*' -a \
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this resolve a specific warning?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It resolves warnings about several files in Swift-Markdown that don't have the license header comment that DocC expects.

Currently, anyone who runs the benchmark tools (even just to read the help text) have to delete that build directory in order for bin/test to pass locally. This fixes that.

\! -name '.' -a \
\( "${matching_files[@]}" \) -a \
\( \! \( "${exceptions[@]}" \) \) \) | while read line; do
Expand Down