Skip to content

Commit

Permalink
fix remaining warnings & enable -warnings-as-errors in CI (#3000)
Browse files Browse the repository at this point in the history
### Motivation:

Warnings are annoying.

### Modifications:

- Remove unnecessary use of `Foundation.Thread` which isn't Sendable.
- Remove now unnecessary `@retroactive`s.
- Enable `-warnings-as-errors` in CI

### Result:

- Warnings can't sneak in as easily anymore
- No more warnings left in `swift-nio`

```
$ rm -rf .build/arm64-apple-macosx/ && swift build --build-tests -Xswiftc -warnings-as-errors > /dev/null
echo $?
$ echo $?
0
```

Co-authored-by: Cory Benfield <[email protected]>
  • Loading branch information
weissi and Lukasa authored Nov 26, 2024
1 parent d2cc4d7 commit 49b9d97
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 34 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ jobs:
with:
linux_5_9_arguments_override: "-Xswiftc -warnings-as-errors --explicit-target-dependency-import-check error"
linux_5_10_arguments_override: "-Xswiftc -warnings-as-errors --explicit-target-dependency-import-check error"
linux_6_0_arguments_override: "--explicit-target-dependency-import-check error"
linux_nightly_6_0_arguments_override: "--explicit-target-dependency-import-check error"
linux_6_0_arguments_override: "-Xswiftc -warnings-as-errors --explicit-target-dependency-import-check error"
linux_nightly_6_0_arguments_override: "-Xswiftc -warnings-as-errors --explicit-target-dependency-import-check error"
linux_nightly_main_arguments_override: "--explicit-target-dependency-import-check error"

cxx-interop:
Expand Down
6 changes: 0 additions & 6 deletions Sources/NIOFoundationCompat/ByteBuffer-foundation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -377,15 +377,9 @@ extension ByteBufferAllocator {
}

// MARK: - Conformances
#if compiler(>=6.0)
extension ByteBufferView: @retroactive ContiguousBytes {}
extension ByteBufferView: @retroactive DataProtocol {}
extension ByteBufferView: @retroactive MutableDataProtocol {}
#else
extension ByteBufferView: ContiguousBytes {}
extension ByteBufferView: DataProtocol {}
extension ByteBufferView: MutableDataProtocol {}
#endif

extension ByteBufferView {
public typealias Regions = CollectionOfOne<ByteBufferView>
Expand Down
2 changes: 1 addition & 1 deletion Tests/NIOPosixTests/EchoServerClientTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ class EchoServerClientTest: XCTestCase {

try withTemporaryUnixDomainSocketPathName { udsPath in
// Bootstrap should not overwrite an existing file unless it is a socket
FileManager.default.createFile(atPath: udsPath, contents: nil, attributes: nil)
_ = FileManager.default.createFile(atPath: udsPath, contents: nil, attributes: nil)
let bootstrap = ServerBootstrap(group: group)

XCTAssertThrowsError(
Expand Down
29 changes: 11 additions & 18 deletions Tests/NIOPosixTests/NIOThreadPoolTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -78,18 +78,15 @@ class NIOThreadPoolTest: XCTestCase {

// The lock here is arguably redundant with the dispatchgroup, but let's make
// this test thread-safe even if I screw up.
let lock = NIOLock()
let threadOne: NIOLockedValueBox<Thread?> = NIOLockedValueBox(Thread?.none)
let threadTwo: NIOLockedValueBox<Thread?> = NIOLockedValueBox(Thread?.none)
let threadOne: NIOLockedValueBox<NIOThread?> = NIOLockedValueBox(NIOThread?.none)
let threadTwo: NIOLockedValueBox<NIOThread?> = NIOLockedValueBox(NIOThread?.none)

completionGroup.enter()
pool.submit { s in
precondition(s == .active)
lock.withLock { () -> Void in
threadOne.withLockedValue { threadOne in
XCTAssertEqual(threadOne, nil)
threadOne = Thread.current
}
threadOne.withLockedValue { threadOne in
XCTAssertEqual(threadOne, nil)
threadOne = NIOThread.current
}
completionGroup.leave()
}
Expand All @@ -99,22 +96,18 @@ class NIOThreadPoolTest: XCTestCase {
completionGroup.enter()
pool.submit { s in
precondition(s == .active)
lock.withLock { () -> Void in
threadTwo.withLockedValue { threadTwo in
XCTAssertEqual(threadTwo, nil)
threadTwo = Thread.current
}
threadTwo.withLockedValue { threadTwo in
XCTAssertEqual(threadTwo, nil)
threadTwo = NIOThread.current
}
completionGroup.leave()
}

completionGroup.wait()

lock.withLock { () -> Void in
XCTAssertNotNil(threadOne)
XCTAssertNotNil(threadTwo)
XCTAssertEqual(threadOne.withLockedValue { $0 }, threadTwo.withLockedValue { $0 })
}
XCTAssertNotNil(threadOne.withLockedValue { $0 })
XCTAssertNotNil(threadTwo.withLockedValue { $0 })
XCTAssertEqual(threadOne.withLockedValue { $0 }, threadTwo.withLockedValue { $0 })
}

func testAsyncThreadPool() async throws {
Expand Down
6 changes: 1 addition & 5 deletions Tests/NIOPosixTests/SALEventLoopTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,12 @@ final class SALEventLoopTests: XCTestCase, SALTest {
}

// Now execute 10 tasks.
var i = 0
for _ in 0..<10 {
thisLoop.execute {
i &+= 1
}
thisLoop.execute {}
}

// Now enqueue a "last" task.
thisLoop.execute {
i &+= 1
promise.succeed(())
}

Expand Down
4 changes: 2 additions & 2 deletions Tests/NIOPosixTests/TestUtils.swift
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func withTemporaryUnixDomainSocketPathName<T>(
shortEnoughPath = path
restoreSavedCWD = false
} catch SocketAddressError.unixDomainSocketPathTooLong {
FileManager.default.changeCurrentDirectoryPath(
_ = FileManager.default.changeCurrentDirectoryPath(
URL(fileURLWithPath: path).deletingLastPathComponent().absoluteString
)
shortEnoughPath = URL(fileURLWithPath: path).lastPathComponent
Expand All @@ -141,7 +141,7 @@ func withTemporaryUnixDomainSocketPathName<T>(
try? FileManager.default.removeItem(atPath: path)
}
if restoreSavedCWD {
FileManager.default.changeCurrentDirectoryPath(saveCurrentDirectory)
_ = FileManager.default.changeCurrentDirectoryPath(saveCurrentDirectory)
}
}
return try body(shortEnoughPath)
Expand Down

0 comments on commit 49b9d97

Please sign in to comment.