introduce "UseAccessLevelOnImports" generator option to support access level on import statements

Author: Skoti

Documentation/PLUGIN.md

@@ -182,6 +182,22 @@ exposed via public API, so even if `ImplementationOnlyImports` is set to `true`,
 this will only work if the `Visibility` is set to `internal`.
 
 
+##### Generation Option: `UseAccessLevelOnImports` - imports preceded by a visibility modifier (`public`, `package`, `internal`)
+
+By default, the code generator does not precede any imports with a visibility modifier.
+You can change this with the `UseAccessLevelOnImports` option:
+
+```
+$ protoc --swift_opt=UseAccessLevelOnImports=[value] --swift_out=. foo/bar/*.proto mumble/*.proto
+```
+
+The possible values for `UseAccessLevelOnImports` are:
+
+* `false` (default): Generates plain import directives without a visibility modifier.
+* `true`: Imports of internal dependencies and any modules defined in the module
+mappings will be preceded by a visibility modifier corresponding to the visibility of the generated types - see `Visibility` option. 
+
+
 ### Building your project
 
 After copying the `.pb.swift` files into your project, you will need

Plugins/SwiftProtobufPlugin/plugin.swift

@@ -85,6 +85,8 @@ struct SwiftProtobufPlugin {
             var fileNaming: FileNaming?
             /// Whether internal imports should be annotated as `@_implementationOnly`.
             var implementationOnlyImports: Bool?
+            /// Whether import statements should be preceded with visibility.
+            var useAccessLevelOnImports: Bool?
         }
 
         /// The path to the `protoc` binary.
@@ -188,6 +190,11 @@ struct SwiftProtobufPlugin {
             protocArgs.append("--swift_opt=ImplementationOnlyImports=\(implementationOnlyImports)")
         }
 
+        // Add the useAccessLevelOnImports only imports flag if it was set
+        if let useAccessLevelOnImports = invocation.useAccessLevelOnImports {
+            protocArgs.append("--swift_opt=UseAccessLevelOnImports=\(useAccessLevelOnImports)")
+        }
+
         var inputFiles = [Path]()
         var outputFiles = [Path]()
 

Sources/SwiftProtobufPluginLibrary/ProtoFileToModuleMappings.swift

