Browse Source

Fix various warnings (#1935)

Motivation:

Swift 6 compiler emits various warnings which we can fix/suppress.

Modifications:

- Add `@unchecked Sendable` where appropriate
- Add a `@retroactive` where appropriate
- Remove use of an inappropriate retroactive conformance

Result:

Fewer warnings
George Barnett 1 year ago
parent
commit
84534555bb

+ 3 - 1
Sources/Examples/Echo/Implementation/Interceptors.swift

@@ -20,7 +20,9 @@ import NIOCore
 // All client interceptors derive from the 'ClientInterceptor' base class. We know the request and
 // response types for all Echo RPCs are the same: so we'll use them concretely here, allowing us
 // to access fields on each type as we intercept them.
-class LoggingEchoClientInterceptor: ClientInterceptor<Echo_EchoRequest, Echo_EchoResponse> {
+class LoggingEchoClientInterceptor: ClientInterceptor<Echo_EchoRequest, Echo_EchoResponse>,
+  @unchecked Sendable
+{
   /// Called when the interceptor has received a request part to handle.
   ///
   /// - Parameters:

+ 3 - 7
Sources/GRPCInteroperabilityTestsImplementation/GRPCTestingConvenienceMethods.swift

@@ -40,13 +40,9 @@ extension Grpc_Testing_Payload {
 
 // MARK: - Bool value
 
-extension Grpc_Testing_BoolValue: ExpressibleByBooleanLiteral {
-  public typealias BooleanLiteralType = Bool
-
-  public init(booleanLiteral value: Bool) {
-    self = .with {
-      $0.value = value
-    }
+extension Grpc_Testing_BoolValue {
+  public init(_ value: Bool) {
+    self = .with { $0.value = value }
   }
 }
 

+ 7 - 7
Sources/GRPCInteroperabilityTestsImplementation/InteroperabilityTestCases.swift

@@ -180,13 +180,13 @@ class ClientCompressedUnary: InteroperabilityTest {
     let client = Grpc_Testing_TestServiceNIOClient(channel: connection)
 
     let compressedRequest = Grpc_Testing_SimpleRequest.with { request in
-      request.expectCompressed = true
+      request.expectCompressed = .init(true)
       request.responseSize = 314_159
       request.payload = .zeros(count: 271_828)
     }
 
     var uncompressedRequest = compressedRequest
-    uncompressedRequest.expectCompressed = false
+    uncompressedRequest.expectCompressed = .init(false)
 
     // For unary RPCs we disable compression at the call level.
 
@@ -268,7 +268,7 @@ class ServerCompressedUnary: InteroperabilityTest {
     let client = Grpc_Testing_TestServiceNIOClient(channel: connection)
 
     let compressedRequest = Grpc_Testing_SimpleRequest.with { request in
-      request.responseCompressed = true
+      request.responseCompressed = .init(true)
       request.responseSize = 314_159
       request.payload = .zeros(count: 271_828)
     }
@@ -427,7 +427,7 @@ class ClientCompressedStreaming: InteroperabilityTest {
     // compression here will stop that header from being sent.
     let probe = client.streamingInputCall()
     let probeRequest: Grpc_Testing_StreamingInputCallRequest = .with { request in
-      request.expectCompressed = true
+      request.expectCompressed = .init(true)
       request.payload = .zeros(count: 27182)
     }
 
@@ -443,7 +443,7 @@ class ClientCompressedStreaming: InteroperabilityTest {
     // The first message is identical to the probe message, we'll reuse that.
     // The second should not be compressed.
     let secondMessage: Grpc_Testing_StreamingInputCallRequest = .with { request in
-      request.expectCompressed = false
+      request.expectCompressed = .init(false)
       request.payload = .zeros(count: 45904)
     }
 
@@ -565,11 +565,11 @@ class ServerCompressedStreaming: InteroperabilityTest {
     let request: Grpc_Testing_StreamingOutputCallRequest = .with { request in
       request.responseParameters = [
         .with {
-          $0.compressed = true
+          $0.compressed = .init(true)
           $0.size = 31415
         },
         .with {
-          $0.compressed = false
+          $0.compressed = .init(false)
           $0.size = 92653
         },
       ]

+ 1 - 2
Sources/GRPCReflectionService/Server/ReflectionServiceV1.swift

@@ -191,8 +191,7 @@ extension Result<Grpc_Reflection_V1_ServerReflectionResponse.OneOf_MessageRespon
     request: Grpc_Reflection_V1_ServerReflectionRequest
   ) -> Grpc_Reflection_V1_ServerReflectionResponse {
     let result = self.recover().attachRequest(request)
-    // Safe to '!' as the failure type is 'Never'.
-    return try! result.get()
+    return result.get()
   }
 }
 

+ 14 - 2
Sources/GRPCReflectionService/Server/ReflectionServiceV1Alpha.swift

@@ -197,8 +197,7 @@ extension Result<Grpc_Reflection_V1alpha_ServerReflectionResponse.OneOf_MessageR
     request: Grpc_Reflection_V1alpha_ServerReflectionRequest
   ) -> Grpc_Reflection_V1alpha_ServerReflectionResponse {
     let result = self.recover().attachRequest(request)
-    // Safe to '!' as the failure type is 'Never'.
-    return try! result.get()
+    return result.get()
   }
 }
 
@@ -212,3 +211,16 @@ where Success == Grpc_Reflection_V1alpha_ServerReflectionResponse.OneOf_MessageR
     }
   }
 }
+
+#if compiler(<6.0)
+extension Result where Failure == Never {
+  func get() -> Success {
+    switch self {
+    case .success(let success):
+      return success
+    case .failure:
+      fatalError("Unreachable")
+    }
+  }
+}
+#endif

+ 9 - 1
Tests/GRPCHTTP2CoreTests/GRPCStreamStateMachineTests.swift

@@ -2722,7 +2722,7 @@ extension XCTestCase {
 }
 
 @available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *)
-extension GRPCStreamStateMachine.OnNextOutboundFrame: Equatable {
+extension GRPCStreamStateMachine.OnNextOutboundFrame {
   public static func == (
     lhs: GRPCStreamStateMachine.OnNextOutboundFrame,
     rhs: GRPCStreamStateMachine.OnNextOutboundFrame
@@ -2741,3 +2741,11 @@ extension GRPCStreamStateMachine.OnNextOutboundFrame: Equatable {
     }
   }
 }
+
+#if compiler(>=6.0)
+@available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *)
+extension GRPCStreamStateMachine.OnNextOutboundFrame: @retroactive Equatable {}
+#else
+@available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *)
+extension GRPCStreamStateMachine.OnNextOutboundFrame: Equatable {}
+#endif

+ 7 - 3
Tests/GRPCTests/ClientInterceptorPipelineTests.swift

@@ -136,7 +136,9 @@ class ClientInterceptorPipelineTests: GRPCTestCase {
     var cancelled = false
     var timedOut = false
 
-    class FailOnCancel<Request, Response>: ClientInterceptor<Request, Response> {
+    class FailOnCancel<Request, Response>: ClientInterceptor<Request, Response>,
+      @unchecked Sendable
+    {
       override func cancel(
         promise: EventLoopPromise<Void>?,
         context: ClientInterceptorContext<Request, Response>
@@ -299,7 +301,9 @@ class ClientInterceptorPipelineTests: GRPCTestCase {
 // MARK: - Test Interceptors
 
 /// A simple interceptor which records and then forwards and request and response parts it sees.
-class RecordingInterceptor<Request, Response>: ClientInterceptor<Request, Response> {
+class RecordingInterceptor<Request, Response>: ClientInterceptor<Request, Response>, @unchecked
+  Sendable
+{
   var requestParts: [GRPCClientRequestPart<Request>] = []
   var responseParts: [GRPCClientResponsePart<Response>] = []
 
@@ -322,7 +326,7 @@ class RecordingInterceptor<Request, Response>: ClientInterceptor<Request, Respon
 }
 
 /// An interceptor which reverses string request messages.
-class StringRequestReverser: ClientInterceptor<String, String> {
+class StringRequestReverser: ClientInterceptor<String, String>, @unchecked Sendable {
   override func send(
     _ part: GRPCClientRequestPart<String>,
     promise: EventLoopPromise<Void>?,

+ 1 - 1
Tests/GRPCTests/EchoHelpers/Interceptors/DelegatingClientInterceptor.swift

@@ -22,7 +22,7 @@ import SwiftProtobuf
 final class DelegatingClientInterceptor<
   Request: Message,
   Response: Message
->: ClientInterceptor<Request, Response> {
+>: ClientInterceptor<Request, Response>, @unchecked Sendable {
   typealias RequestPart = GRPCClientRequestPart<Request>
   typealias ResponsePart = GRPCClientResponsePart<Response>
   typealias Context = ClientInterceptorContext<Request, Response>

+ 7 - 1
Tests/GRPCTests/GRPCPingHandlerTests.swift

@@ -360,7 +360,13 @@ class GRPCPingHandlerTests: GRPCTestCase {
   }
 }
 
-extension PingHandler.Action: Equatable {
+#if compiler(>=6.0)
+extension PingHandler.Action: @retroactive Equatable {}
+#else
+extension PingHandler.Action: Equatable {}
+#endif
+
+extension PingHandler.Action {
   public static func == (lhs: PingHandler.Action, rhs: PingHandler.Action) -> Bool {
     switch (lhs, rhs) {
     case (.none, .none):

+ 2 - 2
Tests/GRPCTests/InterceptedRPCCancellationTests.swift

@@ -79,7 +79,7 @@ final class InterceptedRPCCancellationTests: GRPCTestCase {
 final class MagicRequiredServerInterceptor<
   Request: Message,
   Response: Message
->: ServerInterceptor<Request, Response> {
+>: ServerInterceptor<Request, Response>, @unchecked Sendable {
   override func receive(
     _ part: GRPCServerRequestPart<Request>,
     context: ServerInterceptorContext<Request, Response>
@@ -103,7 +103,7 @@ final class MagicRequiredServerInterceptor<
 final class MagicAddingClientInterceptor<
   Request: Message,
   Response: Message
->: ClientInterceptor<Request, Response> {
+>: ClientInterceptor<Request, Response>, @unchecked Sendable {
   private let channel: GRPCChannel
   private var requestParts = CircularBuffer<GRPCClientRequestPart<Request>>()
   private var retry: Call<Request, Response>?

+ 12 - 5
Tests/GRPCTests/InterceptorsTests.swift

@@ -176,7 +176,9 @@ private class HelloWorldClientInterceptorFactory:
   }
 }
 
-class RemoteAddressExistsInterceptor<Request, Response>: ServerInterceptor<Request, Response> {
+class RemoteAddressExistsInterceptor<Request, Response>:
+  ServerInterceptor<Request, Response>, @unchecked Sendable
+{
   override func receive(
     _ part: GRPCServerRequestPart<Request>,
     context: ServerInterceptorContext<Request, Response>
@@ -187,7 +189,8 @@ class RemoteAddressExistsInterceptor<Request, Response>: ServerInterceptor<Reque
 }
 
 class NotReallyAuthServerInterceptor<Request: Message, Response: Message>:
-  ServerInterceptor<Request, Response>
+  ServerInterceptor<Request, Response>,
+  @unchecked Sendable
 {
   override func receive(
     _ part: GRPCServerRequestPart<Request>,
@@ -220,7 +223,7 @@ final class HelloWorldServerInterceptorFactory: Helloworld_GreeterServerIntercep
 }
 
 class NotReallyAuthClientInterceptor<Request: Message, Response: Message>:
-  ClientInterceptor<Request, Response>
+  ClientInterceptor<Request, Response>, @unchecked Sendable
 {
   private let client: Helloworld_GreeterNIOClient
 
@@ -323,7 +326,9 @@ class NotReallyAuthClientInterceptor<Request: Message, Response: Message>:
   }
 }
 
-final class EchoReverseInterceptor: ClientInterceptor<Echo_EchoRequest, Echo_EchoResponse> {
+final class EchoReverseInterceptor: ClientInterceptor<Echo_EchoRequest, Echo_EchoResponse>,
+  @unchecked Sendable
+{
   override func send(
     _ part: GRPCClientRequestPart<Echo_EchoRequest>,
     promise: EventLoopPromise<Void>?,
@@ -398,7 +403,9 @@ final class CountOnCloseInterceptors: Echo_EchoServerInterceptorFactoryProtocol
   }
 }
 
-final class CountOnCloseServerInterceptor: ServerInterceptor<Echo_EchoRequest, Echo_EchoResponse> {
+final class CountOnCloseServerInterceptor: ServerInterceptor<Echo_EchoRequest, Echo_EchoResponse>,
+  @unchecked Sendable
+{
   private let counter: ManagedAtomic<Int>
 
   init(counter: ManagedAtomic<Int>) {

+ 15 - 5
Tests/GRPCTests/ServerInterceptorPipelineTests.swift

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+import NIOConcurrencyHelpers
 import NIOCore
 import NIOEmbedded
 import NIOHPACK
@@ -125,16 +126,25 @@ class ServerInterceptorPipelineTests: GRPCTestCase {
 }
 
 internal class RecordingServerInterceptor<Request, Response>:
-  ServerInterceptor<Request, Response>
+  ServerInterceptor<Request, Response>, @unchecked Sendable
 {
-  var requestParts: [GRPCServerRequestPart<Request>] = []
-  var responseParts: [GRPCServerResponsePart<Response>] = []
+  private let lock = NIOLock()
+  private var _requestParts: [GRPCServerRequestPart<Request>] = []
+  private var _responseParts: [GRPCServerResponsePart<Response>] = []
+
+  var requestParts: [GRPCServerRequestPart<Request>] {
+    self.lock.withLock { self._requestParts }
+  }
+
+  var responseParts: [GRPCServerResponsePart<Response>] {
+    self.lock.withLock { self._responseParts }
+  }
 
   override func receive(
     _ part: GRPCServerRequestPart<Request>,
     context: ServerInterceptorContext<Request, Response>
   ) {
-    self.requestParts.append(part)
+    self.lock.withLock { self._requestParts.append(part) }
     context.receive(part)
   }
 
@@ -143,7 +153,7 @@ internal class RecordingServerInterceptor<Request, Response>:
     promise: EventLoopPromise<Void>?,
     context: ServerInterceptorContext<Request, Response>
   ) {
-    self.responseParts.append(part)
+    self.lock.withLock { self._responseParts.append(part) }
     context.send(part, promise: promise)
   }
 }

+ 5 - 2
Tests/GRPCTests/ServerInterceptorTests.swift

@@ -204,7 +204,10 @@ final class EchoInterceptorFactory: Echo_EchoServerInterceptorFactoryProtocol {
   }
 }
 
-class ExtraRequestPartEmitter: ServerInterceptor<Echo_EchoRequest, Echo_EchoResponse> {
+class ExtraRequestPartEmitter:
+  ServerInterceptor<Echo_EchoRequest, Echo_EchoResponse>,
+  @unchecked Sendable
+{
   enum Part {
     case metadata
     case message
@@ -293,7 +296,7 @@ class EchoFromInterceptor: Echo_EchoProvider {
 
   // Since all methods use the same request/response types, we can use a single interceptor to
   // respond to all of them.
-  class Interceptor: ServerInterceptor<Echo_EchoRequest, Echo_EchoResponse> {
+  class Interceptor: ServerInterceptor<Echo_EchoRequest, Echo_EchoResponse>, @unchecked Sendable {
     private var collectedRequests: [Echo_EchoRequest] = []
 
     override func receive(