diff --git a/.swift-format b/.swift-format index 7fa06fb305..e458d31e83 100644 --- a/.swift-format +++ b/.swift-format @@ -49,7 +49,7 @@ "OrderedImports" : true, "ReplaceForEachWithForLoop" : true, "ReturnVoidInsteadOfEmptyTuple" : true, - "UseEarlyExits" : false, + "UseEarlyExits" : true, "UseExplicitNilCheckInConditions" : false, "UseLetInEveryBoundCaseVariable" : false, "UseShorthandTypeNames" : true, diff --git a/IntegrationTests/tests_04_performance/test_01_resources/test_ping_pong_1000_reqs_1_conn.swift b/IntegrationTests/tests_04_performance/test_01_resources/test_ping_pong_1000_reqs_1_conn.swift index ebcad8f0b5..b75ce037eb 100644 --- a/IntegrationTests/tests_04_performance/test_01_resources/test_ping_pong_1000_reqs_1_conn.swift +++ b/IntegrationTests/tests_04_performance/test_01_resources/test_ping_pong_1000_reqs_1_conn.swift @@ -27,12 +27,11 @@ private final class PongDecoder: ByteToMessageDecoder { typealias InboundOut = UInt8 public func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) -> DecodingState { - if let ping = buffer.readInteger(as: UInt8.self) { - context.fireChannelRead(Self.wrapInboundOut(ping)) - return .continue - } else { + guard let ping = buffer.readInteger(as: UInt8.self) else { return .needMoreData } + context.fireChannelRead(Self.wrapInboundOut(ping)) + return .continue } public func decodeLast( diff --git a/Sources/NIOConcurrencyHelpers/atomics.swift b/Sources/NIOConcurrencyHelpers/atomics.swift index 1eb276c8c9..43e456584d 100644 --- a/Sources/NIOConcurrencyHelpers/atomics.swift +++ b/Sources/NIOConcurrencyHelpers/atomics.swift @@ -501,12 +501,11 @@ public final class AtomicBox { return true } else { let currentPtrBits = self.storage.load() - if currentPtrBits == 0 || currentPtrBits == expectedPtrBits { - sys_sched_yield() - continue - } else { + guard currentPtrBits == 0 || currentPtrBits == expectedPtrBits else { return false } + sys_sched_yield() + continue } } } diff --git a/Sources/NIOCore/AsyncSequences/NIOAsyncWriter.swift b/Sources/NIOCore/AsyncSequences/NIOAsyncWriter.swift index 64b74b1818..b1a24c8f09 100644 --- a/Sources/NIOCore/AsyncSequences/NIOAsyncWriter.swift +++ b/Sources/NIOCore/AsyncSequences/NIOAsyncWriter.swift @@ -1077,18 +1077,37 @@ extension NIOAsyncWriter { let delegate, let error ): - if bufferedYieldIDs.contains(yieldID) { - // This yield was buffered before we became finished so we still have to deliver it - self._state = .modifying + guard bufferedYieldIDs.contains(yieldID) else { + // We are already finished and still tried to write something + return .throwError(NIOAsyncWriterError.alreadyFinished()) + } + // This yield was buffered before we became finished so we still have to deliver it + self._state = .modifying - if let index = cancelledYields.firstIndex(of: yieldID) { - // We already marked the yield as cancelled. We have to remove it and - // throw a CancellationError. - cancelledYields.remove(at: index) + if let index = cancelledYields.firstIndex(of: yieldID) { + // We already marked the yield as cancelled. We have to remove it and + // throw a CancellationError. + cancelledYields.remove(at: index) + self._state = .writerFinished( + isWritable: isWritable, + inDelegateOutcall: inDelegateOutcall, + suspendedYields: suspendedYields, + cancelledYields: cancelledYields, + bufferedYieldIDs: bufferedYieldIDs, + delegate: delegate, + error: error + ) + + return .throwError(CancellationError()) + } else { + // Yield hasn't been marked as cancelled. + + switch (isWritable, inDelegateOutcall) { + case (true, false): self._state = .writerFinished( isWritable: isWritable, - inDelegateOutcall: inDelegateOutcall, + inDelegateOutcall: true, // We are now making a call to the delegate suspendedYields: suspendedYields, cancelledYields: cancelledYields, bufferedYieldIDs: bufferedYieldIDs, @@ -1096,39 +1115,19 @@ extension NIOAsyncWriter { error: error ) - return .throwError(CancellationError()) - } else { - // Yield hasn't been marked as cancelled. - - switch (isWritable, inDelegateOutcall) { - case (true, false): - self._state = .writerFinished( - isWritable: isWritable, - inDelegateOutcall: true, // We are now making a call to the delegate - suspendedYields: suspendedYields, - cancelledYields: cancelledYields, - bufferedYieldIDs: bufferedYieldIDs, - delegate: delegate, - error: error - ) - - return .callDidYield(delegate) - case (true, true), (false, _): - self._state = .writerFinished( - isWritable: isWritable, - inDelegateOutcall: inDelegateOutcall, - suspendedYields: suspendedYields, - cancelledYields: cancelledYields, - bufferedYieldIDs: bufferedYieldIDs, - delegate: delegate, - error: error - ) - return .suspendTask - } + return .callDidYield(delegate) + case (true, true), (false, _): + self._state = .writerFinished( + isWritable: isWritable, + inDelegateOutcall: inDelegateOutcall, + suspendedYields: suspendedYields, + cancelledYields: cancelledYields, + bufferedYieldIDs: bufferedYieldIDs, + delegate: delegate, + error: error + ) + return .suspendTask } - } else { - // We are already finished and still tried to write something - return .throwError(NIOAsyncWriterError.alreadyFinished()) } case .finished(let sinkError): @@ -1216,28 +1215,7 @@ extension NIOAsyncWriter { var suspendedYields, let delegate ): - if let index = suspendedYields.firstIndex(where: { $0.yieldID == yieldID }) { - self._state = .modifying - // We have a suspended yield for the id. We need to resume the continuation now. - - // Removing can be quite expensive if it produces a gap in the array. - // Since we are not expecting a lot of elements in this array it should be fine - // to just remove. If this turns out to be a performance pitfall, we can - // swap the elements before removing. So that we always remove the last element. - let suspendedYield = suspendedYields.remove(at: index) - - // We are keeping the elements that the yield produced. - self._state = .streaming( - isWritable: isWritable, - inDelegateOutcall: inDelegateOutcall, - cancelledYields: cancelledYields, - suspendedYields: suspendedYields, - delegate: delegate - ) - - return .resumeContinuationWithCancellationError(suspendedYield.continuation) - - } else { + guard let index = suspendedYields.firstIndex(where: { $0.yieldID == yieldID }) else { self._state = .modifying // There is no suspended yield. This can mean that we either already yielded // or that the call to `yield` is coming afterwards. We need to store @@ -1254,6 +1232,25 @@ extension NIOAsyncWriter { return .none } + self._state = .modifying + // We have a suspended yield for the id. We need to resume the continuation now. + + // Removing can be quite expensive if it produces a gap in the array. + // Since we are not expecting a lot of elements in this array it should be fine + // to just remove. If this turns out to be a performance pitfall, we can + // swap the elements before removing. So that we always remove the last element. + let suspendedYield = suspendedYields.remove(at: index) + + // We are keeping the elements that the yield produced. + self._state = .streaming( + isWritable: isWritable, + inDelegateOutcall: inDelegateOutcall, + cancelledYields: cancelledYields, + suspendedYields: suspendedYields, + delegate: delegate + ) + + return .resumeContinuationWithCancellationError(suspendedYield.continuation) case .writerFinished( let isWritable, @@ -1267,30 +1264,7 @@ extension NIOAsyncWriter { guard bufferedYieldIDs.contains(yieldID) else { return .none } - if let index = suspendedYields.firstIndex(where: { $0.yieldID == yieldID }) { - self._state = .modifying - // We have a suspended yield for the id. We need to resume the continuation now. - - // Removing can be quite expensive if it produces a gap in the array. - // Since we are not expecting a lot of elements in this array it should be fine - // to just remove. If this turns out to be a performance pitfall, we can - // swap the elements before removing. So that we always remove the last element. - let suspendedYield = suspendedYields.remove(at: index) - - // We are keeping the elements that the yield produced. - self._state = .writerFinished( - isWritable: isWritable, - inDelegateOutcall: inDelegateOutcall, - suspendedYields: suspendedYields, - cancelledYields: cancelledYields, - bufferedYieldIDs: bufferedYieldIDs, - delegate: delegate, - error: error - ) - - return .resumeContinuationWithCancellationError(suspendedYield.continuation) - - } else { + guard let index = suspendedYields.firstIndex(where: { $0.yieldID == yieldID }) else { self._state = .modifying // There is no suspended yield. This can mean that we either already yielded // or that the call to `yield` is coming afterwards. We need to store @@ -1309,6 +1283,27 @@ extension NIOAsyncWriter { return .none } + self._state = .modifying + // We have a suspended yield for the id. We need to resume the continuation now. + + // Removing can be quite expensive if it produces a gap in the array. + // Since we are not expecting a lot of elements in this array it should be fine + // to just remove. If this turns out to be a performance pitfall, we can + // swap the elements before removing. So that we always remove the last element. + let suspendedYield = suspendedYields.remove(at: index) + + // We are keeping the elements that the yield produced. + self._state = .writerFinished( + isWritable: isWritable, + inDelegateOutcall: inDelegateOutcall, + suspendedYields: suspendedYields, + cancelledYields: cancelledYields, + bufferedYieldIDs: bufferedYieldIDs, + delegate: delegate, + error: error + ) + + return .resumeContinuationWithCancellationError(suspendedYield.continuation) case .finished: // We are already finished and there is nothing to do @@ -1347,27 +1342,7 @@ extension NIOAsyncWriter { let delegate ): // We are currently streaming and the writer got finished. - if suspendedYields.isEmpty { - if inDelegateOutcall { - // We are in an outcall already and have to buffer - // the didTerminate call. - self._state = .writerFinished( - isWritable: isWritable, - inDelegateOutcall: inDelegateOutcall, - suspendedYields: .init(), - cancelledYields: cancelledYields, - bufferedYieldIDs: .init(), - delegate: delegate, - error: error - ) - return .none - } else { - // We have no elements left and are not in an outcall so we - // can transition to finished directly - self._state = .finished(sinkError: nil) - return .callDidTerminate(delegate) - } - } else { + guard suspendedYields.isEmpty else { // There are still suspended writer tasks which we need to deliver once we become writable again self._state = .writerFinished( isWritable: isWritable, @@ -1381,6 +1356,24 @@ extension NIOAsyncWriter { return .none } + guard inDelegateOutcall else { + // We have no elements left and are not in an outcall so we + // can transition to finished directly + self._state = .finished(sinkError: nil) + return .callDidTerminate(delegate) + } + // We are in an outcall already and have to buffer + // the didTerminate call. + self._state = .writerFinished( + isWritable: isWritable, + inDelegateOutcall: inDelegateOutcall, + suspendedYields: .init(), + cancelledYields: cancelledYields, + bufferedYieldIDs: .init(), + delegate: delegate, + error: error + ) + return .none case .writerFinished, .finished: // We are already finished and there is nothing to do @@ -1459,17 +1452,7 @@ extension NIOAsyncWriter { precondition(inDelegateOutcall, "We must be in a delegate outcall when we unbuffer events") // We have to resume the other suspended yields now. - if suspendedYields.isEmpty { - // There are no other writer suspended writer tasks so we can just return - self._state = .streaming( - isWritable: isWritable, - inDelegateOutcall: false, - cancelledYields: cancelledYields, - suspendedYields: suspendedYields, - delegate: delegate - ) - return .none - } else { + guard suspendedYields.isEmpty else { // We have to resume the other suspended yields now. self._state = .streaming( isWritable: isWritable, @@ -1480,6 +1463,15 @@ extension NIOAsyncWriter { ) return .resumeContinuations(suspendedYields) } + // There are no other writer suspended writer tasks so we can just return + self._state = .streaming( + isWritable: isWritable, + inDelegateOutcall: false, + cancelledYields: cancelledYields, + suspendedYields: suspendedYields, + delegate: delegate + ) + return .none case .writerFinished( let isWritable, @@ -1491,11 +1483,7 @@ extension NIOAsyncWriter { let error ): precondition(inDelegateOutcall, "We must be in a delegate outcall when we unbuffer events") - if suspendedYields.isEmpty { - // We were the last writer task and can now call didTerminate - self._state = .finished(sinkError: nil) - return .callDidTerminate(delegate, error) - } else { + guard suspendedYields.isEmpty else { // There are still other writer tasks that need to be resumed self._state = .modifying @@ -1511,6 +1499,9 @@ extension NIOAsyncWriter { return .resumeContinuations(suspendedYields) } + // We were the last writer task and can now call didTerminate + self._state = .finished(sinkError: nil) + return .callDidTerminate(delegate, error) case .finished: return .none diff --git a/Sources/NIOCore/AsyncSequences/NIOThrowingAsyncSequenceProducer.swift b/Sources/NIOCore/AsyncSequences/NIOThrowingAsyncSequenceProducer.swift index 7b95ee86f2..ca87c0086b 100644 --- a/Sources/NIOCore/AsyncSequences/NIOThrowingAsyncSequenceProducer.swift +++ b/Sources/NIOCore/AsyncSequences/NIOThrowingAsyncSequenceProducer.swift @@ -1197,27 +1197,7 @@ extension NIOThrowingAsyncSequenceProducer { ): self._state = .modifying - if let element = buffer.popFirst() { - // We have an element to fulfil the demand right away. - - let shouldProduceMore = backPressureStrategy.didConsume(bufferDepth: buffer.count) - - self._state = .streaming( - backPressureStrategy: backPressureStrategy, - buffer: buffer, - continuation: nil, - hasOutstandingDemand: shouldProduceMore, - iteratorInitialized: iteratorInitialized - ) - - if shouldProduceMore && !hasOutstandingDemand { - // We didn't have any demand but now we do, so we need to inform the delegate. - return .returnElementAndCallProduceMore(element) - } else { - // We don't have any new demand, so we can just return the element. - return .returnElement(element) - } - } else { + guard let element = buffer.popFirst() else { // There is nothing in the buffer to fulfil the demand so we need to suspend. // We are not interacting with the back-pressure strategy here because // we are doing this inside `next(:)` @@ -1231,25 +1211,42 @@ extension NIOThrowingAsyncSequenceProducer { return .suspendTask } + // We have an element to fulfil the demand right away. + + let shouldProduceMore = backPressureStrategy.didConsume(bufferDepth: buffer.count) + + self._state = .streaming( + backPressureStrategy: backPressureStrategy, + buffer: buffer, + continuation: nil, + hasOutstandingDemand: shouldProduceMore, + iteratorInitialized: iteratorInitialized + ) + + guard shouldProduceMore && !hasOutstandingDemand else { + // We don't have any new demand, so we can just return the element. + return .returnElement(element) + } + // We didn't have any demand but now we do, so we need to inform the delegate. + return .returnElementAndCallProduceMore(element) case .sourceFinished(var buffer, let iteratorInitialized, let failure): self._state = .modifying // Check if we have an element left in the buffer and return it - if let element = buffer.popFirst() { - self._state = .sourceFinished( - buffer: buffer, - iteratorInitialized: iteratorInitialized, - failure: failure - ) - - return .returnElement(element) - } else { + guard let element = buffer.popFirst() else { // We are returning the queued failure now and can transition to finished self._state = .finished(iteratorInitialized: iteratorInitialized) return .returnFailureAndCallDidTerminate(failure) } + self._state = .sourceFinished( + buffer: buffer, + iteratorInitialized: iteratorInitialized, + failure: failure + ) + + return .returnElement(element) case .cancelled(let iteratorInitialized): self._state = .finished(iteratorInitialized: iteratorInitialized) @@ -1299,11 +1296,10 @@ extension NIOThrowingAsyncSequenceProducer { iteratorInitialized: iteratorInitialized ) - if shouldProduceMore && !hasOutstandingDemand { - return .callProduceMore - } else { + guard shouldProduceMore && !hasOutstandingDemand else { return .none } + return .callProduceMore case .streaming(_, _, .some(_), _, _), .sourceFinished, .finished, .cancelled: preconditionFailure("This should have already been handled by `next()`") diff --git a/Sources/NIOCore/ByteBuffer-aux.swift b/Sources/NIOCore/ByteBuffer-aux.swift index fcfa74ce3b..22c9efa094 100644 --- a/Sources/NIOCore/ByteBuffer-aux.swift +++ b/Sources/NIOCore/ByteBuffer-aux.swift @@ -119,13 +119,14 @@ extension ByteBuffer { @inlinable mutating func _setStringSlowpath(_ string: String, at index: Int) -> Int { // slow path, let's try to force the string to be native - if let written = (string + "").utf8.withContiguousStorageIfAvailable({ utf8Bytes in - self.setBytes(utf8Bytes, at: index) - }) { - return written - } else { + guard + let written = (string + "").utf8.withContiguousStorageIfAvailable({ utf8Bytes in + self.setBytes(utf8Bytes, at: index) + }) + else { return self.setBytes(string.utf8, at: index) } + return written } /// Write `string` into this `ByteBuffer` at `index` using UTF-8 encoding. Does not move the writer index. @@ -140,14 +141,15 @@ extension ByteBuffer { // Do not implement setString via setSubstring. Before Swift version 5.3, // Substring.UTF8View did not implement withContiguousStorageIfAvailable // and therefore had no fast access to the backing storage. - if let written = string.utf8.withContiguousStorageIfAvailable({ utf8Bytes in - self.setBytes(utf8Bytes, at: index) - }) { - // fast path, directly available - return written - } else { + guard + let written = string.utf8.withContiguousStorageIfAvailable({ utf8Bytes in + self.setBytes(utf8Bytes, at: index) + }) + else { return self._setStringSlowpath(string, at: index) } + // fast path, directly available + return written } /// Write `string` null terminated into this `ByteBuffer` at `index` using UTF-8 encoding. Does not move the writer index. @@ -262,15 +264,16 @@ extension ByteBuffer { public mutating func setSubstring(_ substring: Substring, at index: Int) -> Int { // Substring.UTF8View implements withContiguousStorageIfAvailable starting with // Swift version 5.3. - if let written = substring.utf8.withContiguousStorageIfAvailable({ utf8Bytes in - self.setBytes(utf8Bytes, at: index) - }) { - // fast path, directly available - return written - } else { + guard + let written = substring.utf8.withContiguousStorageIfAvailable({ utf8Bytes in + self.setBytes(utf8Bytes, at: index) + }) + else { // slow path, convert to string return self.setString(String(substring), at: index) } + // fast path, directly available + return written } // MARK: DispatchData APIs @@ -844,13 +847,12 @@ extension Optional where Wrapped == ByteBuffer { @discardableResult @inlinable public mutating func setOrWriteBuffer(_ buffer: inout ByteBuffer) -> Int { - if self == nil { - let readableBytes = buffer.readableBytes - self = buffer - buffer.moveReaderIndex(to: buffer.writerIndex) - return readableBytes - } else { + guard self == nil else { return self!.writeBuffer(&buffer) } + let readableBytes = buffer.readableBytes + self = buffer + buffer.moveReaderIndex(to: buffer.writerIndex) + return readableBytes } } diff --git a/Sources/NIOCore/ByteBuffer-core.swift b/Sources/NIOCore/ByteBuffer-core.swift index 8027fcc637..e782c77184 100644 --- a/Sources/NIOCore/ByteBuffer-core.swift +++ b/Sources/NIOCore/ByteBuffer-core.swift @@ -537,14 +537,15 @@ public struct ByteBuffer { @inlinable mutating func _setBytes(_ bytes: Bytes, at index: _Index) -> _Capacity where Bytes.Element == UInt8 { - if let written = bytes.withContiguousStorageIfAvailable({ bytes in - self._setBytes(UnsafeRawBufferPointer(bytes), at: index) - }) { - // fast path, we've got access to the contiguous bytes - return written - } else { + guard + let written = bytes.withContiguousStorageIfAvailable({ bytes in + self._setBytes(UnsafeRawBufferPointer(bytes), at: index) + }) + else { return self._setSlowPath(bytes: bytes, at: index) } + // fast path, we've got access to the contiguous bytes + return written } // MARK: Public Core API @@ -1190,11 +1191,10 @@ extension ByteBuffer { /// - returns: The return value of `body`. @inlinable public mutating func modifyIfUniquelyOwned(_ body: (inout ByteBuffer) throws -> T) rethrows -> T? { - if isKnownUniquelyReferenced(&self._storage) { - return try body(&self) - } else { + guard isKnownUniquelyReferenced(&self._storage) else { return nil } + return try body(&self) } } diff --git a/Sources/NIOCore/ByteBuffer-hexdump.swift b/Sources/NIOCore/ByteBuffer-hexdump.swift index fa5dde44e8..6e6bcd7d13 100644 --- a/Sources/NIOCore/ByteBuffer-hexdump.swift +++ b/Sources/NIOCore/ByteBuffer-hexdump.swift @@ -248,18 +248,16 @@ extension ByteBuffer { public func hexDump(format: HexDumpFormat) -> String { switch format.value { case .plain(let maxBytes): - if let maxBytes = maxBytes { - return self.hexDumpPlain(maxBytes: maxBytes) - } else { + guard let maxBytes = maxBytes else { return self.hexDumpPlain() } + return self.hexDumpPlain(maxBytes: maxBytes) case .detailed(let maxBytes): - if let maxBytes = maxBytes { - return self.hexDumpDetailed(maxBytes: maxBytes) - } else { + guard let maxBytes = maxBytes else { return self.hexdumpDetailed() } + return self.hexDumpDetailed(maxBytes: maxBytes) } } } diff --git a/Sources/NIOCore/ChannelOption.swift b/Sources/NIOCore/ChannelOption.swift index a4e0746a04..d1029d8df5 100644 --- a/Sources/NIOCore/ChannelOption.swift +++ b/Sources/NIOCore/ChannelOption.swift @@ -392,12 +392,11 @@ extension ChannelOptions { var hasSet = false self._storage = self._storage.map { currentKeyAndValue in let (currentKey, _) = currentKeyAndValue - if let currentKey = currentKey as? Option, currentKey == newKey { - hasSet = true - return (currentKey, (newValue, applier)) - } else { + guard let currentKey = currentKey as? Option, currentKey == newKey else { return currentKeyAndValue } + hasSet = true + return (currentKey, (newValue, applier)) } if !hasSet { self._storage.append((newKey, (newValue, applier))) diff --git a/Sources/NIOCore/ChannelPipeline.swift b/Sources/NIOCore/ChannelPipeline.swift index aa0ab19275..508b9a2d7c 100644 --- a/Sources/NIOCore/ChannelPipeline.swift +++ b/Sources/NIOCore/ChannelPipeline.swift @@ -570,11 +570,10 @@ public final class ChannelPipeline: ChannelInvoker { internal func _contextSync(_ body: (ChannelHandlerContext) -> Bool) -> Result { self.eventLoop.assertInEventLoop() - if let context = self.contextForPredicate0(body) { - return .success(context) - } else { + guard let context = self.contextForPredicate0(body) else { return .failure(ChannelPipelineError.notFound) } + return .success(context) } /// Returns a `ChannelHandlerContext` which matches. diff --git a/Sources/NIOCore/CircularBuffer.swift b/Sources/NIOCore/CircularBuffer.swift index 360b3dfb0d..7a07a505b0 100644 --- a/Sources/NIOCore/CircularBuffer.swift +++ b/Sources/NIOCore/CircularBuffer.swift @@ -456,11 +456,10 @@ extension CircularBuffer { /// Returns the number of element in the ring. @inlinable public var count: Int { - if self.tailBackingIndex >= self.headBackingIndex { - return self.tailBackingIndex &- self.headBackingIndex - } else { + guard self.tailBackingIndex >= self.headBackingIndex else { return self._buffer.count &- (self.headBackingIndex &- self.tailBackingIndex) } + return self.tailBackingIndex &- self.headBackingIndex } /// The total number of elements that the ring can contain without allocating new storage. @@ -538,11 +537,10 @@ extension CircularBuffer: RangeReplaceableCollection { /// - Complexity: O(1) @inlinable public mutating func popFirst() -> Element? { - if count > 0 { - return self.removeFirst() - } else { + guard count > 0 else { return nil } + return self.removeFirst() } /// Removes and returns the last element of the `CircularBuffer`. @@ -557,11 +555,10 @@ extension CircularBuffer: RangeReplaceableCollection { /// - Complexity: O(1) @inlinable public mutating func popLast() -> Element? { - if count > 0 { - return self.removeLast() - } else { + guard count > 0 else { return nil } + return self.removeLast() } /// Removes the specified number of elements from the end of the diff --git a/Sources/NIOCore/Codec.swift b/Sources/NIOCore/Codec.swift index 6b9d59c4f7..452224113c 100644 --- a/Sources/NIOCore/Codec.swift +++ b/Sources/NIOCore/Codec.swift @@ -304,12 +304,11 @@ extension B2MDBuffer { var buffer = self.buffers.removeFirst() buffer.writeBuffers(self.buffers) self.buffers.removeAll(keepingCapacity: self.buffers.capacity < 16) // don't grow too much - if buffer.readableBytes > 0 || allowEmptyBuffer { - self.state = .processingInProgress - return .available(buffer) - } else { + guard buffer.readableBytes > 0 || allowEmptyBuffer else { return .nothingAvailable } + self.state = .processingInProgress + return .available(buffer) case .ready: assert(self.buffers.isEmpty) if allowEmptyBuffer { @@ -618,12 +617,11 @@ extension ByteToMessageHandler { self.tryDecodeWrites() continue case .didProcess(.needMoreData): - if self.queuedWrites.count > 0 { - self.tryDecodeWrites() - continue // we might have received more, so let's spin once more - } else { + guard self.queuedWrites.count > 0 else { return .didProcess(.needMoreData) } + self.tryDecodeWrites() + continue // we might have received more, so let's spin once more case .cannotProcessReentrantly: return .cannotProcessReentrantly } diff --git a/Sources/NIOCore/EventLoop.swift b/Sources/NIOCore/EventLoop.swift index 1f38c73a31..c451d61360 100644 --- a/Sources/NIOCore/EventLoop.swift +++ b/Sources/NIOCore/EventLoop.swift @@ -502,11 +502,10 @@ public struct TimeAmount: Hashable, Sendable { @inlinable static func _cappedNanoseconds(amount: Int64, multiplier: Int64) -> Int64 { let nanosecondsMultiplication = amount.multipliedReportingOverflow(by: multiplier) - if nanosecondsMultiplication.overflow { - return amount >= 0 ? .max : .min - } else { + guard nanosecondsMultiplication.overflow else { return nanosecondsMultiplication.partialValue } + return amount >= 0 ? .max : .min } } @@ -852,12 +851,11 @@ extension EventLoop { /// - returns: a succeeded `EventLoopFuture`. @inlinable public func makeSucceededFuture(_ value: Success) -> EventLoopFuture { - if Success.self == Void.self { - // The as! will always succeed because we previously checked that Success.self == Void.self. - return self.makeSucceededVoidFuture() as! EventLoopFuture - } else { + guard Success.self == Void.self else { return EventLoopFuture(eventLoop: self, value: value) } + // The as! will always succeed because we previously checked that Success.self == Void.self. + return self.makeSucceededVoidFuture() as! EventLoopFuture } /// Creates and returns a new `EventLoopFuture` that is marked as succeeded or failed with the value held by `result`. diff --git a/Sources/NIOCore/EventLoopFuture+WithEventLoop.swift b/Sources/NIOCore/EventLoopFuture+WithEventLoop.swift index bf76a0e97e..906821892d 100644 --- a/Sources/NIOCore/EventLoopFuture+WithEventLoop.swift +++ b/Sources/NIOCore/EventLoopFuture+WithEventLoop.swift @@ -49,14 +49,13 @@ extension EventLoopFuture { switch self._value! { case .success(let t): let futureU = callback(t, eventLoop) - if futureU.eventLoop.inEventLoop { - return futureU._addCallback { - next._setValue(value: futureU._value!) - } - } else { + guard futureU.eventLoop.inEventLoop else { futureU.cascade(to: next) return CallbackList() } + return futureU._addCallback { + next._setValue(value: futureU._value!) + } case .failure(let error): return next._setValue(value: .failure(error)) } @@ -87,14 +86,13 @@ extension EventLoopFuture { return next._setValue(value: .success(t)) case .failure(let e): let t = callback(e, eventLoop) - if t.eventLoop.inEventLoop { - return t._addCallback { - next._setValue(value: t._value!) - } - } else { + guard t.eventLoop.inEventLoop else { t.cascade(to: next) return CallbackList() } + return t._addCallback { + next._setValue(value: t._value!) + } } } return next.futureResult @@ -136,14 +134,13 @@ extension EventLoopFuture { return body } - if self.eventLoop.inEventLoop { - return fold0(eventLoop: self.eventLoop) - } else { + guard self.eventLoop.inEventLoop else { let promise = self.eventLoop.makePromise(of: Value.self) self.eventLoop.execute { [eventLoop = self.eventLoop] in fold0(eventLoop: eventLoop).cascade(to: promise) } return promise.futureResult } + return fold0(eventLoop: self.eventLoop) } } diff --git a/Sources/NIOCore/EventLoopFuture.swift b/Sources/NIOCore/EventLoopFuture.swift index 65b18c048c..baf0602094 100644 --- a/Sources/NIOCore/EventLoopFuture.swift +++ b/Sources/NIOCore/EventLoopFuture.swift @@ -486,14 +486,13 @@ extension EventLoopFuture { switch self._value! { case .success(let t): let futureU = callback(t) - if futureU.eventLoop.inEventLoop { - return futureU._addCallback { - next._setValue(value: futureU._value!) - } - } else { + guard futureU.eventLoop.inEventLoop else { futureU.cascade(to: next) return CallbackList() } + return futureU._addCallback { + next._setValue(value: futureU._value!) + } case .failure(let error): return next._setValue(value: .failure(error)) } @@ -621,16 +620,15 @@ extension EventLoopFuture { @inlinable func _map(_ callback: @escaping MapCallback) -> EventLoopFuture { - if NewValue.self == Value.self && NewValue.self == Void.self { - self.whenSuccess(callback as! @Sendable (Value) -> Void) - return self as! EventLoopFuture - } else { + guard NewValue.self == Value.self && NewValue.self == Void.self else { let next = EventLoopPromise.makeUnleakablePromise(eventLoop: self.eventLoop) self._whenComplete { next._setValue(value: self._value!.map(callback)) } return next.futureResult } + self.whenSuccess(callback as! @Sendable (Value) -> Void) + return self as! EventLoopFuture } /// When the current `EventLoopFuture` is in an error state, run the provided callback, which @@ -662,14 +660,13 @@ extension EventLoopFuture { return next._setValue(value: .success(t)) case .failure(let e): let t = callback(e) - if t.eventLoop.inEventLoop { - return t._addCallback { - next._setValue(value: t._value!) - } - } else { + guard t.eventLoop.inEventLoop else { t.cascade(to: next) return CallbackList() } + return t._addCallback { + next._setValue(value: t._value!) + } } } return next.futureResult @@ -1087,15 +1084,14 @@ extension EventLoopFuture { return body } - if self.eventLoop.inEventLoop { - return fold0() - } else { + guard self.eventLoop.inEventLoop else { let promise = self.eventLoop.makePromise(of: Value.self) self.eventLoop.execute { fold0().cascade(to: promise) } return promise.futureResult } + return fold0() } } diff --git a/Sources/NIOCore/GlobalSingletons.swift b/Sources/NIOCore/GlobalSingletons.swift index 384dfc6d71..5ca11af85c 100644 --- a/Sources/NIOCore/GlobalSingletons.swift +++ b/Sources/NIOCore/GlobalSingletons.swift @@ -89,16 +89,15 @@ extension NIOSingletons { desired: 1, ordering: .relaxed ) - if exchanged { - // Never been set, we're committing to the default (enabled). - assert(original == 0) - return true - } else { + guard exchanged else { // This has been set before, 1: enabled; -1 disabled. assert(original != 0) assert(original == -1 || original == 1) return original > 0 } + // Never been set, we're committing to the default (enabled). + assert(original == 0) + return true } set { diff --git a/Sources/NIOCore/IO.swift b/Sources/NIOCore/IO.swift index 49eb52e4d4..d49de00c1e 100644 --- a/Sources/NIOCore/IO.swift +++ b/Sources/NIOCore/IO.swift @@ -121,11 +121,10 @@ public struct IOError: Swift.Error { /// - reason: what failed /// - returns: the constructed reason. private func reasonForError(errnoCode: CInt, reason: String) -> String { - if let errorDescC = strerror(errnoCode) { - return "\(reason): \(String(cString: errorDescC)) (errno: \(errnoCode))" - } else { + guard let errorDescC = strerror(errnoCode) else { return "\(reason): Broken strerror, unknown error: \(errnoCode)" } + return "\(reason): \(String(cString: errorDescC)) (errno: \(errnoCode))" } #if os(Windows) diff --git a/Sources/NIOCore/MarkedCircularBuffer.swift b/Sources/NIOCore/MarkedCircularBuffer.swift index 1881c824cb..baf7d4e541 100644 --- a/Sources/NIOCore/MarkedCircularBuffer.swift +++ b/Sources/NIOCore/MarkedCircularBuffer.swift @@ -98,22 +98,20 @@ public struct MarkedCircularBuffer: CustomStringConvertible { public func isMarked(index: Index) -> Bool { assert(index >= self.startIndex, "index must not be negative") precondition(index < self.endIndex, "index \(index) out of range (0..<\(self._buffer.count))") - if let markedIndexOffset = self._markedIndexOffset { - return self.index(self.startIndex, offsetBy: markedIndexOffset) == index - } else { + guard let markedIndexOffset = self._markedIndexOffset else { return false } + return self.index(self.startIndex, offsetBy: markedIndexOffset) == index } /// Returns the index of the marked element. @inlinable public var markedElementIndex: Index? { - if let markedIndexOffset = self._markedIndexOffset { - assert(markedIndexOffset >= 0) - return self.index(self.startIndex, offsetBy: markedIndexOffset) - } else { + guard let markedIndexOffset = self._markedIndexOffset else { return nil } + assert(markedIndexOffset >= 0) + return self.index(self.startIndex, offsetBy: markedIndexOffset) } /// Returns the marked element. diff --git a/Sources/NIOCore/NIOAny.swift b/Sources/NIOCore/NIOAny.swift index 1f687b134b..8b751b749d 100644 --- a/Sources/NIOCore/NIOAny.swift +++ b/Sources/NIOCore/NIOAny.swift @@ -83,11 +83,10 @@ public struct NIOAny { /// - returns: The wrapped `ByteBuffer` or `nil` if the wrapped message is not a `ByteBuffer`. @inlinable func tryAsByteBuffer() -> ByteBuffer? { - if case .ioData(.byteBuffer(let bb)) = self._storage { - return bb - } else { + guard case .ioData(.byteBuffer(let bb)) = self._storage else { return nil } + return bb } /// Force unwrapping the wrapped message as `ByteBuffer`. @@ -109,11 +108,10 @@ public struct NIOAny { /// - returns: The wrapped `IOData` or `nil` if the wrapped message is not a `IOData`. @inlinable func tryAsIOData() -> IOData? { - if case .ioData(let data) = self._storage { - return data - } else { + guard case .ioData(let data) = self._storage else { return nil } + return data } /// Force unwrapping the wrapped message as `IOData`. @@ -135,11 +133,10 @@ public struct NIOAny { /// - returns: The wrapped `FileRegion` or `nil` if the wrapped message is not a `FileRegion`. @inlinable func tryAsFileRegion() -> FileRegion? { - if case .ioData(.fileRegion(let f)) = self._storage { - return f - } else { + guard case .ioData(.fileRegion(let f)) = self._storage else { return nil } + return f } /// Force unwrapping the wrapped message as `FileRegion`. @@ -161,11 +158,10 @@ public struct NIOAny { /// - returns: The wrapped `AddressedEnvelope` or `nil` if the wrapped message is not an `AddressedEnvelope`. @inlinable func tryAsByteEnvelope() -> AddressedEnvelope? { - if case .bufferEnvelope(let e) = self._storage { - return e - } else { + guard case .bufferEnvelope(let e) = self._storage else { return nil } + return e } /// Force unwrapping the wrapped message as `AddressedEnvelope`. diff --git a/Sources/NIOCore/SingleStepByteToMessageDecoder.swift b/Sources/NIOCore/SingleStepByteToMessageDecoder.swift index 1fed8965ac..bae7cb9a39 100644 --- a/Sources/NIOCore/SingleStepByteToMessageDecoder.swift +++ b/Sources/NIOCore/SingleStepByteToMessageDecoder.swift @@ -53,12 +53,11 @@ public protocol NIOSingleStepByteToMessageDecoder: ByteToMessageDecoder { // MARK: NIOSingleStepByteToMessageDecoder: ByteToMessageDecoder extension NIOSingleStepByteToMessageDecoder { public mutating func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { - if let message = try self.decode(buffer: &buffer) { - context.fireChannelRead(Self.wrapInboundOut(message)) - return .continue - } else { + guard let message = try self.decode(buffer: &buffer) else { return .needMoreData } + context.fireChannelRead(Self.wrapInboundOut(message)) + return .continue } public mutating func decodeLast( @@ -66,12 +65,11 @@ extension NIOSingleStepByteToMessageDecoder { buffer: inout ByteBuffer, seenEOF: Bool ) throws -> DecodingState { - if let message = try self.decodeLast(buffer: &buffer, seenEOF: seenEOF) { - context.fireChannelRead(Self.wrapInboundOut(message)) - return .continue - } else { + guard let message = try self.decodeLast(buffer: &buffer, seenEOF: seenEOF) else { return .needMoreData } + context.fireChannelRead(Self.wrapInboundOut(message)) + return .continue } } @@ -257,11 +255,10 @@ public final class NIOSingleStepByteToMessageProcessor Decoder.InboundOut? { - if decodeMode == .normal { - return try self.decoder.decode(buffer: &buffer) - } else { + guard decodeMode == .normal else { return try self.decoder.decodeLast(buffer: &buffer, seenEOF: seenEOF) } + return try self.decoder.decode(buffer: &buffer) } while let message = try self._withNonCoWBuffer(decodeOnce) { diff --git a/Sources/NIOCore/SocketAddresses.swift b/Sources/NIOCore/SocketAddresses.swift index 0c52ba7a8e..98710662e9 100644 --- a/Sources/NIOCore/SocketAddresses.swift +++ b/Sources/NIOCore/SocketAddresses.swift @@ -496,20 +496,19 @@ public enum SocketAddress: CustomStringConvertible, Sendable { } } - if let info = info, let addrPointer = info.pointee.ai_addr { - let addressBytes = UnsafeRawPointer(addrPointer) - switch NIOBSDSocket.AddressFamily(rawValue: info.pointee.ai_family) { - case .inet: - return .v4(.init(address: addressBytes.load(as: sockaddr_in.self), host: host)) - case .inet6: - return .v6(.init(address: addressBytes.load(as: sockaddr_in6.self), host: host)) - default: - throw SocketAddressError.unsupported - } - } else { + guard let info = info, let addrPointer = info.pointee.ai_addr else { // this is odd, getaddrinfo returned NULL throw SocketAddressError.unsupported } + let addressBytes = UnsafeRawPointer(addrPointer) + switch NIOBSDSocket.AddressFamily(rawValue: info.pointee.ai_family) { + case .inet: + return .v4(.init(address: addressBytes.load(as: sockaddr_in.self), host: host)) + case .inet6: + return .v6(.init(address: addressBytes.load(as: sockaddr_in6.self), host: host)) + default: + throw SocketAddressError.unsupported + } #endif } } diff --git a/Sources/NIOCrashTester/OutputGrepper.swift b/Sources/NIOCrashTester/OutputGrepper.swift index 7802fe83b9..751245fba6 100644 --- a/Sources/NIOCrashTester/OutputGrepper.swift +++ b/Sources/NIOCrashTester/OutputGrepper.swift @@ -92,12 +92,11 @@ private struct NewlineFramer: ByteToMessageDecoder { typealias InboundOut = String func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { - if let firstNewline = buffer.readableBytesView.firstIndex(of: UInt8(ascii: "\n")) { - let length = firstNewline - buffer.readerIndex + 1 - context.fireChannelRead(Self.wrapInboundOut(String(buffer.readString(length: length)!.dropLast()))) - return .continue - } else { + guard let firstNewline = buffer.readableBytesView.firstIndex(of: UInt8(ascii: "\n")) else { return .needMoreData } + let length = firstNewline - buffer.readerIndex + 1 + context.fireChannelRead(Self.wrapInboundOut(String(buffer.readString(length: length)!.dropLast()))) + return .continue } } diff --git a/Sources/NIOCrashTester/main.swift b/Sources/NIOCrashTester/main.swift index 6234947aea..358a2a8dd8 100644 --- a/Sources/NIOCrashTester/main.swift +++ b/Sources/NIOCrashTester/main.swift @@ -33,11 +33,10 @@ struct CrashTest { extension Process { var binaryPath: String? { get { - if #available(macOS 10.13, *) { - return self.executableURL?.path - } else { + guard #available(macOS 10.13, *) else { return self.launchPath } + return self.executableURL?.path } set { if #available(macOS 10.13, *) { @@ -114,11 +113,10 @@ func main() throws { } let output = try result.get() - if output.range(of: regex, options: .regularExpression) != nil { - return .crashedAsExpected - } else { + guard output.range(of: regex, options: .regularExpression) != nil else { return .regexDidNotMatch(regex: regex, output: output) } + return .crashedAsExpected } func usage() { diff --git a/Sources/NIOEmbedded/AsyncTestingChannel.swift b/Sources/NIOEmbedded/AsyncTestingChannel.swift index 11900d8353..d6fec89319 100644 --- a/Sources/NIOEmbedded/AsyncTestingChannel.swift +++ b/Sources/NIOEmbedded/AsyncTestingChannel.swift @@ -100,11 +100,10 @@ public final class NIOAsyncTestingChannel: Channel { /// `true` if the ``NIOAsyncTestingChannel`` was `clean` on ``NIOAsyncTestingChannel/finish()``, ie. there is no unconsumed inbound, outbound, or /// pending outbound data left on the `Channel`. public var isClean: Bool { - if case .clean = self { - return true - } else { + guard case .clean = self else { return false } + return true } /// `true` if the ``NIOAsyncTestingChannel`` if there was unconsumed inbound, outbound, or pending outbound data left @@ -131,11 +130,10 @@ public final class NIOAsyncTestingChannel: Channel { /// Returns `true` is the buffer was empty. public var isEmpty: Bool { - if case .empty = self { - return true - } else { + guard case .empty = self else { return false } + return true } /// Returns `true` if the buffer was non-empty. @@ -335,15 +333,14 @@ public final class NIOAsyncTestingChannel: Channel { // This can never actually throw. return try! await self.testingEventLoop.executeInContext { let c = self.channelcore! - if c.outboundBuffer.isEmpty && c.inboundBuffer.isEmpty && c.pendingOutboundBuffer.isEmpty { - return .clean - } else { + guard c.outboundBuffer.isEmpty && c.inboundBuffer.isEmpty && c.pendingOutboundBuffer.isEmpty else { return .leftOvers( inbound: c.inboundBuffer, outbound: c.outboundBuffer, pendingOutbound: c.pendingOutboundBuffer.map { $0.0 } ) } + return .clean } } @@ -544,12 +541,11 @@ public final class NIOAsyncTestingChannel: Channel { /// - see: `Channel.setOption` @inlinable public func setOption(_ option: Option, value: Option.Value) -> EventLoopFuture { - if self.eventLoop.inEventLoop { - self.setOptionSync(option, value: value) - return self.eventLoop.makeSucceededVoidFuture() - } else { + guard self.eventLoop.inEventLoop else { return self.eventLoop.submit { self.setOptionSync(option, value: value) } } + self.setOptionSync(option, value: value) + return self.eventLoop.makeSucceededVoidFuture() } @inlinable @@ -565,11 +561,10 @@ public final class NIOAsyncTestingChannel: Channel { /// - see: `Channel.getOption` @inlinable public func getOption(_ option: Option) -> EventLoopFuture { - if self.eventLoop.inEventLoop { - return self.eventLoop.makeSucceededFuture(self.getOptionSync(option)) - } else { + guard self.eventLoop.inEventLoop else { return self.eventLoop.submit { self.getOptionSync(option) } } + return self.eventLoop.makeSucceededFuture(self.getOptionSync(option)) } @inlinable diff --git a/Sources/NIOEmbedded/Embedded.swift b/Sources/NIOEmbedded/Embedded.swift index 8f808e3f02..a257c3977c 100644 --- a/Sources/NIOEmbedded/Embedded.swift +++ b/Sources/NIOEmbedded/Embedded.swift @@ -47,11 +47,10 @@ internal struct EmbeddedScheduledTask { extension EmbeddedScheduledTask: Comparable { static func < (lhs: EmbeddedScheduledTask, rhs: EmbeddedScheduledTask) -> Bool { - if lhs.readyTime == rhs.readyTime { - return lhs.insertOrder < rhs.insertOrder - } else { + guard lhs.readyTime == rhs.readyTime else { return lhs.readyTime < rhs.readyTime } + return lhs.insertOrder < rhs.insertOrder } static func == (lhs: EmbeddedScheduledTask, rhs: EmbeddedScheduledTask) -> Bool { @@ -347,21 +346,19 @@ class EmbeddedChannelCore: ChannelCore { @usableFromInline func localAddress0() throws -> SocketAddress { self.eventLoop.preconditionInEventLoop() - if let localAddress = self.localAddress { - return localAddress - } else { + guard let localAddress = self.localAddress else { throw ChannelError.operationUnsupported } + return localAddress } @usableFromInline func remoteAddress0() throws -> SocketAddress { self.eventLoop.preconditionInEventLoop() - if let remoteAddress = self.remoteAddress { - return remoteAddress - } else { + guard let remoteAddress = self.remoteAddress else { throw ChannelError.operationUnsupported } + return remoteAddress } @usableFromInline @@ -521,11 +518,10 @@ public final class EmbeddedChannel: Channel { /// `true` if the `EmbeddedChannel` was `clean` on `finish`, ie. there is no unconsumed inbound, outbound, or /// pending outbound data left on the `Channel`. public var isClean: Bool { - if case .clean = self { - return true - } else { + guard case .clean = self else { return false } + return true } /// `true` if the `EmbeddedChannel` if there was unconsumed inbound, outbound, or pending outbound data left @@ -551,11 +547,10 @@ public final class EmbeddedChannel: Channel { /// Returns `true` is the buffer was empty. public var isEmpty: Bool { - if case .empty = self { - return true - } else { + guard case .empty = self else { return false } + return true } /// Returns `true` if the buffer was non-empty. @@ -644,15 +639,14 @@ public final class EmbeddedChannel: Channel { self.embeddedEventLoop.run() try throwIfErrorCaught() let c = self.channelcore - if c.outboundBuffer.isEmpty && c.inboundBuffer.isEmpty && c.pendingOutboundBuffer.isEmpty { - return .clean - } else { + guard c.outboundBuffer.isEmpty && c.inboundBuffer.isEmpty && c.pendingOutboundBuffer.isEmpty else { return .leftOvers( inbound: Array(c.inboundBuffer), outbound: Array(c.outboundBuffer), pendingOutbound: c.pendingOutboundBuffer.map { $0.0 } ) } + return .clean } /// Synchronously closes the `EmbeddedChannel`. diff --git a/Sources/NIOFileSystem/BufferedReader.swift b/Sources/NIOFileSystem/BufferedReader.swift index d66aa5d95e..cbe00da844 100644 --- a/Sources/NIOFileSystem/BufferedReader.swift +++ b/Sources/NIOFileSystem/BufferedReader.swift @@ -79,9 +79,7 @@ public struct BufferedReader { let byteCount = Int(count.bytes) guard byteCount > 0 else { return ByteBuffer() } - if let bytes = self.buffer.readSlice(length: byteCount) { - return bytes - } else { + guard let bytes = self.buffer.readSlice(length: byteCount) else { // Not enough bytes: read enough for the caller and to fill the buffer back to capacity. var buffer = self.buffer self.buffer = ByteBuffer() @@ -104,6 +102,7 @@ public struct BufferedReader { return buffer } + return bytes } /// Reads from the current position in the file until `predicate` returns `false` and returns diff --git a/Sources/NIOFileSystem/FileChunks.swift b/Sources/NIOFileSystem/FileChunks.swift index b42d76e18d..2d91f05563 100644 --- a/Sources/NIOFileSystem/FileChunks.swift +++ b/Sources/NIOFileSystem/FileChunks.swift @@ -164,25 +164,23 @@ private struct FileChunkProducer: Sendable { try self.state.withLockedValue { state in state.produceMore() }.flatMap { - if let (descriptor, range) = $0 { - if let range { - let remainingBytes = range.upperBound - range.lowerBound - let clampedLength = min(self.length, remainingBytes) - return descriptor.readChunk( - fromAbsoluteOffset: range.lowerBound, - length: clampedLength - ).mapError { error in - .read(usingSyscall: .pread, error: error, path: self.path, location: .here()) - } - } else { - return descriptor.readChunk(length: self.length).mapError { error in - .read(usingSyscall: .read, error: error, path: self.path, location: .here()) - } - } - } else { + guard let (descriptor, range) = $0 else { // nil means done: return empty to indicate the stream should finish. return .success(ByteBuffer()) } + guard let range else { + return descriptor.readChunk(length: self.length).mapError { error in + .read(usingSyscall: .read, error: error, path: self.path, location: .here()) + } + } + let remainingBytes = range.upperBound - range.lowerBound + let clampedLength = min(self.length, remainingBytes) + return descriptor.readChunk( + fromAbsoluteOffset: range.lowerBound, + length: clampedLength + ).mapError { error in + .read(usingSyscall: .pread, error: error, path: self.path, location: .here()) + } }.get() } @@ -291,9 +289,7 @@ private struct ProducerState: Sendable { mutating func produceMore() -> Result<(FileDescriptor, Range?)?, FileSystemError> { switch self.state { case let .producing(state): - if let descriptor = state.handle.descriptorIfAvailable() { - return .success((descriptor, state.range)) - } else { + guard let descriptor = state.handle.descriptorIfAvailable() else { let error = FileSystemError( code: .closed, message: "Cannot read from closed file ('\(state.handle.path)').", @@ -302,6 +298,7 @@ private struct ProducerState: Sendable { ) return .failure(error) } + return .success((descriptor, state.range)) case .done: return .success(nil) } diff --git a/Sources/NIOFileSystem/FileHandleProtocol.swift b/Sources/NIOFileSystem/FileHandleProtocol.swift index d58e33065a..e1696c8562 100644 --- a/Sources/NIOFileSystem/FileHandleProtocol.swift +++ b/Sources/NIOFileSystem/FileHandleProtocol.swift @@ -354,56 +354,7 @@ extension ReadableFileHandleProtocol { } let isSeekable = !(info.type == .fifo || info.type == .socket) - if isSeekable { - // Limit the size of single shot reads. If the system is busy then avoid slowing down other - // work by blocking an I/O executor thread while doing a large read. The limit is somewhat - // arbitrary. - let singleShotReadLimit = 64 * 1024 * 1024 - - // If the file size isn't 0 but the read size is, then it means that - // we are intending to read an empty fragment: we can just return - // fast and skip any reads. - // If the file size is 0, we can't conclude anything about the size - // of the file, as `stat` will return 0 for unbounded files. - // If this happens, just read in chunks to make sure the whole file - // is read (up to `maximumSizeAllowed` bytes). - var forceChunkedRead = false - if fileSize > 0 { - if readSize == 0 { - return ByteBuffer() - } - } else { - forceChunkedRead = true - } - - if !forceChunkedRead, readSize <= singleShotReadLimit { - return try await self.readChunk( - fromAbsoluteOffset: offset, - length: .bytes(Int64(readSize)) - ) - } else { - var accumulator = ByteBuffer() - accumulator.reserveCapacity(readSize) - - for try await chunk in self.readChunks(in: offset..., chunkLength: .mebibytes(8)) { - accumulator.writeImmutableBuffer(chunk) - if accumulator.readableBytes > maximumSizeAllowed.bytes { - throw FileSystemError( - code: .resourceExhausted, - message: """ - There are more bytes to read than the maximum size allowed \ - (\(maximumSizeAllowed)). Read the file in chunks or increase the maximum size \ - allowed. - """, - cause: nil, - location: .here() - ) - } - } - - return accumulator - } - } else { + guard isSeekable else { guard offset == 0 else { throw FileSystemError( code: .unsupported, @@ -433,6 +384,53 @@ extension ReadableFileHandleProtocol { return accumulator } + // Limit the size of single shot reads. If the system is busy then avoid slowing down other + // work by blocking an I/O executor thread while doing a large read. The limit is somewhat + // arbitrary. + let singleShotReadLimit = 64 * 1024 * 1024 + + // If the file size isn't 0 but the read size is, then it means that + // we are intending to read an empty fragment: we can just return + // fast and skip any reads. + // If the file size is 0, we can't conclude anything about the size + // of the file, as `stat` will return 0 for unbounded files. + // If this happens, just read in chunks to make sure the whole file + // is read (up to `maximumSizeAllowed` bytes). + var forceChunkedRead = false + if fileSize > 0 { + if readSize == 0 { + return ByteBuffer() + } + } else { + forceChunkedRead = true + } + + guard !forceChunkedRead, readSize <= singleShotReadLimit else { + var accumulator = ByteBuffer() + accumulator.reserveCapacity(readSize) + + for try await chunk in self.readChunks(in: offset..., chunkLength: .mebibytes(8)) { + accumulator.writeImmutableBuffer(chunk) + if accumulator.readableBytes > maximumSizeAllowed.bytes { + throw FileSystemError( + code: .resourceExhausted, + message: """ + There are more bytes to read than the maximum size allowed \ + (\(maximumSizeAllowed)). Read the file in chunks or increase the maximum size \ + allowed. + """, + cause: nil, + location: .here() + ) + } + } + + return accumulator + } + return try await self.readChunk( + fromAbsoluteOffset: offset, + length: .bytes(Int64(readSize)) + ) } } diff --git a/Sources/NIOFileSystem/FileSystem.swift b/Sources/NIOFileSystem/FileSystem.swift index 16692a7854..bba8af9a3e 100644 --- a/Sources/NIOFileSystem/FileSystem.swift +++ b/Sources/NIOFileSystem/FileSystem.swift @@ -820,13 +820,12 @@ extension FileSystem { } // Drop the last component and loop around. - if let component = path.lastComponent { - path.removeLastComponent() - droppedComponents.append(component) - } else { + guard let component = path.lastComponent else { // Should only happen if the path is empty or contains just the root. return .failure(.mkdir(errno: errno, path: path, location: .here())) } + path.removeLastComponent() + droppedComponents.append(component) } } @@ -859,12 +858,11 @@ extension FileSystem { return result.map { FileInfo(platformSpecificStatus: $0) }.flatMapError { errno in - if errno == .noSuchFileOrDirectory { - return .success(nil) - } else { + guard errno == .noSuchFileOrDirectory else { let name = infoAboutSymbolicLink ? "lstat" : "stat" return .failure(.stat(name, errno: errno, path: path, location: .here())) } + return .success(nil) } } @@ -1317,19 +1315,18 @@ extension FileSystem { case .success: return .success(finalPath) case let .failure(error): - if let systemCallError = error.cause as? FileSystemError.SystemCallError { - switch systemCallError.errno { - // If the file at the generated path already exists, we generate a new file path. - case .fileExists, .isDirectory: - break - default: - let fileSystemError = FileSystemError( - message: "Unable to create temporary directory '\(template)'.", - wrapping: error - ) - return .failure(fileSystemError) - } - } else { + guard let systemCallError = error.cause as? FileSystemError.SystemCallError else { + let fileSystemError = FileSystemError( + message: "Unable to create temporary directory '\(template)'.", + wrapping: error + ) + return .failure(fileSystemError) + } + switch systemCallError.errno { + // If the file at the generated path already exists, we generate a new file path. + case .fileExists, .isDirectory: + break + default: let fileSystemError = FileSystemError( message: "Unable to create temporary directory '\(template)'.", wrapping: error diff --git a/Sources/NIOFileSystem/FileSystemError.swift b/Sources/NIOFileSystem/FileSystemError.swift index 801fcd699c..0408422707 100644 --- a/Sources/NIOFileSystem/FileSystemError.swift +++ b/Sources/NIOFileSystem/FileSystemError.swift @@ -60,24 +60,22 @@ public struct FileSystemError: Error, Sendable { extension FileSystemError: CustomStringConvertible { public var description: String { - if let cause = self.cause { - return "\(self.code): \(self.message) (\(cause))" - } else { + guard let cause = self.cause else { return "\(self.code): \(self.message)" } + return "\(self.code): \(self.message) (\(cause))" } } extension FileSystemError: CustomDebugStringConvertible { public var debugDescription: String { - if let cause = self.cause { - return """ - \(String(reflecting: self.code)): \(String(reflecting: self.message)) \ - (\(String(reflecting: cause))) - """ - } else { + guard let cause = self.cause else { return "\(String(reflecting: self.code)): \(String(reflecting: self.message))" } + return """ + \(String(reflecting: self.code)): \(String(reflecting: self.message)) \ + (\(String(reflecting: cause))) + """ } } diff --git a/Sources/NIOFileSystem/Internal/BufferedStream.swift b/Sources/NIOFileSystem/Internal/BufferedStream.swift index 6653e4f8c0..6c8a70e2d5 100644 --- a/Sources/NIOFileSystem/Internal/BufferedStream.swift +++ b/Sources/NIOFileSystem/Internal/BufferedStream.swift @@ -998,23 +998,18 @@ extension BufferedStream { mutating func sequenceDeinitialized() -> SequenceDeinitializedAction? { switch self._state { case .initial(let initial): - if initial.iteratorInitialized { - // An iterator was created and we deinited the sequence. - // This is an expected pattern and we just continue on normal. - return .none - } else { + guard initial.iteratorInitialized else { // No iterator was created so we can transition to finished right away. self._state = .finished(iteratorInitialized: false) return .callOnTermination(initial.onTermination) } + // An iterator was created and we deinited the sequence. + // This is an expected pattern and we just continue on normal. + return .none case .streaming(let streaming): - if streaming.iteratorInitialized { - // An iterator was created and we deinited the sequence. - // This is an expected pattern and we just continue on normal. - return .none - } else { + guard streaming.iteratorInitialized else { // No iterator was created so we can transition to finished right away. self._state = .finished(iteratorInitialized: false) @@ -1023,18 +1018,20 @@ extension BufferedStream { streaming.onTermination ) } + // An iterator was created and we deinited the sequence. + // This is an expected pattern and we just continue on normal. + return .none case .sourceFinished(let sourceFinished): - if sourceFinished.iteratorInitialized { - // An iterator was created and we deinited the sequence. - // This is an expected pattern and we just continue on normal. - return .none - } else { + guard sourceFinished.iteratorInitialized else { // No iterator was created so we can transition to finished right away. self._state = .finished(iteratorInitialized: false) return .callOnTermination(sourceFinished.onTermination) } + // An iterator was created and we deinited the sequence. + // This is an expected pattern and we just continue on normal. + return .none case .finished: // We are already finished so there is nothing left to clean up. @@ -1174,15 +1171,7 @@ extension BufferedStream { return .callOnTermination(initial.onTermination) case .streaming(let streaming): - if streaming.buffer.isEmpty { - // We can transition to finished right away since the buffer is empty now - self._state = .finished(iteratorInitialized: streaming.iteratorInitialized) - - return .failProducersAndCallOnTermination( - Array(streaming.producerContinuations.map { $0.1 }), - streaming.onTermination - ) - } else { + guard streaming.buffer.isEmpty else { // The continuation must be `nil` if the buffer has elements precondition(streaming.consumerContinuation == nil) @@ -1199,6 +1188,13 @@ extension BufferedStream { Array(streaming.producerContinuations.map { $0.1 }) ) } + // We can transition to finished right away since the buffer is empty now + self._state = .finished(iteratorInitialized: streaming.iteratorInitialized) + + return .failProducersAndCallOnTermination( + Array(streaming.producerContinuations.map { $0.1 }), + streaming.onTermination + ) case .sourceFinished, .finished: // This is normal and we just have to tolerate it @@ -1296,28 +1292,27 @@ extension BufferedStream { streaming.hasOutstandingDemand = shouldProduceMore let callbackToken = shouldProduceMore ? nil : self.nextCallbackToken() - if let consumerContinuation = streaming.consumerContinuation { - guard let element = streaming.buffer.popFirst() else { - // We got a yield of an empty sequence. We just tolerate this. - self._state = .streaming(streaming) - - return .init(callbackToken: callbackToken) - } - - // We got a consumer continuation and an element. We can resume the consumer now - streaming.consumerContinuation = nil - self._state = .streaming(streaming) - return .init( - callbackToken: callbackToken, - continuationAndElement: (consumerContinuation, element) - ) - } else { + guard let consumerContinuation = streaming.consumerContinuation else { // We don't have a suspended consumer so we just buffer the elements self._state = .streaming(streaming) return .init( callbackToken: callbackToken ) } + guard let element = streaming.buffer.popFirst() else { + // We got a yield of an empty sequence. We just tolerate this. + self._state = .streaming(streaming) + + return .init(callbackToken: callbackToken) + } + + // We got a consumer continuation and an element. We can resume the consumer now + streaming.consumerContinuation = nil + self._state = .streaming(streaming) + return .init( + callbackToken: callbackToken, + continuationAndElement: (consumerContinuation, element) + ) case .sourceFinished, .finished: // If the source has finished we are dropping the elements. @@ -1391,16 +1386,11 @@ extension BufferedStream { fatalError("AsyncStream internal inconsistency") case .streaming(var streaming): - if let index = streaming.producerContinuations.firstIndex(where: { - $0.0 == callbackToken.id - }) { - // We have an enqueued producer that we need to resume now - self._state = .modify - let continuation = streaming.producerContinuations.remove(at: index).1 - self._state = .streaming(streaming) - - return .resumeProducerWithCancellationError(continuation) - } else { + guard + let index = streaming.producerContinuations.firstIndex(where: { + $0.0 == callbackToken.id + }) + else { // The task that yields was cancelled before yielding so the cancellation handler // got invoked right away self._state = .modify @@ -1409,6 +1399,12 @@ extension BufferedStream { return .none } + // We have an enqueued producer that we need to resume now + self._state = .modify + let continuation = streaming.producerContinuations.remove(at: index).1 + self._state = .streaming(streaming) + + return .resumeProducerWithCancellationError(continuation) case .sourceFinished, .finished: // Since we are unlocking between yielding and suspending the yield @@ -1455,24 +1451,7 @@ extension BufferedStream { return .callOnTermination(initial.onTermination) case .streaming(let streaming): - if let consumerContinuation = streaming.consumerContinuation { - // We have a continuation, this means our buffer must be empty - // Furthermore, we can now transition to finished - // and resume the continuation with the failure - precondition(streaming.buffer.isEmpty, "Expected an empty buffer") - precondition( - streaming.producerContinuations.isEmpty, - "Expected no suspended producers" - ) - - self._state = .finished(iteratorInitialized: streaming.iteratorInitialized) - - return .resumeConsumerAndCallOnTermination( - consumerContinuation: consumerContinuation, - failure: failure, - onTermination: streaming.onTermination - ) - } else { + guard let consumerContinuation = streaming.consumerContinuation else { self._state = .sourceFinished( .init( iteratorInitialized: streaming.iteratorInitialized, @@ -1486,6 +1465,22 @@ extension BufferedStream { producerContinuations: Array(streaming.producerContinuations.map { $0.1 }) ) } + // We have a continuation, this means our buffer must be empty + // Furthermore, we can now transition to finished + // and resume the continuation with the failure + precondition(streaming.buffer.isEmpty, "Expected an empty buffer") + precondition( + streaming.producerContinuations.isEmpty, + "Expected no suspended producers" + ) + + self._state = .finished(iteratorInitialized: streaming.iteratorInitialized) + + return .resumeConsumerAndCallOnTermination( + consumerContinuation: consumerContinuation, + failure: failure, + onTermination: streaming.onTermination + ) case .sourceFinished, .finished: // If the source has finished, finishing again has no effect. @@ -1537,25 +1532,7 @@ extension BufferedStream { self._state = .modify - if let element = streaming.buffer.popFirst() { - // We have an element to fulfil the demand right away. - let shouldProduceMore = streaming.backPressureStrategy.didConsume( - bufferDepth: streaming.buffer.count - ) - streaming.hasOutstandingDemand = shouldProduceMore - - if shouldProduceMore { - // There is demand and we have to resume our producers - let producers = Array(streaming.producerContinuations.map { $0.1 }) - streaming.producerContinuations.removeAll() - self._state = .streaming(streaming) - return .returnElementAndResumeProducers(element, producers) - } else { - // We don't have any new demand, so we can just return the element. - self._state = .streaming(streaming) - return .returnElement(element) - } - } else { + guard let element = streaming.buffer.popFirst() else { // There is nothing in the buffer to fulfil the demand so we need to suspend. // We are not interacting with the back-pressure strategy here because // we are doing this inside `suspendNext` @@ -1563,16 +1540,28 @@ extension BufferedStream { return .suspendTask } + // We have an element to fulfil the demand right away. + let shouldProduceMore = streaming.backPressureStrategy.didConsume( + bufferDepth: streaming.buffer.count + ) + streaming.hasOutstandingDemand = shouldProduceMore + + guard shouldProduceMore else { + // We don't have any new demand, so we can just return the element. + self._state = .streaming(streaming) + return .returnElement(element) + } + // There is demand and we have to resume our producers + let producers = Array(streaming.producerContinuations.map { $0.1 }) + streaming.producerContinuations.removeAll() + self._state = .streaming(streaming) + return .returnElementAndResumeProducers(element, producers) case .sourceFinished(var sourceFinished): // Check if we have an element left in the buffer and return it self._state = .modify - if let element = sourceFinished.buffer.popFirst() { - self._state = .sourceFinished(sourceFinished) - - return .returnElement(element) - } else { + guard let element = sourceFinished.buffer.popFirst() else { // We are returning the queued failure now and can transition to finished self._state = .finished(iteratorInitialized: sourceFinished.iteratorInitialized) @@ -1581,6 +1570,9 @@ extension BufferedStream { sourceFinished.onTermination ) } + self._state = .sourceFinished(sourceFinished) + + return .returnElement(element) case .finished: return .returnNil @@ -1629,46 +1621,40 @@ extension BufferedStream { self._state = .modify // We have to check here again since we might have a producer interleave next and suspendNext - if let element = streaming.buffer.popFirst() { - // We have an element to fulfil the demand right away. - - let shouldProduceMore = streaming.backPressureStrategy.didConsume( - bufferDepth: streaming.buffer.count - ) - streaming.hasOutstandingDemand = shouldProduceMore - - if shouldProduceMore { - // There is demand and we have to resume our producers - let producers = Array(streaming.producerContinuations.map { $0.1 }) - streaming.producerContinuations.removeAll() - self._state = .streaming(streaming) - return .resumeConsumerWithElementAndProducers( - continuation, - element, - producers - ) - } else { - // We don't have any new demand, so we can just return the element. - self._state = .streaming(streaming) - return .resumeConsumerWithElement(continuation, element) - } - } else { + guard let element = streaming.buffer.popFirst() else { // There is nothing in the buffer to fulfil the demand so we to store the continuation. streaming.consumerContinuation = continuation self._state = .streaming(streaming) return .none } + // We have an element to fulfil the demand right away. + + let shouldProduceMore = streaming.backPressureStrategy.didConsume( + bufferDepth: streaming.buffer.count + ) + streaming.hasOutstandingDemand = shouldProduceMore + + guard shouldProduceMore else { + // We don't have any new demand, so we can just return the element. + self._state = .streaming(streaming) + return .resumeConsumerWithElement(continuation, element) + } + // There is demand and we have to resume our producers + let producers = Array(streaming.producerContinuations.map { $0.1 }) + streaming.producerContinuations.removeAll() + self._state = .streaming(streaming) + return .resumeConsumerWithElementAndProducers( + continuation, + element, + producers + ) case .sourceFinished(var sourceFinished): // Check if we have an element left in the buffer and return it self._state = .modify - if let element = sourceFinished.buffer.popFirst() { - self._state = .sourceFinished(sourceFinished) - - return .resumeConsumerWithElement(continuation, element) - } else { + guard let element = sourceFinished.buffer.popFirst() else { // We are returning the queued failure now and can transition to finished self._state = .finished(iteratorInitialized: sourceFinished.iteratorInitialized) @@ -1678,6 +1664,9 @@ extension BufferedStream { sourceFinished.onTermination ) } + self._state = .sourceFinished(sourceFinished) + + return .resumeConsumerWithElement(continuation, element) case .finished: return .resumeConsumerWithNil(continuation) @@ -1707,21 +1696,20 @@ extension BufferedStream { case .streaming(let streaming): self._state = .finished(iteratorInitialized: streaming.iteratorInitialized) - if let consumerContinuation = streaming.consumerContinuation { - precondition( - streaming.producerContinuations.isEmpty, - "Internal inconsistency. Unexpected producer continuations." - ) - return .resumeConsumerWithCancellationErrorAndCallOnTermination( - consumerContinuation, - streaming.onTermination - ) - } else { + guard let consumerContinuation = streaming.consumerContinuation else { return .failProducersAndCallOnTermination( Array(streaming.producerContinuations.map { $0.1 }), streaming.onTermination ) } + precondition( + streaming.producerContinuations.isEmpty, + "Internal inconsistency. Unexpected producer continuations." + ) + return .resumeConsumerWithCancellationErrorAndCallOnTermination( + consumerContinuation, + streaming.onTermination + ) case .sourceFinished, .finished: return .none diff --git a/Sources/NIOFileSystem/Internal/System Calls/Errno.swift b/Sources/NIOFileSystem/Internal/System Calls/Errno.swift index 0ac8876aff..a3c47d7289 100644 --- a/Sources/NIOFileSystem/Internal/System Calls/Errno.swift +++ b/Sources/NIOFileSystem/Internal/System Calls/Errno.swift @@ -70,16 +70,14 @@ public func valueOrErrno( while true { Errno.clear() let result = fn() - if result == -1 { - let errno = Errno._current - if errno == .interrupted, retryOnInterrupt { - continue - } else { - return .failure(errno) - } - } else { + guard result == -1 else { return .success(result) } + let errno = Errno._current + guard errno == .interrupted, retryOnInterrupt else { + return .failure(errno) + } + continue } } diff --git a/Sources/NIOFileSystem/Internal/System Calls/FileDescriptor+Syscalls.swift b/Sources/NIOFileSystem/Internal/System Calls/FileDescriptor+Syscalls.swift index d49c549056..4ba64d0450 100644 --- a/Sources/NIOFileSystem/Internal/System Calls/FileDescriptor+Syscalls.swift +++ b/Sources/NIOFileSystem/Internal/System Calls/FileDescriptor+Syscalls.swift @@ -51,12 +51,11 @@ extension FileDescriptor { let oFlag = mode.rawValue | options.rawValue let rawValue = valueOrErrno(retryOnInterrupt: retryOnInterrupt) { path.withPlatformString { - if let permissions = permissions { - return system_fdopenat(self.rawValue, $0, oFlag, permissions.rawValue) - } else { + guard let permissions = permissions else { precondition(!options.contains(.create), "Create must be given permissions") return system_fdopenat(self.rawValue, $0, oFlag) } + return system_fdopenat(self.rawValue, $0, oFlag, permissions.rawValue) } } @@ -279,11 +278,10 @@ extension FileDescriptor { bufferPointer = buffer } - if let offset { - return try self.read(fromAbsoluteOffset: offset, into: bufferPointer) - } else { + guard let offset else { return try self.read(into: bufferPointer) } + return try self.read(fromAbsoluteOffset: offset, into: bufferPointer) } return buffer } diff --git a/Sources/NIOFileSystem/Internal/SystemFileHandle.swift b/Sources/NIOFileSystem/Internal/SystemFileHandle.swift index 6f560c0de0..7037fbaf9a 100644 --- a/Sources/NIOFileSystem/Internal/SystemFileHandle.swift +++ b/Sources/NIOFileSystem/Internal/SystemFileHandle.swift @@ -161,11 +161,10 @@ extension SystemFileHandle.SendableView { _ execute: (FileDescriptor) throws -> R, onUnavailable: () -> FileSystemError ) throws -> R { - if let descriptor = self.descriptorIfAvailable() { - return try execute(descriptor) - } else { + guard let descriptor = self.descriptorIfAvailable() else { throw onUnavailable() } + return try execute(descriptor) } /// Executes a closure with the file descriptor if it's available otherwise returns the result @@ -174,11 +173,10 @@ extension SystemFileHandle.SendableView { _ execute: (FileDescriptor) -> Result, onUnavailable: () -> FileSystemError ) -> Result { - if let descriptor = self.descriptorIfAvailable() { - return execute(descriptor) - } else { + guard let descriptor = self.descriptorIfAvailable() else { return .failure(onUnavailable()) } + return execute(descriptor) } } @@ -738,11 +736,10 @@ extension SystemFileHandle.SendableView { let linkAtResult = linkAtEmptyPath().flatMapError { errno in // ENOENT means we likely didn't have the 'CAP_DAC_READ_SEARCH' capability // so try again by linking to the descriptor via procfs. - if errno == .noSuchFileOrDirectory { - return linkAtProcFS() - } else { + guard errno == .noSuchFileOrDirectory else { return .failure(errno) } + return linkAtProcFS() }.mapError { errno in FileSystemError.link( errno: errno, @@ -756,24 +753,7 @@ extension SystemFileHandle.SendableView { case .failure(.noSuchFileOrDirectory): result = linkAtProcFS().flatMapError { errno in - if errno == .fileExists, !exclusive { - return Libc.remove(desiredPath).mapError { errno in - FileSystemError.remove( - errno: errno, - path: desiredPath, - location: .here() - ) - }.flatMap { - linkAtProcFS().mapError { errno in - FileSystemError.link( - errno: errno, - from: createdPath, - to: desiredPath, - location: .here() - ) - } - } - } else { + guard errno == .fileExists, !exclusive else { let error = FileSystemError.link( errno: errno, from: createdPath, @@ -782,6 +762,22 @@ extension SystemFileHandle.SendableView { ) return .failure(error) } + return Libc.remove(desiredPath).mapError { errno in + FileSystemError.remove( + errno: errno, + path: desiredPath, + location: .here() + ) + }.flatMap { + linkAtProcFS().mapError { errno in + FileSystemError.link( + errno: errno, + from: createdPath, + to: desiredPath, + location: .here() + ) + } + } } case .failure(let errno): @@ -1029,36 +1025,35 @@ extension SystemFileHandle: ReadableFileHandleProtocol { fromAbsoluteOffset: offset, length: length.bytes ).flatMapError { error in - if let errno = error as? Errno, errno == .illegalSeek { - guard offset == 0 else { - return .failure( - FileSystemError( - code: .unsupported, - message: "File is unseekable.", - cause: nil, - location: .here() - ) - ) - } - - return descriptor.readChunk(length: length.bytes).mapError { error in + guard let errno = error as? Errno, errno == .illegalSeek else { + return .failure( FileSystemError.read( - usingSyscall: .read, + usingSyscall: .pread, error: error, path: sendableView.path, location: .here() ) - } - } else { + ) + } + guard offset == 0 else { return .failure( - FileSystemError.read( - usingSyscall: .pread, - error: error, - path: sendableView.path, + FileSystemError( + code: .unsupported, + message: "File is unseekable.", + cause: nil, location: .here() ) ) } + + return descriptor.readChunk(length: length.bytes).mapError { error in + FileSystemError.read( + usingSyscall: .read, + error: error, + path: sendableView.path, + location: .here() + ) + } } .get() } onUnavailable: { @@ -1093,28 +1088,7 @@ extension SystemFileHandle: WritableFileHandleProtocol { try sendableView._withUnsafeDescriptor { descriptor in try descriptor.write(contentsOf: bytes, toAbsoluteOffset: offset) .flatMapError { error in - if let errno = error as? Errno, errno == .illegalSeek { - guard offset == 0 else { - return .failure( - FileSystemError( - code: .unsupported, - message: "File is unseekable.", - cause: nil, - location: .here() - ) - ) - } - - return descriptor.write(contentsOf: bytes) - .mapError { error in - FileSystemError.write( - usingSyscall: .write, - error: error, - path: sendableView.path, - location: .here() - ) - } - } else { + guard let errno = error as? Errno, errno == .illegalSeek else { return .failure( FileSystemError.write( usingSyscall: .pwrite, @@ -1124,6 +1098,26 @@ extension SystemFileHandle: WritableFileHandleProtocol { ) ) } + guard offset == 0 else { + return .failure( + FileSystemError( + code: .unsupported, + message: "File is unseekable.", + cause: nil, + location: .here() + ) + ) + } + + return descriptor.write(contentsOf: bytes) + .mapError { error in + FileSystemError.write( + usingSyscall: .write, + error: error, + path: sendableView.path, + location: .here() + ) + } } .get() } onUnavailable: { @@ -1336,27 +1330,7 @@ extension SystemFileHandle { let truncate = options.contains(.truncate) let delayMaterialization = transactional && isWritable && (exclusiveCreate || truncate) - if delayMaterialization { - // When opening in this mode we can more "atomically" create the file, that is, by not - // leaving the user with a half written file should e.g. the system crash or throw an - // error while writing. On non-Android Linux we do this by opening the directory for - // the path with `O_TMPFILE` and creating a hard link when closing the file. On other - // platforms we generate a dot file with a randomised suffix name and rename it to the - // destination. - #if os(Android) - let temporaryHardLink = false - #else - let temporaryHardLink = true - #endif - return Self.syncOpenWithMaterialization( - atPath: path, - mode: mode, - options: options, - permissions: permissions, - threadPool: threadPool, - useTemporaryFileIfPossible: temporaryHardLink - ) - } else { + guard delayMaterialization else { return Self.syncOpen( atPath: path, mode: mode, @@ -1365,6 +1339,25 @@ extension SystemFileHandle { threadPool: threadPool ) } + // When opening in this mode we can more "atomically" create the file, that is, by not + // leaving the user with a half written file should e.g. the system crash or throw an + // error while writing. On non-Android Linux we do this by opening the directory for + // the path with `O_TMPFILE` and creating a hard link when closing the file. On other + // platforms we generate a dot file with a randomised suffix name and rename it to the + // destination. + #if os(Android) + let temporaryHardLink = false + #else + let temporaryHardLink = true + #endif + return Self.syncOpenWithMaterialization( + atPath: path, + mode: mode, + options: options, + permissions: permissions, + threadPool: threadPool, + useTemporaryFileIfPossible: temporaryHardLink + ) } static func syncOpen( diff --git a/Sources/NIOFileSystem/OpenOptions.swift b/Sources/NIOFileSystem/OpenOptions.swift index 454826897a..1bf901c553 100644 --- a/Sources/NIOFileSystem/OpenOptions.swift +++ b/Sources/NIOFileSystem/OpenOptions.swift @@ -199,11 +199,10 @@ extension OpenOptions { extension OpenOptions.Write { @_spi(Testing) public var permissionsForRegularFile: FilePermissions? { - if let newFile = self.newFile { - return newFile.permissions ?? .defaultsForRegularFile - } else { + guard let newFile = self.newFile else { return nil } + return newFile.permissions ?? .defaultsForRegularFile } var descriptorOptions: FileDescriptor.OpenOptions { diff --git a/Sources/NIOFoundationCompat/ByteBuffer-foundation.swift b/Sources/NIOFoundationCompat/ByteBuffer-foundation.swift index 7aedfb17b0..452862ee45 100644 --- a/Sources/NIOFoundationCompat/ByteBuffer-foundation.swift +++ b/Sources/NIOFoundationCompat/ByteBuffer-foundation.swift @@ -120,12 +120,7 @@ extension ByteBuffer { } return self.withUnsafeReadableBytesWithStorageManagement { ptr, storageRef in - if doCopy { - return Data( - bytes: UnsafeMutableRawPointer(mutating: ptr.baseAddress!.advanced(by: index)), - count: Int(length) - ) - } else { + guard doCopy else { _ = storageRef.retain() return Data( bytesNoCopy: UnsafeMutableRawPointer(mutating: ptr.baseAddress!.advanced(by: index)), @@ -133,6 +128,10 @@ extension ByteBuffer { deallocator: .custom { _, _ in storageRef.release() } ) } + return Data( + bytes: UnsafeMutableRawPointer(mutating: ptr.baseAddress!.advanced(by: index)), + count: Int(length) + ) } } diff --git a/Sources/NIOHTTP1/ByteCollectionUtils.swift b/Sources/NIOHTTP1/ByteCollectionUtils.swift index 95b6f8dff5..aeb30328b0 100644 --- a/Sources/NIOHTTP1/ByteCollectionUtils.swift +++ b/Sources/NIOHTTP1/ByteCollectionUtils.swift @@ -59,11 +59,10 @@ extension Sequence where Self.Element == UInt8 { } } - if let maybeResult = maybeMaybeResult, let result = maybeResult { - return result - } else { + guard let maybeResult = maybeMaybeResult, let result = maybeResult else { return self.elementsEqual(to, by: { ($0 & 0xdf) == ($1 & 0xdf) }) } + return result } } diff --git a/Sources/NIOHTTP1/HTTPDecoder.swift b/Sources/NIOHTTP1/HTTPDecoder.swift index eb33c620ba..036113e25f 100644 --- a/Sources/NIOHTTP1/HTTPDecoder.swift +++ b/Sources/NIOHTTP1/HTTPDecoder.swift @@ -430,18 +430,13 @@ private class BetterHTTPParser { // If we have a richer error than the errno code, and the errno is internal, we'll use it. Otherwise, we use the // error from http_parser. let err = self.httpErrno ?? parserErrno - if err == HPE_INTERNAL || err == HPE_USER, let richerError = self.richerError { - throw richerError - } else { + guard err == HPE_INTERNAL || err == HPE_USER, let richerError = self.richerError else { throw HTTPParserError.httpError(fromCHTTPParserErrno: err)! } + throw richerError } - if let firstNonDiscardableOffset = self.firstNonDiscardableOffset { - self.httpParserOffset += bytesRead - firstNonDiscardableOffset - self.firstNonDiscardableOffset = 0 - return firstNonDiscardableOffset - } else { + guard let firstNonDiscardableOffset = self.firstNonDiscardableOffset else { // By definition we've consumed all of the http parser offset at this stage. There may still be bytes // left in the buffer though: we didn't consume them because they aren't ours to consume, as they may belong // to an upgraded protocol. @@ -452,6 +447,9 @@ private class BetterHTTPParser { self.httpParserOffset = 0 return consumedBytes } + self.httpParserOffset += bytesRead - firstNonDiscardableOffset + self.firstNonDiscardableOffset = 0 + return firstNonDiscardableOffset } } diff --git a/Sources/NIOHTTP1/HTTPEncoder.swift b/Sources/NIOHTTP1/HTTPEncoder.swift index 19bc6109ac..345ba87cdd 100644 --- a/Sources/NIOHTTP1/HTTPEncoder.swift +++ b/Sources/NIOHTTP1/HTTPEncoder.swift @@ -135,12 +135,11 @@ private func correctlyFrameTransportHeaders( if headers.contains(name: "content-length") { return .contentLength } - if version.major == 1 && version.minor >= 1 { - headers.replaceOrAdd(name: "transfer-encoding", value: "chunked") - return .chunked - } else { + guard version.major == 1 && version.minor >= 1 else { return .neither } + headers.replaceOrAdd(name: "transfer-encoding", value: "chunked") + return .chunked case .unlikely: if headers.contains(name: "content-length") { return .contentLength @@ -155,11 +154,10 @@ private func correctlyFrameTransportHeaders( /// Validates the response/request headers to ensure that we correctly send the body with chunked transfer /// encoding, when needed. private func messageIsChunked(headers: HTTPHeaders, version: HTTPVersion) -> Bool { - if version.major == 1 && version.minor >= 1 { - return headers.first(name: "transfer-encoding") == "chunked" ? true : false - } else { + guard version.major == 1 && version.minor >= 1 else { return false } + return headers.first(name: "transfer-encoding") == "chunked" ? true : false } /// A `ChannelOutboundHandler` that can serialize HTTP requests. diff --git a/Sources/NIOHTTP1/HTTPHeaders+Validation.swift b/Sources/NIOHTTP1/HTTPHeaders+Validation.swift index b5a29c695f..57a3c2ba3f 100644 --- a/Sources/NIOHTTP1/HTTPHeaders+Validation.swift +++ b/Sources/NIOHTTP1/HTTPHeaders+Validation.swift @@ -43,11 +43,10 @@ extension String { let fastResult = self.utf8.withContiguousStorageIfAvailable { ptr in ptr.allSatisfy { $0.isValidHeaderFieldValueByte } } - if let fastResult = fastResult { - return fastResult - } else { + guard let fastResult = fastResult else { return self.utf8._isValidHeaderFieldValue_slowPath } + return fastResult } /// Validates a given header field name against the definition in RFC 9110. @@ -71,11 +70,10 @@ extension String { let fastResult = self.utf8.withContiguousStorageIfAvailable { ptr in ptr.allSatisfy { $0.isValidHeaderFieldNameByte } } - if let fastResult = fastResult { - return fastResult - } else { + guard let fastResult = fastResult else { return self.utf8._isValidHeaderFieldName_slowPath } + return fastResult } } diff --git a/Sources/NIOHTTP1/NIOHTTPObjectAggregator.swift b/Sources/NIOHTTP1/NIOHTTPObjectAggregator.swift index 3821e6e2b4..b0b53e8e47 100644 --- a/Sources/NIOHTTP1/NIOHTTPObjectAggregator.swift +++ b/Sources/NIOHTTP1/NIOHTTPObjectAggregator.swift @@ -256,12 +256,11 @@ public final class NIOHTTPServerRequestAggregator: ChannelInboundHandler, Remova content: inout ByteBuffer, message: InboundIn ) -> HTTPResponseHead? { - if content.readableBytes > self.maxContentLength - self.buffer.readableBytes { - return self.handleOversizeMessage(message: message) - } else { + guard content.readableBytes > self.maxContentLength - self.buffer.readableBytes else { self.buffer.writeBuffer(&content) return nil } + return self.handleOversizeMessage(message: message) } private func endAggregation(context: ChannelHandlerContext, trailingHeaders: HTTPHeaders?) { diff --git a/Sources/NIOHTTP1/NIOTypedHTTPClientUpgraderStateMachine.swift b/Sources/NIOHTTP1/NIOTypedHTTPClientUpgraderStateMachine.swift index be51bd22f5..86c0d69f21 100644 --- a/Sources/NIOHTTP1/NIOTypedHTTPClientUpgraderStateMachine.swift +++ b/Sources/NIOHTTP1/NIOTypedHTTPClientUpgraderStateMachine.swift @@ -273,25 +273,23 @@ struct NIOTypedHTTPClientUpgraderStateMachine { case .upgrading(let upgrading): switch result { case .success(let value): - if !upgrading.buffer.isEmpty { - self.state = .unbuffering(.init(buffer: upgrading.buffer)) - return .startUnbuffering(value) - } else { + guard !upgrading.buffer.isEmpty else { self.state = .finished return .removeHandler(value) } + self.state = .unbuffering(.init(buffer: upgrading.buffer)) + return .startUnbuffering(value) case .failure(let error): - if !upgrading.buffer.isEmpty { - // So we failed to upgrade. There is nothing really that we can do here. - // We are unbuffering the reads but there shouldn't be any handler in the pipeline - // that expects a specific type of reads anyhow. - self.state = .unbuffering(.init(buffer: upgrading.buffer)) - return .fireErrorCaughtAndStartUnbuffering(error) - } else { + guard !upgrading.buffer.isEmpty else { self.state = .finished return .fireErrorCaughtAndRemoveHandler(error) } + // So we failed to upgrade. There is nothing really that we can do here. + // We are unbuffering the reads but there shouldn't be any handler in the pipeline + // that expects a specific type of reads anyhow. + self.state = .unbuffering(.init(buffer: upgrading.buffer)) + return .fireErrorCaughtAndStartUnbuffering(error) } case .finished: @@ -318,15 +316,14 @@ struct NIOTypedHTTPClientUpgraderStateMachine { case .unbuffering(var unbuffering): self.state = .modifying - if let element = unbuffering.buffer.popFirst() { - self.state = .unbuffering(unbuffering) - - return .fireChannelRead(element) - } else { + guard let element = unbuffering.buffer.popFirst() else { self.state = .finished return .fireChannelReadCompleteAndRemoveHandler } + self.state = .unbuffering(unbuffering) + + return .fireChannelRead(element) case .modifying: fatalError("Internal inconsistency in HTTPClientUpgradeStateMachine") diff --git a/Sources/NIOHTTP1/NIOTypedHTTPServerUpgraderStateMachine.swift b/Sources/NIOHTTP1/NIOTypedHTTPServerUpgraderStateMachine.swift index 80fd018944..4aac100757 100644 --- a/Sources/NIOHTTP1/NIOTypedHTTPServerUpgraderStateMachine.swift +++ b/Sources/NIOHTTP1/NIOTypedHTTPServerUpgraderStateMachine.swift @@ -96,16 +96,15 @@ struct NIOTypedHTTPServerUpgraderStateMachine { return .unwrapData case .awaitingUpgrader(var awaitingUpgrader): - if awaitingUpgrader.seenFirstRequest { - // We should buffer the data since we have seen the full request. - self.state = .modifying - awaitingUpgrader.buffer.append(data) - self.state = .awaitingUpgrader(awaitingUpgrader) - return nil - } else { + guard awaitingUpgrader.seenFirstRequest else { // We shouldn't buffer. This means we are still expecting HTTP parts. return .unwrapData } + // We should buffer the data since we have seen the full request. + self.state = .modifying + awaitingUpgrader.buffer.append(data) + self.state = .awaitingUpgrader(awaitingUpgrader) + return nil case .upgraderReady: // We have not seen the end of the HTTP request so this @@ -258,25 +257,23 @@ struct NIOTypedHTTPServerUpgraderStateMachine { case .upgrading(let upgrading): switch result { case .success(let value): - if !upgrading.buffer.isEmpty { - self.state = .unbuffering(.init(buffer: upgrading.buffer)) - return .startUnbuffering(value) - } else { + guard !upgrading.buffer.isEmpty else { self.state = .finished return .removeHandler(value) } + self.state = .unbuffering(.init(buffer: upgrading.buffer)) + return .startUnbuffering(value) case .failure(let error): - if !upgrading.buffer.isEmpty { - // So we failed to upgrade. There is nothing really that we can do here. - // We are unbuffering the reads but there shouldn't be any handler in the pipeline - // that expects a specific type of reads anyhow. - self.state = .unbuffering(.init(buffer: upgrading.buffer)) - return .fireErrorCaughtAndStartUnbuffering(error) - } else { + guard !upgrading.buffer.isEmpty else { self.state = .finished return .fireErrorCaughtAndRemoveHandler(error) } + // So we failed to upgrade. There is nothing really that we can do here. + // We are unbuffering the reads but there shouldn't be any handler in the pipeline + // that expects a specific type of reads anyhow. + self.state = .unbuffering(.init(buffer: upgrading.buffer)) + return .fireErrorCaughtAndStartUnbuffering(error) } case .finished: @@ -320,11 +317,7 @@ struct NIOTypedHTTPServerUpgraderStateMachine { case .awaitingUpgrader(let awaitingUpgrader): switch result { case .success(.some((let upgrader, let responseHeaders, let proto))): - if awaitingUpgrader.seenFirstRequest { - // We have seen the end of the request. So we can upgrade now. - self.state = .upgrading(.init(buffer: awaitingUpgrader.buffer)) - return .startUpgrading(upgrader: upgrader, responseHeaders: responseHeaders, proto: proto) - } else { + guard awaitingUpgrader.seenFirstRequest else { // We have not yet seen the end so we have to wait until that happens self.state = .upgraderReady( .init( @@ -337,6 +330,9 @@ struct NIOTypedHTTPServerUpgraderStateMachine { ) return nil } + // We have seen the end of the request. So we can upgrade now. + self.state = .upgrading(.init(buffer: awaitingUpgrader.buffer)) + return .startUpgrading(upgrader: upgrader, responseHeaders: responseHeaders, proto: proto) case .success(.none): // There was no upgrader to handle the request. We just run the not upgrading @@ -345,13 +341,12 @@ struct NIOTypedHTTPServerUpgraderStateMachine { return .runNotUpgradingInitializer case .failure(let error): - if !awaitingUpgrader.buffer.isEmpty { - self.state = .unbuffering(.init(buffer: awaitingUpgrader.buffer)) - return .fireErrorCaughtAndStartUnbuffering(error) - } else { + guard !awaitingUpgrader.buffer.isEmpty else { self.state = .finished return .fireErrorCaughtAndRemoveHandler(error) } + self.state = .unbuffering(.init(buffer: awaitingUpgrader.buffer)) + return .fireErrorCaughtAndStartUnbuffering(error) } case .upgrading, .unbuffering, .finished: @@ -377,15 +372,14 @@ struct NIOTypedHTTPServerUpgraderStateMachine { case .unbuffering(var unbuffering): self.state = .modifying - if let element = unbuffering.buffer.popFirst() { - self.state = .unbuffering(unbuffering) - - return .fireChannelRead(element) - } else { + guard let element = unbuffering.buffer.popFirst() else { self.state = .finished return .fireChannelReadCompleteAndRemoveHandler } + self.state = .unbuffering(unbuffering) + + return .fireChannelRead(element) case .modifying: fatalError("Internal inconsistency in HTTPServerUpgradeStateMachine") diff --git a/Sources/NIOHTTP1Server/main.swift b/Sources/NIOHTTP1Server/main.swift index 728512fac8..313de3615b 100644 --- a/Sources/NIOHTTP1Server/main.swift +++ b/Sources/NIOHTTP1Server/main.swift @@ -17,11 +17,10 @@ import NIOPosix extension String { func chopPrefix(_ prefix: String) -> String? { - if self.unicodeScalars.starts(with: prefix.unicodeScalars) { - return String(self[self.index(self.startIndex, offsetBy: prefix.count)...]) - } else { + guard self.unicodeScalars.starts(with: prefix.unicodeScalars) else { return nil } + return String(self[self.index(self.startIndex, offsetBy: prefix.count)...]) } func containsDotDot() -> Bool { @@ -429,17 +428,16 @@ private final class HTTPHandler: ChannelInboundHandler { self.completeResponse(context, trailers: nil, promise: p) return p.futureResult }.flatMapError { error in - if !responseStarted { - let response = httpResponseHead(request: request, status: .ok) - context.write(Self.wrapOutboundOut(.head(response)), promise: nil) - var buffer = context.channel.allocator.buffer(capacity: 100) - buffer.writeString("fail: \(error)") - context.write(Self.wrapOutboundOut(.body(.byteBuffer(buffer))), promise: nil) - self.state.responseComplete() - return context.writeAndFlush(Self.wrapOutboundOut(.end(nil))) - } else { + guard !responseStarted else { return context.close() } + let response = httpResponseHead(request: request, status: .ok) + context.write(Self.wrapOutboundOut(.head(response)), promise: nil) + var buffer = context.channel.allocator.buffer(capacity: 100) + buffer.writeString("fail: \(error)") + context.write(Self.wrapOutboundOut(.body(.byteBuffer(buffer))), promise: nil) + self.state.responseComplete() + return context.writeAndFlush(Self.wrapOutboundOut(.end(nil))) }.whenComplete { (_: Result) in _ = try? file.close() } diff --git a/Sources/NIOPerformanceTester/ByteToMessageDecoderDecodeManySmallsBenchmark.swift b/Sources/NIOPerformanceTester/ByteToMessageDecoderDecodeManySmallsBenchmark.swift index 623a58f0c2..c506e1cbfb 100644 --- a/Sources/NIOPerformanceTester/ByteToMessageDecoderDecodeManySmallsBenchmark.swift +++ b/Sources/NIOPerformanceTester/ByteToMessageDecoderDecodeManySmallsBenchmark.swift @@ -44,11 +44,10 @@ final class ByteToMessageDecoderDecodeManySmallsBenchmark: Benchmark { typealias InboundOut = Never func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { - if buffer.readSlice(length: 16) == nil { - return .needMoreData - } else { + guard buffer.readSlice(length: 16) == nil else { return .continue } + return .needMoreData } } } diff --git a/Sources/NIOPerformanceTester/UDPBenchmark.swift b/Sources/NIOPerformanceTester/UDPBenchmark.swift index 090b261bb8..fc72312ffa 100644 --- a/Sources/NIOPerformanceTester/UDPBenchmark.swift +++ b/Sources/NIOPerformanceTester/UDPBenchmark.swift @@ -167,13 +167,12 @@ extension UDPBenchmark { switch self.state { case .running(var running): running.responsesToRecieve &-= 1 - if running.responsesToRecieve == 0, running.requestsToSend == 0 { - self.state = .stopped - return .finished(running.promise) - } else { + guard running.responsesToRecieve == 0, running.requestsToSend == 0 else { self.state = .running(running) return .write } + self.state = .stopped + return .finished(running.promise) case .stopped: fatalError("Received too many messages") diff --git a/Sources/NIOPosix/BSDSocketAPIPosix.swift b/Sources/NIOPosix/BSDSocketAPIPosix.swift index 852ab5b4d2..1519b8d1f9 100644 --- a/Sources/NIOPosix/BSDSocketAPIPosix.swift +++ b/Sources/NIOPosix/BSDSocketAPIPosix.swift @@ -246,11 +246,10 @@ extension NIOBSDSocket { } // Only unlink the existing file if it is a socket - if sb.st_mode & S_IFSOCK == S_IFSOCK { - try Posix.unlink(pathname: path) - } else { + guard sb.st_mode & S_IFSOCK == S_IFSOCK else { throw UnixDomainSocketPathWrongType() } + try Posix.unlink(pathname: path) } catch let err as IOError { // If the filepath did not exist, we consider it cleaned up if err.errnoCode == ENOENT { diff --git a/Sources/NIOPosix/BaseSocketChannel+SocketOptionProvider.swift b/Sources/NIOPosix/BaseSocketChannel+SocketOptionProvider.swift index c50429abd3..859f6a5aa0 100644 --- a/Sources/NIOPosix/BaseSocketChannel+SocketOptionProvider.swift +++ b/Sources/NIOPosix/BaseSocketChannel+SocketOptionProvider.swift @@ -33,17 +33,16 @@ extension BaseSocketChannel: SocketOptionProvider { name: NIOBSDSocket.Option, value: Value ) -> EventLoopFuture { - if eventLoop.inEventLoop { - let promise = eventLoop.makePromise(of: Void.self) - executeAndComplete(promise) { - try setSocketOption0(level: level, name: name, value: value) - } - return promise.futureResult - } else { + guard eventLoop.inEventLoop else { return eventLoop.submit { try self.setSocketOption0(level: level, name: name, value: value) } } + let promise = eventLoop.makePromise(of: Void.self) + executeAndComplete(promise) { + try setSocketOption0(level: level, name: name, value: value) + } + return promise.futureResult } #if !os(Windows) @@ -59,17 +58,16 @@ extension BaseSocketChannel: SocketOptionProvider { level: NIOBSDSocket.OptionLevel, name: NIOBSDSocket.Option ) -> EventLoopFuture { - if eventLoop.inEventLoop { - let promise = eventLoop.makePromise(of: Value.self) - executeAndComplete(promise) { - try getSocketOption0(level: level, name: name) - } - return promise.futureResult - } else { + guard eventLoop.inEventLoop else { return eventLoop.submit { try self.getSocketOption0(level: level, name: name) } } + let promise = eventLoop.makePromise(of: Value.self) + executeAndComplete(promise) { + try getSocketOption0(level: level, name: name) + } + return promise.futureResult } func setSocketOption0(level: NIOBSDSocket.OptionLevel, name: NIOBSDSocket.Option, value: Value) throws { diff --git a/Sources/NIOPosix/BaseSocketChannel.swift b/Sources/NIOPosix/BaseSocketChannel.swift index 98cd75a57d..b6bac1301a 100644 --- a/Sources/NIOPosix/BaseSocketChannel.swift +++ b/Sources/NIOPosix/BaseSocketChannel.swift @@ -272,13 +272,12 @@ class BaseSocketChannel: SelectableChannel, Chan // This is called from arbitrary threads. internal var addressesCached: AddressCache { get { - if self.eventLoop.inEventLoop { - return self._addressCache - } else { + guard self.eventLoop.inEventLoop else { return self._offEventLoopLock.withLock { self._addressCache } } + return self._addressCache } set { self.eventLoop.preconditionInEventLoop() @@ -291,13 +290,12 @@ class BaseSocketChannel: SelectableChannel, Chan // This is called from arbitrary threads. private var bufferAllocatorCached: ByteBufferAllocator { get { - if self.eventLoop.inEventLoop { - return self._bufferAllocatorCache - } else { + guard self.eventLoop.inEventLoop else { return self._offEventLoopLock.withLock { self._bufferAllocatorCache } } + return self._bufferAllocatorCache } set { self.eventLoop.preconditionInEventLoop() @@ -608,13 +606,12 @@ class BaseSocketChannel: SelectableChannel, Chan } public final func setOption(_ option: Option, value: Option.Value) -> EventLoopFuture { - if eventLoop.inEventLoop { - let promise = eventLoop.makePromise(of: Void.self) - executeAndComplete(promise) { try self.setOption0(option, value: value) } - return promise.futureResult - } else { + guard eventLoop.inEventLoop else { return eventLoop.submit { try self.setOption0(option, value: value) } } + let promise = eventLoop.makePromise(of: Void.self) + executeAndComplete(promise) { try self.setOption0(option, value: value) } + return promise.futureResult } func setOption0(_ option: Option, value: Option.Value) throws { @@ -654,15 +651,14 @@ class BaseSocketChannel: SelectableChannel, Chan } public func getOption(_ option: Option) -> EventLoopFuture { - if eventLoop.inEventLoop { - do { - return self.eventLoop.makeSucceededFuture(try self.getOption0(option)) - } catch { - return self.eventLoop.makeFailedFuture(error) - } - } else { + guard eventLoop.inEventLoop else { return self.eventLoop.submit { try self.getOption0(option) } } + do { + return self.eventLoop.makeSucceededFuture(try self.getOption0(option)) + } catch { + return self.eventLoop.makeFailedFuture(error) + } } func getOption0(_ option: Option) throws -> Option.Value { diff --git a/Sources/NIOPosix/BaseStreamSocketChannel.swift b/Sources/NIOPosix/BaseStreamSocketChannel.swift index 605f551464..9b6ef48ceb 100644 --- a/Sources/NIOPosix/BaseStreamSocketChannel.swift +++ b/Sources/NIOPosix/BaseStreamSocketChannel.swift @@ -125,22 +125,7 @@ class BaseStreamSocketChannel: BaseSocketChannel // the end of the loop to not do an allocation when the loop exits. switch readResult { case .processed(let bytesRead): - if bytesRead > 0 { - self.recvBufferPool.record(actualReadBytes: bytesRead) - self.readPending = false - - assert(self.isActive) - self.pipeline.syncOperations.fireChannelRead(NIOAny(buffer)) - result = .some - - if buffer.writableBytes > 0 { - // If we did not fill the whole buffer with read(...) we should stop reading and wait until we get notified again. - // Otherwise chances are good that the next read(...) call will either read nothing or only a very small amount of data. - // Also this will allow us to call fireChannelReadComplete() which may give the user the chance to flush out all pending - // writes. - return result - } - } else { + guard bytesRead > 0 else { if self.inputShutdown { // We received a EOF because we called shutdown on the fd by ourself, unregister from the Selector and return self.readPending = false @@ -150,6 +135,20 @@ class BaseStreamSocketChannel: BaseSocketChannel // end-of-file throw ChannelError._eof } + self.recvBufferPool.record(actualReadBytes: bytesRead) + self.readPending = false + + assert(self.isActive) + self.pipeline.syncOperations.fireChannelRead(NIOAny(buffer)) + result = .some + + if buffer.writableBytes > 0 { + // If we did not fill the whole buffer with read(...) we should stop reading and wait until we get notified again. + // Otherwise chances are good that the next read(...) call will either read nothing or only a very small amount of data. + // Also this will allow us to call fireChannelReadComplete() which may give the user the chance to flush out all pending + // writes. + return result + } case .wouldBlock(let bytesRead): assert(bytesRead == 0) return result diff --git a/Sources/NIOPosix/Bootstrap.swift b/Sources/NIOPosix/Bootstrap.swift index 6deaae1e1c..ae1c1ef94e 100644 --- a/Sources/NIOPosix/Bootstrap.swift +++ b/Sources/NIOPosix/Bootstrap.swift @@ -792,16 +792,15 @@ public final class ClientBootstrap: NIOClientTCPBootstrapProtocol { private var protocolHandlers: Optional<@Sendable () -> [ChannelHandler]> private var _channelInitializer: ChannelInitializerCallback private var channelInitializer: ChannelInitializerCallback { - if let protocolHandlers = self.protocolHandlers { - let channelInitializer = _channelInitializer - return { channel in - channelInitializer(channel).flatMap { - channel.pipeline.addHandlers(protocolHandlers(), position: .first) - } - } - } else { + guard let protocolHandlers = self.protocolHandlers else { return self._channelInitializer } + let channelInitializer = _channelInitializer + return { channel in + channelInitializer(channel).flatMap { + channel.pipeline.addHandlers(protocolHandlers(), position: .first) + } + } } @usableFromInline internal var _channelOptions: ChannelOptions.Storage @@ -1110,11 +1109,10 @@ public final class ClientBootstrap: NIOClientTCPBootstrapProtocol { } } - if eventLoop.inEventLoop { - return setupChannel() - } else { + guard eventLoop.inEventLoop else { return eventLoop.flatSubmit { setupChannel() } } + return setupChannel() } private func initializeAndRegisterNewChannel( @@ -1143,13 +1141,12 @@ public final class ClientBootstrap: NIOClientTCPBootstrapProtocol { func setupChannel() -> EventLoopFuture { eventLoop.assertInEventLoop() return channelOptions.applyAllChannelOptions(to: channel).flatMap { - if let bindTarget = self.bindTarget { - return channel.bind(to: bindTarget).flatMap { - channelInitializer(channel) - } - } else { + guard let bindTarget = self.bindTarget else { return channelInitializer(channel) } + return channel.bind(to: bindTarget).flatMap { + channelInitializer(channel) + } }.flatMap { eventLoop.assertInEventLoop() return channel.registerAndDoSynchronously(body) @@ -1161,13 +1158,12 @@ public final class ClientBootstrap: NIOClientTCPBootstrapProtocol { } } - if eventLoop.inEventLoop { - return setupChannel() - } else { + guard eventLoop.inEventLoop else { return eventLoop.flatSubmit { setupChannel() } } + return setupChannel() } } @@ -1415,16 +1411,15 @@ extension ClientBootstrap { channelOptions .applyAllChannelOptions(to: channel) .flatMap { - if let bindTarget = bindTarget { - return - channel - .bind(to: bindTarget) - .flatMap { - channelInitializer(channel) - } - } else { + guard let bindTarget = bindTarget else { return channelInitializer(channel) } + return + channel + .bind(to: bindTarget) + .flatMap { + channelInitializer(channel) + } }.flatMap { (result: ChannelInitializerResult) in eventLoop.assertInEventLoop() return registration(channel).map { @@ -1440,13 +1435,12 @@ extension ClientBootstrap { } } - if eventLoop.inEventLoop { - return setupChannel() - } else { + guard eventLoop.inEventLoop else { return eventLoop.flatSubmit { setupChannel() } } + return setupChannel() } } @@ -1718,13 +1712,12 @@ public final class DatagramBootstrap { } } - if eventLoop.inEventLoop { - return setupChannel() - } else { + guard eventLoop.inEventLoop else { return eventLoop.flatSubmit { setupChannel() } } + return setupChannel() } } @@ -2012,13 +2005,12 @@ extension DatagramBootstrap { } } - if eventLoop.inEventLoop { - return setupChannel() - } else { + guard eventLoop.inEventLoop else { return eventLoop.flatSubmit { setupChannel() } } + return setupChannel() } } @@ -2230,11 +2222,10 @@ public final class NIOPipeBootstrap { let eventLoop = self.group.next() let channelInitializer = self.channelInitializer return { channel in - if let channelInitializer = channelInitializer { - return channelInitializer(channel).map { channel } - } else { + guard let channelInitializer = channelInitializer else { return eventLoop.makeSucceededFuture(channel) } + return channelInitializer(channel).map { channel } } }() @@ -2455,13 +2446,12 @@ extension NIOPipeBootstrap { } } - if eventLoop.inEventLoop { - return setupChannel() - } else { + guard eventLoop.inEventLoop else { return eventLoop.flatSubmit { setupChannel() } } + return setupChannel() } } diff --git a/Sources/NIOPosix/GetaddrinfoResolver.swift b/Sources/NIOPosix/GetaddrinfoResolver.swift index c5a7e5bfa7..512dca2975 100644 --- a/Sources/NIOPosix/GetaddrinfoResolver.swift +++ b/Sources/NIOPosix/GetaddrinfoResolver.swift @@ -104,14 +104,13 @@ internal class GetaddrinfoResolver: Resolver { if let offloadQueue = offloadQueueTSV.currentValue { return offloadQueue } else { - if MultiThreadedEventLoopGroup.currentEventLoop != nil { - // Okay, we're on an SelectableEL thread. Let's stuff our queue into the thread local. - let offloadQueue = DispatchQueue(label: "io.swiftnio.GetaddrinfoResolver.offloadQueue") - offloadQueueTSV.currentValue = offloadQueue - return offloadQueue - } else { + guard MultiThreadedEventLoopGroup.currentEventLoop != nil else { return DispatchQueue.global() } + // Okay, we're on an SelectableEL thread. Let's stuff our queue into the thread local. + let offloadQueue = DispatchQueue(label: "io.swiftnio.GetaddrinfoResolver.offloadQueue") + offloadQueueTSV.currentValue = offloadQueue + return offloadQueue } } diff --git a/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift b/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift index 6e245b3862..50d2d5324e 100644 --- a/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift +++ b/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift @@ -318,17 +318,16 @@ public final class MultiThreadedEventLoopGroup: EventLoopGroup { /// /// - returns: The `EventLoop`. public func any() -> EventLoop { - if let loop = Self.currentSelectableEventLoop, + guard let loop = Self.currentSelectableEventLoop, // We are on `loop`'s thread, so we may ask for the its parent group. loop.parentGroupCallableFromThisEventLoopOnly() === self - { - // Nice, we can return this. - loop.assertInEventLoop() - return loop - } else { + else { // Oh well, let's just vend the next one then. return self.next() } + // Nice, we can return this. + loop.assertInEventLoop() + return loop } /// Shut this `MultiThreadedEventLoopGroup` down which causes the `EventLoop`s and their associated threads to be @@ -543,11 +542,10 @@ extension ScheduledTask: CustomStringConvertible { extension ScheduledTask: Comparable { @usableFromInline static func < (lhs: ScheduledTask, rhs: ScheduledTask) -> Bool { - if lhs.readyTime == rhs.readyTime { - return lhs.id < rhs.id - } else { + guard lhs.readyTime == rhs.readyTime else { return lhs.readyTime < rhs.readyTime } + return lhs.id < rhs.id } @usableFromInline diff --git a/Sources/NIOPosix/NonBlockingFileIO.swift b/Sources/NIOPosix/NonBlockingFileIO.swift index 2e1c3d565b..698354c679 100644 --- a/Sources/NIOPosix/NonBlockingFileIO.swift +++ b/Sources/NIOPosix/NonBlockingFileIO.swift @@ -377,20 +377,19 @@ public struct NonBlockingFileIO: Sendable { while bytesRead < byteCount { let n = try buf.writeWithUnsafeMutableBytes(minimumWritableBytes: byteCount - bytesRead) { ptr in let res = try fileHandle.withUnsafeFileDescriptor { descriptor -> IOResult in - if let offset = fromOffset { - return try Posix.pread( - descriptor: descriptor, - pointer: ptr.baseAddress!, - size: byteCount - bytesRead, - offset: off_t(offset) + off_t(bytesRead) - ) - } else { + guard let offset = fromOffset else { return try Posix.read( descriptor: descriptor, pointer: ptr.baseAddress!, size: byteCount - bytesRead ) } + return try Posix.pread( + descriptor: descriptor, + pointer: ptr.baseAddress!, + size: byteCount - bytesRead, + offset: off_t(offset) + off_t(bytesRead) + ) } switch res { case .processed(let n): @@ -514,20 +513,19 @@ public struct NonBlockingFileIO: Sendable { let n = try buf.readWithUnsafeReadableBytes { ptr in precondition(ptr.count == byteCount - offsetAccumulator) let res: IOResult = try fileHandle.withUnsafeFileDescriptor { descriptor in - if let toOffset = toOffset { - return try Posix.pwrite( - descriptor: descriptor, - pointer: ptr.baseAddress!, - size: byteCount - offsetAccumulator, - offset: off_t(toOffset + Int64(offsetAccumulator)) - ) - } else { + guard let toOffset = toOffset else { return try Posix.write( descriptor: descriptor, pointer: ptr.baseAddress!, size: byteCount - offsetAccumulator ) } + return try Posix.pwrite( + descriptor: descriptor, + pointer: ptr.baseAddress!, + size: byteCount - offsetAccumulator, + offset: off_t(toOffset + Int64(offsetAccumulator)) + ) } switch res { case .processed(let n): diff --git a/Sources/NIOPosix/PendingDatagramWritesManager.swift b/Sources/NIOPosix/PendingDatagramWritesManager.swift index 31365ca987..7120fbaf2b 100644 --- a/Sources/NIOPosix/PendingDatagramWritesManager.swift +++ b/Sources/NIOPosix/PendingDatagramWritesManager.swift @@ -91,12 +91,11 @@ private func doPendingDatagramWriteVectorOperation( // plausible given that a similar limit exists for TCP. For now we assume it's present // in UDP until I can do some research to validate the existence of this limit. guard Socket.writevLimitBytes - toWrite >= p.data.readableBytes else { - if c == 0 { - // The first buffer is larger than the writev limit. Let's throw, and fall back to linear processing. - throw IOError(errnoCode: EMSGSIZE, reason: "synthetic error for overlarge write") - } else { + guard c == 0 else { break } + // The first buffer is larger than the writev limit. Let's throw, and fall back to linear processing. + throw IOError(errnoCode: EMSGSIZE, reason: "synthetic error for overlarge write") } // Must not write more than writevLimitIOVectors in one go @@ -209,15 +208,14 @@ private struct PendingDatagramWritesState { /// Check if there are no outstanding writes. public var isEmpty: Bool { - if self.pendingWrites.isEmpty { - assert(self.chunks == 0) - assert(self.bytes == 0) - assert(!self.pendingWrites.hasMark) - return true - } else { + guard self.pendingWrites.isEmpty else { assert(self.chunks > 0 && self.bytes >= 0) return false } + assert(self.chunks == 0) + assert(self.bytes == 0) + assert(!self.pendingWrites.hasMark) + return true } /// Add a new write and optionally the corresponding promise to the list of outstanding writes. @@ -252,11 +250,10 @@ private struct PendingDatagramWritesState { ) -> (DatagramWritePromiseFiller?, OneWriteOperationResult) { switch data { case .processed(let written): - if let messages = messages { - return didVectorWrite(written: written, messages: messages) - } else { + guard let messages = messages else { return didScalarWrite(written: written) } + return didVectorWrite(written: written, messages: messages) case .wouldBlock: return (nil, .wouldBlock) } @@ -475,22 +472,7 @@ final class PendingDatagramWritesManager: PendingWritesManager { /// - warning: If the socket is connected, then the `envelope.remoteAddress` _must_ match the /// address of the connected peer, otherwise this function will throw a fatal error. func add(envelope: AddressedEnvelope, promise: EventLoopPromise?) -> Bool { - if let remoteAddress = self.state.remoteAddress { - precondition( - envelope.remoteAddress == remoteAddress, - """ - Remote address of AddressedEnvelope does not match remote address of connected socket. - """ - ) - return self.add( - PendingDatagramWrite( - data: envelope.data, - promise: promise, - address: nil, - metadata: envelope.metadata - ) - ) - } else { + guard let remoteAddress = self.state.remoteAddress else { return self.add( PendingDatagramWrite( data: envelope.data, @@ -500,6 +482,20 @@ final class PendingDatagramWritesManager: PendingWritesManager { ) ) } + precondition( + envelope.remoteAddress == remoteAddress, + """ + Remote address of AddressedEnvelope does not match remote address of connected socket. + """ + ) + return self.add( + PendingDatagramWrite( + data: envelope.data, + promise: promise, + address: nil, + metadata: envelope.metadata + ) + ) } /// Add a pending write, without an `AddressedEnvelope`, on a connected socket. diff --git a/Sources/NIOPosix/PendingWritesManager.swift b/Sources/NIOPosix/PendingWritesManager.swift index 10c22cd204..bcd787a0a7 100644 --- a/Sources/NIOPosix/PendingWritesManager.swift +++ b/Sources/NIOPosix/PendingWritesManager.swift @@ -156,14 +156,13 @@ private struct PendingStreamWritesState { /// Check if there are no outstanding writes. public var isEmpty: Bool { - if self.pendingWrites.isEmpty { - assert(self.bytes == 0) - assert(!self.pendingWrites.hasMark) - return true - } else { + guard self.pendingWrites.isEmpty else { assert(self.bytes >= 0) return false } + assert(self.bytes == 0) + assert(!self.pendingWrites.hasMark) + return true } /// Add a new write and optionally the corresponding promise to the list of outstanding writes. @@ -211,23 +210,22 @@ private struct PendingStreamWritesState { var unaccountedWrites = written for _ in 0..= headItemReadableBytes { - unaccountedWrites -= headItemReadableBytes - // we wrote at least the whole head item, so drop it and succeed the promise - if let promise = self.fullyWrittenFirst() { - if let p = promise0 { - p.futureResult.cascade(to: promise) - } else { - promise0 = promise - } - } - } else { + guard unaccountedWrites >= headItemReadableBytes else { // we could only write a part of the head item, so don't drop it but remember what we wrote self.partiallyWrittenFirst(bytes: unaccountedWrites) // may try again depending on the writeSpinCount return (promise0, .writtenPartially) } + unaccountedWrites -= headItemReadableBytes + // we wrote at least the whole head item, so drop it and succeed the promise + if let promise = self.fullyWrittenFirst() { + if let p = promise0 { + p.futureResult.cascade(to: promise) + } else { + promise0 = promise + } + } } assert( unaccountedWrites == 0, diff --git a/Sources/NIOPosix/Pool.swift b/Sources/NIOPosix/Pool.swift index e46f57537b..870ff1d8f1 100644 --- a/Sources/NIOPosix/Pool.swift +++ b/Sources/NIOPosix/Pool.swift @@ -33,11 +33,10 @@ class Pool { } func get() -> Element { - if elements.isEmpty { - return Element() - } else { + guard elements.isEmpty else { return elements.removeLast() } + return Element() } func put(_ e: Element) { diff --git a/Sources/NIOPosix/PooledRecvBufferAllocator.swift b/Sources/NIOPosix/PooledRecvBufferAllocator.swift index f34621cdc3..4c648780f1 100644 --- a/Sources/NIOPosix/PooledRecvBufferAllocator.swift +++ b/Sources/NIOPosix/PooledRecvBufferAllocator.swift @@ -47,14 +47,13 @@ internal struct PooledRecvBufferAllocator { /// Returns the number of buffers in the pool. var count: Int { - if self.buffer == nil { - // Empty or switched to `buffers` for storage. - return self.buffers.count - } else { + guard self.buffer == nil else { // `buffer` is non-nil; `buffers` must be empty and the count must be 1. assert(self.buffers.isEmpty) return 1 } + // Empty or switched to `buffers` for storage. + return self.buffers.count } /// Update the capacity of the underlying buffer pool. @@ -93,13 +92,12 @@ internal struct PooledRecvBufferAllocator { _ body: (inout ByteBuffer) throws -> Result ) rethrows -> (ByteBuffer, Result) { // Reuse an existing buffer if we can do so without CoWing. - if let bufferAndResult = try self.reuseExistingBuffer(body) { - return bufferAndResult - } else { + guard let bufferAndResult = try self.reuseExistingBuffer(body) else { // No available buffers or the allocator does not offer up buffer sizes; directly // allocate a new one. return try self.allocateNewBuffer(using: allocator, body) } + return bufferAndResult } private mutating func reuseExistingBuffer( @@ -146,17 +144,16 @@ internal struct PooledRecvBufferAllocator { // We have a stored buffer, either: // 1. We have capacity to add more and use `buffers` for storage, or // 2. Our capacity is 1; we can't use `buffers` for storage. - if self.capacity > 1 { - self.buffer = nil - self.buffers.reserveCapacity(self.capacity) - self.buffers.append(buffer) - self.buffers.append(newBuffer) - self.lastUsedIndex = self.buffers.index(before: self.buffers.endIndex) - return try self.modifyBuffer(atIndex: self.lastUsedIndex, body) - } else { + guard self.capacity > 1 else { let result = try body(&newBuffer) return (newBuffer, result) } + self.buffer = nil + self.buffers.reserveCapacity(self.capacity) + self.buffers.append(buffer) + self.buffers.append(newBuffer) + self.lastUsedIndex = self.buffers.index(before: self.buffers.endIndex) + return try self.modifyBuffer(atIndex: self.lastUsedIndex, body) } else { // There's no stored buffer which could be due to: // 1. this is the first buffer we allocate (i.e. buffers is empty, we already know diff --git a/Sources/NIOPosix/RawSocketBootstrap.swift b/Sources/NIOPosix/RawSocketBootstrap.swift index 5f507fb5e9..65355587de 100644 --- a/Sources/NIOPosix/RawSocketBootstrap.swift +++ b/Sources/NIOPosix/RawSocketBootstrap.swift @@ -196,13 +196,12 @@ public final class NIORawSocketBootstrap { } } - if eventLoop.inEventLoop { - return setupChannel() - } else { + guard eventLoop.inEventLoop else { return eventLoop.flatSubmit { setupChannel() } } + return setupChannel() } } @@ -360,13 +359,12 @@ extension NIORawSocketBootstrap { } } - if eventLoop.inEventLoop { - return setupChannel() - } else { + guard eventLoop.inEventLoop else { return eventLoop.flatSubmit { setupChannel() } } + return setupChannel() } } diff --git a/Sources/NIOPosix/SelectableEventLoop.swift b/Sources/NIOPosix/SelectableEventLoop.swift index c8f9204cfb..89884f0cf1 100644 --- a/Sources/NIOPosix/SelectableEventLoop.swift +++ b/Sources/NIOPosix/SelectableEventLoop.swift @@ -404,14 +404,13 @@ internal final class SelectableEventLoop: EventLoop { self._immediateTasks.append(task) } - if self._pendingTaskPop == false { - // Our job to wake the selector. - self._pendingTaskPop = true - return true - } else { + guard self._pendingTaskPop == false else { // There is already an event-loop-tick scheduled, we don't need to wake the selector. return false } + // Our job to wake the selector. + self._pendingTaskPop = true + return true } } @@ -475,12 +474,11 @@ internal final class SelectableEventLoop: EventLoop { } let nextReady = deadline.readyIn(.now()) - if nextReady <= .nanoseconds(0) { - // Something is ready to be processed just do a non-blocking select of events. - return .now - } else { + guard nextReady <= .nanoseconds(0) else { return .blockUntilTimeout(nextReady) } + // Something is ready to be processed just do a non-blocking select of events. + return .now } private func run(_ task: UnderlyingTask) { @@ -790,12 +788,11 @@ internal final class SelectableEventLoop: EventLoop { } } else { let goAhead = self.externalStateLock.withLock { () -> Bool in - if self.externalState == .open { - self.externalState = .closing - return true - } else { + guard self.externalState == .open else { return false } + self.externalState = .closing + return true } guard goAhead else { queue.async { diff --git a/Sources/NIOPosix/SelectorGeneric.swift b/Sources/NIOPosix/SelectorGeneric.swift index f865021bd7..689d198705 100644 --- a/Sources/NIOPosix/SelectorGeneric.swift +++ b/Sources/NIOPosix/SelectorGeneric.swift @@ -23,13 +23,12 @@ internal enum SelectorLifecycleState { extension Optional { internal func withUnsafeOptionalPointer(_ body: (UnsafePointer?) throws -> T) rethrows -> T { - if var this = self { - return try withUnsafePointer(to: &this) { x in - try body(x) - } - } else { + guard var this = self else { return try body(nil) } + return try withUnsafePointer(to: &this) { x in + try body(x) + } } } @@ -344,13 +343,12 @@ extension Selector: CustomStringConvertible { "Selector { descriptor = \(self.selectorFD) }" } - if NIOThread.current == self.myThread { - return makeDescription() - } else { + guard NIOThread.current == self.myThread else { return self.externalSelectorFDLock.withLock { makeDescription() } } + return makeDescription() } } @@ -403,11 +401,10 @@ extension Selector where R == NIORegistration { } }.map { future in future.flatMapErrorThrowing { error in - if let error = error as? ChannelError, error == .alreadyClosed { - return () - } else { + guard let error = error as? ChannelError, error == .alreadyClosed else { throw error } + return () } } diff --git a/Sources/NIOPosix/SocketChannel.swift b/Sources/NIOPosix/SocketChannel.swift index a299a8ec75..2fea00a213 100644 --- a/Sources/NIOPosix/SocketChannel.swift +++ b/Sources/NIOPosix/SocketChannel.swift @@ -336,24 +336,23 @@ final class ServerSocketChannel: BaseSocketChannel { guard self.isOpen else { throw ChannelError._eof } - if let accepted = try self.socket.accept(setNonBlocking: true) { - readPending = false - result = .some - do { - let chan = try SocketChannel( - socket: accepted, - parent: self, - eventLoop: group.next() as! SelectableEventLoop - ) - assert(self.isActive) - self.pipeline.syncOperations.fireChannelRead(NIOAny(chan)) - } catch { - try? accepted.close() - throw error - } - } else { + guard let accepted = try self.socket.accept(setNonBlocking: true) else { break } + readPending = false + result = .some + do { + let chan = try SocketChannel( + socket: accepted, + parent: self, + eventLoop: group.next() as! SelectableEventLoop + ) + assert(self.isActive) + self.pipeline.syncOperations.fireChannelRead(NIOAny(chan)) + } catch { + try? accepted.close() + throw error + } } return result } diff --git a/Sources/NIOPosix/System.swift b/Sources/NIOPosix/System.swift index b9f6b9710e..599f6fe0b1 100644 --- a/Sources/NIOPosix/System.swift +++ b/Sources/NIOPosix/System.swift @@ -581,11 +581,10 @@ internal enum Posix { sysAccept(descriptor, addr, len) } - if case .processed(let fd) = result { - return fd - } else { + guard case .processed(let fd) = result else { return nil } + return fd } @inline(never) diff --git a/Sources/NIOTCPEchoClient/Client.swift b/Sources/NIOTCPEchoClient/Client.swift index e624a78667..a385d0e548 100644 --- a/Sources/NIOTCPEchoClient/Client.swift +++ b/Sources/NIOTCPEchoClient/Client.swift @@ -97,14 +97,13 @@ private final class NewlineDelimiterCoder: ByteToMessageDecoder, MessageToByteEn func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { let readableBytes = buffer.readableBytesView - if let firstLine = readableBytes.firstIndex(of: self.newLine).map({ readableBytes[..<$0] }) { - buffer.moveReaderIndex(forwardBy: firstLine.count + 1) - // Fire a read without a newline - context.fireChannelRead(Self.wrapInboundOut(String(buffer: ByteBuffer(firstLine)))) - return .continue - } else { + guard let firstLine = readableBytes.firstIndex(of: self.newLine).map({ readableBytes[..<$0] }) else { return .needMoreData } + buffer.moveReaderIndex(forwardBy: firstLine.count + 1) + // Fire a read without a newline + context.fireChannelRead(Self.wrapInboundOut(String(buffer: ByteBuffer(firstLine)))) + return .continue } func encode(data: String, out: inout ByteBuffer) throws { diff --git a/Sources/NIOTCPEchoServer/Server.swift b/Sources/NIOTCPEchoServer/Server.swift index 451d687b89..e70fbcce1e 100644 --- a/Sources/NIOTCPEchoServer/Server.swift +++ b/Sources/NIOTCPEchoServer/Server.swift @@ -107,14 +107,13 @@ private final class NewlineDelimiterCoder: ByteToMessageDecoder, MessageToByteEn func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { let readableBytes = buffer.readableBytesView - if let firstLine = readableBytes.firstIndex(of: self.newLine).map({ readableBytes[..<$0] }) { - buffer.moveReaderIndex(forwardBy: firstLine.count + 1) - // Fire a read without a newline - context.fireChannelRead(Self.wrapInboundOut(String(buffer: ByteBuffer(firstLine)))) - return .continue - } else { + guard let firstLine = readableBytes.firstIndex(of: self.newLine).map({ readableBytes[..<$0] }) else { return .needMoreData } + buffer.moveReaderIndex(forwardBy: firstLine.count + 1) + // Fire a read without a newline + context.fireChannelRead(Self.wrapInboundOut(String(buffer: ByteBuffer(firstLine)))) + return .continue } func encode(data: String, out: inout ByteBuffer) throws { diff --git a/Sources/NIOTLS/ProtocolNegotiationHandlerStateMachine.swift b/Sources/NIOTLS/ProtocolNegotiationHandlerStateMachine.swift index 4dca2ae527..4fe329d46c 100644 --- a/Sources/NIOTLS/ProtocolNegotiationHandlerStateMachine.swift +++ b/Sources/NIOTLS/ProtocolNegotiationHandlerStateMachine.swift @@ -53,20 +53,19 @@ struct ProtocolNegotiationHandlerStateMachine { @inlinable mutating func userInboundEventTriggered(event: Any) -> UserInboundEventTriggeredAction { - if case .handshakeCompleted(let negotiated) = event as? TLSUserEvent { - switch self.state { - case .initial: - self.state = .waitingForUser(buffer: .init()) - - return .invokeUserClosure(.init(negotiated: negotiated)) - case .waitingForUser, .unbuffering: - preconditionFailure("Unexpectedly received two TLSUserEvents") - - case .finished: - // This is weird but we can tolerate it and just forward the event - return .fireUserInboundEventTriggered - } - } else { + guard case .handshakeCompleted(let negotiated) = event as? TLSUserEvent else { + return .fireUserInboundEventTriggered + } + switch self.state { + case .initial: + self.state = .waitingForUser(buffer: .init()) + + return .invokeUserClosure(.init(negotiated: negotiated)) + case .waitingForUser, .unbuffering: + preconditionFailure("Unexpectedly received two TLSUserEvents") + + case .finished: + // This is weird but we can tolerate it and just forward the event return .fireUserInboundEventTriggered } } @@ -114,22 +113,20 @@ struct ProtocolNegotiationHandlerStateMachine { switch result { case .success(let value): - if !buffer.isEmpty { - self.state = .unbuffering(buffer: buffer) - return .startUnbuffering(value) - } else { + guard !buffer.isEmpty else { self.state = .finished return .removeHandler(value) } + self.state = .unbuffering(buffer: buffer) + return .startUnbuffering(value) case .failure(let error): - if !buffer.isEmpty { - self.state = .unbuffering(buffer: buffer) - return .fireErrorCaughtAndStartUnbuffering(error) - } else { + guard !buffer.isEmpty else { self.state = .finished return .fireErrorCaughtAndRemoveHandler(error) } + self.state = .unbuffering(buffer: buffer) + return .fireErrorCaughtAndStartUnbuffering(error) } case .unbuffering: @@ -154,15 +151,14 @@ struct ProtocolNegotiationHandlerStateMachine { preconditionFailure("Invalid state \(self.state)") case .unbuffering(var buffer): - if let element = buffer.popFirst() { - self.state = .unbuffering(buffer: buffer) - - return .fireChannelRead(element) - } else { + guard let element = buffer.popFirst() else { self.state = .finished return .fireChannelReadCompleteAndRemoveHandler } + self.state = .unbuffering(buffer: buffer) + + return .fireChannelRead(element) } } diff --git a/Sources/NIOTLS/SNIHandler.swift b/Sources/NIOTLS/SNIHandler.swift index 18efcac439..a9c71fdc97 100644 --- a/Sources/NIOTLS/SNIHandler.swift +++ b/Sources/NIOTLS/SNIHandler.swift @@ -404,11 +404,10 @@ public final class SNIHandler: ByteToMessageDecoder { } return UnsafeRawBufferPointer(rebasing: ptr.prefix(nameLength)).decodeStringValidatingASCII() } - if let hostname = hostname { - return hostname - } else { + guard let hostname = hostname else { throw InternalSNIErrors.invalidRecord } + return hostname } return nil } diff --git a/Sources/NIOTestUtils/NIOHTTP1TestServer.swift b/Sources/NIOTestUtils/NIOHTTP1TestServer.swift index b4326f5c58..3d180dfaee 100644 --- a/Sources/NIOTestUtils/NIOHTTP1TestServer.swift +++ b/Sources/NIOTestUtils/NIOHTTP1TestServer.swift @@ -218,11 +218,10 @@ public final class NIOHTTP1TestServer { return } channel.pipeline.configureHTTPServerPipeline().flatMap { - if self.aggregateBody { - return channel.pipeline.addHandler(AggregateBodyHandler()) - } else { + guard self.aggregateBody else { return self.eventLoop.makeSucceededVoidFuture() } + return channel.pipeline.addHandler(AggregateBodyHandler()) }.flatMap { channel.pipeline.addHandler(WebServerHandler(webServer: self)) }.whenSuccess { @@ -306,11 +305,10 @@ extension NIOHTTP1TestServer { public func writeOutbound(_ data: HTTPServerResponsePart) throws { self.eventLoop.assertNotInEventLoop() try self.eventLoop.flatSubmit { () -> EventLoopFuture in - if let channel = self.currentClientChannel { - return channel.writeAndFlush(data) - } else { + guard let channel = self.currentClientChannel else { return self.eventLoop.makeFailedFuture(ChannelError.ioOnClosedChannel) } + return channel.writeAndFlush(data) }.wait() } diff --git a/Sources/_NIODataStructures/Heap.swift b/Sources/_NIODataStructures/Heap.swift index 9af0c0622b..7cdb510320 100644 --- a/Sources/_NIODataStructures/Heap.swift +++ b/Sources/_NIODataStructures/Heap.swift @@ -114,12 +114,11 @@ internal struct Heap { @discardableResult @inlinable internal mutating func remove(value: Element) -> Bool { - if let idx = self.storage.firstIndex(of: value) { - self._remove(index: idx) - return true - } else { + guard let idx = self.storage.firstIndex(of: value) else { return false } + self._remove(index: idx) + return true } @inlinable diff --git a/Tests/NIOCoreTests/SingleStepByteToMessageDecoderTest.swift b/Tests/NIOCoreTests/SingleStepByteToMessageDecoderTest.swift index 60921984db..77757c55fa 100644 --- a/Tests/NIOCoreTests/SingleStepByteToMessageDecoderTest.swift +++ b/Tests/NIOCoreTests/SingleStepByteToMessageDecoderTest.swift @@ -284,24 +284,22 @@ public final class NIOSingleStepByteToMessageDecoderTest: XCTestCase { var state: Int = 1 mutating func decode(buffer: inout ByteBuffer) throws -> InboundOut? { - if buffer.readSlice(length: self.state) != nil { - defer { - self.state += 1 - } - return self.state - } else { + guard buffer.readSlice(length: self.state) != nil else { return nil } + defer { + self.state += 1 + } + return self.state } mutating func decodeLast(buffer: inout ByteBuffer, seenEOF: Bool) throws -> InboundOut? { XCTAssertTrue(seenEOF) - if self.state > 0 { - self.state = 0 - return buffer.readableBytes * -1 - } else { + guard self.state > 0 else { return nil } + self.state = 0 + return buffer.readableBytes * -1 } } let allocator = ByteBufferAllocator() diff --git a/Tests/NIOCoreTests/XCTest+Extensions.swift b/Tests/NIOCoreTests/XCTest+Extensions.swift index 1717542d9e..ab32218d4b 100644 --- a/Tests/NIOCoreTests/XCTest+Extensions.swift +++ b/Tests/NIOCoreTests/XCTest+Extensions.swift @@ -47,11 +47,10 @@ func assertNoThrowWithValue( return try body() } catch { XCTFail("\(message.map { $0 + ": " } ?? "")unexpected error \(error) thrown", file: (file), line: line) - if let defaultValue = defaultValue { - return defaultValue - } else { + guard let defaultValue = defaultValue else { throw error } + return defaultValue } } @@ -80,11 +79,10 @@ private var temporaryDirectory: String { #if os(Linux) return "/tmp" #else - if #available(macOS 10.12, iOS 10, tvOS 10, watchOS 3, *) { - return FileManager.default.temporaryDirectory.path - } else { + guard #available(macOS 10.12, iOS 10, tvOS 10, watchOS 3, *) else { return "/tmp" } + return FileManager.default.temporaryDirectory.path #endif // os #endif // targetEnvironment } diff --git a/Tests/NIODataStructuresTests/HeapTests.swift b/Tests/NIODataStructuresTests/HeapTests.swift index 186ae4ad11..d4194b1035 100644 --- a/Tests/NIODataStructuresTests/HeapTests.swift +++ b/Tests/NIODataStructuresTests/HeapTests.swift @@ -120,9 +120,7 @@ extension Heap { func checkHeapProperty(index: Int) -> Bool { let li = self.leftIndex(index) let ri = self.rightIndex(index) - if index >= self.storage.count { - return true - } else { + guard index >= self.storage.count else { let me = self.storage[index] var lCond = true var rCond = true @@ -136,6 +134,7 @@ extension Heap { } return lCond && rCond && checkHeapProperty(index: li) && checkHeapProperty(index: ri) } + return true } return checkHeapProperty(index: 0) } diff --git a/Tests/NIOEmbeddedTests/TestUtils.swift b/Tests/NIOEmbeddedTests/TestUtils.swift index 02fe152e7d..e5cab88165 100644 --- a/Tests/NIOEmbeddedTests/TestUtils.swift +++ b/Tests/NIOEmbeddedTests/TestUtils.swift @@ -41,15 +41,7 @@ func assert( extension EventLoopFuture { var isFulfilled: Bool { - if self.eventLoop.inEventLoop { - // Easy, we're on the EventLoop. Let's just use our knowledge that we run completed future callbacks - // immediately. - var fulfilled = false - self.whenComplete { _ in - fulfilled = true - } - return fulfilled - } else { + guard self.eventLoop.inEventLoop else { let lock = NIOLock() let group = DispatchGroup() var fulfilled = false // protected by lock @@ -65,6 +57,13 @@ extension EventLoopFuture { group.wait() // this is very nasty but this is for tests only, so... return lock.withLock { fulfilled } } + // Easy, we're on the EventLoop. Let's just use our knowledge that we run completed future callbacks + // immediately. + var fulfilled = false + self.whenComplete { _ in + fulfilled = true + } + return fulfilled } } diff --git a/Tests/NIOFileSystemIntegrationTests/FileSystemTests.swift b/Tests/NIOFileSystemIntegrationTests/FileSystemTests.swift index 673f119f78..11c5557d55 100644 --- a/Tests/NIOFileSystemIntegrationTests/FileSystemTests.swift +++ b/Tests/NIOFileSystemIntegrationTests/FileSystemTests.swift @@ -34,12 +34,11 @@ extension FileSystem { _ function: String = #function, inTemporaryDirectory: Bool = true ) async throws -> FilePath { - if inTemporaryDirectory { - let directory = try await self.temporaryDirectory - return self.temporaryFilePath(function, inDirectory: directory) - } else { + guard inTemporaryDirectory else { return self.temporaryFilePath(function, inDirectory: nil) } + let directory = try await self.temporaryDirectory + return self.temporaryFilePath(function, inDirectory: directory) } func temporaryFilePath( @@ -51,11 +50,10 @@ extension FileSystem { let random = UInt32.random(in: .min ... .max) let fileName = "\(functionName)-\(random)" - if let directory = directory { - return directory.appending(fileName) - } else { + guard let directory = directory else { return FilePath(fileName) } + return directory.appending(fileName) } } diff --git a/Tests/NIOFileSystemTests/Internal/SyscallTests.swift b/Tests/NIOFileSystemTests/Internal/SyscallTests.swift index e971c5f943..6749e3816d 100644 --- a/Tests/NIOFileSystemTests/Internal/SyscallTests.swift +++ b/Tests/NIOFileSystemTests/Internal/SyscallTests.swift @@ -423,14 +423,13 @@ final class SyscallTests: XCTestCase { var shouldInterrupt = true let r2: Result = valueOrErrno(retryOnInterrupt: true) { - if shouldInterrupt { - shouldInterrupt = false - Errno._current = .interrupted - return -1 - } else { + guard shouldInterrupt else { Errno._current = .permissionDenied return -1 } + shouldInterrupt = false + Errno._current = .interrupted + return -1 } XCTAssertFalse(shouldInterrupt) XCTAssertEqual(r2, .failure(.permissionDenied)) @@ -451,14 +450,13 @@ final class SyscallTests: XCTestCase { var shouldInterrupt = true let r2: Result = nothingOrErrno(retryOnInterrupt: true) { - if shouldInterrupt { - shouldInterrupt = false - Errno._current = .interrupted - return -1 - } else { + guard shouldInterrupt else { Errno._current = .permissionDenied return -1 } + shouldInterrupt = false + Errno._current = .interrupted + return -1 } XCTAssertFalse(shouldInterrupt) XCTAssertThrowsError(try r2.get()) { error in @@ -478,13 +476,12 @@ final class SyscallTests: XCTestCase { var shouldInterrupt = true let r2: Result = optionalValueOrErrno(retryOnInterrupt: true) { - if shouldInterrupt { - shouldInterrupt = false - Errno._current = .interrupted - return nil - } else { + guard shouldInterrupt else { return "foo" } + shouldInterrupt = false + Errno._current = .interrupted + return nil } XCTAssertFalse(shouldInterrupt) XCTAssertEqual(r2, .success("foo")) diff --git a/Tests/NIOHTTP1Tests/HTTPServerUpgradeTests.swift b/Tests/NIOHTTP1Tests/HTTPServerUpgradeTests.swift index 343b6b9138..fe1ba837f9 100644 --- a/Tests/NIOHTTP1Tests/HTTPServerUpgradeTests.swift +++ b/Tests/NIOHTTP1Tests/HTTPServerUpgradeTests.swift @@ -1791,14 +1791,13 @@ final class TypedHTTPServerUpgradeTestCase: HTTPServerUpgradeTestCase { configuration: configuration ) .flatMap { result in - if result { - return channel.pipeline.context(handlerType: NIOTypedHTTPServerUpgradeHandler.self) - .map { - upgradeCompletionHandler($0) - } - } else { + guard result else { return channel.eventLoop.makeSucceededVoidFuture() } + return channel.pipeline.context(handlerType: NIOTypedHTTPServerUpgradeHandler.self) + .map { + upgradeCompletionHandler($0) + } } } .flatMap { _ in diff --git a/Tests/NIOHTTP1Tests/HTTPTest.swift b/Tests/NIOHTTP1Tests/HTTPTest.swift index b1cf126e80..88ab8c0860 100644 --- a/Tests/NIOHTTP1Tests/HTTPTest.swift +++ b/Tests/NIOHTTP1Tests/HTTPTest.swift @@ -124,17 +124,16 @@ class HTTPTest: XCTestCase { XCTAssertNoThrow(try EventLoopFuture.andAllSucceed(writeFutures, on: channel.eventLoop).wait()) XCTAssertEqual(2 * expecteds.count, step) - if body != nil { - XCTAssertGreaterThan(allBodyDatas.count, 0) - let firstBodyData = allBodyDatas[0] - for bodyData in allBodyDatas { - XCTAssertEqual(firstBodyData, bodyData) - } - return String(decoding: firstBodyData, as: Unicode.UTF8.self) - } else { + guard body != nil else { XCTAssertEqual(0, allBodyDatas.count, "left with \(allBodyDatas)") return nil } + XCTAssertGreaterThan(allBodyDatas.count, 0) + let firstBodyData = allBodyDatas[0] + for bodyData in allBodyDatas { + XCTAssertEqual(firstBodyData, bodyData) + } + return String(decoding: firstBodyData, as: Unicode.UTF8.self) } // send all bytes in one go diff --git a/Tests/NIOPosixTests/AsyncChannelBootstrapTests.swift b/Tests/NIOPosixTests/AsyncChannelBootstrapTests.swift index b1fc060feb..529b52cf69 100644 --- a/Tests/NIOPosixTests/AsyncChannelBootstrapTests.swift +++ b/Tests/NIOPosixTests/AsyncChannelBootstrapTests.swift @@ -46,20 +46,19 @@ private final class LineDelimiterCoder: ByteToMessageDecoder, MessageToByteEncod func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { let readable = buffer.withUnsafeReadableBytes { $0.firstIndex(of: self.newLine) } if let readable = readable { - if let id = self.inboundID { - let data = buffer.readSlice(length: readable - 1)! - let inboundID = buffer.readInteger(as: UInt8.self)! - buffer.moveReaderIndex(forwardBy: 1) - - if id == inboundID { - context.fireChannelRead(Self.wrapInboundOut(data)) - } - return .continue - } else { + guard let id = self.inboundID else { context.fireChannelRead(Self.wrapInboundOut(buffer.readSlice(length: readable)!)) buffer.moveReaderIndex(forwardBy: 1) return .continue } + let data = buffer.readSlice(length: readable - 1)! + let inboundID = buffer.readInteger(as: UInt8.self)! + buffer.moveReaderIndex(forwardBy: 1) + + if id == inboundID { + context.fireChannelRead(Self.wrapInboundOut(data)) + } + return .continue } return .needMoreData } diff --git a/Tests/NIOPosixTests/ChannelTests.swift b/Tests/NIOPosixTests/ChannelTests.swift index 8b86e834a4..0fce149100 100644 --- a/Tests/NIOPosixTests/ChannelTests.swift +++ b/Tests/NIOPosixTests/ChannelTests.swift @@ -287,53 +287,32 @@ public final class ChannelTests: XCTestCase { singleState += 1 everythingState += 1 } - if let expected = expectedSingleWritabilities { - if expected.count > singleState { - XCTAssertGreaterThan(returns.count, everythingState) - XCTAssertEqual( - expected[singleState], - buf.count, - "in single write \(singleState) (overall \(everythingState)), \(expected[singleState]) bytes expected but \(buf.count) actual" - ) - return returns[everythingState] - } else { - XCTFail( - "single write call \(singleState) but less than \(expected.count) expected", - file: (file), - line: line - ) - return IOResult.wouldBlock(-1 * (everythingState + 1)) - } - } else { + guard let expected = expectedSingleWritabilities else { XCTFail("single write called on \(buf) but no single writes expected", file: (file), line: line) return IOResult.wouldBlock(-1 * (everythingState + 1)) } + guard expected.count > singleState else { + XCTFail( + "single write call \(singleState) but less than \(expected.count) expected", + file: (file), + line: line + ) + return IOResult.wouldBlock(-1 * (everythingState + 1)) + } + XCTAssertGreaterThan(returns.count, everythingState) + XCTAssertEqual( + expected[singleState], + buf.count, + "in single write \(singleState) (overall \(everythingState)), \(expected[singleState]) bytes expected but \(buf.count) actual" + ) + return returns[everythingState] }, vectorBufferWriteOperation: { ptrs in defer { multiState += 1 everythingState += 1 } - if let expected = expectedVectorWritabilities { - if expected.count > multiState { - XCTAssertGreaterThan(returns.count, everythingState) - XCTAssertEqual( - expected[multiState], - ptrs.map { numericCast($0.iov_len) }, - "in vector write \(multiState) (overall \(everythingState)), \(expected[multiState]) byte counts expected but \(ptrs.map { $0.iov_len }) actual", - file: (file), - line: line - ) - return returns[everythingState] - } else { - XCTFail( - "vector write call \(multiState) but less than \(expected.count) expected", - file: (file), - line: line - ) - return IOResult.wouldBlock(-1 * (everythingState + 1)) - } - } else { + guard let expected = expectedVectorWritabilities else { XCTFail( "vector write called on \(ptrs) but no vector writes expected", file: (file), @@ -341,6 +320,23 @@ public final class ChannelTests: XCTestCase { ) return IOResult.wouldBlock(-1 * (everythingState + 1)) } + guard expected.count > multiState else { + XCTFail( + "vector write call \(multiState) but less than \(expected.count) expected", + file: (file), + line: line + ) + return IOResult.wouldBlock(-1 * (everythingState + 1)) + } + XCTAssertGreaterThan(returns.count, everythingState) + XCTAssertEqual( + expected[multiState], + ptrs.map { numericCast($0.iov_len) }, + "in vector write \(multiState) (overall \(everythingState)), \(expected[multiState]) byte counts expected but \(ptrs.map { $0.iov_len }) actual", + file: (file), + line: line + ) + return returns[everythingState] }, scalarFileWriteOperation: { _, start, end in defer { @@ -356,24 +352,7 @@ public final class ChannelTests: XCTestCase { return IOResult.wouldBlock(-1 * (everythingState + 1)) } - if expected.count > fileState { - XCTAssertGreaterThan(returns.count, everythingState) - XCTAssertEqual( - expected[fileState].0, - start, - "in file write \(fileState) (overall \(everythingState)), \(expected[fileState].0) expected as start index but \(start) actual", - file: (file), - line: line - ) - XCTAssertEqual( - expected[fileState].1, - end, - "in file write \(fileState) (overall \(everythingState)), \(expected[fileState].1) expected as end index but \(end) actual", - file: (file), - line: line - ) - return returns[everythingState] - } else { + guard expected.count > fileState else { XCTFail( "file write call \(fileState) but less than \(expected.count) expected", file: (file), @@ -381,6 +360,22 @@ public final class ChannelTests: XCTestCase { ) return IOResult.wouldBlock(-1 * (everythingState + 1)) } + XCTAssertGreaterThan(returns.count, everythingState) + XCTAssertEqual( + expected[fileState].0, + start, + "in file write \(fileState) (overall \(everythingState)), \(expected[fileState].0) expected as start index but \(start) actual", + file: (file), + line: line + ) + XCTAssertEqual( + expected[fileState].1, + end, + "in file write \(fileState) (overall \(everythingState)), \(expected[fileState].1) expected as end index but \(end) actual", + file: (file), + line: line + ) + return returns[everythingState] } ) if everythingState > 0 { @@ -2216,11 +2211,10 @@ public final class ChannelTests: XCTestCase { try super.init(protocolFamily: protocolFamily, type: .stream, setNonBlocking: true) } override func connect(to address: SocketAddress) throws -> Bool { - if address.port == 123 { - return true - } else { + guard address.port == 123 else { return try super.connect(to: address) } + return true } } class ReadDoesNotHappen: ChannelInboundHandler { @@ -2422,13 +2416,12 @@ public final class ChannelTests: XCTestCase { self.firstReadHappened = true } XCTAssertGreaterThan(pointer.count, 0) - if self.firstReadHappened { - // this is a copy of the exact error that'd come out of the real Socket.read - throw IOError.init(errnoCode: ECONNRESET, reason: "read(descriptor:pointer:size:)") - } else { + guard self.firstReadHappened else { pointer[0] = 0xff return .processed(1) } + // this is a copy of the exact error that'd come out of the real Socket.read + throw IOError.init(errnoCode: ECONNRESET, reason: "read(descriptor:pointer:size:)") } } class VerifyThingsAreRightHandler: ChannelInboundHandler { diff --git a/Tests/NIOPosixTests/CodecTest.swift b/Tests/NIOPosixTests/CodecTest.swift index a6ffbe8802..097218c0b2 100644 --- a/Tests/NIOPosixTests/CodecTest.swift +++ b/Tests/NIOPosixTests/CodecTest.swift @@ -683,15 +683,14 @@ public final class ByteToMessageDecoderTest: XCTestCase { var state: Int = 1 mutating func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { - if buffer.readSlice(length: self.state) != nil { - defer { - self.state += 1 - } - context.fireChannelRead(Self.wrapInboundOut(self.state)) - return .continue - } else { + guard buffer.readSlice(length: self.state) != nil else { return .needMoreData } + defer { + self.state += 1 + } + context.fireChannelRead(Self.wrapInboundOut(self.state)) + return .continue } mutating func decodeLast( @@ -737,20 +736,19 @@ public final class ByteToMessageDecoderTest: XCTestCase { mutating func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { XCTAssertGreaterThan(self.state, 0) - if let slice = buffer.readSlice(length: self.state) { - self.state >>= 1 - for i in 0..>= 1 + for i in 0.. DecodingState { - if let slice = buffer.readSlice(length: 16) { - context.fireChannelRead(Self.wrapInboundOut(slice)) - context.channel.close().whenFailure { error in - XCTFail("unexpected error: \(error)") - } - return .continue - } else { + guard let slice = buffer.readSlice(length: 16) else { return .needMoreData } + context.fireChannelRead(Self.wrapInboundOut(slice)) + context.channel.close().whenFailure { error in + XCTFail("unexpected error: \(error)") + } + return .continue } mutating func decodeLast( @@ -832,15 +829,14 @@ public final class ByteToMessageDecoderTest: XCTestCase { typealias InboundOut = ByteBuffer mutating func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { - if let slice = buffer.readSlice(length: 16) { - context.fireChannelRead(Self.wrapInboundOut(slice)) - context.pipeline.removeHandler(context: context).whenFailure { error in - XCTFail("unexpected error: \(error)") - } - return .continue - } else { + guard let slice = buffer.readSlice(length: 16) else { return .needMoreData } + context.fireChannelRead(Self.wrapInboundOut(slice)) + context.pipeline.removeHandler(context: context).whenFailure { error in + XCTFail("unexpected error: \(error)") + } + return .continue } mutating func decodeLast( @@ -879,15 +875,14 @@ public final class ByteToMessageDecoderTest: XCTestCase { typealias InboundOut = ByteBuffer mutating func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { - if let slice = buffer.readSlice(length: 16) { - context.fireChannelRead(Self.wrapInboundOut(slice)) - context.close().whenFailure { error in - XCTFail("unexpected error: \(error)") - } - return .continue - } else { + guard let slice = buffer.readSlice(length: 16) else { return .needMoreData } + context.fireChannelRead(Self.wrapInboundOut(slice)) + context.close().whenFailure { error in + XCTFail("unexpected error: \(error)") + } + return .continue } mutating func decodeLast( @@ -1187,12 +1182,11 @@ public final class ByteToMessageDecoderTest: XCTestCase { } func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { - if let string = buffer.readString(length: 1) { - context.fireChannelRead(Self.wrapInboundOut(string)) - return .continue - } else { + guard let string = buffer.readString(length: 1) else { return .needMoreData } + context.fireChannelRead(Self.wrapInboundOut(string)) + return .continue } func decodeLast( @@ -1238,20 +1232,19 @@ public final class ByteToMessageDecoderTest: XCTestCase { func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { self.decodeRun += 1 - if let string = buffer.readString(length: 1) { - context.fireChannelRead(Self.wrapInboundOut("I: \(self.decodeRun): \(string)")) - XCTAssertNoThrow( - try (context.channel as! EmbeddedChannel).writeOutbound("O: \(self.decodeRun): \(string)") - ) - if self.decodeRun == 1 { - var buffer = context.channel.allocator.buffer(capacity: 1) - buffer.writeStaticString("X") - XCTAssertNoThrow(try (context.channel as! EmbeddedChannel).writeInbound(buffer)) - } - return .continue - } else { + guard let string = buffer.readString(length: 1) else { return .needMoreData } + context.fireChannelRead(Self.wrapInboundOut("I: \(self.decodeRun): \(string)")) + XCTAssertNoThrow( + try (context.channel as! EmbeddedChannel).writeOutbound("O: \(self.decodeRun): \(string)") + ) + if self.decodeRun == 1 { + var buffer = context.channel.allocator.buffer(capacity: 1) + buffer.writeStaticString("X") + XCTAssertNoThrow(try (context.channel as! EmbeddedChannel).writeInbound(buffer)) + } + return .continue } func decodeLast( @@ -1966,12 +1959,11 @@ private class PairOfBytesDecoder: ByteToMessageDecoder { } func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { - if let slice = buffer.readSlice(length: 2) { - context.fireChannelRead(Self.wrapInboundOut(slice)) - return .continue - } else { + guard let slice = buffer.readSlice(length: 2) else { return .needMoreData } + context.fireChannelRead(Self.wrapInboundOut(slice)) + return .continue } func decodeLast(context: ChannelHandlerContext, buffer: inout ByteBuffer, seenEOF: Bool) throws -> DecodingState { @@ -1989,11 +1981,10 @@ public final class MessageToByteHandlerTest: XCTestCase { typealias OutboundIn = Int public func encode(data value: Int, out: inout ByteBuffer) throws { - if value == 0 { - out.writeInteger(value) - } else { + guard value == 0 else { throw HandlerError() } + out.writeInteger(value) } } diff --git a/Tests/NIOPosixTests/DatagramChannelTests.swift b/Tests/NIOPosixTests/DatagramChannelTests.swift index 40e0b3dcd0..d7c6c3e834 100644 --- a/Tests/NIOPosixTests/DatagramChannelTests.swift +++ b/Tests/NIOPosixTests/DatagramChannelTests.swift @@ -1206,12 +1206,11 @@ class DatagramChannelTests: XCTestCase { to destinationChannel: Channel, wrappingInAddressedEnvelope shouldWrapInAddressedEnvelope: Bool ) -> EventLoopFuture { - if shouldWrapInAddressedEnvelope { - let envelope = AddressedEnvelope(remoteAddress: destinationChannel.localAddress!, data: data) - return sourceChannel.write(envelope) - } else { + guard shouldWrapInAddressedEnvelope else { return sourceChannel.write(data) } + let envelope = AddressedEnvelope(remoteAddress: destinationChannel.localAddress!, data: data) + return sourceChannel.write(envelope) } func bufferWriteOfHelloWorld( diff --git a/Tests/NIOPosixTests/EchoServerClientTest.swift b/Tests/NIOPosixTests/EchoServerClientTest.swift index 16b74efc4c..cf40bc679e 100644 --- a/Tests/NIOPosixTests/EchoServerClientTest.swift +++ b/Tests/NIOPosixTests/EchoServerClientTest.swift @@ -898,14 +898,13 @@ class EchoServerClientTest: XCTestCase { return channel.eventLoop.makeSucceededFuture(()) }.bind(host: host, port: 0).wait() } catch let e as SocketAddressError { - if case .unknown(host, port: 0) = e { - // this can happen if the system isn't configured for both IPv4 and IPv6 - continue - } else { + guard case .unknown(host, port: 0) = e else { // nope, that's a different socket error XCTFail("unexpected SocketAddressError: \(e)") break } + // this can happen if the system isn't configured for both IPv4 and IPv6 + continue } catch { // other unknown error XCTFail("unexpected error: \(error)") diff --git a/Tests/NIOPosixTests/PendingDatagramWritesManagerTests.swift b/Tests/NIOPosixTests/PendingDatagramWritesManagerTests.swift index 988b235197..82f4a434e5 100644 --- a/Tests/NIOPosixTests/PendingDatagramWritesManagerTests.swift +++ b/Tests/NIOPosixTests/PendingDatagramWritesManagerTests.swift @@ -101,109 +101,54 @@ class PendingDatagramWritesManagerTests: XCTestCase { singleState += 1 everythingState += 1 } - if let expected = expectedSingleWritabilities { - if expected.count > singleState { - XCTAssertGreaterThan(returns.count, everythingState) - XCTAssertEqual( - expected[singleState].0, - buf.count, - "in single write \(singleState) (overall \(everythingState)), \(expected[singleState].0) bytes expected but \(buf.count) actual", - file: (file), - line: line - ) - XCTAssertEqual( - expected[singleState].1, - addr.map(SocketAddress.init), - "in single write \(singleState) (overall \(everythingState)), \(expected[singleState].1) address expected but \(String(describing: addr.map(SocketAddress.init))) received", - file: (file), - line: line - ) - XCTAssertEqual( - expected[singleState].1.expectedSize, - len, - "in single write \(singleState) (overall \(everythingState)), \(expected[singleState].1.expectedSize) socklen expected but \(len) received", - file: (file), - line: line - ) - - switch returns[everythingState] { - case .success(let r): - return r - case .failure(let e): - throw e - } - } else { - XCTFail( - "single write call \(singleState) but less than \(expected.count) expected", - file: (file), - line: line - ) - return IOResult.wouldBlock(-1 * (everythingState + 1)) - } - } else { + guard let expected = expectedSingleWritabilities else { XCTFail("single write called on \(buf) but no single writes expected", file: (file), line: line) return IOResult.wouldBlock(-1 * (everythingState + 1)) } + guard expected.count > singleState else { + XCTFail( + "single write call \(singleState) but less than \(expected.count) expected", + file: (file), + line: line + ) + return IOResult.wouldBlock(-1 * (everythingState + 1)) + } + XCTAssertGreaterThan(returns.count, everythingState) + XCTAssertEqual( + expected[singleState].0, + buf.count, + "in single write \(singleState) (overall \(everythingState)), \(expected[singleState].0) bytes expected but \(buf.count) actual", + file: (file), + line: line + ) + XCTAssertEqual( + expected[singleState].1, + addr.map(SocketAddress.init), + "in single write \(singleState) (overall \(everythingState)), \(expected[singleState].1) address expected but \(String(describing: addr.map(SocketAddress.init))) received", + file: (file), + line: line + ) + XCTAssertEqual( + expected[singleState].1.expectedSize, + len, + "in single write \(singleState) (overall \(everythingState)), \(expected[singleState].1.expectedSize) socklen expected but \(len) received", + file: (file), + line: line + ) + + switch returns[everythingState] { + case .success(let r): + return r + case .failure(let e): + throw e + } }, vectorWriteOperation: { ptrs in defer { multiState += 1 everythingState += 1 } - if let expected = expectedVectorWritabilities { - if expected.count > multiState { - XCTAssertGreaterThan(returns.count, everythingState) - XCTAssertEqual( - ptrs.map { $0.msg_hdr.msg_iovlen }, - Array(repeating: 1, count: ptrs.count), - "mustn't write more than one iovec element per datagram", - file: (file), - line: line - ) - XCTAssertEqual( - expected[multiState].map { numericCast($0.0) }, - ptrs.map { $0.msg_hdr.msg_iov.pointee.iov_len }, - "in vector write \(multiState) (overall \(everythingState)), \(expected[multiState]) byte counts expected but \(ptrs.map { $0.msg_hdr.msg_iov.pointee.iov_len }) actual", - file: (file), - line: line - ) - XCTAssertEqual( - ptrs.map { Int($0.msg_len) }, - Array(repeating: 0, count: ptrs.count), - "in vector write \(multiState) (overall \(everythingState)), \(expected[multiState]) byte counts expected but \(ptrs.map { $0.msg_len }) actual", - file: (file), - line: line - ) - XCTAssertEqual( - expected[multiState].map { $0.1 }, - ptrs.map { SocketAddress($0.msg_hdr.msg_name.assumingMemoryBound(to: sockaddr.self)) }, - "in vector write \(multiState) (overall \(everythingState)), \(expected[multiState].map { $0.1 }) addresses expected but \(ptrs.map { SocketAddress($0.msg_hdr.msg_name.assumingMemoryBound(to: sockaddr.self)) }) actual", - file: (file), - line: line - ) - XCTAssertEqual( - expected[multiState].map { $0.1.expectedSize }, - ptrs.map { $0.msg_hdr.msg_namelen }, - "in vector write \(multiState) (overall \(everythingState)), \(expected[multiState].map { $0.1.expectedSize }) address lengths expected but \(ptrs.map { $0.msg_hdr.msg_namelen }) actual", - file: (file), - line: line - ) - - switch returns[everythingState] { - case .success(let r): - return r - case .failure(let e): - throw e - } - } else { - XCTFail( - "vector write call \(multiState) but less than \(expected.count) expected", - file: (file), - line: line - ) - return IOResult.wouldBlock(-1 * (everythingState + 1)) - } - } else { + guard let expected = expectedVectorWritabilities else { XCTFail( "vector write called on \(ptrs) but no vector writes expected", file: (file), @@ -211,6 +156,57 @@ class PendingDatagramWritesManagerTests: XCTestCase { ) return IOResult.wouldBlock(-1 * (everythingState + 1)) } + guard expected.count > multiState else { + XCTFail( + "vector write call \(multiState) but less than \(expected.count) expected", + file: (file), + line: line + ) + return IOResult.wouldBlock(-1 * (everythingState + 1)) + } + XCTAssertGreaterThan(returns.count, everythingState) + XCTAssertEqual( + ptrs.map { $0.msg_hdr.msg_iovlen }, + Array(repeating: 1, count: ptrs.count), + "mustn't write more than one iovec element per datagram", + file: (file), + line: line + ) + XCTAssertEqual( + expected[multiState].map { numericCast($0.0) }, + ptrs.map { $0.msg_hdr.msg_iov.pointee.iov_len }, + "in vector write \(multiState) (overall \(everythingState)), \(expected[multiState]) byte counts expected but \(ptrs.map { $0.msg_hdr.msg_iov.pointee.iov_len }) actual", + file: (file), + line: line + ) + XCTAssertEqual( + ptrs.map { Int($0.msg_len) }, + Array(repeating: 0, count: ptrs.count), + "in vector write \(multiState) (overall \(everythingState)), \(expected[multiState]) byte counts expected but \(ptrs.map { $0.msg_len }) actual", + file: (file), + line: line + ) + XCTAssertEqual( + expected[multiState].map { $0.1 }, + ptrs.map { SocketAddress($0.msg_hdr.msg_name.assumingMemoryBound(to: sockaddr.self)) }, + "in vector write \(multiState) (overall \(everythingState)), \(expected[multiState].map { $0.1 }) addresses expected but \(ptrs.map { SocketAddress($0.msg_hdr.msg_name.assumingMemoryBound(to: sockaddr.self)) }) actual", + file: (file), + line: line + ) + XCTAssertEqual( + expected[multiState].map { $0.1.expectedSize }, + ptrs.map { $0.msg_hdr.msg_namelen }, + "in vector write \(multiState) (overall \(everythingState)), \(expected[multiState].map { $0.1.expectedSize }) address lengths expected but \(ptrs.map { $0.msg_hdr.msg_namelen }) actual", + file: (file), + line: line + ) + + switch returns[everythingState] { + case .success(let r): + return r + case .failure(let e): + throw e + } } ) result = r diff --git a/Tests/NIOPosixTests/SALChannelTests.swift b/Tests/NIOPosixTests/SALChannelTests.swift index d4d5e45f5c..608822fd49 100644 --- a/Tests/NIOPosixTests/SALChannelTests.swift +++ b/Tests/NIOPosixTests/SALChannelTests.swift @@ -424,18 +424,18 @@ final class SALChannelTest: XCTestCase, SALTest { // Then we register the inbound channel. try self.assertRegister { selectable, eventSet, registration in - if case (.socketChannel(let channel), let registrationEventSet) = - (registration.channel, registration.interested) - { - - XCTAssertEqual(localAddress, channel.localAddress) - XCTAssertEqual(remoteAddress, channel.remoteAddress) - XCTAssertEqual(eventSet, registrationEventSet) - XCTAssertEqual(.reset, eventSet) - return true - } else { + guard + case (.socketChannel(let channel), let registrationEventSet) = + (registration.channel, registration.interested) + else { return false } + + XCTAssertEqual(localAddress, channel.localAddress) + XCTAssertEqual(remoteAddress, channel.remoteAddress) + XCTAssertEqual(eventSet, registrationEventSet) + XCTAssertEqual(.reset, eventSet) + return true } try self.assertReregister { selectable, eventSet in XCTAssertEqual([.reset, .readEOF], eventSet) @@ -497,18 +497,18 @@ final class SALChannelTest: XCTestCase, SALTest { // Then we register the inbound channel from the first accept. try self.assertRegister { selectable, eventSet, registration in - if case (.socketChannel(let channel), let registrationEventSet) = - (registration.channel, registration.interested) - { - - XCTAssertEqual(localAddress, channel.localAddress) - XCTAssertEqual(remoteAddress, channel.remoteAddress) - XCTAssertEqual(eventSet, registrationEventSet) - XCTAssertEqual(.reset, eventSet) - return true - } else { + guard + case (.socketChannel(let channel), let registrationEventSet) = + (registration.channel, registration.interested) + else { return false } + + XCTAssertEqual(localAddress, channel.localAddress) + XCTAssertEqual(remoteAddress, channel.remoteAddress) + XCTAssertEqual(eventSet, registrationEventSet) + XCTAssertEqual(.reset, eventSet) + return true } try self.assertReregister { selectable, eventSet in XCTAssertEqual([.reset, .readEOF], eventSet) @@ -808,18 +808,18 @@ final class SALChannelTest: XCTestCase, SALTest { // Then we register the inbound channel. try self.assertRegister { selectable, eventSet, registration in - if case (.socketChannel(let channel), let registrationEventSet) = - (registration.channel, registration.interested) - { - - XCTAssertEqual(localAddress, channel.localAddress) - XCTAssertEqual(remoteAddress, channel.remoteAddress) - XCTAssertEqual(eventSet, registrationEventSet) - XCTAssertEqual(.reset, eventSet) - return true - } else { + guard + case (.socketChannel(let channel), let registrationEventSet) = + (registration.channel, registration.interested) + else { return false } + + XCTAssertEqual(localAddress, channel.localAddress) + XCTAssertEqual(remoteAddress, channel.remoteAddress) + XCTAssertEqual(eventSet, registrationEventSet) + XCTAssertEqual(.reset, eventSet) + return true } try self.assertReregister { selectable, event in XCTAssertEqual([.reset, .readEOF], event) @@ -888,18 +888,18 @@ final class SALChannelTest: XCTestCase, SALTest { // Then we register the inbound channel. try self.assertRegister { selectable, eventSet, registration in - if case (.socketChannel(let channel), let registrationEventSet) = - (registration.channel, registration.interested) - { - - XCTAssertEqual(localAddress, channel.localAddress) - XCTAssertEqual(remoteAddress, channel.remoteAddress) - XCTAssertEqual(eventSet, registrationEventSet) - XCTAssertEqual(.reset, eventSet) - return true - } else { + guard + case (.socketChannel(let channel), let registrationEventSet) = + (registration.channel, registration.interested) + else { return false } + + XCTAssertEqual(localAddress, channel.localAddress) + XCTAssertEqual(remoteAddress, channel.remoteAddress) + XCTAssertEqual(eventSet, registrationEventSet) + XCTAssertEqual(.reset, eventSet) + return true } try self.assertReregister { selectable, event in XCTAssertEqual([.reset, .readEOF], event) @@ -976,18 +976,18 @@ final class SALChannelTest: XCTestCase, SALTest { // Then we register the inbound channel. try self.assertRegister { selectable, eventSet, registration in - if case (.socketChannel(let channel), let registrationEventSet) = - (registration.channel, registration.interested) - { - - XCTAssertEqual(localAddress, channel.localAddress) - XCTAssertEqual(remoteAddress, channel.remoteAddress) - XCTAssertEqual(eventSet, registrationEventSet) - XCTAssertEqual(.reset, eventSet) - return true - } else { + guard + case (.socketChannel(let channel), let registrationEventSet) = + (registration.channel, registration.interested) + else { return false } + + XCTAssertEqual(localAddress, channel.localAddress) + XCTAssertEqual(remoteAddress, channel.remoteAddress) + XCTAssertEqual(eventSet, registrationEventSet) + XCTAssertEqual(.reset, eventSet) + return true } try self.assertReregister { selectable, event in XCTAssertEqual([.reset, .readEOF], event) diff --git a/Tests/NIOPosixTests/SocketChannelTest.swift b/Tests/NIOPosixTests/SocketChannelTest.swift index 80133abbdd..5391f73cf0 100644 --- a/Tests/NIOPosixTests/SocketChannelTest.swift +++ b/Tests/NIOPosixTests/SocketChannelTest.swift @@ -757,15 +757,14 @@ public final class SocketChannelTest: XCTestCase { let shouldAcceptsFail = ManagedAtomic(true) override func accept(setNonBlocking: Bool = false) throws -> Socket? { XCTAssertTrue(setNonBlocking) - if self.shouldAcceptsFail.load(ordering: .relaxed) { - throw NIOFcntlFailedError() - } else { + guard self.shouldAcceptsFail.load(ordering: .relaxed) else { return try Socket( protocolFamily: .inet, type: .stream, setNonBlocking: false ) } + throw NIOFcntlFailedError() } } diff --git a/Tests/NIOPosixTests/SyscallAbstractionLayer.swift b/Tests/NIOPosixTests/SyscallAbstractionLayer.swift index ab5b075110..f2aa8ad14d 100644 --- a/Tests/NIOPosixTests/SyscallAbstractionLayer.swift +++ b/Tests/NIOPosixTests/SyscallAbstractionLayer.swift @@ -83,38 +83,35 @@ final class LockedBox { } func waitForEmptyAndSet(_ value: T) throws { - if self.condition.lock(whenValue: 0, timeoutSeconds: SAL.defaultTimeout) { - defer { - self.condition.unlock(withValue: 1) - } - self._value = value - } else { + guard self.condition.lock(whenValue: 0, timeoutSeconds: SAL.defaultTimeout) else { throw TimeoutError(self.description) } + defer { + self.condition.unlock(withValue: 1) + } + self._value = value } func takeValue() throws -> T { - if self.condition.lock(whenValue: 1, timeoutSeconds: SAL.defaultTimeout) { - defer { - self.condition.unlock(withValue: 0) - } - let value = self._value! - self._value = nil - return value - } else { + guard self.condition.lock(whenValue: 1, timeoutSeconds: SAL.defaultTimeout) else { throw TimeoutError(self.description) } + defer { + self.condition.unlock(withValue: 0) + } + let value = self._value! + self._value = nil + return value } func waitForValue() throws -> T { - if self.condition.lock(whenValue: 1, timeoutSeconds: SAL.defaultTimeout) { - defer { - self.condition.unlock(withValue: 1) - } - return self._value! - } else { + guard self.condition.lock(whenValue: 1, timeoutSeconds: SAL.defaultTimeout) else { throw TimeoutError(self.description) } + defer { + self.condition.unlock(withValue: 1) + } + return self._value! } } @@ -186,11 +183,10 @@ private protocol UserKernelInterface { extension UserKernelInterface { fileprivate func waitForKernelReturn() throws -> KernelToUser { let value = try self.kernelToUser.takeValue() - if case .error(let error) = value { - throw error - } else { + guard case .error(let error) = value else { return value } + throw error } } @@ -223,21 +219,19 @@ internal class HookedSelector: NIOPosix.Selector, UserKernelInt ) ) let ret = try self.waitForKernelReturn() - if case .returnVoid = ret { - return - } else { + guard case .returnVoid = ret else { throw UnexpectedKernelReturn(ret) } + return } override func reregister(selectable: S, interested: SelectorEventSet) throws { try self.userToKernel.waitForEmptyAndSet(.reregister(selectable, interested)) let ret = try self.waitForKernelReturn() - if case .returnVoid = ret { - return - } else { + guard case .returnVoid = ret else { throw UnexpectedKernelReturn(ret) } + return } override func whenReady( @@ -247,25 +241,23 @@ internal class HookedSelector: NIOPosix.Selector, UserKernelInt ) throws { try self.userToKernel.waitForEmptyAndSet(.whenReady(strategy)) let ret = try self.waitForKernelReturn() - if case .returnSelectorEvent(let event) = ret { - loopStart() - if let event = event { - try body(event) - } - return - } else { + guard case .returnSelectorEvent(let event) = ret else { throw UnexpectedKernelReturn(ret) } + loopStart() + if let event = event { + try body(event) + } + return } override func deregister(selectable: S) throws { try self.userToKernel.waitForEmptyAndSet(.deregister(selectable)) let ret = try self.waitForKernelReturn() - if case .returnVoid = ret { - return - } else { + guard case .returnVoid = ret else { throw UnexpectedKernelReturn(ret) } + return } override func wakeup() throws { @@ -292,53 +284,48 @@ class HookedServerSocket: ServerSocket, UserKernelInterface { try self.withUnsafeHandle { fd in try self.userToKernel.waitForEmptyAndSet(.disableSIGPIPE(fd)) let ret = try self.waitForKernelReturn() - if case .returnVoid = ret { - return - } else { + guard case .returnVoid = ret else { throw UnexpectedKernelReturn(ret) } + return } } override func localAddress() throws -> SocketAddress { try self.userToKernel.waitForEmptyAndSet(.localAddress) let ret = try self.waitForKernelReturn() - if case .returnSocketAddress(let address) = ret { - return address - } else { + guard case .returnSocketAddress(let address) = ret else { throw UnexpectedKernelReturn(ret) } + return address } override func remoteAddress() throws -> SocketAddress { try self.userToKernel.waitForEmptyAndSet(.remoteAddress) let ret = try self.waitForKernelReturn() - if case .returnSocketAddress(let address) = ret { - return address - } else { + guard case .returnSocketAddress(let address) = ret else { throw UnexpectedKernelReturn(ret) } + return address } override func bind(to address: SocketAddress) throws { try self.userToKernel.waitForEmptyAndSet(.bind(address)) let ret = try self.waitForKernelReturn() - if case .returnVoid = ret { - return - } else { + guard case .returnVoid = ret else { throw UnexpectedKernelReturn(ret) } + return } override func listen(backlog: Int32 = 128) throws { try self.withUnsafeHandle { fd in try self.userToKernel.waitForEmptyAndSet(.listen(fd, backlog)) let ret = try self.waitForKernelReturn() - if case .returnVoid = ret { - return - } else { + guard case .returnVoid = ret else { throw UnexpectedKernelReturn(ret) } + return } } @@ -362,11 +349,10 @@ class HookedServerSocket: ServerSocket, UserKernelInterface { try self.userToKernel.waitForEmptyAndSet(.close(fd)) let ret = try self.waitForKernelReturn() - if case .returnVoid = ret { - return - } else { + guard case .returnVoid = ret else { throw UnexpectedKernelReturn(ret) } + return } } @@ -388,54 +374,49 @@ class HookedSocket: Socket, UserKernelInterface { try self.withUnsafeHandle { fd in try self.userToKernel.waitForEmptyAndSet(.disableSIGPIPE(fd)) let ret = try self.waitForKernelReturn() - if case .returnVoid = ret { - return - } else { + guard case .returnVoid = ret else { throw UnexpectedKernelReturn(ret) } + return } } override func localAddress() throws -> SocketAddress { try self.userToKernel.waitForEmptyAndSet(.localAddress) let ret = try self.waitForKernelReturn() - if case .returnSocketAddress(let address) = ret { - return address - } else { + guard case .returnSocketAddress(let address) = ret else { throw UnexpectedKernelReturn(ret) } + return address } override func remoteAddress() throws -> SocketAddress { try self.userToKernel.waitForEmptyAndSet(.remoteAddress) let ret = try self.waitForKernelReturn() - if case .returnSocketAddress(let address) = ret { - return address - } else { + guard case .returnSocketAddress(let address) = ret else { throw UnexpectedKernelReturn(ret) } + return address } override func connect(to address: SocketAddress) throws -> Bool { try self.userToKernel.waitForEmptyAndSet(.connect(address)) let ret = try self.waitForKernelReturn() - if case .returnBool(let success) = ret { - return success - } else { + guard case .returnBool(let success) = ret else { throw UnexpectedKernelReturn(ret) } + return success } override func read(pointer: UnsafeMutableRawBufferPointer) throws -> IOResult { try self.userToKernel.waitForEmptyAndSet(.read(pointer.count)) let ret = try self.waitForKernelReturn() - if case .returnBytes(let buffer) = ret { - assert(buffer.readableBytes <= pointer.count) - pointer.copyBytes(from: buffer.readableBytesView) - return .processed(buffer.readableBytes) - } else { + guard case .returnBytes(let buffer) = ret else { throw UnexpectedKernelReturn(ret) } + assert(buffer.readableBytes <= pointer.count) + pointer.copyBytes(from: buffer.readableBytesView) + return .processed(buffer.readableBytes) } override func write(pointer: UnsafeRawBufferPointer) throws -> IOResult { @@ -444,11 +425,10 @@ class HookedSocket: Socket, UserKernelInterface { buffer.writeBytes(pointer) try self.userToKernel.waitForEmptyAndSet(.write(fd, buffer)) let ret = try self.waitForKernelReturn() - if case .returnIOResultInt(let result) = ret { - return result - } else { + guard case .returnIOResultInt(let result) = ret else { throw UnexpectedKernelReturn(ret) } + return result } } @@ -467,11 +447,10 @@ class HookedSocket: Socket, UserKernelInterface { try self.userToKernel.waitForEmptyAndSet(.writev(fd, buffers)) let ret = try self.waitForKernelReturn() - if case .returnIOResultInt(let result) = ret { - return result - } else { + guard case .returnIOResultInt(let result) = ret else { throw UnexpectedKernelReturn(ret) } + return result } } @@ -480,41 +459,37 @@ class HookedSocket: Socket, UserKernelInterface { try self.userToKernel.waitForEmptyAndSet(.close(fd)) let ret = try self.waitForKernelReturn() - if case .returnVoid = ret { - return - } else { + guard case .returnVoid = ret else { throw UnexpectedKernelReturn(ret) } + return } override func getOption(level: NIOBSDSocket.OptionLevel, name: NIOBSDSocket.Option) throws -> T { try self.userToKernel.waitForEmptyAndSet(.getOption(level, name)) let ret = try self.waitForKernelReturn() - if case .returnAny(let any) = ret { - return any as! T - } else { + guard case .returnAny(let any) = ret else { throw UnexpectedKernelReturn(ret) } + return any as! T } override func setOption(level: NIOBSDSocket.OptionLevel, name: NIOBSDSocket.Option, value: T) throws { try self.userToKernel.waitForEmptyAndSet(.setOption(level, name, value)) let ret = try self.waitForKernelReturn() - if case .returnVoid = ret { - return - } else { + guard case .returnVoid = ret else { throw UnexpectedKernelReturn(ret) } + return } override func bind(to address: SocketAddress) throws { try self.userToKernel.waitForEmptyAndSet(.bind(address)) let ret = try self.waitForKernelReturn() - if case .returnVoid = ret { - return - } else { + guard case .returnVoid = ret else { throw UnexpectedKernelReturn(ret) } + return } } @@ -526,12 +501,11 @@ extension HookedSelector { matcher: (UserToKernel) throws -> Bool ) throws { let syscall = try self.userToKernel.takeValue() - if try matcher(syscall) { - try self.kernelToUser.waitForEmptyAndSet(result) - } else { + guard try matcher(syscall) else { XCTFail("unexpected syscall \(syscall)", file: (file), line: line) throw UnexpectedSyscall(syscall) } + try self.kernelToUser.waitForEmptyAndSet(result) } /// This function will wait for an event loop wakeup until it unblocks. If the event loop @@ -542,11 +516,10 @@ extension HookedSelector { SAL.printIfDebug("\(#function)") try self.wakeups.takeValue() try self.assertSyscallAndReturn(.returnSelectorEvent(nil), file: (file), line: line) { syscall in - if case .whenReady(.block) = syscall { - return true - } else { + guard case .whenReady(.block) = syscall else { return false } + return true } } @@ -746,18 +719,18 @@ extension SALTest { try self.assertLocalAddress(address: localAddress) try self.assertRemoteAddress(address: remoteAddress) try self.assertRegister { selectable, eventSet, registration in - if case (.socketChannel(let channel), let registrationEventSet) = - (registration.channel, registration.interested) - { - - XCTAssertEqual(localAddress, channel.localAddress) - XCTAssertEqual(remoteAddress, channel.remoteAddress) - XCTAssertEqual(eventSet, registrationEventSet) - XCTAssertEqual(.reset, eventSet) - return true - } else { + guard + case (.socketChannel(let channel), let registrationEventSet) = + (registration.channel, registration.interested) + else { return false } + + XCTAssertEqual(localAddress, channel.localAddress) + XCTAssertEqual(remoteAddress, channel.remoteAddress) + XCTAssertEqual(eventSet, registrationEventSet) + XCTAssertEqual(.reset, eventSet) + return true } try self.assertReregister { selectable, eventSet in XCTAssertEqual([.reset, .readEOF], eventSet) @@ -788,18 +761,18 @@ extension SALTest { try self.assertLocalAddress(address: localAddress) try self.assertListen(expectedFD: .max, expectedBacklog: 128) try self.assertRegister { selectable, eventSet, registration in - if case (.serverSocketChannel(let channel), let registrationEventSet) = - (registration.channel, registration.interested) - { - - XCTAssertEqual(localAddress, channel.localAddress) - XCTAssertEqual(nil, channel.remoteAddress) - XCTAssertEqual(eventSet, registrationEventSet) - XCTAssertEqual(.reset, eventSet) - return true - } else { + guard + case (.serverSocketChannel(let channel), let registrationEventSet) = + (registration.channel, registration.interested) + else { return false } + + XCTAssertEqual(localAddress, channel.localAddress) + XCTAssertEqual(nil, channel.remoteAddress) + XCTAssertEqual(eventSet, registrationEventSet) + XCTAssertEqual(.reset, eventSet) + return true } try self.assertReregister { selectable, eventSet in XCTAssertEqual([.reset, .readEOF], eventSet) @@ -869,11 +842,10 @@ extension SALTest { file: (file), line: line ) { syscall in - if case .whenReady = syscall { - return true - } else { + guard case .whenReady = syscall else { return false } + return true } } @@ -896,11 +868,10 @@ extension SALTest { ret = .error(error) } try self.selector.assertSyscallAndReturn(ret, file: (file), line: line) { syscall in - if case .disableSIGPIPE(expectedFD) = syscall { - return true - } else { + guard case .disableSIGPIPE(expectedFD) = syscall else { return false } + return true } } @@ -913,11 +884,10 @@ extension SALTest { file: (file), line: line ) { syscall in - if case .localAddress = syscall { - return true - } else { + guard case .localAddress = syscall else { return false } + return true } } @@ -928,11 +898,10 @@ extension SALTest { file: (file), line: line ) { syscall in - if case .remoteAddress = syscall { - return true - } else { + guard case .remoteAddress = syscall else { return false } + return true } } @@ -945,34 +914,31 @@ extension SALTest { ) throws { SAL.printIfDebug("\(#function)") try self.selector.assertSyscallAndReturn(.returnBool(result), file: (file), line: line) { syscall in - if case .connect(let address) = syscall { - return address == expectedAddress - } else { + guard case .connect(let address) = syscall else { return false } + return address == expectedAddress } } func assertBind(expectedAddress: SocketAddress, file: StaticString = #filePath, line: UInt = #line) throws { SAL.printIfDebug("\(#function)") try self.selector.assertSyscallAndReturn(.returnVoid, file: (file), line: line) { syscall in - if case .bind(let address) = syscall { - return address == expectedAddress - } else { + guard case .bind(let address) = syscall else { return false } + return address == expectedAddress } } func assertClose(expectedFD: CInt, file: StaticString = #filePath, line: UInt = #line) throws { SAL.printIfDebug("\(#function)") try self.selector.assertSyscallAndReturn(.returnVoid, file: (file), line: line) { syscall in - if case .close(let fd) = syscall { - XCTAssertEqual(expectedFD, fd, file: (file), line: line) - return true - } else { + guard case .close(let fd) = syscall else { return false } + XCTAssertEqual(expectedFD, fd, file: (file), line: line) + return true } } @@ -985,11 +951,10 @@ extension SALTest { ) throws { SAL.printIfDebug("\(#function)") try self.selector.assertSyscallAndReturn(.returnAny(value), file: (file), line: line) { syscall in - if case .getOption(expectedLevel, expectedOption) = syscall { - return true - } else { + guard case .getOption(expectedLevel, expectedOption) = syscall else { return false } + return true } } @@ -1002,11 +967,10 @@ extension SALTest { ) throws { SAL.printIfDebug("\(#function)") try self.selector.assertSyscallAndReturn(.returnVoid, file: (file), line: line) { syscall in - if case .setOption(expectedLevel, expectedOption, let value) = syscall { - return valueMatcher(value) - } else { + guard case .setOption(expectedLevel, expectedOption, let value) = syscall else { return false } + return valueMatcher(value) } } @@ -1017,11 +981,10 @@ extension SALTest { ) throws { SAL.printIfDebug("\(#function)") try self.selector.assertSyscallAndReturn(.returnVoid, file: (file), line: line) { syscall in - if case .register(let selectable, let eventSet, let registration) = syscall { - return try matcher(selectable, eventSet, registration) - } else { + guard case .register(let selectable, let eventSet, let registration) = syscall else { return false } + return try matcher(selectable, eventSet, registration) } } @@ -1032,11 +995,10 @@ extension SALTest { ) throws { SAL.printIfDebug("\(#function)") try self.selector.assertSyscallAndReturn(.returnVoid, file: (file), line: line) { syscall in - if case .reregister(let selectable, let eventSet) = syscall { - return try matcher(selectable, eventSet) - } else { + guard case .reregister(let selectable, let eventSet) = syscall else { return false } + return try matcher(selectable, eventSet) } } @@ -1047,11 +1009,10 @@ extension SALTest { ) throws { SAL.printIfDebug("\(#function)") try self.selector.assertSyscallAndReturn(.returnVoid, file: (file), line: line) { syscall in - if case .deregister(let selectable) = syscall { - return try matcher(selectable) - } else { + guard case .deregister(let selectable) = syscall else { return false } + return try matcher(selectable) } } @@ -1064,11 +1025,10 @@ extension SALTest { ) throws { SAL.printIfDebug("\(#function)") try self.selector.assertSyscallAndReturn(.returnIOResultInt(`return`), file: (file), line: line) { syscall in - if case .write(let actualFD, let actualBytes) = syscall { - return expectedFD == actualFD && expectedBytes == actualBytes - } else { + guard case .write(let actualFD, let actualBytes) = syscall else { return false } + return expectedFD == actualFD && expectedBytes == actualBytes } } @@ -1081,11 +1041,10 @@ extension SALTest { ) throws { SAL.printIfDebug("\(#function)") try self.selector.assertSyscallAndReturn(.returnIOResultInt(`return`), file: (file), line: line) { syscall in - if case .writev(let actualFD, let actualBytes) = syscall { - return expectedFD == actualFD && expectedBytes == actualBytes - } else { + guard case .writev(let actualFD, let actualBytes) = syscall else { return false } + return expectedFD == actualFD && expectedBytes == actualBytes } } @@ -1102,12 +1061,11 @@ extension SALTest { file: (file), line: line ) { syscall in - if case .read(let amount) = syscall { - XCTAssertEqual(expectedBufferSpace, amount, file: (file), line: line) - return true - } else { + guard case .read(let amount) = syscall else { return false } + XCTAssertEqual(expectedBufferSpace, amount, file: (file), line: line) + return true } } @@ -1123,13 +1081,12 @@ extension SALTest { file: (file), line: line ) { syscall in - if case .listen(let fd, let backlog) = syscall { - XCTAssertEqual(fd, expectedFD, file: (file), line: line) - XCTAssertEqual(backlog, expectedBacklog, file: (file), line: line) - return true - } else { + guard case .listen(let fd, let backlog) = syscall else { return false } + XCTAssertEqual(fd, expectedFD, file: (file), line: line) + XCTAssertEqual(backlog, expectedBacklog, file: (file), line: line) + return true } } @@ -1146,13 +1103,12 @@ extension SALTest { file: (file), line: line ) { syscall in - if case .accept(let fd, let nonBlocking) = syscall { - XCTAssertEqual(fd, expectedFD, file: (file), line: line) - XCTAssertEqual(nonBlocking, expectedNonBlocking, file: (file), line: line) - return true - } else { + guard case .accept(let fd, let nonBlocking) = syscall else { return false } + XCTAssertEqual(fd, expectedFD, file: (file), line: line) + XCTAssertEqual(nonBlocking, expectedNonBlocking, file: (file), line: line) + return true } } @@ -1169,13 +1125,12 @@ extension SALTest { file: (file), line: line ) { syscall in - if case .accept(let fd, let nonBlocking) = syscall { - XCTAssertEqual(fd, expectedFD, file: (file), line: line) - XCTAssertEqual(nonBlocking, expectedNonBlocking, file: (file), line: line) - return true - } else { + guard case .accept(let fd, let nonBlocking) = syscall else { return false } + XCTAssertEqual(fd, expectedFD, file: (file), line: line) + XCTAssertEqual(nonBlocking, expectedNonBlocking, file: (file), line: line) + return true } } diff --git a/Tests/NIOPosixTests/SystemCallWrapperHelpers.swift b/Tests/NIOPosixTests/SystemCallWrapperHelpers.swift index d2041f9286..f2ccfc57f1 100644 --- a/Tests/NIOPosixTests/SystemCallWrapperHelpers.swift +++ b/Tests/NIOPosixTests/SystemCallWrapperHelpers.swift @@ -88,22 +88,21 @@ func runSystemCallWrapperPerformanceTest( for _ in 0..( return try body() } catch { XCTFail("\(message.map { $0 + ": " } ?? "")unexpected error \(error) thrown", file: (file), line: line) - if let defaultValue = defaultValue { - return defaultValue - } else { + guard let defaultValue = defaultValue else { throw error } + return defaultValue } } @@ -842,15 +840,7 @@ func forEachCrossConnectedStreamChannelPair( extension EventLoopFuture { var isFulfilled: Bool { - if self.eventLoop.inEventLoop { - // Easy, we're on the EventLoop. Let's just use our knowledge that we run completed future callbacks - // immediately. - var fulfilled = false - self.whenComplete { _ in - fulfilled = true - } - return fulfilled - } else { + guard self.eventLoop.inEventLoop else { let lock = NIOLock() let group = DispatchGroup() var fulfilled = false // protected by lock @@ -866,5 +856,12 @@ extension EventLoopFuture { group.wait() // this is very nasty but this is for tests only, so... return lock.withLock { fulfilled } } + // Easy, we're on the EventLoop. Let's just use our knowledge that we run completed future callbacks + // immediately. + var fulfilled = false + self.whenComplete { _ in + fulfilled = true + } + return fulfilled } } diff --git a/Tests/NIOTestUtilsTests/NIOHTTP1TestServerTest.swift b/Tests/NIOTestUtilsTests/NIOHTTP1TestServerTest.swift index 293fb624bc..734882e89c 100644 --- a/Tests/NIOTestUtilsTests/NIOHTTP1TestServerTest.swift +++ b/Tests/NIOTestUtilsTests/NIOHTTP1TestServerTest.swift @@ -547,10 +547,9 @@ func assertNoThrowWithValue( return try body() } catch { XCTFail("\(message.map { $0 + ": " } ?? "")unexpected error \(error) thrown", file: (file), line: line) - if let defaultValue = defaultValue { - return defaultValue - } else { + guard let defaultValue = defaultValue else { throw error } + return defaultValue } } diff --git a/Tests/NIOWebSocketTests/WebSocketFrameDecoderTest.swift b/Tests/NIOWebSocketTests/WebSocketFrameDecoderTest.swift index 8a0a7f1944..8774aabe17 100644 --- a/Tests/NIOWebSocketTests/WebSocketFrameDecoderTest.swift +++ b/Tests/NIOWebSocketTests/WebSocketFrameDecoderTest.swift @@ -115,11 +115,10 @@ public final class WebSocketFrameDecoderTest: XCTestCase { // an encoder because we want to fail gracefully if a frame is written. let f = self.decoderChannel.pipeline.context(handlerType: ByteToMessageHandler.self) .flatMapThrowing { - if let handler = $0.handler as? RemovableChannelHandler { - return handler - } else { + guard let handler = $0.handler as? RemovableChannelHandler else { throw ChannelError.unremovableHandler } + return handler }.flatMap { self.decoderChannel.pipeline.removeHandler($0) }