@@ -20,7 +20,7 @@ private let defaultSwiftProtobufModuleName = "SwiftProtobuf"
 public struct ProtoFileToModuleMappings {
 
   /// Errors raised from parsing mappings
-  public enum LoadError: Error {
+  public enum LoadError: Error, Equatable {
     /// Raised if the path wasn't found.
     case failToOpen(path: String)
     /// Raised if an mapping entry in the protobuf doesn't have a module name.

Sources/protoc-gen-swift/Descriptor+Extensions.swift

@@ -49,17 +49,15 @@ extension FileDescriptor {
   // Aside: This could be moved into the plugin library, but it doesn't seem
   // like anyone else would need the logic. Swift GRPC support probably stick
   // with the support for the module mappings.
-  public func computeImports(
+  func computeImports(
     namer: SwiftProtobufNamer,
-    reexportPublicImports: Bool,
-    asImplementationOnly: Bool
+    directive: GeneratorOptions.ImportDirective,
+    reexportPublicImports: Bool
   ) -> String {
     // The namer should be configured with the module this file generated for.
     assert(namer.targetModule == (namer.mappings.moduleName(forFile: self) ?? ""))
     // Both options can't be enabled.
-    assert(!reexportPublicImports ||
-           !asImplementationOnly ||
-           reexportPublicImports != asImplementationOnly)
+    assert(!reexportPublicImports || directive != .implementationOnly)
 
     guard namer.mappings.hasMappings else {
       // No module mappings? Everything must be the same module, so no Swift
@@ -72,7 +70,7 @@ extension FileDescriptor {
       return ""
     }
 
-    let directive = asImplementationOnly ? "@_implementationOnly import" : "import"
+    let importSnippet = directive.snippet
     var imports = Set<String>()
     for dependency in dependencies {
       if SwiftProtobufInfo.isBundledProto(file: dependency) {
@@ -86,16 +84,19 @@ extension FileDescriptor {
       if let depModule = namer.mappings.moduleName(forFile: dependency),
          depModule != namer.targetModule {
         // Different module, import it.
-        imports.insert("\(directive) \(depModule)")
+        imports.insert("\(importSnippet) \(depModule)")
       }
     }
 
     // If not re-exporting imports, then there is nothing special needed for
     // `import public` files, as any transitive `import public` directives
     // would have already re-exported the types, so everything this file needs
     // will be covered by the above imports.
-    let exportingImports: [String] =
-      reexportPublicImports ? computeSymbolReExports(namer: namer) : [String]()
+    let exportingImports: [String] = reexportPublicImports
+      ? computeSymbolReExports(
+        namer: namer,
+        useAccessLevelOnImports: directive.isAccessLevel)
+      : [String]()
 
     var result = imports.sorted().joined(separator: "\n")
     if !exportingImports.isEmpty {
@@ -109,7 +110,7 @@ extension FileDescriptor {
   }
 
   // Internal helper to `computeImports(...)`.
-  private func computeSymbolReExports(namer: SwiftProtobufNamer) -> [String] {
+  private func computeSymbolReExports(namer: SwiftProtobufNamer, useAccessLevelOnImports: Bool) -> [String] {
     var result = [String]()
 
     // To handle re-exporting, recursively walk all the `import public` files
@@ -119,6 +120,7 @@ extension FileDescriptor {
     // authored code.
     var toScan = publicDependencies
     var visited = Set<String>()
+    let exportedImportDirective = "@_exported\(useAccessLevelOnImports ? " public" : "") import"
     while let dependency = toScan.popLast() {
       let dependencyName = dependency.name
       if visited.contains(dependencyName) { continue }
@@ -144,16 +146,16 @@ extension FileDescriptor {
       // chained imports.
 
       for m in dependency.messages {
-        result.append("@_exported import struct \(namer.fullName(message: m))")
+        result.append("\(exportedImportDirective) struct \(namer.fullName(message: m))")
       }
       for e in dependency.enums {
-        result.append("@_exported import enum \(namer.fullName(enum: e))")
+        result.append("\(exportedImportDirective) enum \(namer.fullName(enum: e))")
       }
       // There is nothing we can do for the Swift extensions declared on the
       // extended Messages, best we can do is expose the raw extensions
       // themselves.
       for e in dependency.extensions {
-        result.append("@_exported import let \(namer.fullName(extensionField: e))")
+        result.append("\(exportedImportDirective) let \(namer.fullName(extensionField: e))")
       }
     }
     return result

Sources/protoc-gen-swift/Docs.docc/index.md

@@ -188,6 +188,22 @@ exposed via public API, so even if `ImplementationOnlyImports` is set to `true`,
 this will only work if the `Visibility` is set to `internal`. 
 
 
+##### Generation Option: `UseAccessLevelOnImports` - imports preceded by a visibility modifier (`public`, `package`, `internal`)
+
+By default, the code generator does not precede any imports with a visibility modifier.
+You can change this with the `UseAccessLevelOnImports` option:
+
+```
+$ protoc --swift_opt=UseAccessLevelOnImports=[value] --swift_out=. foo/bar/*.proto mumble/*.proto
+```
+
+The possible values for `UseAccessLevelOnImports` are:
+
+* `false` (default): Generates plain import directives without a visibility modifier.
+* `true`: Imports of internal dependencies and any modules defined in the module
+mappings will be preceded by a visibility modifier corresponding to the visibility of the generated types - see `Visibility` option. 
+
+
 ### Building your project
 
 After copying the `.pb.swift` files into your project, you will need

Sources/protoc-gen-swift/FileGenerator.swift

@@ -100,14 +100,13 @@ class FileGenerator {
         if fileDescriptor.isBundledProto {
             p.print("// 'import \(namer.swiftProtobufModuleName)' suppressed, this proto file is meant to be bundled in the runtime.")
         } else {
-            let directive = self.generatorOptions.implementationOnlyImports ? "@_implementationOnly import" : "import"
-            p.print("\(directive) \(namer.swiftProtobufModuleName)")
+            p.print("\(generatorOptions.importDirective.snippet) \(namer.swiftProtobufModuleName)")
         }
 
         let neededImports = fileDescriptor.computeImports(
           namer: namer,
-          reexportPublicImports: self.generatorOptions.visibility != .internal,
-          asImplementationOnly: self.generatorOptions.implementationOnlyImports)
+          directive: generatorOptions.importDirective,
+          reexportPublicImports: generatorOptions.visibility != .internal)
         if !neededImports.isEmpty {
             p.print()
             p.print(neededImports)

Sources/protoc-gen-swift/GeneratorOptions.swift

@@ -30,29 +30,44 @@ class GeneratorOptions {
     }
   }
 
-  enum Visibility {
+  enum Visibility: String {
     case `internal`
     case `public`
     case `package`
 
     init?(flag: String) {
-      switch flag.lowercased() {
-      case "internal":
-        self = .internal
-      case "public":
-        self = .public
-      case "package":
-        self = .package
-      default:
-        return nil
+      self.init(rawValue: flag.lowercased())
+    }
+  }
+
+  enum ImportDirective: Equatable {
+    case accessLevel(Visibility)
+    case plain
+    case implementationOnly
+
+    var isAccessLevel: Bool {
+        switch self {
+        case .accessLevel: true
+        default: false
+        }
+    }
+
+    var snippet: String {
+      switch self {
+      case let .accessLevel(visibility):
+        "\(visibility) import"
+      case .plain:
+        "import"
+      case .implementationOnly:
+        "@_implementationOnly import"
       }
     }
   }
 
   let outputNaming: OutputNaming
   let protoToModuleMappings: ProtoFileToModuleMappings
   let visibility: Visibility
-  let implementationOnlyImports: Bool
+  let importDirective: ImportDirective
   let experimentalStripNonfunctionalCodegen: Bool
 
   /// A string snippet to insert for the visibility
@@ -64,6 +79,7 @@ class GeneratorOptions {
     var visibility: Visibility = .internal
     var swiftProtobufModuleName: String? = nil
     var implementationOnlyImports: Bool = false
+    var useAccessLevelOnImports = false
     var experimentalStripNonfunctionalCodegen: Bool = false
 
     for pair in parameter.parsedPairs {
@@ -102,6 +118,13 @@ class GeneratorOptions {
           throw GenerationError.invalidParameterValue(name: pair.key,
                                                       value: pair.value)
         }
+      case "UseAccessLevelOnImports":
+        if let value = Bool(pair.value) {
+          useAccessLevelOnImports = value
+        } else {
+          throw GenerationError.invalidParameterValue(name: pair.key,
+                                                      value: pair.value)
+        }
       case "experimental_strip_nonfunctional_codegen":
         if pair.value.isEmpty {  // Also support option without any value.
           experimentalStripNonfunctionalCodegen = true
@@ -140,13 +163,22 @@ class GeneratorOptions {
       visibilitySourceSnippet = "package "
     }
 
-    self.implementationOnlyImports = implementationOnlyImports
     self.experimentalStripNonfunctionalCodegen = experimentalStripNonfunctionalCodegen
 
+    self.importDirective = switch (implementationOnlyImports, useAccessLevelOnImports) {
+    case (false, false): .plain
+    case (false, true): .accessLevel(visibility)
+    case (true, false): .implementationOnly
+    case (true, true): throw GenerationError.message(message: """
+      When using access levels on imports the @_implementationOnly option is unnecessary.
+      Disable @_implementationOnly imports.
+      """)
+    }
+
     // ------------------------------------------------------------------------
     // Now do "cross option" validations.
 
-    if self.implementationOnlyImports && self.visibility != .internal {
+    if implementationOnlyImports && self.visibility != .internal {
       throw GenerationError.message(message: """
         Cannot use @_implementationOnly imports when the proto visibility is public or package.
         Either change the visibility to internal, or disable @_implementationOnly imports.

Tests/SwiftProtobufPluginLibraryTests/Test_ProtoFileToModuleMappings.swift

@@ -13,19 +13,6 @@ import SwiftProtobuf
 import SwiftProtobufTestHelpers
 @testable import SwiftProtobufPluginLibrary
 
-// Support equality to simplify testing of getting the correct errors.
-extension ProtoFileToModuleMappings.LoadError: Equatable {
-  public static func ==(lhs: ProtoFileToModuleMappings.LoadError, rhs: ProtoFileToModuleMappings.LoadError) -> Bool {
-    switch (lhs, rhs) {
-    case (.entryMissingModuleName(let l), .entryMissingModuleName(let r)): return l == r
-    case (.entryHasNoProtoPaths(let l), .entryHasNoProtoPaths(let r)): return l == r
-    case (.duplicateProtoPathMapping(let l1, let l2, let l3),
-          .duplicateProtoPathMapping(let r1, let r2, let r3)): return l1 == r1 && l2 == r2 && l3 == r3
-    default: return false
-    }
-  }
-}
-
 // Helpers to make test cases.
 
 fileprivate typealias FileDescriptorProto = Google_Protobuf_FileDescriptorProto