diff --git a/CompileTests/MultiModule/Sources/ImportsAPublicly/imports_a_publicly.pb.swift b/CompileTests/MultiModule/Sources/ImportsAPublicly/imports_a_publicly.pb.swift index 556b2ed19..99f4f3e88 100644 --- a/CompileTests/MultiModule/Sources/ImportsAPublicly/imports_a_publicly.pb.swift +++ b/CompileTests/MultiModule/Sources/ImportsAPublicly/imports_a_publicly.pb.swift @@ -10,7 +10,10 @@ import Foundation import SwiftProtobuf -import ModuleA +// Use of 'import public' causes re-exports: +@_exported import enum ModuleA.E +@_exported import let ModuleA.Extensions_ext_str +@_exported import struct ModuleA.A // If the compiler emits an error on this type, it is because this file // was generated by a version of the `protoc` Swift plug-in that is @@ -62,6 +65,11 @@ extension ImportsAPublicly: SwiftProtobuf.Message, SwiftProtobuf._MessageImpleme 12: .same(proto: "e"), ] + public var isInitialized: Bool { + if let v = self._a, !v.isInitialized {return false} + return true + } + public mutating func decodeMessage(decoder: inout D) throws { while let fieldNumber = try decoder.nextFieldNumber() { // The use of inline closures is to circumvent an issue where the compiler diff --git a/CompileTests/MultiModule/Sources/ImportsImportsAPublicly/imports_imports_a_publicly.pb.swift b/CompileTests/MultiModule/Sources/ImportsImportsAPublicly/imports_imports_a_publicly.pb.swift index 8a345f5d0..7e1ec8739 100644 --- a/CompileTests/MultiModule/Sources/ImportsImportsAPublicly/imports_imports_a_publicly.pb.swift +++ b/CompileTests/MultiModule/Sources/ImportsImportsAPublicly/imports_imports_a_publicly.pb.swift @@ -10,8 +10,11 @@ import Foundation import SwiftProtobuf -import ImportsAPublicly -import ModuleA +// Use of 'import public' causes re-exports: +@_exported import enum ModuleA.E +@_exported import let ModuleA.Extensions_ext_str +@_exported import struct ImportsAPublicly.ImportsAPublicly +@_exported import struct ModuleA.A // If the compiler emits an error on this type, it is because this file // was generated by a version of the `protoc` Swift plug-in that is @@ -63,6 +66,11 @@ extension ImportsImportsAPublicly: SwiftProtobuf.Message, SwiftProtobuf._Message 22: .same(proto: "e"), ] + public var isInitialized: Bool { + if let v = self._a, !v.isInitialized {return false} + return true + } + public mutating func decodeMessage(decoder: inout D) throws { while let fieldNumber = try decoder.nextFieldNumber() { // The use of inline closures is to circumvent an issue where the compiler diff --git a/CompileTests/MultiModule/Sources/ModuleA/a.pb.swift b/CompileTests/MultiModule/Sources/ModuleA/a.pb.swift index e456acc57..165a10475 100644 --- a/CompileTests/MultiModule/Sources/ModuleA/a.pb.swift +++ b/CompileTests/MultiModule/Sources/ModuleA/a.pb.swift @@ -49,7 +49,7 @@ public enum E: SwiftProtobuf.Enum { } -public struct A: Sendable { +public struct A: SwiftProtobuf.ExtensibleMessage, Sendable { // SwiftProtobuf.Message conformance is added in an extension below. See the // `Message` and `Message+*Additions` files in the SwiftProtobuf library for // methods supported on all messages. @@ -67,9 +67,57 @@ public struct A: Sendable { public init() {} + public var _protobuf_extensionFieldValues = SwiftProtobuf.ExtensionFieldValueSet() fileprivate var _e: E? = nil } +// MARK: - Extension support defined in a.proto. + +// MARK: - Extension Properties + +// Swift Extensions on the exteneded Messages to add easy access to the declared +// extension fields. The names are based on the extension field name from the proto +// declaration. To avoid naming collisions, the names are prefixed with the name of +// the scope where the extend directive occurs. + +extension A { + + public var extStr: String { + get {return getExtensionValue(ext: Extensions_ext_str) ?? String()} + set {setExtensionValue(ext: Extensions_ext_str, value: newValue)} + } + /// Returns true if extension `Extensions_ext_str` + /// has been explicitly set. + public var hasExtStr: Bool { + return hasExtensionValue(ext: Extensions_ext_str) + } + /// Clears the value of extension `Extensions_ext_str`. + /// Subsequent reads from it will return its default value. + public mutating func clearExtStr() { + clearExtensionValue(ext: Extensions_ext_str) + } + +} + +// MARK: - File's ExtensionMap: A_Extensions + +/// A `SwiftProtobuf.SimpleExtensionMap` that includes all of the extensions defined by +/// this .proto file. It can be used any place an `SwiftProtobuf.ExtensionMap` is needed +/// in parsing, or it can be combined with other `SwiftProtobuf.SimpleExtensionMap`s to create +/// a larger `SwiftProtobuf.SimpleExtensionMap`. +public let A_Extensions: SwiftProtobuf.SimpleExtensionMap = [ + Extensions_ext_str +] + +// Extension Objects - The only reason these might be needed is when manually +// constructing a `SimpleExtensionMap`, otherwise, use the above _Extension Properties_ +// accessors for the extension fields on the messages directly. + +public let Extensions_ext_str = SwiftProtobuf.MessageExtension, A>( + _protobuf_fieldNumber: 100, + fieldName: "ext_str" +) + // MARK: - Code below here is support for the SwiftProtobuf runtime. extension E: SwiftProtobuf._ProtoNameProviding { @@ -86,6 +134,11 @@ extension A: SwiftProtobuf.Message, SwiftProtobuf._MessageImplementationBase, Sw 1: .same(proto: "e"), ] + public var isInitialized: Bool { + if !_protobuf_extensionFieldValues.isInitialized {return false} + return true + } + public mutating func decodeMessage(decoder: inout D) throws { while let fieldNumber = try decoder.nextFieldNumber() { // The use of inline closures is to circumvent an issue where the compiler @@ -93,6 +146,8 @@ extension A: SwiftProtobuf.Message, SwiftProtobuf._MessageImplementationBase, Sw // enabled. https://github.com/apple/swift-protobuf/issues/1034 switch fieldNumber { case 1: try { try decoder.decodeSingularEnumField(value: &self._e) }() + case 100..<1001: + try { try decoder.decodeExtensionField(values: &_protobuf_extensionFieldValues, messageType: A.self, fieldNumber: fieldNumber) }() default: break } } @@ -106,12 +161,14 @@ extension A: SwiftProtobuf.Message, SwiftProtobuf._MessageImplementationBase, Sw try { if let v = self._e { try visitor.visitSingularEnumField(value: v, fieldNumber: 1) } }() + try visitor.visitExtensionFields(fields: _protobuf_extensionFieldValues, start: 100, end: 1001) try unknownFields.traverse(visitor: &visitor) } public static func ==(lhs: A, rhs: A) -> Bool { if lhs._e != rhs._e {return false} if lhs.unknownFields != rhs.unknownFields {return false} + if lhs._protobuf_extensionFieldValues != rhs._protobuf_extensionFieldValues {return false} return true } } diff --git a/CompileTests/MultiModule/Tests/Test1/test1.swift b/CompileTests/MultiModule/Tests/Test1/test1.swift index f186277bd..474a7c09c 100644 --- a/CompileTests/MultiModule/Tests/Test1/test1.swift +++ b/CompileTests/MultiModule/Tests/Test1/test1.swift @@ -1,5 +1,5 @@ import ImportsAPublicly -import ModuleA // Needed because `import public` doesn't help Swift +// Don't need to import ModuleA because of the file being a `import public` import XCTest diff --git a/CompileTests/MultiModule/Tests/Test1/uses_a_transitively.pb.swift b/CompileTests/MultiModule/Tests/Test1/uses_a_transitively.pb.swift index 3bb4d6701..e88624091 100644 --- a/CompileTests/MultiModule/Tests/Test1/uses_a_transitively.pb.swift +++ b/CompileTests/MultiModule/Tests/Test1/uses_a_transitively.pb.swift @@ -11,7 +11,6 @@ import Foundation import SwiftProtobuf import ImportsAPublicly -import ModuleA // If the compiler emits an error on this type, it is because this file // was generated by a version of the `protoc` Swift plug-in that is @@ -63,6 +62,11 @@ extension UsesATransitively: SwiftProtobuf.Message, SwiftProtobuf._MessageImplem 102: .same(proto: "e"), ] + public var isInitialized: Bool { + if let v = self._a, !v.isInitialized {return false} + return true + } + public mutating func decodeMessage(decoder: inout D) throws { while let fieldNumber = try decoder.nextFieldNumber() { // The use of inline closures is to circumvent an issue where the compiler diff --git a/CompileTests/MultiModule/Tests/Test2/test2.swift b/CompileTests/MultiModule/Tests/Test2/test2.swift index e20ebd130..45d62fbb5 100644 --- a/CompileTests/MultiModule/Tests/Test2/test2.swift +++ b/CompileTests/MultiModule/Tests/Test2/test2.swift @@ -1,5 +1,5 @@ import ImportsImportsAPublicly -import ModuleA // Needed because `import public` doesn't help Swift +// Don't need to import ModuleA because of the file being a `import public` import XCTest diff --git a/CompileTests/MultiModule/Tests/Test2/uses_a_transitively2.pb.swift b/CompileTests/MultiModule/Tests/Test2/uses_a_transitively2.pb.swift index 7dd0c210e..6c056cb8a 100644 --- a/CompileTests/MultiModule/Tests/Test2/uses_a_transitively2.pb.swift +++ b/CompileTests/MultiModule/Tests/Test2/uses_a_transitively2.pb.swift @@ -10,9 +10,7 @@ import Foundation import SwiftProtobuf -import ImportsAPublicly import ImportsImportsAPublicly -import ModuleA // If the compiler emits an error on this type, it is because this file // was generated by a version of the `protoc` Swift plug-in that is @@ -64,6 +62,11 @@ extension UsesATransitively2: SwiftProtobuf.Message, SwiftProtobuf._MessageImple 122: .same(proto: "e"), ] + public var isInitialized: Bool { + if let v = self._a, !v.isInitialized {return false} + return true + } + public mutating func decodeMessage(decoder: inout D) throws { while let fieldNumber = try decoder.nextFieldNumber() { // The use of inline closures is to circumvent an issue where the compiler diff --git a/Protos/CompileTests/MultiModule/Sources/ModuleA/a.proto b/Protos/CompileTests/MultiModule/Sources/ModuleA/a.proto index 7a2ef4496..75c1506a7 100644 --- a/Protos/CompileTests/MultiModule/Sources/ModuleA/a.proto +++ b/Protos/CompileTests/MultiModule/Sources/ModuleA/a.proto @@ -6,4 +6,9 @@ enum E { message A { optional E e = 1; + extensions 100 to 1000; +} + +extend A { + optional string ext_str = 100; } diff --git a/Reference/CompileTests/MultiModule/Sources/ImportsAPublicly/imports_a_publicly.pb.swift b/Reference/CompileTests/MultiModule/Sources/ImportsAPublicly/imports_a_publicly.pb.swift index 556b2ed19..99f4f3e88 100644 --- a/Reference/CompileTests/MultiModule/Sources/ImportsAPublicly/imports_a_publicly.pb.swift +++ b/Reference/CompileTests/MultiModule/Sources/ImportsAPublicly/imports_a_publicly.pb.swift @@ -10,7 +10,10 @@ import Foundation import SwiftProtobuf -import ModuleA +// Use of 'import public' causes re-exports: +@_exported import enum ModuleA.E +@_exported import let ModuleA.Extensions_ext_str +@_exported import struct ModuleA.A // If the compiler emits an error on this type, it is because this file // was generated by a version of the `protoc` Swift plug-in that is @@ -62,6 +65,11 @@ extension ImportsAPublicly: SwiftProtobuf.Message, SwiftProtobuf._MessageImpleme 12: .same(proto: "e"), ] + public var isInitialized: Bool { + if let v = self._a, !v.isInitialized {return false} + return true + } + public mutating func decodeMessage(decoder: inout D) throws { while let fieldNumber = try decoder.nextFieldNumber() { // The use of inline closures is to circumvent an issue where the compiler diff --git a/Reference/CompileTests/MultiModule/Sources/ImportsImportsAPublicly/imports_imports_a_publicly.pb.swift b/Reference/CompileTests/MultiModule/Sources/ImportsImportsAPublicly/imports_imports_a_publicly.pb.swift index 8a345f5d0..7e1ec8739 100644 --- a/Reference/CompileTests/MultiModule/Sources/ImportsImportsAPublicly/imports_imports_a_publicly.pb.swift +++ b/Reference/CompileTests/MultiModule/Sources/ImportsImportsAPublicly/imports_imports_a_publicly.pb.swift @@ -10,8 +10,11 @@ import Foundation import SwiftProtobuf -import ImportsAPublicly -import ModuleA +// Use of 'import public' causes re-exports: +@_exported import enum ModuleA.E +@_exported import let ModuleA.Extensions_ext_str +@_exported import struct ImportsAPublicly.ImportsAPublicly +@_exported import struct ModuleA.A // If the compiler emits an error on this type, it is because this file // was generated by a version of the `protoc` Swift plug-in that is @@ -63,6 +66,11 @@ extension ImportsImportsAPublicly: SwiftProtobuf.Message, SwiftProtobuf._Message 22: .same(proto: "e"), ] + public var isInitialized: Bool { + if let v = self._a, !v.isInitialized {return false} + return true + } + public mutating func decodeMessage(decoder: inout D) throws { while let fieldNumber = try decoder.nextFieldNumber() { // The use of inline closures is to circumvent an issue where the compiler diff --git a/Reference/CompileTests/MultiModule/Sources/ModuleA/a.pb.swift b/Reference/CompileTests/MultiModule/Sources/ModuleA/a.pb.swift index e456acc57..165a10475 100644 --- a/Reference/CompileTests/MultiModule/Sources/ModuleA/a.pb.swift +++ b/Reference/CompileTests/MultiModule/Sources/ModuleA/a.pb.swift @@ -49,7 +49,7 @@ public enum E: SwiftProtobuf.Enum { } -public struct A: Sendable { +public struct A: SwiftProtobuf.ExtensibleMessage, Sendable { // SwiftProtobuf.Message conformance is added in an extension below. See the // `Message` and `Message+*Additions` files in the SwiftProtobuf library for // methods supported on all messages. @@ -67,9 +67,57 @@ public struct A: Sendable { public init() {} + public var _protobuf_extensionFieldValues = SwiftProtobuf.ExtensionFieldValueSet() fileprivate var _e: E? = nil } +// MARK: - Extension support defined in a.proto. + +// MARK: - Extension Properties + +// Swift Extensions on the exteneded Messages to add easy access to the declared +// extension fields. The names are based on the extension field name from the proto +// declaration. To avoid naming collisions, the names are prefixed with the name of +// the scope where the extend directive occurs. + +extension A { + + public var extStr: String { + get {return getExtensionValue(ext: Extensions_ext_str) ?? String()} + set {setExtensionValue(ext: Extensions_ext_str, value: newValue)} + } + /// Returns true if extension `Extensions_ext_str` + /// has been explicitly set. + public var hasExtStr: Bool { + return hasExtensionValue(ext: Extensions_ext_str) + } + /// Clears the value of extension `Extensions_ext_str`. + /// Subsequent reads from it will return its default value. + public mutating func clearExtStr() { + clearExtensionValue(ext: Extensions_ext_str) + } + +} + +// MARK: - File's ExtensionMap: A_Extensions + +/// A `SwiftProtobuf.SimpleExtensionMap` that includes all of the extensions defined by +/// this .proto file. It can be used any place an `SwiftProtobuf.ExtensionMap` is needed +/// in parsing, or it can be combined with other `SwiftProtobuf.SimpleExtensionMap`s to create +/// a larger `SwiftProtobuf.SimpleExtensionMap`. +public let A_Extensions: SwiftProtobuf.SimpleExtensionMap = [ + Extensions_ext_str +] + +// Extension Objects - The only reason these might be needed is when manually +// constructing a `SimpleExtensionMap`, otherwise, use the above _Extension Properties_ +// accessors for the extension fields on the messages directly. + +public let Extensions_ext_str = SwiftProtobuf.MessageExtension, A>( + _protobuf_fieldNumber: 100, + fieldName: "ext_str" +) + // MARK: - Code below here is support for the SwiftProtobuf runtime. extension E: SwiftProtobuf._ProtoNameProviding { @@ -86,6 +134,11 @@ extension A: SwiftProtobuf.Message, SwiftProtobuf._MessageImplementationBase, Sw 1: .same(proto: "e"), ] + public var isInitialized: Bool { + if !_protobuf_extensionFieldValues.isInitialized {return false} + return true + } + public mutating func decodeMessage(decoder: inout D) throws { while let fieldNumber = try decoder.nextFieldNumber() { // The use of inline closures is to circumvent an issue where the compiler @@ -93,6 +146,8 @@ extension A: SwiftProtobuf.Message, SwiftProtobuf._MessageImplementationBase, Sw // enabled. https://github.com/apple/swift-protobuf/issues/1034 switch fieldNumber { case 1: try { try decoder.decodeSingularEnumField(value: &self._e) }() + case 100..<1001: + try { try decoder.decodeExtensionField(values: &_protobuf_extensionFieldValues, messageType: A.self, fieldNumber: fieldNumber) }() default: break } } @@ -106,12 +161,14 @@ extension A: SwiftProtobuf.Message, SwiftProtobuf._MessageImplementationBase, Sw try { if let v = self._e { try visitor.visitSingularEnumField(value: v, fieldNumber: 1) } }() + try visitor.visitExtensionFields(fields: _protobuf_extensionFieldValues, start: 100, end: 1001) try unknownFields.traverse(visitor: &visitor) } public static func ==(lhs: A, rhs: A) -> Bool { if lhs._e != rhs._e {return false} if lhs.unknownFields != rhs.unknownFields {return false} + if lhs._protobuf_extensionFieldValues != rhs._protobuf_extensionFieldValues {return false} return true } } diff --git a/Reference/CompileTests/MultiModule/Tests/Test1/uses_a_transitively.pb.swift b/Reference/CompileTests/MultiModule/Tests/Test1/uses_a_transitively.pb.swift index 3bb4d6701..e88624091 100644 --- a/Reference/CompileTests/MultiModule/Tests/Test1/uses_a_transitively.pb.swift +++ b/Reference/CompileTests/MultiModule/Tests/Test1/uses_a_transitively.pb.swift @@ -11,7 +11,6 @@ import Foundation import SwiftProtobuf import ImportsAPublicly -import ModuleA // If the compiler emits an error on this type, it is because this file // was generated by a version of the `protoc` Swift plug-in that is @@ -63,6 +62,11 @@ extension UsesATransitively: SwiftProtobuf.Message, SwiftProtobuf._MessageImplem 102: .same(proto: "e"), ] + public var isInitialized: Bool { + if let v = self._a, !v.isInitialized {return false} + return true + } + public mutating func decodeMessage(decoder: inout D) throws { while let fieldNumber = try decoder.nextFieldNumber() { // The use of inline closures is to circumvent an issue where the compiler diff --git a/Reference/CompileTests/MultiModule/Tests/Test2/uses_a_transitively2.pb.swift b/Reference/CompileTests/MultiModule/Tests/Test2/uses_a_transitively2.pb.swift index 7dd0c210e..6c056cb8a 100644 --- a/Reference/CompileTests/MultiModule/Tests/Test2/uses_a_transitively2.pb.swift +++ b/Reference/CompileTests/MultiModule/Tests/Test2/uses_a_transitively2.pb.swift @@ -10,9 +10,7 @@ import Foundation import SwiftProtobuf -import ImportsAPublicly import ImportsImportsAPublicly -import ModuleA // If the compiler emits an error on this type, it is because this file // was generated by a version of the `protoc` Swift plug-in that is @@ -64,6 +62,11 @@ extension UsesATransitively2: SwiftProtobuf.Message, SwiftProtobuf._MessageImple 122: .same(proto: "e"), ] + public var isInitialized: Bool { + if let v = self._a, !v.isInitialized {return false} + return true + } + public mutating func decodeMessage(decoder: inout D) throws { while let fieldNumber = try decoder.nextFieldNumber() { // The use of inline closures is to circumvent an issue where the compiler diff --git a/Sources/SwiftProtobufPluginLibrary/ProtoFileToModuleMappings.swift b/Sources/SwiftProtobufPluginLibrary/ProtoFileToModuleMappings.swift index df21faf33..5b8549521 100644 --- a/Sources/SwiftProtobufPluginLibrary/ProtoFileToModuleMappings.swift +++ b/Sources/SwiftProtobufPluginLibrary/ProtoFileToModuleMappings.swift @@ -132,6 +132,14 @@ public struct ProtoFileToModuleMappings { } } + // NOTE: This api is only used by gRPC (or things like it), with + // `import public` now re-exporting things, this likely can go away or just + // be reduced just the above loop, without the need for special casing the + // `import public` cases. It will come down to what should expectations + // be for protobuf messages, enums, and extensions with repsect to something + // that generates on top if it. i.e. - should they re-export things or + // should only the generated proto code do it? + // Protocol Buffers has the concept of "public imports", these are imports // into a file that expose everything from within the file to the new // context. From the docs - diff --git a/Sources/SwiftProtobufPluginLibrary/SwiftProtobufNamer.swift b/Sources/SwiftProtobufPluginLibrary/SwiftProtobufNamer.swift index 83942d7b2..0dab8324c 100644 --- a/Sources/SwiftProtobufPluginLibrary/SwiftProtobufNamer.swift +++ b/Sources/SwiftProtobufPluginLibrary/SwiftProtobufNamer.swift @@ -17,8 +17,8 @@ import Foundation public final class SwiftProtobufNamer { var filePrefixCache = [String:String]() var enumValueRelativeNameCache = [String:String]() - var mappings: ProtoFileToModuleMappings - var targetModule: String + public let mappings: ProtoFileToModuleMappings + public let targetModule: String public var swiftProtobufModuleName: String { return mappings.swiftProtobufModuleName } diff --git a/Sources/SwiftProtobufTestHelpers/Descriptor+TestHelpers.swift b/Sources/SwiftProtobufTestHelpers/Descriptor+TestHelpers.swift index 263e988db..817b853d4 100644 --- a/Sources/SwiftProtobufTestHelpers/Descriptor+TestHelpers.swift +++ b/Sources/SwiftProtobufTestHelpers/Descriptor+TestHelpers.swift @@ -11,12 +11,13 @@ import SwiftProtobuf public extension Google_Protobuf_FileDescriptorProto { - init(name: String, dependencies: [String] = [], publicDependencies: [Int32] = []) { + init(name: String, dependencies: [String] = [], publicDependencies: [Int32] = [], package: String = "") { for idx in publicDependencies { precondition(Int(idx) <= dependencies.count) } self.init() self.name = name - dependency = dependencies - publicDependency = publicDependencies + self.dependency = dependencies + self.publicDependency = publicDependencies + self.package = package } init(textFormatStrings: [String]) throws { let s = textFormatStrings.joined(separator: "\n") + "\n" @@ -27,7 +28,7 @@ public extension Google_Protobuf_FileDescriptorProto { public extension Google_Protobuf_FileDescriptorSet { init(files: [Google_Protobuf_FileDescriptorProto]) { self.init() - file = files + self.file = files } init(file: Google_Protobuf_FileDescriptorProto) { self.init() diff --git a/Sources/protoc-gen-swift/Descriptor+Extensions.swift b/Sources/protoc-gen-swift/Descriptor+Extensions.swift index 0cb60effe..0aaa00e67 100644 --- a/Sources/protoc-gen-swift/Descriptor+Extensions.swift +++ b/Sources/protoc-gen-swift/Descriptor+Extensions.swift @@ -14,6 +14,150 @@ extension FileDescriptor { var isBundledProto: Bool { return SwiftProtobufInfo.isBundledProto(file: self) } + + // Returns a string of any import lines for the give file based on the file's + // imports. The string may include multiple lines. + // + // Protocol Buffers has the concept of "public imports", these are imports + // into a file that expose everything from within the file to the new + // context. From the docs - + // https://protobuf.dev/programming-guides/proto/#importing + // `import public` dependencies can be transitively relied upon by anyone + // importing the proto containing the import public statement. + // To properly expose the types for use, it means in each file, the public + // imports from the dependencies (recursively) have to be hoisted and + // reexported. This way someone importing a given module still sees the type + // even when moved. + // + // NOTE: There is a weakness for Swift with protobuf extensions. To make + // the protobuf extensions easier to use, a Swift extension is declared with + // field exposed as a property on the exteneded message. There is no way + // to reexport the Swift `extension` and/or added properties. But the raw + // types are re-exported to minimize the breaking of code if a type is moved + // between files/modules. + // + // `reexportPublicImports` will cause the `import public` types to be + // reexported to avoid breaking downstream code using a type that might have + // moved between .proto files. + // + // `asImplementationOnly` will cause all of the import directives to be + // marked as `@_implementationOnly`. It will also cause all of the `file`'s + // `publicDependencies` to instead be recursively pulled up as direct imports + // to ensure the generate file compiles, and no `import public` files are + // re-exported. + // + // 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( + namer: SwiftProtobufNamer, + reexportPublicImports: Bool, + asImplementationOnly: 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) + + guard namer.mappings.hasMappings else { + // No module mappings? Everything must be the same module, so no Swift + // imports will be needed. + return "" + } + + if dependencies.isEmpty { + // No proto dependencies (imports), then no Swift imports will be needed. + return "" + } + + let directive = asImplementationOnly ? "@_implementationOnly import" : "import" + var imports = Set() + for dependency in dependencies { + if SwiftProtobufInfo.isBundledProto(file: dependency) { + continue // No import needed for the runtime, that's always added. + } + if reexportPublicImports && publicDependencies.contains(where: { $0 === dependency }) { + // When re-exporting, the `import public` types will be imported + // instead of importing the module. + continue + } + if let depModule = namer.mappings.moduleName(forFile: dependency), + depModule != namer.targetModule { + // Different module, import it. + imports.insert("\(directive) \(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]() + + var result = imports.sorted().joined(separator: "\n") + if !exportingImports.isEmpty { + if !result.isEmpty { + result.append("\n") + } + result.append("// Use of 'import public' causes re-exports:\n") + result.append(exportingImports.sorted().joined(separator: "\n")) + } + return result + } + + // Internal helper to `computeImports(...)`. + private func computeSymbolReExports(namer: SwiftProtobufNamer) -> [String] { + var result = [String]() + + // To handle re-exporting, recuively walk all the `import public` files + // and make this module do a Swift exporting import of the specific + // symbols. That will keep any type that gets moved between .proto files + // still exposed from the same modules so as not to break developer + // authored code. + var toScan = publicDependencies + var visited = Set() + while let dependency = toScan.popLast() { + let dependencyName = dependency.name + if visited.contains(dependencyName) { continue } + visited.insert(dependencyName) + + if SwiftProtobufInfo.isBundledProto(file: dependency) { + continue // Bundlined file, nothing to do. + } + guard let depModule = namer.mappings.moduleName(forFile: dependency) else { + continue // No mapping, assume same module, nothing to do. + } + if depModule == namer.targetModule { + // Same module, nothing to do (that generated file will do any re-exports). + continue + } + + toScan.append(contentsOf: dependency.publicDependencies) + + // NOTE: This re-exports/imports from the module that defines the type. + // If Xcode/SwiftPM ever were to do some sort of "layering checks" to + // ensure there is a direct dependency on the thing being imported, this + // could be updated do the re-export/import from the middle step in + // chained imports. + + for m in dependency.messages { + result.append("@_exported import struct \(namer.fullName(message: m))") + } + for e in dependency.enums { + result.append("@_exported import 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 extenions + // themselves. + for e in dependency.extensions { + result.append("@_exported import let \(namer.fullName(extensionField: e))") + } + } + return result + } } extension Descriptor { diff --git a/Sources/protoc-gen-swift/FileGenerator.swift b/Sources/protoc-gen-swift/FileGenerator.swift index e4055805a..18e46bbe3 100644 --- a/Sources/protoc-gen-swift/FileGenerator.swift +++ b/Sources/protoc-gen-swift/FileGenerator.swift @@ -90,25 +90,20 @@ class FileGenerator { p.print("\(comments)import Foundation") - // Import all other imports as @_implementationOnly if the option is - // set, to avoid exposing internal types to users. - let visibilityAnnotation: String - if self.generatorOptions.implementationOnlyImports { - precondition(self.generatorOptions.visibility == .internal) - visibilityAnnotation = "@_implementationOnly " - } else { - visibilityAnnotation = "" - } if fileDescriptor.isBundledProto { p.print("// 'import \(namer.swiftProtobufModuleName)' suppressed, this proto file is meant to be bundled in the runtime.") } else { - p.print("\(visibilityAnnotation)import \(namer.swiftProtobufModuleName)") + let directive = self.generatorOptions.implementationOnlyImports ? "@_implementationOnly import" : "import" + p.print("\(directive) \(namer.swiftProtobufModuleName)") } - if let neededImports = generatorOptions.protoToModuleMappings.neededModules(forFile: fileDescriptor) { + + let neededImports = fileDescriptor.computeImports( + namer: namer, + reexportPublicImports: self.generatorOptions.visibility != .internal, + asImplementationOnly: self.generatorOptions.implementationOnlyImports) + if !neededImports.isEmpty { p.print() - for i in neededImports { - p.print("\(visibilityAnnotation)import \(i)") - } + p.print(neededImports) } p.print() diff --git a/Tests/protoc-gen-swiftTests/Test_DescriptorExtensions.swift b/Tests/protoc-gen-swiftTests/Test_DescriptorExtensions.swift index 712070cb1..1c2a342eb 100644 --- a/Tests/protoc-gen-swiftTests/Test_DescriptorExtensions.swift +++ b/Tests/protoc-gen-swiftTests/Test_DescriptorExtensions.swift @@ -13,6 +13,8 @@ import SwiftProtobuf import SwiftProtobufPluginLibrary @testable import protoc_gen_swift +fileprivate typealias FileDescriptorProto = Google_Protobuf_FileDescriptorProto + final class Test_DescriptorExtensions: XCTestCase { func testExtensionRanges() throws { @@ -194,4 +196,271 @@ final class Test_DescriptorExtensions: XCTestCase { XCTAssertIdentical(aliasInfo.original(of: e.values[5]), e.values[0]) } + func test_File_computeImports_noImportPublic() { + let configText = """ + mapping { module_name: "foo", proto_file_path: "file" } + mapping { module_name: "bar", proto_file_path: "dir1/file" } + mapping { module_name: "baz", proto_file_path: ["dir2/file","file4"] } + mapping { module_name: "foo", proto_file_path: "file5" } + """ + + let config = try! SwiftProtobuf_GenSwift_ModuleMappings(textFormatString: configText) + let mapper = try! ProtoFileToModuleMappings(moduleMappingsProto: config) + + let fileProtos = [ + FileDescriptorProto(name: "file"), + FileDescriptorProto(name: "google/protobuf/any.proto", package: "google.protobuf"), + FileDescriptorProto(name: "dir1/file", dependencies: ["file"]), + FileDescriptorProto(name: "dir2/file", dependencies: ["google/protobuf/any.proto"]), + FileDescriptorProto(name: "file4", dependencies: ["dir2/file", "dir1/file", "file"]), + FileDescriptorProto(name: "file5", dependencies: ["file"]), + ] + let descSet = DescriptorSet(protos: fileProtos) + + // ( filename, imports, implOnly imports ) + let tests: [(String, String, String)] = [ + ( "file", "", "" ), + ( "dir1/file", "import foo", "@_implementationOnly import foo" ), + ( "dir2/file", "", "" ), + ( "file4", "import bar\nimport foo", "@_implementationOnly import bar\n@_implementationOnly import foo" ), + ( "file5", "", "" ), + ] + + for (name, expected, expectedImplOnly) in tests { + let fileDesc = descSet.files.filter{ $0.name == name }.first! + do { // reexportPublicImports: false, asImplementationOnly: false + let namer = + SwiftProtobufNamer(currentFile: fileDesc, + protoFileToModuleMappings: mapper) + let result = fileDesc.computeImports(namer: namer, reexportPublicImports: false, asImplementationOnly: false) + XCTAssertEqual(result, expected, "Looking for \(name)") + } + do { // reexportPublicImports: true, asImplementationOnly: false - No `import publc`, same as previous + let namer = + SwiftProtobufNamer(currentFile: fileDesc, + protoFileToModuleMappings: mapper) + let result = fileDesc.computeImports(namer: namer, reexportPublicImports: true, asImplementationOnly: false) + XCTAssertEqual(result, expected, "Looking for \(name)") + } + do { // reexportPublicImports: false, asImplementationOnly: true + let namer = + SwiftProtobufNamer(currentFile: fileDesc, + protoFileToModuleMappings: mapper) + let result = fileDesc.computeImports(namer: namer, reexportPublicImports: false, asImplementationOnly: true) + XCTAssertEqual(result, expectedImplOnly, "Looking for \(name)") + } + } + } + + func test_File_computeImports_PublicImports() { + // See the notes on computeImports(namer:reexportPublicImports:asImplementationOnly:) + // about how public import complicate things. + + // Given: + // + // + File: a.proto + // message A {} + // + // enum E { + // E_UNSET = 0; + // E_A = 1; + // E_B = 2; + // } + // + // + File: imports_a_publicly.proto + // import public "a.proto"; + // + // message ImportsAPublicly { + // optional A a = 1; + // optional E e = 2; + // } + // + // + File: imports_imports_a_publicly.proto + // import public "imports_a_publicly.proto"; + // + // message ImportsImportsAPublicly { + // optional A a = 1; + // } + // + // + File: uses_a_transitively.proto + // import "imports_a_publicly.proto"; + // + // message UsesATransitively { + // optional A a = 1; + // } + // + // + File: uses_a_transitively2.proto + // import "imports_imports_a_publicly.proto"; + // + // message UsesATransitively2 { + // optional A a = 1; + // } + // + // With a mapping file of: + // + // mapping { + // module_name: "A" + // proto_file_path: "a.proto" + // } + // mapping { + // module_name: "ImportsAPublicly" + // proto_file_path: "imports_a_publicly.proto" + // } + // mapping { + // module_name: "ImportsImportsAPublicly" + // proto_file_path: "imports_imports_a_publicly.proto" + // } + + let configText = """ + mapping { module_name: "A", proto_file_path: "a.proto" } + mapping { module_name: "ImportsAPublicly", proto_file_path: "imports_a_publicly.proto" } + mapping { module_name: "ImportsImportsAPublicly", proto_file_path: "imports_imports_a_publicly.proto" } + """ + + let config = try! SwiftProtobuf_GenSwift_ModuleMappings(textFormatString: configText) + let mapper = try! ProtoFileToModuleMappings(moduleMappingsProto: config) + + let fileProtos = [ + try! FileDescriptorProto(textFormatString: """ + name: "a.proto" + message_type { + name: "A" + } + enum_type { + name: "E" + value { + name: "E_UNSET" + number: 0 + } + value { + name: "E_A" + number: 1 + } + value { + name: "E_B" + number: 2 + } + } + """), + try! FileDescriptorProto(textFormatString: """ + name: "imports_a_publicly.proto" + dependency: "a.proto" + message_type { + name: "ImportsAPublicly" + field { + name: "a" + number: 1 + label: LABEL_OPTIONAL + type: TYPE_MESSAGE + type_name: ".A" + json_name: "a" + } + field { + name: "e" + number: 2 + label: LABEL_OPTIONAL + type: TYPE_ENUM + type_name: ".E" + json_name: "e" + } + } + public_dependency: 0 + """), + try! FileDescriptorProto(textFormatString: """ + name: "imports_imports_a_publicly.proto" + dependency: "imports_a_publicly.proto" + message_type { + name: "ImportsImportsAPublicly" + field { + name: "a" + number: 1 + label: LABEL_OPTIONAL + type: TYPE_MESSAGE + type_name: ".A" + json_name: "a" + } + } + public_dependency: 0 + """), + try! FileDescriptorProto(textFormatString: """ + name: "uses_a_transitively.proto" + dependency: "imports_a_publicly.proto" + message_type { + name: "UsesATransitively" + field { + name: "a" + number: 1 + label: LABEL_OPTIONAL + type: TYPE_MESSAGE + type_name: ".A" + json_name: "a" + } + } + """), + try! FileDescriptorProto(textFormatString: """ + name: "uses_a_transitively2.proto" + dependency: "imports_imports_a_publicly.proto" + message_type { + name: "UsesATransitively2" + field { + name: "a" + number: 1 + label: LABEL_OPTIONAL + type: TYPE_MESSAGE + type_name: ".A" + json_name: "a" + } + } + """), + ] + let descSet = DescriptorSet(protos: fileProtos) + + // ( filename, imports, reExportPublicImports imports, implOnly imports ) + let tests: [(String, String, String, String)] = [ + ( "a.proto", + "", "", "" ), + ( "imports_a_publicly.proto", + "import A", + "// Use of 'import public' causes re-exports:\n@_exported import enum A.E\n@_exported import struct A.A", + "@_implementationOnly import A" ), + ( "imports_imports_a_publicly.proto", + "import ImportsAPublicly", + "// Use of 'import public' causes re-exports:\n@_exported import enum A.E\n@_exported import struct A.A\n@_exported import struct ImportsAPublicly.ImportsAPublicly", + "@_implementationOnly import ImportsAPublicly" ), + ( "uses_a_transitively.proto", + "import ImportsAPublicly", // this reexports A, so we don't directly pull in A. + "import ImportsAPublicly", // just a plain `import`, nothing to re-export. + "@_implementationOnly import ImportsAPublicly" ), + ( "uses_a_transitively2.proto", + "import ImportsImportsAPublicly", // this chain reexports A, so we don't directly pull in A. + "import ImportsImportsAPublicly", // just a plain `import`, nothing to re-export. + "@_implementationOnly import ImportsImportsAPublicly" ), + ] + + for (name, expected, expectedReExport, expectedImplOnly) in tests { + let fileDesc = descSet.files.filter{ $0.name == name }.first! + do { // reexportPublicImports: false, asImplementationOnly: false + let namer = + SwiftProtobufNamer(currentFile: fileDesc, + protoFileToModuleMappings: mapper) + let result = fileDesc.computeImports(namer: namer, reexportPublicImports: false, asImplementationOnly: false) + XCTAssertEqual(result, expected, "Looking for \(name)") + } + do { // reexportPublicImports: true, asImplementationOnly: false + let namer = + SwiftProtobufNamer(currentFile: fileDesc, + protoFileToModuleMappings: mapper) + let result = fileDesc.computeImports(namer: namer, reexportPublicImports: true, asImplementationOnly: false) + XCTAssertEqual(result, expectedReExport, "Looking for \(name)") + } + do { // reexportPublicImports: false, asImplementationOnly: true + let namer = + SwiftProtobufNamer(currentFile: fileDesc, + protoFileToModuleMappings: mapper) + let result = fileDesc.computeImports(namer: namer, reexportPublicImports: false, asImplementationOnly: true) + XCTAssertEqual(result, expectedImplOnly, "Looking for \(name)") + } + } + } + }