Only import SwiftProtobuf when WKTs are used as input/output (#86)

Modification:

The code generator imports SwiftProtobuf if the file being generated has
a dependency on any of the protos bundled by SwiftProtobuf. This can
yield unnecessary imports which may warn if used in conjunction with
access levels on imports.

The problem is that the grpc generator checks for dependencies on the
FileDescriptor which may include messages defined in the same file as a
service. If any of those messages depend on a well known type (WKT) then
an import will be added even though the code being generated doesn't
include message code.

Modifications:

- Only include a SwiftProtobuf import if an input or output type for an
RPC is a bundled proto.
- Fix a test where a WKT is used in a message but not as an input/output
type for an RPC.

Result:

Fewer warnings

Author: glbrntt

Sources/GRPCProtobufCodeGen/ProtobufCodeGenParser.swift

@@ -110,15 +110,26 @@ extension ProtobufCodeGenParser {
     var codeDependencies: [Dependency] = [
       Dependency(module: self.moduleNames.grpcProtobuf, accessLevel: .internal)
     ]
+
     // If there's a dependency on a bundled proto then add the SwiftProtobuf import.
     //
     // Importing SwiftProtobuf unconditionally results in warnings in the generated
     // code if access-levels are used on imports and no bundled protos are used.
-    let dependsOnBundledProto = file.dependencies.contains { descriptor in
-      SwiftProtobufInfo.isBundledProto(file: descriptor)
+    //
+    // The import is only needed if a bundled proto is used as the input or output type
+    // for an RPC. The file may have a dependency on a bundled proto without requiring
+    // the SwiftProtobuf import (e.g. a message depending on a bundle proto may be
+    // defined in the same file as the service, the dependency will be in the '.pb.swift'
+    // along with the message code).
+    let needsProtobufImport = file.services.contains { service in
+      service.methods.contains { method in
+        let inputIsBundled = SwiftProtobufInfo.isBundledProto(file: method.inputType.file)
+        let outputIsBundled = SwiftProtobufInfo.isBundledProto(file: method.outputType.file)
+        return inputIsBundled || outputIsBundled
+      }
     }
 
-    if dependsOnBundledProto {
+    if needsProtobufImport {
       let dependency = Dependency(
         module: self.moduleNames.swiftProtobuf,
         accessLevel: self.accessLevel

Sources/protoc-gen-grpc-swift-2/Options.swift

@@ -62,6 +62,8 @@ struct GeneratorOptions {
   }
 
   init(pairs: [(key: String, value: String)]) throws {
+    var moduleMapPath: String?
+
     for pair in pairs {
       switch pair.key {
       case "Visibility":
@@ -87,14 +89,7 @@ struct GeneratorOptions {
 
       case "ProtoPathModuleMappings":
         if !pair.value.isEmpty {
-          do {
-            self.protoToModuleMappings = try ProtoFileToModuleMappings(path: pair.value)
-          } catch let e {
-            throw GenerationError.wrappedError(
-              message: "Parameter 'ProtoPathModuleMappings=\(pair.value)'",
-              error: e
-            )
-          }
+          moduleMapPath = pair.value
         }
 
       case "FileNaming":
@@ -164,6 +159,24 @@ struct GeneratorOptions {
         throw GenerationError.unknownParameter(name: pair.key)
       }
     }
+
+    if let moduleMapPath = moduleMapPath {
+      do {
+        self.protoToModuleMappings = try ProtoFileToModuleMappings(
+          path: moduleMapPath,
+          swiftProtobufModuleName: self.config.moduleNames.swiftProtobuf
+        )
+      } catch let e {
+        throw GenerationError.wrappedError(
+          message: "Parameter 'ProtoPathModuleMappings=\(moduleMapPath)'",
+          error: e
+        )
+      }
+    } else {
+      self.protoToModuleMappings = ProtoFileToModuleMappings(
+        swiftProtobufModuleName: self.config.moduleNames.swiftProtobuf
+      )
+    }
   }
 
   static func parseParameter(string: String?) -> [(key: String, value: String)] {

Tests/GRPCProtobufCodeGenTests/Generated/test-service.pb

@@ -66,8 +66,7 @@ struct ProtobufCodeGenParserTests {
     @available(gRPCSwiftProtobuf 2.0, *)
     func dependencies() throws {
       let expected: [GRPCCodeGen.Dependency] = [
-        .init(module: "GRPCProtobuf", accessLevel: .internal),  // Always an internal import
-        .init(module: "SwiftProtobuf", accessLevel: .internal),
+        .init(module: "GRPCProtobuf", accessLevel: .internal)  // Always an internal import
       ]
       #expect(try self.codeGen.dependencies == expected)
     }
@@ -266,5 +265,37 @@ struct ProtobufCodeGenParserTests {
       ]
       #expect(codeGen.dependencies == expected)
     }
+
+    @Test("Generate with different module names")
+    @available(gRPCSwiftProtobuf 2.0, *)
+    func generateWithDifferentModuleNames() throws {
+      var config = ProtobufCodeGenerator.Config.defaults
+      let defaultNames = config.moduleNames
+
+      config.accessLevel = .public
+      config.indentation = 2
+      config.moduleNames.grpcCore = String(config.moduleNames.grpcCore.reversed())
+      config.moduleNames.grpcProtobuf = String(config.moduleNames.grpcProtobuf.reversed())
+      config.moduleNames.swiftProtobuf = String(config.moduleNames.swiftProtobuf.reversed())
+
+      let generator = ProtobufCodeGenerator(config: config)
+      let generated = try generator.generateCode(
+        fileDescriptor: Self.fileDescriptor,
+        protoFileModuleMappings: ProtoFileToModuleMappings(
+          swiftProtobufModuleName: config.moduleNames.swiftProtobuf
+        ),
+        extraModuleImports: []
+      )
+
+      // Mustn't contain the default names.
+      #expect(!generated.contains(defaultNames.grpcCore))
+      #expect(!generated.contains(defaultNames.grpcProtobuf))
+      #expect(!generated.contains(defaultNames.swiftProtobuf))
+
+      // Must contain the configured names.
+      #expect(generated.contains(config.moduleNames.grpcCore))
+      #expect(generated.contains(config.moduleNames.grpcProtobuf))
+      #expect(generated.contains(config.moduleNames.swiftProtobuf))
+    }
   }
 }

Tests/GRPCProtobufCodeGenTests/ProtobufCodeGenParserTests.swift

@@ -99,7 +99,6 @@ struct ProtobufCodeGeneratorTests {
 
         import GRPCCore
         import GRPCProtobuf
-        import SwiftProtobuf
 
         // MARK: - test.TestService
 
@@ -1103,35 +1102,6 @@ struct ProtobufCodeGeneratorTests {
       #expect(generated == expected)
     }
 
-    @Test("Generate with different module names")
-    @available(gRPCSwiftProtobuf 2.0, *)
-    func generateWithDifferentModuleNames() throws {
-      var config = ProtobufCodeGenerator.Config.defaults
-      let defaultNames = config.moduleNames
-
-      config.accessLevel = .public
-      config.indentation = 2
-      config.moduleNames.grpcCore = String(config.moduleNames.grpcCore.reversed())
-      config.moduleNames.grpcProtobuf = String(config.moduleNames.grpcProtobuf.reversed())
-      config.moduleNames.swiftProtobuf = String(config.moduleNames.swiftProtobuf.reversed())
-
-      let generator = ProtobufCodeGenerator(config: config)
-      let generated = try generator.generateCode(
-        fileDescriptor: Self.fileDescriptor,
-        protoFileModuleMappings: ProtoFileToModuleMappings(),
-        extraModuleImports: []
-      )
-
-      // Mustn't contain the default names.
-      #expect(!generated.contains(defaultNames.grpcCore))
-      #expect(!generated.contains(defaultNames.grpcProtobuf))
-      #expect(!generated.contains(defaultNames.swiftProtobuf))
-
-      // Must contain the configured names.
-      #expect(generated.contains(config.moduleNames.grpcCore))
-      #expect(generated.contains(config.moduleNames.grpcProtobuf))
-      #expect(generated.contains(config.moduleNames.swiftProtobuf))
-    }
   }
 
   @Suite("File-without-services (foo-messages.proto)")

Tests/GRPCProtobufCodeGenTests/ProtobufCodeGeneratorTests.swift

@@ -3,7 +3,7 @@ syntax = "proto3";
 
 package test;
 
-// Using a WKT forces an "SwiftProtobuf" to be imported in generated code.
+// WKT isn't used in API generated by gRPC, so no import is expected.
 import "google/protobuf/any.proto";
 
 // Service docs.