[CodeGenLib] Add validation step for input (#1753)

Add validation step for input in IDLToStructuredSwiftTranslator

Motivation:

The IDLToStructuredSwiftTranslator should discover errors in the CodeGenerationRequest object, before the specialized translators start their work. This way, if the input is not correct no translator will be started.

Modifications:

Moved the validation functions into a IDLToStructuredSwiftTranslator extension and the associated tests in a separate test file.

Result:

The input validation is not done by each individual specialzed translator, but by the main translator, before transformations take place.

Sources/GRPCCodeGen/Internal/Translator/IDLToStructuredSwiftTranslator.swift

@@ -22,6 +22,7 @@ struct IDLToStructuredSwiftTranslator: Translator {
     client: Bool,
     server: Bool
   ) throws -> StructuredSwiftRepresentation {
+    try self.validateInput(codeGenerationRequest)
     let typealiasTranslator = TypealiasTranslator(client: client, server: server)
     let topComment = Comment.doc(codeGenerationRequest.leadingTrivia)
     let imports: [ImportDescription] = [
@@ -49,6 +50,88 @@ struct IDLToStructuredSwiftTranslator: Translator {
   }
 }
 
+extension IDLToStructuredSwiftTranslator {
+  private func validateInput(_ codeGenerationRequest: CodeGenerationRequest) throws {
+    let servicesByNamespace = Dictionary(
+      grouping: codeGenerationRequest.services,
+      by: { $0.namespace }
+    )
+    try self.checkServiceNamesAreUnique(for: servicesByNamespace)
+    for service in codeGenerationRequest.services {
+      try self.checkMethodNamesAreUnique(in: service)
+    }
+  }
+
+  // Verify service names are unique within each namespace and that services with no namespace
+  // don't have the same names as any of the namespaces.
+  private func checkServiceNamesAreUnique(
+    for servicesByNamespace: [String: [CodeGenerationRequest.ServiceDescriptor]]
+  ) throws {
+    // Check that if there are services in an empty namespace, none have names which match other namespaces
+    if let noNamespaceServices = servicesByNamespace[""] {
+      let namespaces = servicesByNamespace.keys
+      for service in noNamespaceServices {
+        if namespaces.contains(service.name) {
+          throw CodeGenError(
+            code: .nonUniqueServiceName,
+            message: """
+              Services with no namespace must not have the same names as the namespaces. \
+              \(service.name) is used as a name for a service with no namespace and a namespace.
+              """
+          )
+        }
+      }
+    }
+
+    // Check that service names are unique within each namespace.
+    for (namespace, services) in servicesByNamespace {
+      var serviceNames: Set<String> = []
+      for service in services {
+        if serviceNames.contains(service.name) {
+          let errorMessage: String
+          if namespace.isEmpty {
+            errorMessage = """
+              Services in an empty namespace must have unique names. \
+              \(service.name) is used as a name for multiple services without namespaces.
+              """
+          } else {
+            errorMessage = """
+              Services within the same namespace must have unique names. \
+              \(service.name) is used as a name for multiple services in the \(service.namespace) namespace.
+              """
+          }
+          throw CodeGenError(
+            code: .nonUniqueServiceName,
+            message: errorMessage
+          )
+        }
+        serviceNames.insert(service.name)
+      }
+    }
+  }
+
+  // Verify method names are unique for the service.
+  private func checkMethodNamesAreUnique(
+    in service: CodeGenerationRequest.ServiceDescriptor
+  ) throws {
+    let methodNames = service.methods.map { $0.name }
+    var seenNames = Set<String>()
+
+    for methodName in methodNames {
+      if seenNames.contains(methodName) {
+        throw CodeGenError(
+          code: .nonUniqueMethodName,
+          message: """
+            Methods of a service must have unique names. \
+            \(methodName) is used as a name for multiple methods of the \(service.name) service.
+            """
+        )
+      }
+      seenNames.insert(methodName)
+    }
+  }
+}
+
 extension CodeGenerationRequest.ServiceDescriptor {
   var namespacedTypealiasPrefix: String {
     if self.namespace.isEmpty {

Sources/GRPCCodeGen/Internal/Translator/TypealiasTranslator.swift

@@ -66,10 +66,6 @@ struct TypealiasTranslator: SpecializedTranslator {
     let services = codeGenerationRequest.services
     let servicesByNamespace = Dictionary(grouping: services, by: { $0.namespace })
 
-    // Verify service names are unique within each namespace and that services with no namespace
-    // don't have the same names as any of the namespaces.
-    try self.checkServiceNamesAreUnique(for: servicesByNamespace)
-
     // Sorting the keys and the services in each list of the dictionary is necessary
     // so that the generated enums are deterministically ordered.
     for (namespace, services) in servicesByNamespace.sorted(by: { $0.key < $1.key }) {
@@ -85,51 +81,6 @@ struct TypealiasTranslator: SpecializedTranslator {
 }
 
 extension TypealiasTranslator {
-  private func checkServiceNamesAreUnique(
-    for servicesByNamespace: [String: [CodeGenerationRequest.ServiceDescriptor]]
-  ) throws {
-    // Check that if there are services in an empty namespace, none have names which match other namespaces
-    let noNamespaceServices = servicesByNamespace["", default: []]
-    let namespaces = servicesByNamespace.keys
-    for service in noNamespaceServices {
-      if namespaces.contains(service.name) {
-        throw CodeGenError(
-          code: .nonUniqueServiceName,
-          message: """
-            Services with no namespace must not have the same names as the namespaces. \
-            \(service.name) is used as a name for a service with no namespace and a namespace.
-            """
-        )
-      }
-    }
-
-    // Check that service names are unique within each namespace.
-    for (namespace, services) in servicesByNamespace {
-      var serviceNames: Set<String> = []
-      for service in services {
-        if serviceNames.contains(service.name) {
-          let errorMessage: String
-          if namespace.isEmpty {
-            errorMessage = """
-              Services in an empty namespace must have unique names. \
-              \(service.name) is used as a name for multiple services without namespaces.
-              """
-          } else {
-            errorMessage = """
-              Services within the same namespace must have unique names. \
-              \(service.name) is used as a name for multiple services in the \(service.namespace) namespace.
-              """
-          }
-          throw CodeGenError(
-            code: .nonUniqueServiceName,
-            message: errorMessage
-          )
-        }
-        serviceNames.insert(service.name)
-      }
-    }
-  }
-
   private func makeNamespaceEnum(
     for namespace: String,
     containing services: [CodeGenerationRequest.ServiceDescriptor]
@@ -163,9 +114,6 @@ extension TypealiasTranslator {
     var methodsEnum = EnumDescription(name: "Methods")
     let methods = service.methods
 
-    // Verify method names are unique for the service.
-    try self.checkMethodNamesAreUnique(in: service)
-
     // Create the method specific enums.
     for method in methods {
       let methodEnum = self.makeMethodEnum(from: method, in: service)
@@ -196,26 +144,6 @@ extension TypealiasTranslator {
     return .enum(serviceEnum)
   }
 
-  private func checkMethodNamesAreUnique(
-    in service: CodeGenerationRequest.ServiceDescriptor
-  ) throws {
-    let methodNames = service.methods.map { $0.name }
-    var seenNames = Set<String>()
-
-    for methodName in methodNames {
-      if seenNames.contains(methodName) {
-        throw CodeGenError(
-          code: .nonUniqueMethodName,
-          message: """
-            Methods of a service must have unique names. \
-            \(methodName) is used as a name for multiple methods of the \(service.name) service.
-            """
-        )
-      }
-      seenNames.insert(methodName)
-    }
-  }
-
   private func makeMethodEnum(
     from method: CodeGenerationRequest.ServiceDescriptor.MethodDescriptor,
     in service: CodeGenerationRequest.ServiceDescriptor

Tests/GRPCCodeGenTests/Internal/Translator/IDLToStructuredSwiftTranslatorSnippetBasedTests.swift

@@ -0,0 +1,169 @@
+/*
+ * Copyright 2024, gRPC Authors All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#if os(macOS) || os(Linux)  // swift-format doesn't like canImport(Foundation.Process)
+
+import XCTest
+
+@testable import GRPCCodeGen
+
+final class IDLToStructuredSwiftTranslatorSnippetBasedTests: XCTestCase {
+  typealias MethodDescriptor = GRPCCodeGen.CodeGenerationRequest.ServiceDescriptor.MethodDescriptor
+  typealias ServiceDescriptor = GRPCCodeGen.CodeGenerationRequest.ServiceDescriptor
+
+  func testSameNameServicesNoNamespaceError() throws {
+    let serviceA = ServiceDescriptor(
+      documentation: "Documentation for AService",
+      name: "AService",
+      namespace: "",
+      methods: []
+    )
+
+    let codeGenerationRequest = makeCodeGenerationRequest(services: [serviceA, serviceA])
+    let translator = IDLToStructuredSwiftTranslator()
+    XCTAssertThrowsError(
+      ofType: CodeGenError.self,
+      try translator.translate(
+        codeGenerationRequest: codeGenerationRequest,
+        client: true,
+        server: true
+      )
+    ) {
+      error in
+      XCTAssertEqual(
+        error as CodeGenError,
+        CodeGenError(
+          code: .nonUniqueServiceName,
+          message: """
+            Services in an empty namespace must have unique names. \
+            AService is used as a name for multiple services without namespaces.
+            """
+        )
+      )
+    }
+  }
+
+  func testSameNameServicesSameNamespaceError() throws {
+    let serviceA = ServiceDescriptor(
+      documentation: "Documentation for AService",
+      name: "AService",
+      namespace: "namespacea",
+      methods: []
+    )
+
+    let codeGenerationRequest = makeCodeGenerationRequest(services: [serviceA, serviceA])
+    let translator = IDLToStructuredSwiftTranslator()
+    XCTAssertThrowsError(
+      ofType: CodeGenError.self,
+      try translator.translate(
+        codeGenerationRequest: codeGenerationRequest,
+        client: true,
+        server: true
+      )
+    ) {
+      error in
+      XCTAssertEqual(
+        error as CodeGenError,
+        CodeGenError(
+          code: .nonUniqueServiceName,
+          message: """
+            Services within the same namespace must have unique names. \
+            AService is used as a name for multiple services in the namespacea namespace.
+            """
+        )
+      )
+    }
+  }
+
+  func testSameNameMethodsSameServiceError() throws {
+    let methodA = MethodDescriptor(
+      documentation: "Documentation for MethodA",
+      name: "MethodA",
+      isInputStreaming: false,
+      isOutputStreaming: false,
+      inputType: "NamespaceA_ServiceARequest",
+      outputType: "NamespaceA_ServiceAResponse"
+    )
+    let service = ServiceDescriptor(
+      documentation: "Documentation for AService",
+      name: "AService",
+      namespace: "namespacea",
+      methods: [methodA, methodA]
+    )
+
+    let codeGenerationRequest = makeCodeGenerationRequest(services: [service])
+    let translator = IDLToStructuredSwiftTranslator()
+    XCTAssertThrowsError(
+      ofType: CodeGenError.self,
+      try translator.translate(
+        codeGenerationRequest: codeGenerationRequest,
+        client: true,
+        server: true
+      )
+    ) {
+      error in
+      XCTAssertEqual(
+        error as CodeGenError,
+        CodeGenError(
+          code: .nonUniqueMethodName,
+          message: """
+            Methods of a service must have unique names. \
+            MethodA is used as a name for multiple methods of the AService service.
+            """
+        )
+      )
+    }
+  }
+
+  func testSameNameNoNamespaceServiceAndNamespaceError() throws {
+    let serviceA = ServiceDescriptor(
+      documentation: "Documentation for SameName service with no namespace",
+      name: "SameName",
+      namespace: "",
+      methods: []
+    )
+    let serviceB = ServiceDescriptor(
+      documentation: "Documentation for BService",
+      name: "BService",
+      namespace: "SameName",
+      methods: []
+    )
+    let codeGenerationRequest = makeCodeGenerationRequest(services: [serviceA, serviceB])
+    let translator = IDLToStructuredSwiftTranslator()
+    XCTAssertThrowsError(
+      ofType: CodeGenError.self,
+      try translator.translate(
+        codeGenerationRequest: codeGenerationRequest,
+        client: true,
+        server: true
+      )
+    ) {
+      error in
+      XCTAssertEqual(
+        error as CodeGenError,
+        CodeGenError(
+          code: .nonUniqueServiceName,
+          message: """
+            Services with no namespace must not have the same names as the namespaces. \
+            SameName is used as a name for a service with no namespace and a namespace.
+            """
+        )
+      )
+    }
+  }
+}
+
+#endif  // os(macOS) || os(Linux)

Tests/GRPCCodeGenTests/Internal/Translator/TypealiasTranslatorSnippetBasedTests.swift

@@ -497,131 +497,6 @@ final class TypealiasTranslatorSnippetBasedTests: XCTestCase {
       server: true
     )
   }
-
-  func testTypealiasTranslatorSameNameServicesNoNamespaceError() throws {
-    let serviceA = ServiceDescriptor(
-      documentation: "Documentation for AService",
-      name: "AService",
-      namespace: "",
-      methods: []
-    )
-
-    let codeGenerationRequest = makeCodeGenerationRequest(services: [serviceA, serviceA])
-    let translator = TypealiasTranslator(client: true, server: true)
-    XCTAssertThrowsError(
-      ofType: CodeGenError.self,
-      try translator.translate(from: codeGenerationRequest)
-    ) {
-      error in
-      XCTAssertEqual(
-        error as CodeGenError,
-        CodeGenError(
-          code: .nonUniqueServiceName,
-          message: """
-            Services in an empty namespace must have unique names. \
-            AService is used as a name for multiple services without namespaces.
-            """
-        )
-      )
-    }
-  }
-
-  func testTypealiasTranslatorSameNameServicesSameNamespaceError() throws {
-    let serviceA = ServiceDescriptor(
-      documentation: "Documentation for AService",
-      name: "AService",
-      namespace: "namespacea",
-      methods: []
-    )
-
-    let codeGenerationRequest = makeCodeGenerationRequest(services: [serviceA, serviceA])
-    let translator = TypealiasTranslator(client: true, server: true)
-    XCTAssertThrowsError(
-      ofType: CodeGenError.self,
-      try translator.translate(from: codeGenerationRequest)
-    ) {
-      error in
-      XCTAssertEqual(
-        error as CodeGenError,
-        CodeGenError(
-          code: .nonUniqueServiceName,
-          message: """
-            Services within the same namespace must have unique names. \
-            AService is used as a name for multiple services in the namespacea namespace.
-            """
-        )
-      )
-    }
-  }
-
-  func testTypealiasTranslatorSameNameMethodsSameServiceError() throws {
-    let methodA = MethodDescriptor(
-      documentation: "Documentation for MethodA",
-      name: "MethodA",
-      isInputStreaming: false,
-      isOutputStreaming: false,
-      inputType: "NamespaceA_ServiceARequest",
-      outputType: "NamespaceA_ServiceAResponse"
-    )
-    let service = ServiceDescriptor(
-      documentation: "Documentation for AService",
-      name: "AService",
-      namespace: "namespacea",
-      methods: [methodA, methodA]
-    )
-
-    let codeGenerationRequest = makeCodeGenerationRequest(services: [service])
-    let translator = TypealiasTranslator(client: true, server: true)
-    XCTAssertThrowsError(
-      ofType: CodeGenError.self,
-      try translator.translate(from: codeGenerationRequest)
-    ) {
-      error in
-      XCTAssertEqual(
-        error as CodeGenError,
-        CodeGenError(
-          code: .nonUniqueMethodName,
-          message: """
-            Methods of a service must have unique names. \
-            MethodA is used as a name for multiple methods of the AService service.
-            """
-        )
-      )
-    }
-  }
-
-  func testTypealiasTranslatorSameNameNoNamespaceServiceAndNamespaceError() throws {
-    let serviceA = ServiceDescriptor(
-      documentation: "Documentation for SameName service with no namespace",
-      name: "SameName",
-      namespace: "",
-      methods: []
-    )
-    let serviceB = ServiceDescriptor(
-      documentation: "Documentation for BService",
-      name: "BService",
-      namespace: "SameName",
-      methods: []
-    )
-    let codeGenerationRequest = makeCodeGenerationRequest(services: [serviceA, serviceB])
-    let translator = TypealiasTranslator(client: true, server: true)
-    XCTAssertThrowsError(
-      ofType: CodeGenError.self,
-      try translator.translate(from: codeGenerationRequest)
-    ) {
-      error in
-      XCTAssertEqual(
-        error as CodeGenError,
-        CodeGenError(
-          code: .nonUniqueServiceName,
-          message: """
-            Services with no namespace must not have the same names as the namespaces. \
-            SameName is used as a name for a service with no namespace and a namespace.
-            """
-        )
-      )
-    }
-  }
 }
 
 extension TypealiasTranslatorSnippetBasedTests {