From bcb751c0c37cdb8587b8bbcfff26f27971153538 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Wed, 26 Apr 2017 16:12:00 -0400 Subject: [PATCH 01/16] FileDescriptor improvements and NamingUtils. - Expose most of the things interesting fields off a FileDesciptorProto. - Add NamingUtils to hold helpers. - Add a function to calculate the Swift prefix for types given a file's metadata. - Add a unittest for the type prefix function. --- Sources/PluginLibrary/Descriptor.swift | 43 ++++++++++++++-- Sources/PluginLibrary/NamingUtils.swift | 51 +++++++++++++++++++ .../PluginLibraryTests/Test_NamingUtils.swift | 51 +++++++++++++++++++ 3 files changed, 141 insertions(+), 4 deletions(-) create mode 100644 Sources/PluginLibrary/NamingUtils.swift create mode 100644 Tests/PluginLibraryTests/Test_NamingUtils.swift diff --git a/Sources/PluginLibrary/Descriptor.swift b/Sources/PluginLibrary/Descriptor.swift index 236aa061f..806e65151 100644 --- a/Sources/PluginLibrary/Descriptor.swift +++ b/Sources/PluginLibrary/Descriptor.swift @@ -56,24 +56,59 @@ public final class DescriptorSet { } public final class FileDescriptor { + public enum Syntax: String { + case proto2 + case proto3 + + public init?(rawValue: String) { + switch rawValue { + case "proto2", "": + self = .proto2 + case "proto3": + self = .proto3 + default: + return nil + } + } + } + public let proto: Google_Protobuf_FileDescriptorProto - public var name: String { return self.proto.name } + public var name: String { return proto.name } + public var package: String { return proto.package } + + public let syntax: Syntax + + public var dependencies: [String] { return proto.dependency } + public var publicDependencies: [String] { return proto.publicDependency.map { dependencies[Int($0)] } } + public var weakDependencies: [String] { return proto.weakDependency.map { dependencies[Int($0)] } } public let enums: [EnumDescriptor] public let messages: [Descriptor] public let extensions: [FieldDescriptor] public let services: [ServiceDescriptor] + public var fileOptions: Google_Protobuf_FileOptions { return proto.options } + public var isDeprecated: Bool { return proto.options.deprecated } + + /// This will be the Swift prefix file option or the prefix to use built + /// out of the proto package. + public var swiftTypePrefix: String + fileprivate init(proto: Google_Protobuf_FileDescriptorProto, registry: Registry) { self.proto = proto + self.syntax = Syntax(rawValue: proto.syntax)! let prefix: String - let pkg = proto.package - if pkg.isEmpty { + let protoPackage = proto.package + if protoPackage.isEmpty { prefix = "" } else { - prefix = "." + pkg + prefix = "." + protoPackage } + + swiftTypePrefix = NamingUtils.typePrefix(protoPackage: protoPackage, + fileOptions: proto.options) + self.enums = proto.enumType.map { return EnumDescriptor(proto: $0, registry: registry, protoNamePrefix: prefix) } diff --git a/Sources/PluginLibrary/NamingUtils.swift b/Sources/PluginLibrary/NamingUtils.swift new file mode 100644 index 000000000..adb3577cc --- /dev/null +++ b/Sources/PluginLibrary/NamingUtils.swift @@ -0,0 +1,51 @@ +// Sources/PluginLibrary/NamingUtils.swift - Utilities for generating names +// +// Copyright (c) 2014 - 2017 Apple Inc. and the project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See LICENSE.txt for license information: +// https://github.com/apple/swift-protobuf/blob/master/LICENSE.txt +// +// ----------------------------------------------------------------------------- +/// +/// This provides some utilities for generating names. +/// +// ----------------------------------------------------------------------------- + +enum NamingUtils { + + // Returns the type prefix to use for a given + static func typePrefix(protoPackage: String, fileOptions: Google_Protobuf_FileOptions) -> String { + // Explicit option (including blank), wins. + if fileOptions.hasSwiftPrefix { + return fileOptions.swiftPrefix + } + + if protoPackage.isEmpty { + return String() + } + + // Transforms: + // "package.name" -> "Package_Name" + // "package_name" -> "PackageName" + // "pacakge.some_name" -> "Package_SomeName" + var makeUpper = true + var prefix = "" + for c in protoPackage.characters { + if c == "_" { + makeUpper = true + } else if c == "." { + makeUpper = true + prefix += "_" + } else if makeUpper { + prefix += String(c).uppercased() + makeUpper = false + } else { + prefix += String(c) + } + } + // End in an underscore to split off anything that gets added to it. + return prefix + "_" + } + +} diff --git a/Tests/PluginLibraryTests/Test_NamingUtils.swift b/Tests/PluginLibraryTests/Test_NamingUtils.swift new file mode 100644 index 000000000..87cf5adc2 --- /dev/null +++ b/Tests/PluginLibraryTests/Test_NamingUtils.swift @@ -0,0 +1,51 @@ +// Tests/PluginLibraryTests/Test_NamingUtils.swift - Test NamingUtils.swift +// +// Copyright (c) 2014 - 2017 Apple Inc. and the project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See LICENSE.txt for license information: +// https://github.com/apple/swift-protobuf/blob/master/LICENSE.txt +// +// ----------------------------------------------------------------------------- + +import XCTest +@testable import PluginLibrary + +class Test_NamingUtils: XCTestCase { + + func testTypePrefix() throws { + // package, swiftPrefix, expected + let tests: [(String, String?, String)] = [ + ( "", nil, "" ), + ( "", "", "" ), + + ( "foo", nil, "Foo_" ), + ( "FOO", nil, "FOO_" ), + ( "fooBar", nil, "FooBar_" ), + ( "FooBar", nil, "FooBar_" ), + + ( "foo.bar.baz", nil, "Foo_Bar_Baz_" ), + ( "foo_bar_baz", nil, "FooBarBaz_" ), + ( "foo.bar_baz", nil, "Foo_BarBaz_" ), + + ( "foo.BAR_baz", nil, "Foo_BARBaz_" ), + ( "foo.bar_bAZ", nil, "Foo_BarBAZ_" ), + ( "FOO.BAR_BAZ", nil, "FOO_BARBAZ_" ), + + ( "foo.bar.baz", "", "" ), + ( "", "ABC", "ABC" ), + + ( "foo.bar.baz", "ABC", "ABC" ), + ( "foo.bar.baz", "abc", "abc" ), + ( "foo.bar.baz", "aBc", "aBc" ), + ] + for (package, prefix, expected) in tests { + var proto = Google_Protobuf_FileOptions() + if let prefix = prefix { + proto.swiftPrefix = prefix + } + let result = NamingUtils.typePrefix(protoPackage: package, fileOptions: proto) + XCTAssertEqual(result, expected, "Package: \(package), Prefix: \(prefix ?? "nil")") + } + } +} From 4d883f9ee19a00cf976beeecbca0bed9a03623b2 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Wed, 26 Apr 2017 17:33:57 -0400 Subject: [PATCH 02/16] Expose SourceCodeInfo.Location from the FileDescriptor. --- Sources/PluginLibrary/Descriptor.swift | 18 ++++++++++++++ Sources/PluginLibrary/HashableArray.swift | 29 +++++++++++++++++++++++ 2 files changed, 47 insertions(+) create mode 100644 Sources/PluginLibrary/HashableArray.swift diff --git a/Sources/PluginLibrary/Descriptor.swift b/Sources/PluginLibrary/Descriptor.swift index 806e65151..92dcb6979 100644 --- a/Sources/PluginLibrary/Descriptor.swift +++ b/Sources/PluginLibrary/Descriptor.swift @@ -133,6 +133,24 @@ public final class FileDescriptor { self.services.forEach { $0.bind(file: self, registry: registry) } } + // TODO(thomasvl): Eventually hide this and just expose it info off the descriptors so + // paths aren't needed externally. + public func sourceCodeInfoLocation(path: [Int32]) -> Google_Protobuf_SourceCodeInfo.Location? { + guard let location = locationMap[HashableArray(path)] else { + return nil + } + return location + } + + // Lazy so this can be computed on demand, as the imported files won't need + // comments during generation. + private lazy var locationMap: [HashableInt32Array:Google_Protobuf_SourceCodeInfo.Location] = { + var result: [HashableInt32Array:Google_Protobuf_SourceCodeInfo.Location] = [:] + for loc in self.proto.sourceCodeInfo.location { + result[HashableArray(loc.path)] = loc + } + return result + }() } public final class Descriptor { diff --git a/Sources/PluginLibrary/HashableArray.swift b/Sources/PluginLibrary/HashableArray.swift new file mode 100644 index 000000000..e204827ed --- /dev/null +++ b/Sources/PluginLibrary/HashableArray.swift @@ -0,0 +1,29 @@ +// Sources/PluginLibrary/HashableArray.swift - Wrapper array to support hashing +// +// Copyright (c) 2014 - 2017 Apple Inc. and the project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See LICENSE.txt for license information: +// https://github.com/apple/swift-protobuf/blob/master/LICENSE.txt +// +// ----------------------------------------------------------------------------- + +/// Helper type to use an array as a dictionary key. +struct HashableArray: Hashable { + let array: [T] + let hashValue: Int + + init(_ array: [T]) { + self.array = array + var hash = Int(bitPattern: 2166136261) + for i in array { + hash = (hash &* Int(16777619)) ^ i.hashValue + } + hashValue = hash + } + static func ==(lhs: HashableArray, rhs: HashableArray) -> Bool { + return lhs.hashValue == rhs.hashValue && lhs.array == rhs.array + } +} + +typealias HashableInt32Array = HashableArray From fb43115d3d63794edbeee9b3c6dd6f3005b21c04 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Thu, 27 Apr 2017 10:16:35 -0400 Subject: [PATCH 03/16] Track the index in the parent when creating Descriptors. Additionally: - Add enumeratedMap() to Array to get the index and value. - Make OneofDescriptor's fields lazy so it doesn't have to get collected for all imports unless they end up actually being used. --- Sources/PluginLibrary/Array+Extensions.swift | 22 ++++++ Sources/PluginLibrary/Descriptor.swift | 72 ++++++++++++-------- 2 files changed, 65 insertions(+), 29 deletions(-) create mode 100644 Sources/PluginLibrary/Array+Extensions.swift diff --git a/Sources/PluginLibrary/Array+Extensions.swift b/Sources/PluginLibrary/Array+Extensions.swift new file mode 100644 index 000000000..bc3486d0e --- /dev/null +++ b/Sources/PluginLibrary/Array+Extensions.swift @@ -0,0 +1,22 @@ +// Sources/PluginLibrary/Array+Extensions.swift - Additions to Arrays +// +// Copyright (c) 2014 - 2017 Apple Inc. and the project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See LICENSE.txt for license information: +// https://github.com/apple/swift-protobuf/blob/master/LICENSE.txt +// +// ----------------------------------------------------------------------------- + +import Foundation + +extension Array { + /// Like map, but calls the transform with the index and value. + func enumeratedMap(_ transform: (Int, Element) throws -> T) rethrows -> [T] { + var i: Int = -1 + return try map { + i += 1 + return try transform(i, $0) + } + } +} diff --git a/Sources/PluginLibrary/Descriptor.swift b/Sources/PluginLibrary/Descriptor.swift index 92dcb6979..9636b6a3d 100644 --- a/Sources/PluginLibrary/Descriptor.swift +++ b/Sources/PluginLibrary/Descriptor.swift @@ -109,17 +109,17 @@ public final class FileDescriptor { swiftTypePrefix = NamingUtils.typePrefix(protoPackage: protoPackage, fileOptions: proto.options) - self.enums = proto.enumType.map { - return EnumDescriptor(proto: $0, registry: registry, protoNamePrefix: prefix) + self.enums = proto.enumType.enumeratedMap { + return EnumDescriptor(proto: $1, index: $0, registry: registry, protoNamePrefix: prefix) } - self.messages = proto.messageType.map { - return Descriptor(proto: $0, registry: registry, protoNamePrefix: prefix) + self.messages = proto.messageType.enumeratedMap { + return Descriptor(proto: $1, index: $0, registry: registry, protoNamePrefix: prefix) } - self.extensions = proto.extension_p.map { - return FieldDescriptor(proto: $0, registry: registry, isExtension: true) + self.extensions = proto.extension_p.enumeratedMap { + return FieldDescriptor(proto: $1, index: $0, registry: registry, isExtension: true) } - self.services = proto.service.map { - return ServiceDescriptor(proto: $0, registry: registry, protoNamePrefix: prefix) + self.services = proto.service.enumeratedMap { + return ServiceDescriptor(proto: $1, index: $0, registry: registry, protoNamePrefix: prefix) } // Done initializing, register ourselves. @@ -155,6 +155,7 @@ public final class FileDescriptor { public final class Descriptor { public let proto: Google_Protobuf_DescriptorProto + let index: Int public let protoName: String public private(set) weak var file: FileDescriptor! public private(set) weak var containingType: Descriptor? @@ -166,31 +167,28 @@ public final class Descriptor { public let extensions: [FieldDescriptor] fileprivate init(proto: Google_Protobuf_DescriptorProto, + index: Int, registry: Registry, protoNamePrefix prefix: String) { self.proto = proto + self.index = index let protoName = "\(prefix).\(proto.name)" self.protoName = protoName - self.enums = proto.enumType.map { - return EnumDescriptor(proto: $0, registry: registry, protoNamePrefix: protoName) + self.enums = proto.enumType.enumeratedMap { + return EnumDescriptor(proto: $1, index: $0, registry: registry, protoNamePrefix: protoName) } - self.messages = proto.nestedType.map { - return Descriptor(proto: $0, registry: registry, protoNamePrefix: protoName) + self.messages = proto.nestedType.enumeratedMap { + return Descriptor(proto: $1, index: $0, registry: registry, protoNamePrefix: protoName) } - self.fields = proto.field.map { - return FieldDescriptor(proto: $0, registry: registry) + self.fields = proto.field.enumeratedMap { + return FieldDescriptor(proto: $1, index: $0, registry: registry) } - var i: Int32 = 0 - var oneofs = [OneofDescriptor]() - for o in proto.oneofDecl { - let oneofFields = self.fields.filter { $0.oneofIndex == i } - oneofs.append(OneofDescriptor(proto: o, registry: registry, fields: oneofFields)) - i += 1 + self.oneofs = proto.oneofDecl.enumeratedMap { + return OneofDescriptor(proto: $1, index: $0, registry: registry) } - self.oneofs = oneofs - self.extensions = proto.extension_p.map { - return FieldDescriptor(proto: $0, registry: registry, isExtension: true) + self.extensions = proto.extension_p.enumeratedMap { + return FieldDescriptor(proto: $1, index: $0, registry: registry, isExtension: true) } // Done initializing, register ourselves. @@ -210,14 +208,17 @@ public final class Descriptor { public final class EnumDescriptor { public let proto: Google_Protobuf_EnumDescriptorProto + let index: Int public let protoName: String public private(set) weak var file: FileDescriptor! public private(set) weak var containingType: Descriptor? fileprivate init(proto: Google_Protobuf_EnumDescriptorProto, + index: Int, registry: Registry, protoNamePrefix prefix: String) { self.proto = proto + self.index = index self.protoName = "\(prefix).\(proto.name)" // Done initializing, register ourselves. @@ -232,17 +233,21 @@ public final class EnumDescriptor { public final class OneofDescriptor { public let proto: Google_Protobuf_OneofDescriptorProto + let index: Int public private(set) weak var containingType: Descriptor! public var name: String { return proto.name } - public let fields: [FieldDescriptor] + public private(set) lazy var fields: [FieldDescriptor] = { + let myIndex = Int32(self.index) + return self.containingType.fields.filter { $0.oneofIndex == myIndex } + }() fileprivate init(proto: Google_Protobuf_OneofDescriptorProto, - registry: Registry, - fields: [FieldDescriptor]) { + index: Int, + registry: Registry) { self.proto = proto - self.fields = fields + self.index = index } fileprivate func bind(registry: Registry, containingType: Descriptor) { @@ -252,6 +257,7 @@ public final class OneofDescriptor { public final class FieldDescriptor { public let proto: Google_Protobuf_FieldDescriptorProto + let index: Int public private(set) weak var file: FileDescriptor! /// The Descriptor of the message which this is a field of. For extensions, /// this is the extended type. @@ -285,9 +291,11 @@ public final class FieldDescriptor { public private(set) weak var enumType: EnumDescriptor! fileprivate init(proto: Google_Protobuf_FieldDescriptorProto, + index: Int, registry: Registry, isExtension: Bool = false) { self.proto = proto + self.index = index self.isExtension = isExtension } @@ -319,20 +327,23 @@ public final class FieldDescriptor { public final class ServiceDescriptor { public let proto: Google_Protobuf_ServiceDescriptorProto + let index: Int public let protoName: String public private(set) weak var file: FileDescriptor! public let methods: [MethodDescriptor] fileprivate init(proto: Google_Protobuf_ServiceDescriptorProto, + index: Int, registry: Registry, protoNamePrefix prefix: String) { self.proto = proto + self.index = index let protoName = "\(prefix).\(proto.name)" self.protoName = protoName - self.methods = proto.method.map { - return MethodDescriptor(proto: $0, registry: registry) + self.methods = proto.method.enumeratedMap { + return MethodDescriptor(proto: $1, index: $0, registry: registry) } // Done initializing, register ourselves. @@ -347,6 +358,7 @@ public final class ServiceDescriptor { public final class MethodDescriptor { public let proto: Google_Protobuf_MethodDescriptorProto + let index: Int public var name: String { return proto.name } @@ -355,8 +367,10 @@ public final class MethodDescriptor { public private(set) var outputType: Descriptor! fileprivate init(proto: Google_Protobuf_MethodDescriptorProto, + index: Int, registry: Registry) { self.proto = proto + self.index = index } fileprivate func bind(service: ServiceDescriptor, registry: Registry) { From e7cf9ae3bfad95dd2638d439739919b44e231e22 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Thu, 27 Apr 2017 11:32:46 -0400 Subject: [PATCH 04/16] Expose Google_Protobuf_SourceCodeInfo.Location from the descriptors. - Protocol ProvidesLocationPath and implement it on the Descriptors. - Protocol ProvidesSourceCodeLocation and provide default impl for things that also have ProvidesLocationPath. --- .../PluginLibrary/Descriptor+Extensions.swift | 87 +++++++++++++++++++ Sources/PluginLibrary/FieldNumbers.swift | 40 +++++++++ .../PluginLibrary/ProvidesLocationPath.swift | 16 ++++ .../ProvidesSourceCodeLocation.swift | 25 ++++++ 4 files changed, 168 insertions(+) create mode 100644 Sources/PluginLibrary/Descriptor+Extensions.swift create mode 100644 Sources/PluginLibrary/FieldNumbers.swift create mode 100644 Sources/PluginLibrary/ProvidesLocationPath.swift create mode 100644 Sources/PluginLibrary/ProvidesSourceCodeLocation.swift diff --git a/Sources/PluginLibrary/Descriptor+Extensions.swift b/Sources/PluginLibrary/Descriptor+Extensions.swift new file mode 100644 index 000000000..8f3c3ebde --- /dev/null +++ b/Sources/PluginLibrary/Descriptor+Extensions.swift @@ -0,0 +1,87 @@ +// Sources/PluginLibrary/Descriptor+Extensions.swift - Additions to Descriptor +// +// Copyright (c) 2014 - 2017 Apple Inc. and the project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See LICENSE.txt for license information: +// https://github.com/apple/swift-protobuf/blob/master/LICENSE.txt +// +// ----------------------------------------------------------------------------- + +import Foundation +import SwiftProtobuf + +extension FileDescriptor: ProvidesSourceCodeLocation { + public var sourceCodeInfoLocation: Google_Protobuf_SourceCodeInfo.Location? { + // google/protobuf's descriptor.cc says it should be an empty path. + return sourceCodeInfoLocation(path: []) + } +} + +extension Descriptor: ProvidesLocationPath, ProvidesSourceCodeLocation { + public func getLocationPath(path: inout [Int32]) { + if let containingType = containingType { + containingType.getLocationPath(path: &path) + path.append(Google_Protobuf_DescriptorProto.FieldNumbers.nestedType) + } else { + path.append(Google_Protobuf_FileDescriptorProto.FieldNumbers.messageType) + } + path.append(Int32(index)) + } +} + +extension EnumDescriptor: ProvidesLocationPath, ProvidesSourceCodeLocation { + public func getLocationPath(path: inout [Int32]) { + if let containingType = containingType { + containingType.getLocationPath(path: &path) + path.append(Google_Protobuf_DescriptorProto.FieldNumbers.enumType) + } else { + path.append(Google_Protobuf_FileDescriptorProto.FieldNumbers.enumType) + } + path.append(Int32(index)) + } +} + +extension OneofDescriptor: ProvidesLocationPath, ProvidesSourceCodeLocation { + public weak var file: FileDescriptor! { return containingType.file } + + public func getLocationPath(path: inout [Int32]) { + containingType.getLocationPath(path: &path) + path.append(Google_Protobuf_DescriptorProto.FieldNumbers.oneofDecl) + path.append(Int32(index)) + } +} + +extension FieldDescriptor: ProvidesLocationPath, ProvidesSourceCodeLocation { + public func getLocationPath(path: inout [Int32]) { + if isExtension { + if let extensionScope = extensionScope { + extensionScope.getLocationPath(path: &path) + path.append(Google_Protobuf_DescriptorProto.FieldNumbers.extension) + } else { + path.append(Google_Protobuf_FileDescriptorProto.FieldNumbers.extension) + } + } else { + containingType.getLocationPath(path: &path) + path.append(Google_Protobuf_DescriptorProto.FieldNumbers.field) + } + path.append(Int32(index)) + } +} + +extension ServiceDescriptor: ProvidesLocationPath, ProvidesSourceCodeLocation { + public func getLocationPath(path: inout [Int32]) { + path.append(Google_Protobuf_FileDescriptorProto.FieldNumbers.service) + path.append(Int32(index)) + } +} + +extension MethodDescriptor: ProvidesLocationPath, ProvidesSourceCodeLocation { + public weak var file: FileDescriptor! { return service.file } + + public func getLocationPath(path: inout [Int32]) { + service.getLocationPath(path: &path) + path.append(Google_Protobuf_ServiceDescriptorProto.FieldNumbers.method) + path.append(Int32(index)) + } +} diff --git a/Sources/PluginLibrary/FieldNumbers.swift b/Sources/PluginLibrary/FieldNumbers.swift new file mode 100644 index 000000000..435fa403c --- /dev/null +++ b/Sources/PluginLibrary/FieldNumbers.swift @@ -0,0 +1,40 @@ +// Sources/PluginLibrary/FieldNumbers.swift - Proto Field numbers +// +// Copyright (c) 2014 - 2017 Apple Inc. and the project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See LICENSE.txt for license information: +// https://github.com/apple/swift-protobuf/blob/master/LICENSE.txt +// +// ----------------------------------------------------------------------------- +/// +/// Field numbers needed by PluginLibrary since they currently aren't generated. +/// +// ----------------------------------------------------------------------------- + +import Foundation + +extension Google_Protobuf_FileDescriptorProto { + struct FieldNumbers { + static let messageType: Int32 = 4 + static let enumType: Int32 = 5 + static let service: Int32 = 6 + static let `extension`: Int32 = 7 + } +} + +extension Google_Protobuf_DescriptorProto { + struct FieldNumbers { + static let field: Int32 = 2 + static let nestedType: Int32 = 3 + static let enumType: Int32 = 4 + static let `extension`: Int32 = 6 + static let oneofDecl: Int32 = 8 + } +} + +extension Google_Protobuf_ServiceDescriptorProto { + struct FieldNumbers { + static let method: Int32 = 2 + } +} diff --git a/Sources/PluginLibrary/ProvidesLocationPath.swift b/Sources/PluginLibrary/ProvidesLocationPath.swift new file mode 100644 index 000000000..55d9a0bcc --- /dev/null +++ b/Sources/PluginLibrary/ProvidesLocationPath.swift @@ -0,0 +1,16 @@ +// Sources/PluginLibrary/ProvidesLocationPath.swift - Proto Field numbers +// +// Copyright (c) 2014 - 2017 Apple Inc. and the project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See LICENSE.txt for license information: +// https://github.com/apple/swift-protobuf/blob/master/LICENSE.txt +// +// ----------------------------------------------------------------------------- + +import Foundation + +public protocol ProvidesLocationPath { + func getLocationPath(path: inout [Int32]) + weak var file: FileDescriptor! { get } +} diff --git a/Sources/PluginLibrary/ProvidesSourceCodeLocation.swift b/Sources/PluginLibrary/ProvidesSourceCodeLocation.swift new file mode 100644 index 000000000..a3b055fd8 --- /dev/null +++ b/Sources/PluginLibrary/ProvidesSourceCodeLocation.swift @@ -0,0 +1,25 @@ +// Sources/PluginLibrary/ProvidesSourceCodeLocation.swift - SourceCodeInfo.Location provider +// +// Copyright (c) 2014 - 2017 Apple Inc. and the project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See LICENSE.txt for license information: +// https://github.com/apple/swift-protobuf/blob/master/LICENSE.txt +// +// ----------------------------------------------------------------------------- + +import Foundation + +public protocol ProvidesSourceCodeLocation { + var sourceCodeInfoLocation: Google_Protobuf_SourceCodeInfo.Location? { get } +} + +// Default implementation for things that support ProvidesLocationPath. +extension ProvidesSourceCodeLocation where Self: ProvidesLocationPath { + public var sourceCodeInfoLocation: Google_Protobuf_SourceCodeInfo.Location? { + var path = [Int32]() + getLocationPath(path: &path) + return file.sourceCodeInfoLocation(path: path) + } +} + From 5b9d4a94168e62bcc514904480d20f3b6aa281d2 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Thu, 27 Apr 2017 13:59:02 -0400 Subject: [PATCH 05/16] Move the generators over to taking Descriptors to init. This doesn't really chang how the generator classed work, but it standardizes on passing the new Descriptor objects and GeneratorOptions as the first args to them. --- Sources/PluginLibrary/Descriptor.swift | 4 + Sources/protoc-gen-swift/Context.swift | 14 ++-- Sources/protoc-gen-swift/EnumGenerator.swift | 18 +++-- .../protoc-gen-swift/ExtensionGenerator.swift | 35 ++++---- Sources/protoc-gen-swift/FileGenerator.swift | 81 ++++++------------- .../protoc-gen-swift/MessageGenerator.swift | 58 ++++++------- 6 files changed, 97 insertions(+), 113 deletions(-) diff --git a/Sources/PluginLibrary/Descriptor.swift b/Sources/PluginLibrary/Descriptor.swift index 9636b6a3d..87f4b0928 100644 --- a/Sources/PluginLibrary/Descriptor.swift +++ b/Sources/PluginLibrary/Descriptor.swift @@ -160,6 +160,8 @@ public final class Descriptor { public private(set) weak var file: FileDescriptor! public private(set) weak var containingType: Descriptor? + public let isMapEntry: Bool + public let enums: [EnumDescriptor] public let messages: [Descriptor] public let fields: [FieldDescriptor] @@ -175,6 +177,8 @@ public final class Descriptor { let protoName = "\(prefix).\(proto.name)" self.protoName = protoName + isMapEntry = proto.options.mapEntry + self.enums = proto.enumType.enumeratedMap { return EnumDescriptor(proto: $1, index: $0, registry: registry, protoNamePrefix: protoName) } diff --git a/Sources/protoc-gen-swift/Context.swift b/Sources/protoc-gen-swift/Context.swift index 9bbd2f62e..1bd2774f1 100644 --- a/Sources/protoc-gen-swift/Context.swift +++ b/Sources/protoc-gen-swift/Context.swift @@ -122,14 +122,16 @@ class Context { func generateResponse() -> CodeGeneratorResponse { var response = CodeGeneratorResponse() - let explicit = Set(request.fileToGenerate) - for fileProto in request.protoFile where explicit.contains(fileProto.name) { + for name in request.fileToGenerate { + let fileDescriptor = descriptorSet.lookupFileDescriptor(protoName: name) + let fileGenerator = FileGenerator(fileDescriptor: fileDescriptor, generatorOptions: options) var printer = CodePrinter() - let file = FileGenerator(descriptor: fileProto, generatorOptions: options) - file.generateOutputFile(printer: &printer, context: self) - let fileResponse = CodeGeneratorResponse.File(name: file.outputFilename, content: printer.content) - response.file.append(fileResponse) + // TODO(thomasvl): Go to a model where this can throw or return an error which can be + // sent back in the response's error (including the input file name that caused it). + fileGenerator.generateOutputFile(printer: &printer, context: self) + response.file.append(CodeGeneratorResponse.File(name: fileGenerator.outputFilename, + content: printer.content)) } return response } diff --git a/Sources/protoc-gen-swift/EnumGenerator.swift b/Sources/protoc-gen-swift/EnumGenerator.swift index 5b6f66644..e31d7fc65 100644 --- a/Sources/protoc-gen-swift/EnumGenerator.swift +++ b/Sources/protoc-gen-swift/EnumGenerator.swift @@ -22,8 +22,9 @@ private let unrecognizedCaseName = "UNRECOGNIZED" /// Generates a Swift enum from a protobuf enum descriptor. class EnumGenerator { - private let descriptor: Google_Protobuf_EnumDescriptorProto + private let enumDescriptor: EnumDescriptor private let generatorOptions: GeneratorOptions + private let visibility: String private let swiftRelativeName: String private let swiftFullName: String @@ -34,28 +35,31 @@ class EnumGenerator { private let comments: String private let isProto3: Bool - init(descriptor: Google_Protobuf_EnumDescriptorProto, + init(descriptor: EnumDescriptor, + generatorOptions: GeneratorOptions, path: [Int32], parentSwiftName: String?, file: FileGenerator ) { - self.descriptor = descriptor + self.enumDescriptor = descriptor self.generatorOptions = file.generatorOptions + + let proto = descriptor.proto self.visibility = generatorOptions.visibilitySourceSnippet self.isProto3 = file.isProto3 if parentSwiftName == nil { - swiftRelativeName = sanitizeEnumTypeName(file.swiftPrefix + descriptor.name) + swiftRelativeName = sanitizeEnumTypeName(file.swiftPrefix + proto.name) swiftFullName = swiftRelativeName } else { - swiftRelativeName = sanitizeEnumTypeName(descriptor.name) + swiftRelativeName = sanitizeEnumTypeName(proto.name) swiftFullName = parentSwiftName! + "." + swiftRelativeName } - let stripLength: Int = descriptor.stripPrefixLength + let stripLength: Int = proto.stripPrefixLength var i: Int32 = 0 var firstCases = [Int32: EnumCaseGenerator]() var enumCases = [EnumCaseGenerator]() - for v in descriptor.value { + for v in proto.value { var casePath = path casePath.append(Google_Protobuf_EnumValueDescriptorProto.FieldNumbers.number) casePath.append(i) diff --git a/Sources/protoc-gen-swift/ExtensionGenerator.swift b/Sources/protoc-gen-swift/ExtensionGenerator.swift index c6b5f93cd..7b822e80f 100644 --- a/Sources/protoc-gen-swift/ExtensionGenerator.swift +++ b/Sources/protoc-gen-swift/ExtensionGenerator.swift @@ -19,8 +19,9 @@ import PluginLibrary import SwiftProtobuf struct ExtensionGenerator { - let descriptor: Google_Protobuf_FieldDescriptorProto - let generatorOptions: GeneratorOptions + private let fieldDescriptor: FieldDescriptor + private let generatorOptions: GeneratorOptions + let path: [Int32] let protoPackageName: String let swiftDeclaringMessageName: String? @@ -39,18 +40,18 @@ struct ExtensionGenerator { var extensionFieldType: String { let label: String - switch descriptor.label { + switch fieldDescriptor.proto.label { case .optional: label = "Optional" case .required: label = "Required" case .repeated: - if descriptor.options.packed == true { + if fieldDescriptor.proto.options.packed == true { label = "Packed" } else { label = "Repeated" } } let modifier: String - switch descriptor.type { + switch fieldDescriptor.proto.type { case .group: modifier = "Group" case .message: modifier = "Message" case .enum: modifier = "Enum" @@ -60,24 +61,26 @@ struct ExtensionGenerator { } var defaultValue: String { - switch descriptor.label { + switch fieldDescriptor.proto.label { case .repeated: return "[]" default: - return descriptor.getSwiftDefaultValue(context: context, isProto3: false) + return fieldDescriptor.proto.getSwiftDefaultValue(context: context, isProto3: false) } } - init(descriptor: Google_Protobuf_FieldDescriptorProto, path: [Int32], parentProtoPath: String?, swiftDeclaringMessageName: String?, file: FileGenerator, context: Context) { - self.descriptor = descriptor + init(descriptor: FieldDescriptor, generatorOptions: GeneratorOptions, path: [Int32], parentProtoPath: String?, swiftDeclaringMessageName: String?, file: FileGenerator, context: Context) { + self.fieldDescriptor = descriptor self.generatorOptions = file.generatorOptions + + let proto = descriptor.proto self.path = path self.protoPackageName = file.protoPackageName self.swiftDeclaringMessageName = swiftDeclaringMessageName - self.swiftExtendedMessageName = context.getMessageNameForPath(path: descriptor.extendee)! + self.swiftExtendedMessageName = context.getMessageNameForPath(path: proto.extendee)! self.context = context - self.apiType = descriptor.getSwiftApiType(context: context, isProto3: false) + self.apiType = proto.getSwiftApiType(context: context, isProto3: false) self.comments = file.commentsFor(path: path) - self.fieldName = descriptor.isGroup ? descriptor.bareTypeName : descriptor.name + self.fieldName = proto.isGroup ? proto.bareTypeName : proto.name if let parentProtoPath = parentProtoPath, !parentProtoPath.isEmpty { var p = parentProtoPath assert(p.hasPrefix(".")) @@ -89,7 +92,7 @@ struct ExtensionGenerator { let baseName: String if descriptor.type == .group { - let g = context.getMessageForPath(path: descriptor.typeName)! + let g = context.getMessageForPath(path: proto.typeName)! baseName = g.name } else { baseName = descriptor.name @@ -140,12 +143,12 @@ struct ExtensionGenerator { func generateProtobufExtensionDeclarations(printer p: inout CodePrinter) { p.print(comments) let scope = swiftDeclaringMessageName == nil ? "" : "static " - let traitsType = descriptor.getTraitsType(context: context) + let traitsType = fieldDescriptor.proto.getTraitsType(context: context) p.print("\(scope)let \(swiftRelativeExtensionName) = SwiftProtobuf.MessageExtension<\(extensionFieldType)<\(traitsType)>, \(swiftExtendedMessageName)>(\n") p.indent() p.print( - "_protobuf_fieldNumber: \(descriptor.number),\n", + "_protobuf_fieldNumber: \(fieldDescriptor.proto.number),\n", "fieldName: \"\(fieldNamePath)\",\n", "defaultValue: \(defaultValue)\n") p.outdent() @@ -160,7 +163,7 @@ struct ExtensionGenerator { p.print(comments) p.print("\(generatorOptions.visibilitySourceSnippet)var \(swiftFieldName): \(apiType) {\n") p.indent() - if descriptor.label == .repeated { + if fieldDescriptor.proto.label == .repeated { p.print("get {return getExtensionValue(ext: \(swiftFullExtensionName))}\n") } else { p.print("get {return getExtensionValue(ext: \(swiftFullExtensionName)) ?? \(defaultValue)}\n") diff --git a/Sources/protoc-gen-swift/FileGenerator.swift b/Sources/protoc-gen-swift/FileGenerator.swift index 67e7b3fa0..b300d07e4 100644 --- a/Sources/protoc-gen-swift/FileGenerator.swift +++ b/Sources/protoc-gen-swift/FileGenerator.swift @@ -155,57 +155,15 @@ extension Google_Protobuf_FileDescriptorProto { return "" } } - - func locationFor(path: [Int32]) -> Google_Protobuf_SourceCodeInfo.Location? { - if hasSourceCodeInfo { - for l in sourceCodeInfo.location { - if l.path == path { - return l - } - } - } - return nil - } } class FileGenerator { - let descriptor: Google_Protobuf_FileDescriptorProto - let generatorOptions: GeneratorOptions - - init(descriptor: Google_Protobuf_FileDescriptorProto, - generatorOptions: GeneratorOptions) { - self.descriptor = descriptor - self.generatorOptions = generatorOptions - } - - func messageNameForPath(path: String) -> String? { - let base: String - if !descriptor.package.isEmpty { - base = "." + descriptor.package - } else { - base = "" - } - for m in descriptor.messageType { - let messagePath = base + "." + m.name - if messagePath == path { - let swiftName = swiftPrefix + m.name - return swiftName - } - } - Stderr.print("Unable to match \(path) within \(base)") - assert(false) - return nil - } - - var protoPackageName: String {return descriptor.package} - var swiftPrefix: String {return descriptor.swiftPrefix} - var isProto3: Bool {return descriptor.isProto3} - var isWellKnownType: Bool {return descriptor.isWellKnownType} - var baseFilename: String {return descriptor.baseFilename} + private let fileDescriptor: FileDescriptor + let generatorOptions: GeneratorOptions // TODO(private)### var outputFilename: String { let ext = ".pb.swift" - let pathParts = splitPath(pathname: descriptor.name) + let pathParts = splitPath(pathname: fileDescriptor.name) switch generatorOptions.outputNaming { case .FullPath: return pathParts.dir + pathParts.base + ext @@ -218,6 +176,18 @@ class FileGenerator { } } + init(fileDescriptor: FileDescriptor, + generatorOptions: GeneratorOptions) { + self.fileDescriptor = fileDescriptor + self.generatorOptions = generatorOptions + } + + var protoPackageName: String {return fileDescriptor.package} + var swiftPrefix: String {return fileDescriptor.swiftTypePrefix} + var isProto3: Bool {return fileDescriptor.syntax == .proto3} + private var isWellKnownType: Bool {return fileDescriptor.proto.isWellKnownType} + private var baseFilename: String {return fileDescriptor.proto.baseFilename} + func commentsFor(path: [Int32], includeLeadingDetached: Bool = false) -> String { func escapeMarkup(_ text: String) -> String { // Proto file comments don't really have any markup associated with @@ -245,7 +215,7 @@ class FileGenerator { return result } - if let location = descriptor.locationFor(path: path) { + if let location = fileDescriptor.sourceCodeInfoLocation(path: path) { var result = "" if includeLeadingDetached { @@ -268,13 +238,12 @@ class FileGenerator { } func generateOutputFile(printer p: inout CodePrinter, context: Context) { - let inputFilename = descriptor.hasName ? descriptor.name : "" p.print( "/*\n", " * DO NOT EDIT.\n", " *\n", " * Generated by the protocol buffer compiler.\n", - " * Source: \(inputFilename)\n", + " * Source: \(fileDescriptor.name)\n", " *\n", " */\n", "\n") @@ -310,26 +279,26 @@ class FileGenerator { var enums = [EnumGenerator]() let path = [Int32]() var i: Int32 = 0 - for e in descriptor.enumType { + for e in fileDescriptor.enums { let enumPath = path + [5, i] i += 1 - enums.append(EnumGenerator(descriptor: e, path: enumPath, parentSwiftName: nil, file: self)) + enums.append(EnumGenerator(descriptor: e, generatorOptions: generatorOptions, path: enumPath, parentSwiftName: nil, file: self)) } var messages = [MessageGenerator]() i = 0 - for m in descriptor.messageType { + for m in fileDescriptor.messages { let messagePath = path + [4, i] i += 1 - messages.append(MessageGenerator(descriptor: m, path: messagePath, parentSwiftName: nil, parentProtoPath: descriptor.protoPath, file: self, context: context)) + messages.append(MessageGenerator(descriptor: m, generatorOptions: generatorOptions, path: messagePath, parentSwiftName: nil, parentProtoPath: fileDescriptor.proto.protoPath, file: self, context: context)) } var extensions = [ExtensionGenerator]() i = 0 - for e in descriptor.extension_p { + for e in fileDescriptor.extensions { let extPath = path + [7, i] i += 1 - extensions.append(ExtensionGenerator(descriptor: e, path: extPath, parentProtoPath: descriptor.protoPath, swiftDeclaringMessageName: nil, file: self, context: context)) + extensions.append(ExtensionGenerator(descriptor: e, generatorOptions: generatorOptions, path: extPath, parentProtoPath: fileDescriptor.proto.protoPath, swiftDeclaringMessageName: nil, file: self, context: context)) } for e in enums { @@ -348,7 +317,7 @@ class FileGenerator { m.registerExtensions(registry: ®istry) } if !registry.isEmpty { - let pathParts = splitPath(pathname: descriptor.name) + let pathParts = splitPath(pathname: fileDescriptor.name) let filename = pathParts.base + pathParts.suffix p.print( "\n", @@ -371,7 +340,7 @@ class FileGenerator { "/// this .proto file. It can be used any place an `SwiftProtobuf.ExtensionMap` is needed\n", "/// in parsing, or it can be combined with other `SwiftProtobuf.SimpleExtensionMap`s to create\n", "/// a larger `SwiftProtobuf.SimpleExtensionMap`.\n", - "\(generatorOptions.visibilitySourceSnippet)let \(descriptor.swiftPrefix)\(filenameAsIdentifer)_Extensions: SwiftProtobuf.SimpleExtensionMap = [\n") + "\(generatorOptions.visibilitySourceSnippet)let \(fileDescriptor.proto.swiftPrefix)\(filenameAsIdentifer)_Extensions: SwiftProtobuf.SimpleExtensionMap = [\n") p.indent() var separator = "" for e in registry { diff --git a/Sources/protoc-gen-swift/MessageGenerator.swift b/Sources/protoc-gen-swift/MessageGenerator.swift index 68df72f09..73a6a4541 100644 --- a/Sources/protoc-gen-swift/MessageGenerator.swift +++ b/Sources/protoc-gen-swift/MessageGenerator.swift @@ -19,9 +19,9 @@ import PluginLibrary import SwiftProtobuf class MessageGenerator { - private let descriptor: Google_Protobuf_DescriptorProto - private let context: Context + private let descriptor: Descriptor private let generatorOptions: GeneratorOptions + private let context: Context private let visibility: String private let protoFullName: String private let swiftFullName: String @@ -44,33 +44,35 @@ class MessageGenerator { private let comments: String init( - descriptor: Google_Protobuf_DescriptorProto, + descriptor: Descriptor, + generatorOptions: GeneratorOptions, path: [Int32], parentSwiftName: String?, parentProtoPath: String?, file: FileGenerator, context: Context ) { - self.protoMessageName = descriptor.name + self.descriptor = descriptor + self.generatorOptions = generatorOptions + + let proto = descriptor.proto + self.protoMessageName = proto.name self.context = context - self.generatorOptions = context.options self.visibility = generatorOptions.visibilitySourceSnippet self.protoFullName = (parentProtoPath == nil ? "" : (parentProtoPath! + ".")) + self.protoMessageName - self.descriptor = descriptor self.isProto3 = file.isProto3 - self.isExtensible = descriptor.extensionRange.count > 0 + self.isExtensible = proto.extensionRange.count > 0 self.protoPackageName = file.protoPackageName if let parentSwiftName = parentSwiftName { - swiftRelativeName = sanitizeMessageTypeName(descriptor.name) + swiftRelativeName = sanitizeMessageTypeName(proto.name) swiftFullName = parentSwiftName + "." + swiftRelativeName } else { - swiftRelativeName = sanitizeMessageTypeName(file.swiftPrefix + descriptor.name) + swiftRelativeName = sanitizeMessageTypeName(file.swiftPrefix + proto.name) swiftFullName = swiftRelativeName } self.isAnyMessage = (isProto3 && - descriptor.name == "Any" && - file.descriptor.package == "google.protobuf" && - file.descriptor.name == "google/protobuf/any.proto") + descriptor.protoName == ".google.protobuf.Any" && + descriptor.file.name == "google/protobuf/any.proto") var conformance: [String] = ["SwiftProtobuf.Message"] if isExtensible { conformance.append("SwiftProtobuf.ExtensibleMessage") @@ -79,12 +81,12 @@ class MessageGenerator { var i: Int32 = 0 var fields = [MessageFieldGenerator]() - for f in descriptor.field { + for f in proto.field { var fieldPath = path fieldPath.append(Google_Protobuf_DescriptorProto.FieldNumbers.field) fieldPath.append(i) i += 1 - fields.append(MessageFieldGenerator(descriptor: f, path: fieldPath, messageDescriptor: descriptor, file: file, context: context)) + fields.append(MessageFieldGenerator(descriptor: f, path: fieldPath, messageDescriptor: proto, file: file, context: context)) } self.fields = fields fieldsSortedByNumber = fields.sorted {$0.number < $1.number} @@ -92,18 +94,18 @@ class MessageGenerator { i = 0 var extensions = [ExtensionGenerator]() - for e in descriptor.extension_p { + for e in descriptor.extensions { var extPath = path extPath.append(Google_Protobuf_DescriptorProto.FieldNumbers.extension) extPath.append(i) i += 1 - extensions.append(ExtensionGenerator(descriptor: e, path: extPath, parentProtoPath: protoFullName, swiftDeclaringMessageName: swiftFullName, file: file, context: context)) + extensions.append(ExtensionGenerator(descriptor: e, generatorOptions: generatorOptions, path: extPath, parentProtoPath: protoFullName, swiftDeclaringMessageName: swiftFullName, file: file, context: context)) } self.extensions = extensions i = 0 var oneofs = [OneofGenerator]() - for o in descriptor.oneofDecl { + for o in proto.oneofDecl { let oneofFields = fields.filter { $0.descriptor.hasOneofIndex && $0.descriptor.oneofIndex == Int32(i) } @@ -111,30 +113,30 @@ class MessageGenerator { oneofPath.append(Google_Protobuf_DescriptorProto.FieldNumbers.oneofDecl) oneofPath.append(i) i += 1 - let oneof = OneofGenerator(descriptor: o, path: oneofPath, file: file, generatorOptions: generatorOptions, fields: oneofFields, swiftMessageFullName: swiftFullName, parentFieldNumbersSorted: sortedFieldNumbers, parentExtensionRanges: descriptor.extensionRange) + let oneof = OneofGenerator(descriptor: o, path: oneofPath, file: file, generatorOptions: generatorOptions, fields: oneofFields, swiftMessageFullName: swiftFullName, parentFieldNumbersSorted: sortedFieldNumbers, parentExtensionRanges: proto.extensionRange) oneofs.append(oneof) } self.oneofs = oneofs i = 0 var enums = [EnumGenerator]() - for e in descriptor.enumType { + for e in descriptor.enums { var enumPath = path enumPath.append(Google_Protobuf_DescriptorProto.FieldNumbers.enumType) enumPath.append(i) i += 1 - enums.append(EnumGenerator(descriptor: e, path: enumPath, parentSwiftName: swiftFullName, file: file)) + enums.append(EnumGenerator(descriptor: e, generatorOptions: generatorOptions, path: enumPath, parentSwiftName: swiftFullName, file: file)) } self.enums = enums i = 0 var messages = [MessageGenerator]() - for m in descriptor.nestedType where m.options.mapEntry != true { + for m in descriptor.messages where !m.isMapEntry { var msgPath = path msgPath.append(Google_Protobuf_DescriptorProto.FieldNumbers.nestedType) msgPath.append(i) i += 1 - messages.append(MessageGenerator(descriptor: m, path: msgPath, parentSwiftName: swiftFullName, parentProtoPath: protoFullName, file: file, context: context)) + messages.append(MessageGenerator(descriptor: m, generatorOptions: generatorOptions, path: msgPath, parentSwiftName: swiftFullName, parentProtoPath: protoFullName, file: file, context: context)) } self.messages = messages @@ -146,10 +148,10 @@ class MessageGenerator { // reduce the real number of fields and the message might not need heap // storage yet. let useHeapStorage = fields.count > 16 || - hasMessageField(descriptor: descriptor, context: context) + hasMessageField(descriptor: descriptor.proto, context: context) if isAnyMessage { self.storage = AnyMessageStorageClassGenerator( - descriptor: descriptor, + descriptor: proto, fields: fields, oneofs: oneofs, file: file, @@ -157,7 +159,7 @@ class MessageGenerator { context: context) } else if useHeapStorage { self.storage = MessageStorageClassGenerator( - descriptor: descriptor, + descriptor: proto, fields: fields, oneofs: oneofs, file: file, @@ -412,7 +414,7 @@ class MessageGenerator { } } if isExtensible { - p.print("case \(descriptor.swiftExtensionRangeExpressions):\n") + p.print("case \(descriptor.proto.swiftExtensionRangeExpressions):\n") p.indent() p.print("try decoder.decodeExtensionField(values: &_protobuf_extensionFieldValues, messageType: \(swiftRelativeName).self, fieldNumber: fieldNumber)\n") p.outdent() @@ -421,7 +423,7 @@ class MessageGenerator { } else if isExtensible { // Just output a simple if-statement if the message had no fields of its // own but we still need to generate a decode statement for extensions. - p.print("if \(descriptor.swiftExtensionRangeBooleanExpression(variable: "fieldNumber")) {\n") + p.print("if \(descriptor.proto.swiftExtensionRangeBooleanExpression(variable: "fieldNumber")) {\n") p.indent() p.print("try decoder.decodeExtensionField(values: &_protobuf_extensionFieldValues, messageType: \(swiftRelativeName).self, fieldNumber: fieldNumber)\n") p.outdent() @@ -479,7 +481,7 @@ class MessageGenerator { if let storage = storage { storage.generatePreTraverse(printer: &p) } - var ranges = descriptor.extensionRange.makeIterator() + var ranges = descriptor.proto.extensionRange.makeIterator() var nextRange = ranges.next() var currentOneofGenerator: OneofGenerator? var oneofStart = 0 From 168bb3b01375d2eae443ced744761b548098396d Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Thu, 27 Apr 2017 14:21:08 -0400 Subject: [PATCH 06/16] Add EnumValueDescriptor and wire it through to the EnumCaseGenerator. Doesn't really change any generation, just moves things over to the types. --- .../PluginLibrary/Descriptor+Extensions.swift | 10 ++++ Sources/PluginLibrary/Descriptor.swift | 46 ++++++++++++++++++- Sources/PluginLibrary/FieldNumbers.swift | 6 +++ .../protoc-gen-swift/EnumCaseGenerator.swift | 14 ++++-- Sources/protoc-gen-swift/EnumGenerator.swift | 3 +- 5 files changed, 73 insertions(+), 6 deletions(-) diff --git a/Sources/PluginLibrary/Descriptor+Extensions.swift b/Sources/PluginLibrary/Descriptor+Extensions.swift index 8f3c3ebde..8853f79ad 100644 --- a/Sources/PluginLibrary/Descriptor+Extensions.swift +++ b/Sources/PluginLibrary/Descriptor+Extensions.swift @@ -42,6 +42,16 @@ extension EnumDescriptor: ProvidesLocationPath, ProvidesSourceCodeLocation { } } +extension EnumValueDescriptor: ProvidesLocationPath, ProvidesSourceCodeLocation { + public weak var file: FileDescriptor! { return enumType.file } + + public func getLocationPath(path: inout [Int32]) { + enumType.getLocationPath(path: &path) + path.append(Google_Protobuf_EnumDescriptorProto.FieldNumbers.value) + path.append(Int32(index)) + } +} + extension OneofDescriptor: ProvidesLocationPath, ProvidesSourceCodeLocation { public weak var file: FileDescriptor! { return containingType.file } diff --git a/Sources/PluginLibrary/Descriptor.swift b/Sources/PluginLibrary/Descriptor.swift index 87f4b0928..2c79c4d1e 100644 --- a/Sources/PluginLibrary/Descriptor.swift +++ b/Sources/PluginLibrary/Descriptor.swift @@ -12,7 +12,8 @@ /// wrappers around the protos to make a more usable object graph for generation /// and also provides some SwiftProtobuf specific additions that would be useful /// to anyone generating something that uses SwiftProtobufs (like support the -/// `service` messages). +/// `service` messages). It is *not* the intent for these to eventually be used +/// as part of some reflection or generate message api. /// // ----------------------------------------------------------------------------- @@ -217,6 +218,28 @@ public final class EnumDescriptor { public private(set) weak var file: FileDescriptor! public private(set) weak var containingType: Descriptor? + // This is lazy so it is they are created only when needed, that way an + // import doesn't have to do all this work unless the enum is used by + // the importer. + public private(set) lazy var values: [EnumValueDescriptor] = { + var firstValues = [Int32:EnumValueDescriptor]() + var result = [EnumValueDescriptor]() + var i = 0 + for p in self.proto.value { + let aliasing = firstValues[p.number] + let d = EnumValueDescriptor(proto: p, index: i, enumType: self, aliasing: aliasing) + result.append(d) + i += 1 + + if let aliasing = aliasing { + aliasing.aliases.append(d) + } else { + firstValues[d.number] = d + } + } + return result + }() + fileprivate init(proto: Google_Protobuf_EnumDescriptorProto, index: Int, registry: Registry, @@ -235,6 +258,27 @@ public final class EnumDescriptor { } } +public final class EnumValueDescriptor { + public let proto: Google_Protobuf_EnumValueDescriptorProto + let index: Int + public private(set) weak var enumType: EnumDescriptor! + + public var number: Int32 { return proto.number } + + public let aliasOf: EnumValueDescriptor? + public fileprivate(set) var aliases: [EnumValueDescriptor] = [] + + fileprivate init(proto: Google_Protobuf_EnumValueDescriptorProto, + index: Int, + enumType: EnumDescriptor, + aliasing: EnumValueDescriptor?) { + self.proto = proto + self.index = index + self.enumType = enumType + aliasOf = aliasing + } +} + public final class OneofDescriptor { public let proto: Google_Protobuf_OneofDescriptorProto let index: Int diff --git a/Sources/PluginLibrary/FieldNumbers.swift b/Sources/PluginLibrary/FieldNumbers.swift index 435fa403c..61a467719 100644 --- a/Sources/PluginLibrary/FieldNumbers.swift +++ b/Sources/PluginLibrary/FieldNumbers.swift @@ -33,6 +33,12 @@ extension Google_Protobuf_DescriptorProto { } } +extension Google_Protobuf_EnumDescriptorProto { + struct FieldNumbers { + static let value: Int32 = 2 + } +} + extension Google_Protobuf_ServiceDescriptorProto { struct FieldNumbers { static let method: Int32 = 2 diff --git a/Sources/protoc-gen-swift/EnumCaseGenerator.swift b/Sources/protoc-gen-swift/EnumCaseGenerator.swift index 206207691..c28fbb114 100644 --- a/Sources/protoc-gen-swift/EnumCaseGenerator.swift +++ b/Sources/protoc-gen-swift/EnumCaseGenerator.swift @@ -19,7 +19,10 @@ import SwiftProtobuf /// Generates the Swift code for a single enum case. class EnumCaseGenerator { - internal let descriptor: Google_Protobuf_EnumValueDescriptorProto + private let enumValueDescriptor: EnumValueDescriptor + private let generatorOptions: GeneratorOptions + + internal var descriptor: Google_Protobuf_EnumValueDescriptorProto { return enumValueDescriptor.proto } internal let swiftName: String internal let path: [Int32] internal let comments: String @@ -42,14 +45,17 @@ class EnumCaseGenerator { return aliasOfGenerator != nil } - init(descriptor: Google_Protobuf_EnumValueDescriptorProto, + init(descriptor: EnumValueDescriptor, + generatorOptions: GeneratorOptions, path: [Int32], file: FileGenerator, stripLength: Int, aliasing aliasOfGenerator: EnumCaseGenerator? ) { - self.descriptor = descriptor - self.swiftName = descriptor.getSwiftName(stripLength: stripLength) + self.enumValueDescriptor = descriptor + self.generatorOptions = generatorOptions + + self.swiftName = descriptor.proto.getSwiftName(stripLength: stripLength) self.path = path self.comments = file.commentsFor(path: path) self.aliasOfGenerator = aliasOfGenerator diff --git a/Sources/protoc-gen-swift/EnumGenerator.swift b/Sources/protoc-gen-swift/EnumGenerator.swift index e31d7fc65..5f86a66e4 100644 --- a/Sources/protoc-gen-swift/EnumGenerator.swift +++ b/Sources/protoc-gen-swift/EnumGenerator.swift @@ -59,7 +59,7 @@ class EnumGenerator { var i: Int32 = 0 var firstCases = [Int32: EnumCaseGenerator]() var enumCases = [EnumCaseGenerator]() - for v in proto.value { + for v in enumDescriptor.values { var casePath = path casePath.append(Google_Protobuf_EnumValueDescriptorProto.FieldNumbers.number) casePath.append(i) @@ -68,6 +68,7 @@ class EnumGenerator { // Keep track of aliases by recording them as we build the generators. let firstCase = firstCases[v.number] let generator = EnumCaseGenerator(descriptor: v, + generatorOptions: generatorOptions, path: casePath, file: file, stripLength: stripLength, From af623a7116efc3cd14917258f17f2166b0f5105a Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Thu, 27 Apr 2017 15:40:57 -0400 Subject: [PATCH 07/16] Support reusable comment generation. - Add a helper to Google_Protobuf_SourceCodeInfo.Location to do the generation. - Give ProvidesSourceCodeLocation a helper for getting the comments. --- ...e_Protobuf_SourceCodeInfo+Extensions.swift | 65 +++++++++++++++++++ .../ProvidesSourceCodeLocation.swift | 11 ++++ Sources/protoc-gen-swift/FileGenerator.swift | 47 ++------------ 3 files changed, 81 insertions(+), 42 deletions(-) create mode 100644 Sources/PluginLibrary/Google_Protobuf_SourceCodeInfo+Extensions.swift diff --git a/Sources/PluginLibrary/Google_Protobuf_SourceCodeInfo+Extensions.swift b/Sources/PluginLibrary/Google_Protobuf_SourceCodeInfo+Extensions.swift new file mode 100644 index 000000000..b58c9da3f --- /dev/null +++ b/Sources/PluginLibrary/Google_Protobuf_SourceCodeInfo+Extensions.swift @@ -0,0 +1,65 @@ +// Sources/PluginLibrary/Google_Protobuf_SourceCodeInfo+Extensions.swift - SourceCodeInfo Additions +// +// Copyright (c) 2014 - 2017 Apple Inc. and the project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See LICENSE.txt for license information: +// https://github.com/apple/swift-protobuf/blob/master/LICENSE.txt +// +// ----------------------------------------------------------------------------- + +import Foundation + +extension Google_Protobuf_SourceCodeInfo.Location { + + /// Builds a source comment out of the location's comment fields. + /// + /// If leadingDetachedPrefix is not provided, those comments won't + /// be collected. + public func asSourceComment(commentPrefix: String, + leadingDetachedPrefix: String? = nil) -> String { + func escapeMarkup(_ text: String) -> String { + // Proto file comments don't really have any markup associated with + // them. Swift uses something like MarkDown: + // "Markup Formatting Reference" + // https://developer.apple.com/library/content/documentation/Xcode/Reference/xcode_markup_formatting_ref/index.html + // Sadly that format doesn't really lend itself to any form of + // escaping to ensure comments are interpreted markup when they + // really aren't. About the only thing that could be done is to + // try and escape some set of things that could start directives, + // and that gets pretty chatty/ugly pretty quickly. + return text + } + + func prefixLines(text: String, prefix: String) -> String { + var result = String() + var lines = text.components(separatedBy: .newlines) + // Trim any blank lines off the end. + while !lines.isEmpty && lines.last!.trimmingCharacters(in: .whitespaces).isEmpty { + lines.removeLast() + } + for line in lines { + result.append(prefix + line + "\n") + } + return result + } + + var result = String() + + if let leadingDetachedPrefix = leadingDetachedPrefix { + for detached in leadingDetachedComments { + let comment = prefixLines(text: detached, prefix: leadingDetachedPrefix) + if !comment.isEmpty { + result += comment + // Detached comments have blank lines between then (and + // anything that follows them). + result += "\n" + } + } + } + + let comments = hasLeadingComments ? leadingComments : trailingComments + result += prefixLines(text: escapeMarkup(comments), prefix: commentPrefix) + return result + } +} diff --git a/Sources/PluginLibrary/ProvidesSourceCodeLocation.swift b/Sources/PluginLibrary/ProvidesSourceCodeLocation.swift index a3b055fd8..4689d2f14 100644 --- a/Sources/PluginLibrary/ProvidesSourceCodeLocation.swift +++ b/Sources/PluginLibrary/ProvidesSourceCodeLocation.swift @@ -23,3 +23,14 @@ extension ProvidesSourceCodeLocation where Self: ProvidesLocationPath { } } +// Helper to get source comments out of ProvidesSourceCodeLocation +extension ProvidesSourceCodeLocation { + public func protoSourceComments(commentPrefix: String = "///", + leadingDetachedPrefix: String? = nil) -> String { + if let loc = sourceCodeInfoLocation { + return loc.asSourceComment(commentPrefix: commentPrefix, + leadingDetachedPrefix: leadingDetachedPrefix) + } + return String() + } +} diff --git a/Sources/protoc-gen-swift/FileGenerator.swift b/Sources/protoc-gen-swift/FileGenerator.swift index b300d07e4..5165f9c66 100644 --- a/Sources/protoc-gen-swift/FileGenerator.swift +++ b/Sources/protoc-gen-swift/FileGenerator.swift @@ -189,52 +189,15 @@ class FileGenerator { private var baseFilename: String {return fileDescriptor.proto.baseFilename} func commentsFor(path: [Int32], includeLeadingDetached: Bool = false) -> String { - func escapeMarkup(_ text: String) -> String { - // Proto file comments don't really have any markup associated with - // them. Swift uses something like MarkDown: - // "Markup Formatting Reference" - // https://developer.apple.com/library/content/documentation/Xcode/Reference/xcode_markup_formatting_ref/index.html - // Sadly that format doesn't really lend itself to any form of - // escaping to ensure comments are interpreted markup when they - // really aren't. About the only thing that could be done is to - // try and escape some set of things that could start directives, - // and that gets pretty chatty/ugly pretty quickly. - return text - } - - func prefixLines(text: String, prefix: String) -> String { - var result = "" - var lines = text.components(separatedBy: .newlines) - // Trim any blank lines off the end. - while !lines.isEmpty && trimWhitespace(lines.last!).isEmpty { - lines.removeLast() - } - for line in lines { - result.append(prefix + line + "\n") - } - return result - } - if let location = fileDescriptor.sourceCodeInfoLocation(path: path) { - var result = "" - + var detachedPrefix: String? = nil if includeLeadingDetached { - for detached in location.leadingDetachedComments { - let comment = prefixLines(text: detached, prefix: "//") - if !comment.isEmpty { - result += comment - // Detached comments have blank lines between then (and - // anything that follows them). - result += "\n" - } - } + detachedPrefix = "//" } - - let comments = location.hasLeadingComments ? location.leadingComments : location.trailingComments - result += prefixLines(text: escapeMarkup(comments), prefix: "///") - return result + return location.asSourceComment(commentPrefix: "///", + leadingDetachedPrefix: detachedPrefix) } - return "" + return String() } func generateOutputFile(printer p: inout CodePrinter, context: Context) { From 771c287904de95307b93bfcb25708b30dd29fea0 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Thu, 27 Apr 2017 16:12:46 -0400 Subject: [PATCH 08/16] Add a helper for enum behaviors. Rather then adding checking the syntax in multiple places, provide a helper so the other code is easier to follow. --- Sources/PluginLibrary/Descriptor+Extensions.swift | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Sources/PluginLibrary/Descriptor+Extensions.swift b/Sources/PluginLibrary/Descriptor+Extensions.swift index 8853f79ad..9a5b2ca95 100644 --- a/Sources/PluginLibrary/Descriptor+Extensions.swift +++ b/Sources/PluginLibrary/Descriptor+Extensions.swift @@ -12,6 +12,11 @@ import Foundation import SwiftProtobuf extension FileDescriptor: ProvidesSourceCodeLocation { + // True if this file should perserve unknown enums within the enum. + public var hasPreservingUnknownEnumSemantics: Bool { + return syntax == .proto3 + } + public var sourceCodeInfoLocation: Google_Protobuf_SourceCodeInfo.Location? { // google/protobuf's descriptor.cc says it should be an empty path. return sourceCodeInfoLocation(path: []) @@ -31,6 +36,11 @@ extension Descriptor: ProvidesLocationPath, ProvidesSourceCodeLocation { } extension EnumDescriptor: ProvidesLocationPath, ProvidesSourceCodeLocation { + // True if this enum should perserve unknown enums within the enum. + public var hasPreservingUnknownEnumSemantics: Bool { + return file.hasPreservingUnknownEnumSemantics + } + public func getLocationPath(path: inout [Int32]) { if let containingType = containingType { containingType.getLocationPath(path: &path) From fc327868f364123596972b511a28a08429007c53 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Thu, 27 Apr 2017 16:37:24 -0400 Subject: [PATCH 09/16] Move comment generation over to the new code paths. Remove several places paths were calculated, etc. --- .../protoc-gen-swift/EnumCaseGenerator.swift | 12 ++----- Sources/protoc-gen-swift/EnumGenerator.swift | 25 +++---------- .../protoc-gen-swift/ExtensionGenerator.swift | 8 ++--- Sources/protoc-gen-swift/FileGenerator.swift | 35 +++++++++---------- .../protoc-gen-swift/MessageGenerator.swift | 13 ++----- 5 files changed, 29 insertions(+), 64 deletions(-) diff --git a/Sources/protoc-gen-swift/EnumCaseGenerator.swift b/Sources/protoc-gen-swift/EnumCaseGenerator.swift index c28fbb114..5fc7629f5 100644 --- a/Sources/protoc-gen-swift/EnumCaseGenerator.swift +++ b/Sources/protoc-gen-swift/EnumCaseGenerator.swift @@ -24,11 +24,8 @@ class EnumCaseGenerator { internal var descriptor: Google_Protobuf_EnumValueDescriptorProto { return enumValueDescriptor.proto } internal let swiftName: String - internal let path: [Int32] - internal let comments: String internal let aliasOfGenerator: EnumCaseGenerator? - private let visibility: String private var aliases = [Weak]() internal var protoName: String { @@ -47,8 +44,6 @@ class EnumCaseGenerator { init(descriptor: EnumValueDescriptor, generatorOptions: GeneratorOptions, - path: [Int32], - file: FileGenerator, stripLength: Int, aliasing aliasOfGenerator: EnumCaseGenerator? ) { @@ -56,11 +51,7 @@ class EnumCaseGenerator { self.generatorOptions = generatorOptions self.swiftName = descriptor.proto.getSwiftName(stripLength: stripLength) - self.path = path - self.comments = file.commentsFor(path: path) self.aliasOfGenerator = aliasOfGenerator - - self.visibility = file.generatorOptions.visibilitySourceSnippet } /// Registers the given enum case generator as an alias of the receiver. @@ -81,12 +72,13 @@ class EnumCaseGenerator { /// /// - Parameter p: The code printer. func generateCaseOrAlias(printer p: inout CodePrinter) { + let comments = enumValueDescriptor.protoSourceComments() if !comments.isEmpty { p.print("\n") p.print(comments) } if let aliasOf = aliasOfGenerator { - p.print("\(visibility)static let \(swiftName) = \(aliasOf.swiftName)\n") + p.print("\(generatorOptions.visibilitySourceSnippet)static let \(swiftName) = \(aliasOf.swiftName)\n") } else { p.print("case \(swiftName) // = \(number)\n") } diff --git a/Sources/protoc-gen-swift/EnumGenerator.swift b/Sources/protoc-gen-swift/EnumGenerator.swift index 5f86a66e4..1be054506 100644 --- a/Sources/protoc-gen-swift/EnumGenerator.swift +++ b/Sources/protoc-gen-swift/EnumGenerator.swift @@ -31,22 +31,17 @@ class EnumGenerator { private let enumCases: [EnumCaseGenerator] private let enumCasesSortedByNumber: [EnumCaseGenerator] private let defaultCase: EnumCaseGenerator - private let path: [Int32] - private let comments: String - private let isProto3: Bool init(descriptor: EnumDescriptor, generatorOptions: GeneratorOptions, - path: [Int32], parentSwiftName: String?, file: FileGenerator ) { self.enumDescriptor = descriptor - self.generatorOptions = file.generatorOptions + self.generatorOptions = generatorOptions let proto = descriptor.proto self.visibility = generatorOptions.visibilitySourceSnippet - self.isProto3 = file.isProto3 if parentSwiftName == nil { swiftRelativeName = sanitizeEnumTypeName(file.swiftPrefix + proto.name) swiftFullName = swiftRelativeName @@ -56,21 +51,13 @@ class EnumGenerator { } let stripLength: Int = proto.stripPrefixLength - var i: Int32 = 0 var firstCases = [Int32: EnumCaseGenerator]() var enumCases = [EnumCaseGenerator]() for v in enumDescriptor.values { - var casePath = path - casePath.append(Google_Protobuf_EnumValueDescriptorProto.FieldNumbers.number) - casePath.append(i) - i += 1 - // Keep track of aliases by recording them as we build the generators. let firstCase = firstCases[v.number] let generator = EnumCaseGenerator(descriptor: v, generatorOptions: generatorOptions, - path: casePath, - file: file, stripLength: stripLength, aliasing: firstCase) enumCases.append(generator) @@ -84,13 +71,11 @@ class EnumGenerator { self.enumCases = enumCases enumCasesSortedByNumber = enumCases.sorted {$0.number < $1.number} self.defaultCase = self.enumCases[0] - self.path = path - self.comments = file.commentsFor(path: path) } func generateMainEnum(printer p: inout CodePrinter) { p.print("\n") - p.print(comments) + p.print(enumDescriptor.protoSourceComments()) p.print("\(visibility)enum \(swiftRelativeName): SwiftProtobuf.Enum {\n") p.indent() p.print("\(visibility)typealias RawValue = Int\n") @@ -99,7 +84,7 @@ class EnumGenerator { for c in enumCases { c.generateCaseOrAlias(printer: &p) } - if isProto3 { + if enumDescriptor.hasPreservingUnknownEnumSemantics { p.print("case \(unrecognizedCaseName)(Int)\n") } @@ -158,7 +143,7 @@ class EnumGenerator { for c in enumCasesSortedByNumber where !c.isAlias { p.print("case \(c.number): self = .\(c.swiftName)\n") } - if isProto3 { + if enumDescriptor.hasPreservingUnknownEnumSemantics { p.print("default: self = .\(unrecognizedCaseName)(rawValue)\n") } else { p.print("default: return nil\n") @@ -178,7 +163,7 @@ class EnumGenerator { for c in enumCasesSortedByNumber where !c.isAlias { p.print("case .\(c.swiftName): return \(c.number)\n") } - if isProto3 { + if enumDescriptor.hasPreservingUnknownEnumSemantics { p.print("case .\(unrecognizedCaseName)(let i): return i\n") } p.print("}\n") diff --git a/Sources/protoc-gen-swift/ExtensionGenerator.swift b/Sources/protoc-gen-swift/ExtensionGenerator.swift index 7b822e80f..400bcfe81 100644 --- a/Sources/protoc-gen-swift/ExtensionGenerator.swift +++ b/Sources/protoc-gen-swift/ExtensionGenerator.swift @@ -22,7 +22,6 @@ struct ExtensionGenerator { private let fieldDescriptor: FieldDescriptor private let generatorOptions: GeneratorOptions - let path: [Int32] let protoPackageName: String let swiftDeclaringMessageName: String? let context: Context @@ -68,18 +67,17 @@ struct ExtensionGenerator { } } - init(descriptor: FieldDescriptor, generatorOptions: GeneratorOptions, path: [Int32], parentProtoPath: String?, swiftDeclaringMessageName: String?, file: FileGenerator, context: Context) { + init(descriptor: FieldDescriptor, generatorOptions: GeneratorOptions, parentProtoPath: String?, swiftDeclaringMessageName: String?, file: FileGenerator, context: Context) { self.fieldDescriptor = descriptor self.generatorOptions = file.generatorOptions let proto = descriptor.proto - self.path = path self.protoPackageName = file.protoPackageName self.swiftDeclaringMessageName = swiftDeclaringMessageName self.swiftExtendedMessageName = context.getMessageNameForPath(path: proto.extendee)! self.context = context self.apiType = proto.getSwiftApiType(context: context, isProto3: false) - self.comments = file.commentsFor(path: path) + self.comments = descriptor.protoSourceComments() self.fieldName = proto.isGroup ? proto.bareTypeName : proto.name if let parentProtoPath = parentProtoPath, !parentProtoPath.isEmpty { var p = parentProtoPath @@ -160,7 +158,7 @@ struct ExtensionGenerator { p.print("extension \(swiftExtendedMessageName) {\n") p.indent() - p.print(comments) + p.print(comments) p.print("\(generatorOptions.visibilitySourceSnippet)var \(swiftFieldName): \(apiType) {\n") p.indent() if fieldDescriptor.proto.label == .repeated { diff --git a/Sources/protoc-gen-swift/FileGenerator.swift b/Sources/protoc-gen-swift/FileGenerator.swift index 5165f9c66..e8a7bf238 100644 --- a/Sources/protoc-gen-swift/FileGenerator.swift +++ b/Sources/protoc-gen-swift/FileGenerator.swift @@ -218,15 +218,18 @@ class FileGenerator { // the file is an empty path. That never seems to have comments on it. // https://github.com/google/protobuf/issues/2249 opened to figure out // the right way to do this since the syntax entry is optional. - let comments = commentsFor(path: [Google_Protobuf_FileDescriptorProto.FieldNumbers.syntax], - includeLeadingDetached: true) - if !comments.isEmpty { - p.print(comments) - // If the was a leading or tailing comment it won't have a blank - // line, after it, so ensure there is one. - if !comments.hasSuffix("\n\n") { - p.print("\n") - } + let syntaxPath = [Google_Protobuf_FileDescriptorProto.FieldNumbers.syntax] + if let syntaxLocation = fileDescriptor.sourceCodeInfoLocation(path: syntaxPath) { + let comments = syntaxLocation.asSourceComment(commentPrefix: "///", + leadingDetachedPrefix: "//") + if !comments.isEmpty { + p.print(comments) + // If the was a leading or tailing comment it won't have a blank + // line, after it, so ensure there is one. + if !comments.hasSuffix("\n\n") { + p.print("\n") + } + } } p.print("import Foundation\n") @@ -240,16 +243,13 @@ class FileGenerator { generateVersionCheck(printer: &p) var enums = [EnumGenerator]() - let path = [Int32]() - var i: Int32 = 0 for e in fileDescriptor.enums { - let enumPath = path + [5, i] - i += 1 - enums.append(EnumGenerator(descriptor: e, generatorOptions: generatorOptions, path: enumPath, parentSwiftName: nil, file: self)) + enums.append(EnumGenerator(descriptor: e, generatorOptions: generatorOptions, parentSwiftName: nil, file: self)) } var messages = [MessageGenerator]() - i = 0 + var i: Int32 = 0 + let path = [Int32]() for m in fileDescriptor.messages { let messagePath = path + [4, i] i += 1 @@ -257,11 +257,8 @@ class FileGenerator { } var extensions = [ExtensionGenerator]() - i = 0 for e in fileDescriptor.extensions { - let extPath = path + [7, i] - i += 1 - extensions.append(ExtensionGenerator(descriptor: e, generatorOptions: generatorOptions, path: extPath, parentProtoPath: fileDescriptor.proto.protoPath, swiftDeclaringMessageName: nil, file: self, context: context)) + extensions.append(ExtensionGenerator(descriptor: e, generatorOptions: generatorOptions, parentProtoPath: fileDescriptor.proto.protoPath, swiftDeclaringMessageName: nil, file: self, context: context)) } for e in enums { diff --git a/Sources/protoc-gen-swift/MessageGenerator.swift b/Sources/protoc-gen-swift/MessageGenerator.swift index 73a6a4541..f05e99c24 100644 --- a/Sources/protoc-gen-swift/MessageGenerator.swift +++ b/Sources/protoc-gen-swift/MessageGenerator.swift @@ -41,7 +41,6 @@ class MessageGenerator { private let isAnyMessage: Bool private let path: [Int32] - private let comments: String init( descriptor: Descriptor, @@ -92,14 +91,9 @@ class MessageGenerator { fieldsSortedByNumber = fields.sorted {$0.number < $1.number} let sortedFieldNumbers = fieldsSortedByNumber.map { $0.number } - i = 0 var extensions = [ExtensionGenerator]() for e in descriptor.extensions { - var extPath = path - extPath.append(Google_Protobuf_DescriptorProto.FieldNumbers.extension) - extPath.append(i) - i += 1 - extensions.append(ExtensionGenerator(descriptor: e, generatorOptions: generatorOptions, path: extPath, parentProtoPath: protoFullName, swiftDeclaringMessageName: swiftFullName, file: file, context: context)) + extensions.append(ExtensionGenerator(descriptor: e, generatorOptions: generatorOptions, parentProtoPath: protoFullName, swiftDeclaringMessageName: swiftFullName, file: file, context: context)) } self.extensions = extensions @@ -125,7 +119,7 @@ class MessageGenerator { enumPath.append(Google_Protobuf_DescriptorProto.FieldNumbers.enumType) enumPath.append(i) i += 1 - enums.append(EnumGenerator(descriptor: e, generatorOptions: generatorOptions, path: enumPath, parentSwiftName: swiftFullName, file: file)) + enums.append(EnumGenerator(descriptor: e, generatorOptions: generatorOptions, parentSwiftName: swiftFullName, file: file)) } self.enums = enums @@ -141,7 +135,6 @@ class MessageGenerator { self.messages = messages self.path = path - self.comments = file.commentsFor(path: path) // NOTE: This check for fields.count likely isn't completely correct // when the message has one or more oneof{}s. As that will efficively @@ -173,7 +166,7 @@ class MessageGenerator { func generateMainStruct(printer p: inout CodePrinter, file: FileGenerator, parent: MessageGenerator?) { p.print( "\n", - comments, + descriptor.protoSourceComments(), "\(visibility)struct \(swiftRelativeName): \(swiftMessageConformance) {\n") p.indent() if let parent = parent { From 6821af3f45b29643e222aad016445e2550b5633d Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Thu, 27 Apr 2017 16:57:04 -0400 Subject: [PATCH 10/16] Move MessageField and Oneof Generators over to using Descriptors in init. Only update how comments are done to finish removing all the old comment paths. --- .../protoc-gen-swift/ExtensionGenerator.swift | 2 +- Sources/protoc-gen-swift/FileGenerator.swift | 14 +------- .../MessageFieldGenerator.swift | 33 ++++++++++--------- .../protoc-gen-swift/MessageGenerator.swift | 15 +++------ Sources/protoc-gen-swift/OneofGenerator.swift | 17 +++++----- 5 files changed, 33 insertions(+), 48 deletions(-) diff --git a/Sources/protoc-gen-swift/ExtensionGenerator.swift b/Sources/protoc-gen-swift/ExtensionGenerator.swift index 400bcfe81..d0861f4f0 100644 --- a/Sources/protoc-gen-swift/ExtensionGenerator.swift +++ b/Sources/protoc-gen-swift/ExtensionGenerator.swift @@ -69,7 +69,7 @@ struct ExtensionGenerator { init(descriptor: FieldDescriptor, generatorOptions: GeneratorOptions, parentProtoPath: String?, swiftDeclaringMessageName: String?, file: FileGenerator, context: Context) { self.fieldDescriptor = descriptor - self.generatorOptions = file.generatorOptions + self.generatorOptions = generatorOptions let proto = descriptor.proto self.protoPackageName = file.protoPackageName diff --git a/Sources/protoc-gen-swift/FileGenerator.swift b/Sources/protoc-gen-swift/FileGenerator.swift index e8a7bf238..a878a44ea 100644 --- a/Sources/protoc-gen-swift/FileGenerator.swift +++ b/Sources/protoc-gen-swift/FileGenerator.swift @@ -159,7 +159,7 @@ extension Google_Protobuf_FileDescriptorProto { class FileGenerator { private let fileDescriptor: FileDescriptor - let generatorOptions: GeneratorOptions // TODO(private)### + private let generatorOptions: GeneratorOptions var outputFilename: String { let ext = ".pb.swift" @@ -188,18 +188,6 @@ class FileGenerator { private var isWellKnownType: Bool {return fileDescriptor.proto.isWellKnownType} private var baseFilename: String {return fileDescriptor.proto.baseFilename} - func commentsFor(path: [Int32], includeLeadingDetached: Bool = false) -> String { - if let location = fileDescriptor.sourceCodeInfoLocation(path: path) { - var detachedPrefix: String? = nil - if includeLeadingDetached { - detachedPrefix = "//" - } - return location.asSourceComment(commentPrefix: "///", - leadingDetachedPrefix: detachedPrefix) - } - return String() - } - func generateOutputFile(printer p: inout CodePrinter, context: Context) { p.print( "/*\n", diff --git a/Sources/protoc-gen-swift/MessageFieldGenerator.swift b/Sources/protoc-gen-swift/MessageFieldGenerator.swift index 57323c26f..551912af2 100644 --- a/Sources/protoc-gen-swift/MessageFieldGenerator.swift +++ b/Sources/protoc-gen-swift/MessageFieldGenerator.swift @@ -273,7 +273,10 @@ extension Google_Protobuf_FieldDescriptorProto { } struct MessageFieldGenerator { - let descriptor: Google_Protobuf_FieldDescriptorProto + private let fieldDescriptor: FieldDescriptor + private let generatorOptions: GeneratorOptions + + var descriptor: Google_Protobuf_FieldDescriptorProto { return fieldDescriptor.proto } let oneof: Google_Protobuf_OneofDescriptorProto? let jsonName: String? let swiftName: String @@ -282,22 +285,22 @@ struct MessageFieldGenerator { let swiftStorageName: String var protoName: String {return descriptor.name} var number: Int {return Int(descriptor.number)} - let path: [Int32] let comments: String let isProto3: Bool let context: Context - let generatorOptions: GeneratorOptions - init(descriptor: Google_Protobuf_FieldDescriptorProto, - path: [Int32], - messageDescriptor: Google_Protobuf_DescriptorProto, - file: FileGenerator, - context: Context) + init(descriptor: FieldDescriptor, + generatorOptions: GeneratorOptions, + messageDescriptor: Google_Protobuf_DescriptorProto, + file: FileGenerator, + context: Context) { - self.descriptor = descriptor - self.jsonName = descriptor.jsonName + self.fieldDescriptor = descriptor + self.generatorOptions = generatorOptions + + self.jsonName = descriptor.proto.jsonName if descriptor.type == .group { - let g = context.getMessageForPath(path: descriptor.typeName)! + let g = context.getMessageForPath(path: descriptor.proto.typeName)! let lowerName = toLowerCamelCase(g.name) self.swiftName = sanitizeFieldName(lowerName) let sanitizedUpper = sanitizeFieldName(toUpperCamelCase(g.name), basedOn: lowerName) @@ -310,17 +313,15 @@ struct MessageFieldGenerator { self.swiftHasName = "has" + sanitizedUpper self.swiftClearName = "clear" + sanitizedUpper } - if descriptor.hasOneofIndex { - self.oneof = messageDescriptor.oneofDecl[Int(descriptor.oneofIndex)] + if descriptor.proto.hasOneofIndex { + self.oneof = messageDescriptor.oneofDecl[Int(descriptor.proto.oneofIndex)] } else { self.oneof = nil } self.swiftStorageName = "_" + self.swiftName - self.path = path - self.comments = file.commentsFor(path: path) + self.comments = descriptor.protoSourceComments() self.isProto3 = file.isProto3 self.context = context - self.generatorOptions = file.generatorOptions } var fieldMapNames: String { diff --git a/Sources/protoc-gen-swift/MessageGenerator.swift b/Sources/protoc-gen-swift/MessageGenerator.swift index f05e99c24..a2bf68f6f 100644 --- a/Sources/protoc-gen-swift/MessageGenerator.swift +++ b/Sources/protoc-gen-swift/MessageGenerator.swift @@ -78,14 +78,9 @@ class MessageGenerator { } self.swiftMessageConformance = conformance.joined(separator: ", ") - var i: Int32 = 0 var fields = [MessageFieldGenerator]() - for f in proto.field { - var fieldPath = path - fieldPath.append(Google_Protobuf_DescriptorProto.FieldNumbers.field) - fieldPath.append(i) - i += 1 - fields.append(MessageFieldGenerator(descriptor: f, path: fieldPath, messageDescriptor: proto, file: file, context: context)) + for f in descriptor.fields { + fields.append(MessageFieldGenerator(descriptor: f, generatorOptions: generatorOptions, messageDescriptor: proto, file: file, context: context)) } self.fields = fields fieldsSortedByNumber = fields.sorted {$0.number < $1.number} @@ -97,9 +92,9 @@ class MessageGenerator { } self.extensions = extensions - i = 0 + var i: Int32 = 0 var oneofs = [OneofGenerator]() - for o in proto.oneofDecl { + for o in descriptor.oneofs { let oneofFields = fields.filter { $0.descriptor.hasOneofIndex && $0.descriptor.oneofIndex == Int32(i) } @@ -107,7 +102,7 @@ class MessageGenerator { oneofPath.append(Google_Protobuf_DescriptorProto.FieldNumbers.oneofDecl) oneofPath.append(i) i += 1 - let oneof = OneofGenerator(descriptor: o, path: oneofPath, file: file, generatorOptions: generatorOptions, fields: oneofFields, swiftMessageFullName: swiftFullName, parentFieldNumbersSorted: sortedFieldNumbers, parentExtensionRanges: proto.extensionRange) + let oneof = OneofGenerator(descriptor: o, generatorOptions: generatorOptions, file: file, fields: oneofFields, swiftMessageFullName: swiftFullName, parentFieldNumbersSorted: sortedFieldNumbers, parentExtensionRanges: proto.extensionRange) oneofs.append(oneof) } self.oneofs = oneofs diff --git a/Sources/protoc-gen-swift/OneofGenerator.swift b/Sources/protoc-gen-swift/OneofGenerator.swift index db7694b71..458283e60 100644 --- a/Sources/protoc-gen-swift/OneofGenerator.swift +++ b/Sources/protoc-gen-swift/OneofGenerator.swift @@ -29,9 +29,10 @@ extension Google_Protobuf_OneofDescriptorProto { } class OneofGenerator { - let descriptor: Google_Protobuf_OneofDescriptorProto - let path: [Int32] - let generatorOptions: GeneratorOptions + private let oneofDescriptor: OneofDescriptor + private let generatorOptions: GeneratorOptions + + var descriptor: Google_Protobuf_OneofDescriptorProto { return oneofDescriptor.proto } let fields: [MessageFieldGenerator] let fieldsSortedByNumber: [MessageFieldGenerator] let swiftRelativeName: String @@ -40,16 +41,16 @@ class OneofGenerator { let comments: String let oneofIsContinuousInParent: Bool - init(descriptor: Google_Protobuf_OneofDescriptorProto, path: [Int32], file: FileGenerator, generatorOptions: GeneratorOptions, fields: [MessageFieldGenerator], swiftMessageFullName: String, parentFieldNumbersSorted: [Int], parentExtensionRanges: [Google_Protobuf_DescriptorProto.ExtensionRange]) { - self.descriptor = descriptor - self.path = path + init(descriptor: OneofDescriptor, generatorOptions: GeneratorOptions, file: FileGenerator, fields: [MessageFieldGenerator], swiftMessageFullName: String, parentFieldNumbersSorted: [Int], parentExtensionRanges: [Google_Protobuf_DescriptorProto.ExtensionRange]) { + self.oneofDescriptor = descriptor self.generatorOptions = generatorOptions + self.fields = fields self.fieldsSortedByNumber = fields.sorted {$0.number < $1.number} self.isProto3 = file.isProto3 - self.swiftRelativeName = sanitizeOneofTypeName(descriptor.swiftRelativeType) + self.swiftRelativeName = sanitizeOneofTypeName(descriptor.proto.swiftRelativeType) self.swiftFullName = swiftMessageFullName + "." + swiftRelativeName - self.comments = file.commentsFor(path: path) + self.comments = descriptor.protoSourceComments() let first = fieldsSortedByNumber.first!.number let last = fieldsSortedByNumber.last!.number From 2d483cb4a4e00c6413659ed035545172bd1ee556 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Fri, 28 Apr 2017 11:57:32 -0400 Subject: [PATCH 11/16] EnumValue's aliasOf has to be weak to avoid cycles. --- Sources/PluginLibrary/Descriptor.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/PluginLibrary/Descriptor.swift b/Sources/PluginLibrary/Descriptor.swift index 2c79c4d1e..ca2f164f8 100644 --- a/Sources/PluginLibrary/Descriptor.swift +++ b/Sources/PluginLibrary/Descriptor.swift @@ -265,7 +265,7 @@ public final class EnumValueDescriptor { public var number: Int32 { return proto.number } - public let aliasOf: EnumValueDescriptor? + public private(set) weak var aliasOf: EnumValueDescriptor? public fileprivate(set) var aliases: [EnumValueDescriptor] = [] fileprivate init(proto: Google_Protobuf_EnumValueDescriptorProto, From 8a9d06936b97fd38f6f47b21186d0d9ae3fe6dfc Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Fri, 28 Apr 2017 12:49:23 -0400 Subject: [PATCH 12/16] Remove the rest of the path building (leftover from old comments support). --- Sources/protoc-gen-swift/FileGenerator.swift | 6 +----- .../protoc-gen-swift/MessageGenerator.swift | 20 +------------------ 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/Sources/protoc-gen-swift/FileGenerator.swift b/Sources/protoc-gen-swift/FileGenerator.swift index a878a44ea..61ae4dcc7 100644 --- a/Sources/protoc-gen-swift/FileGenerator.swift +++ b/Sources/protoc-gen-swift/FileGenerator.swift @@ -236,12 +236,8 @@ class FileGenerator { } var messages = [MessageGenerator]() - var i: Int32 = 0 - let path = [Int32]() for m in fileDescriptor.messages { - let messagePath = path + [4, i] - i += 1 - messages.append(MessageGenerator(descriptor: m, generatorOptions: generatorOptions, path: messagePath, parentSwiftName: nil, parentProtoPath: fileDescriptor.proto.protoPath, file: self, context: context)) + messages.append(MessageGenerator(descriptor: m, generatorOptions: generatorOptions, parentSwiftName: nil, parentProtoPath: fileDescriptor.proto.protoPath, file: self, context: context)) } var extensions = [ExtensionGenerator]() diff --git a/Sources/protoc-gen-swift/MessageGenerator.swift b/Sources/protoc-gen-swift/MessageGenerator.swift index a2bf68f6f..12facf6a3 100644 --- a/Sources/protoc-gen-swift/MessageGenerator.swift +++ b/Sources/protoc-gen-swift/MessageGenerator.swift @@ -40,12 +40,9 @@ class MessageGenerator { private let isExtensible: Bool private let isAnyMessage: Bool - private let path: [Int32] - init( descriptor: Descriptor, generatorOptions: GeneratorOptions, - path: [Int32], parentSwiftName: String?, parentProtoPath: String?, file: FileGenerator, @@ -98,39 +95,24 @@ class MessageGenerator { let oneofFields = fields.filter { $0.descriptor.hasOneofIndex && $0.descriptor.oneofIndex == Int32(i) } - var oneofPath = path - oneofPath.append(Google_Protobuf_DescriptorProto.FieldNumbers.oneofDecl) - oneofPath.append(i) i += 1 let oneof = OneofGenerator(descriptor: o, generatorOptions: generatorOptions, file: file, fields: oneofFields, swiftMessageFullName: swiftFullName, parentFieldNumbersSorted: sortedFieldNumbers, parentExtensionRanges: proto.extensionRange) oneofs.append(oneof) } self.oneofs = oneofs - i = 0 var enums = [EnumGenerator]() for e in descriptor.enums { - var enumPath = path - enumPath.append(Google_Protobuf_DescriptorProto.FieldNumbers.enumType) - enumPath.append(i) - i += 1 enums.append(EnumGenerator(descriptor: e, generatorOptions: generatorOptions, parentSwiftName: swiftFullName, file: file)) } self.enums = enums - i = 0 var messages = [MessageGenerator]() for m in descriptor.messages where !m.isMapEntry { - var msgPath = path - msgPath.append(Google_Protobuf_DescriptorProto.FieldNumbers.nestedType) - msgPath.append(i) - i += 1 - messages.append(MessageGenerator(descriptor: m, generatorOptions: generatorOptions, path: msgPath, parentSwiftName: swiftFullName, parentProtoPath: protoFullName, file: file, context: context)) + messages.append(MessageGenerator(descriptor: m, generatorOptions: generatorOptions, parentSwiftName: swiftFullName, parentProtoPath: protoFullName, file: file, context: context)) } self.messages = messages - self.path = path - // NOTE: This check for fields.count likely isn't completely correct // when the message has one or more oneof{}s. As that will efficively // reduce the real number of fields and the message might not need heap From 3aebf685d712595b3d8a4f58d0674c40f3c792f7 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Mon, 1 May 2017 13:10:54 -0400 Subject: [PATCH 13/16] Rename hasPreservingUnknownEnumSemantics -> hasUnknownEnumPreservingSemantics --- Sources/PluginLibrary/Descriptor+Extensions.swift | 6 +++--- Sources/protoc-gen-swift/EnumGenerator.swift | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Sources/PluginLibrary/Descriptor+Extensions.swift b/Sources/PluginLibrary/Descriptor+Extensions.swift index 9a5b2ca95..3487a5527 100644 --- a/Sources/PluginLibrary/Descriptor+Extensions.swift +++ b/Sources/PluginLibrary/Descriptor+Extensions.swift @@ -13,7 +13,7 @@ import SwiftProtobuf extension FileDescriptor: ProvidesSourceCodeLocation { // True if this file should perserve unknown enums within the enum. - public var hasPreservingUnknownEnumSemantics: Bool { + public var hasUnknownEnumPreservingSemantics: Bool { return syntax == .proto3 } @@ -37,8 +37,8 @@ extension Descriptor: ProvidesLocationPath, ProvidesSourceCodeLocation { extension EnumDescriptor: ProvidesLocationPath, ProvidesSourceCodeLocation { // True if this enum should perserve unknown enums within the enum. - public var hasPreservingUnknownEnumSemantics: Bool { - return file.hasPreservingUnknownEnumSemantics + public var hasUnknownEnumPreservingSemantics: Bool { + return file.hasUnknownEnumPreservingSemantics } public func getLocationPath(path: inout [Int32]) { diff --git a/Sources/protoc-gen-swift/EnumGenerator.swift b/Sources/protoc-gen-swift/EnumGenerator.swift index 1be054506..2dd9e7534 100644 --- a/Sources/protoc-gen-swift/EnumGenerator.swift +++ b/Sources/protoc-gen-swift/EnumGenerator.swift @@ -84,7 +84,7 @@ class EnumGenerator { for c in enumCases { c.generateCaseOrAlias(printer: &p) } - if enumDescriptor.hasPreservingUnknownEnumSemantics { + if enumDescriptor.hasUnknownEnumPreservingSemantics { p.print("case \(unrecognizedCaseName)(Int)\n") } @@ -143,7 +143,7 @@ class EnumGenerator { for c in enumCasesSortedByNumber where !c.isAlias { p.print("case \(c.number): self = .\(c.swiftName)\n") } - if enumDescriptor.hasPreservingUnknownEnumSemantics { + if enumDescriptor.hasUnknownEnumPreservingSemantics { p.print("default: self = .\(unrecognizedCaseName)(rawValue)\n") } else { p.print("default: return nil\n") @@ -163,7 +163,7 @@ class EnumGenerator { for c in enumCasesSortedByNumber where !c.isAlias { p.print("case .\(c.swiftName): return \(c.number)\n") } - if enumDescriptor.hasPreservingUnknownEnumSemantics { + if enumDescriptor.hasUnknownEnumPreservingSemantics { p.print("case .\(unrecognizedCaseName)(let i): return i\n") } p.print("}\n") From 7c4df41edfa8741c30ac3b987baba23fd652d750 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Mon, 1 May 2017 13:12:34 -0400 Subject: [PATCH 14/16] Remove typealias for HashableInt32Array, wasn't buying much. --- Sources/PluginLibrary/Descriptor.swift | 4 ++-- Sources/PluginLibrary/HashableArray.swift | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/Sources/PluginLibrary/Descriptor.swift b/Sources/PluginLibrary/Descriptor.swift index ca2f164f8..dda5a83cc 100644 --- a/Sources/PluginLibrary/Descriptor.swift +++ b/Sources/PluginLibrary/Descriptor.swift @@ -145,8 +145,8 @@ public final class FileDescriptor { // Lazy so this can be computed on demand, as the imported files won't need // comments during generation. - private lazy var locationMap: [HashableInt32Array:Google_Protobuf_SourceCodeInfo.Location] = { - var result: [HashableInt32Array:Google_Protobuf_SourceCodeInfo.Location] = [:] + private lazy var locationMap: [HashableArray:Google_Protobuf_SourceCodeInfo.Location] = { + var result: [HashableArray:Google_Protobuf_SourceCodeInfo.Location] = [:] for loc in self.proto.sourceCodeInfo.location { result[HashableArray(loc.path)] = loc } diff --git a/Sources/PluginLibrary/HashableArray.swift b/Sources/PluginLibrary/HashableArray.swift index e204827ed..6b85c298b 100644 --- a/Sources/PluginLibrary/HashableArray.swift +++ b/Sources/PluginLibrary/HashableArray.swift @@ -25,5 +25,3 @@ struct HashableArray: Hashable { return lhs.hashValue == rhs.hashValue && lhs.array == rhs.array } } - -typealias HashableInt32Array = HashableArray From eaafacea5ab521f1f27bbe852d37fbe6f5f7de4d Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Mon, 1 May 2017 13:29:45 -0400 Subject: [PATCH 15/16] Convert comment lookup to using IndexPath. - Remove HashableArray.swift. --- .../PluginLibrary/Descriptor+Extensions.swift | 30 +++++++++---------- Sources/PluginLibrary/Descriptor.swift | 11 +++---- Sources/PluginLibrary/FieldNumbers.swift | 22 +++++++------- Sources/PluginLibrary/HashableArray.swift | 27 ----------------- .../PluginLibrary/ProvidesLocationPath.swift | 2 +- .../ProvidesSourceCodeLocation.swift | 2 +- Sources/protoc-gen-swift/FileGenerator.swift | 2 +- ...tobuf_FileDescriptorProto+Extensions.swift | 2 +- 8 files changed, 36 insertions(+), 62 deletions(-) delete mode 100644 Sources/PluginLibrary/HashableArray.swift diff --git a/Sources/PluginLibrary/Descriptor+Extensions.swift b/Sources/PluginLibrary/Descriptor+Extensions.swift index 3487a5527..ccb879541 100644 --- a/Sources/PluginLibrary/Descriptor+Extensions.swift +++ b/Sources/PluginLibrary/Descriptor+Extensions.swift @@ -19,19 +19,19 @@ extension FileDescriptor: ProvidesSourceCodeLocation { public var sourceCodeInfoLocation: Google_Protobuf_SourceCodeInfo.Location? { // google/protobuf's descriptor.cc says it should be an empty path. - return sourceCodeInfoLocation(path: []) + return sourceCodeInfoLocation(path: IndexPath()) } } extension Descriptor: ProvidesLocationPath, ProvidesSourceCodeLocation { - public func getLocationPath(path: inout [Int32]) { + public func getLocationPath(path: inout IndexPath) { if let containingType = containingType { containingType.getLocationPath(path: &path) path.append(Google_Protobuf_DescriptorProto.FieldNumbers.nestedType) } else { path.append(Google_Protobuf_FileDescriptorProto.FieldNumbers.messageType) } - path.append(Int32(index)) + path.append(index) } } @@ -41,39 +41,39 @@ extension EnumDescriptor: ProvidesLocationPath, ProvidesSourceCodeLocation { return file.hasUnknownEnumPreservingSemantics } - public func getLocationPath(path: inout [Int32]) { + public func getLocationPath(path: inout IndexPath) { if let containingType = containingType { containingType.getLocationPath(path: &path) path.append(Google_Protobuf_DescriptorProto.FieldNumbers.enumType) } else { path.append(Google_Protobuf_FileDescriptorProto.FieldNumbers.enumType) } - path.append(Int32(index)) + path.append(index) } } extension EnumValueDescriptor: ProvidesLocationPath, ProvidesSourceCodeLocation { public weak var file: FileDescriptor! { return enumType.file } - public func getLocationPath(path: inout [Int32]) { + public func getLocationPath(path: inout IndexPath) { enumType.getLocationPath(path: &path) path.append(Google_Protobuf_EnumDescriptorProto.FieldNumbers.value) - path.append(Int32(index)) + path.append(index) } } extension OneofDescriptor: ProvidesLocationPath, ProvidesSourceCodeLocation { public weak var file: FileDescriptor! { return containingType.file } - public func getLocationPath(path: inout [Int32]) { + public func getLocationPath(path: inout IndexPath) { containingType.getLocationPath(path: &path) path.append(Google_Protobuf_DescriptorProto.FieldNumbers.oneofDecl) - path.append(Int32(index)) + path.append(index) } } extension FieldDescriptor: ProvidesLocationPath, ProvidesSourceCodeLocation { - public func getLocationPath(path: inout [Int32]) { + public func getLocationPath(path: inout IndexPath) { if isExtension { if let extensionScope = extensionScope { extensionScope.getLocationPath(path: &path) @@ -85,23 +85,23 @@ extension FieldDescriptor: ProvidesLocationPath, ProvidesSourceCodeLocation { containingType.getLocationPath(path: &path) path.append(Google_Protobuf_DescriptorProto.FieldNumbers.field) } - path.append(Int32(index)) + path.append(index) } } extension ServiceDescriptor: ProvidesLocationPath, ProvidesSourceCodeLocation { - public func getLocationPath(path: inout [Int32]) { + public func getLocationPath(path: inout IndexPath) { path.append(Google_Protobuf_FileDescriptorProto.FieldNumbers.service) - path.append(Int32(index)) + path.append(index) } } extension MethodDescriptor: ProvidesLocationPath, ProvidesSourceCodeLocation { public weak var file: FileDescriptor! { return service.file } - public func getLocationPath(path: inout [Int32]) { + public func getLocationPath(path: inout IndexPath) { service.getLocationPath(path: &path) path.append(Google_Protobuf_ServiceDescriptorProto.FieldNumbers.method) - path.append(Int32(index)) + path.append(index) } } diff --git a/Sources/PluginLibrary/Descriptor.swift b/Sources/PluginLibrary/Descriptor.swift index dda5a83cc..1aab67612 100644 --- a/Sources/PluginLibrary/Descriptor.swift +++ b/Sources/PluginLibrary/Descriptor.swift @@ -136,8 +136,8 @@ public final class FileDescriptor { // TODO(thomasvl): Eventually hide this and just expose it info off the descriptors so // paths aren't needed externally. - public func sourceCodeInfoLocation(path: [Int32]) -> Google_Protobuf_SourceCodeInfo.Location? { - guard let location = locationMap[HashableArray(path)] else { + public func sourceCodeInfoLocation(path: IndexPath) -> Google_Protobuf_SourceCodeInfo.Location? { + guard let location = locationMap[path] else { return nil } return location @@ -145,10 +145,11 @@ public final class FileDescriptor { // Lazy so this can be computed on demand, as the imported files won't need // comments during generation. - private lazy var locationMap: [HashableArray:Google_Protobuf_SourceCodeInfo.Location] = { - var result: [HashableArray:Google_Protobuf_SourceCodeInfo.Location] = [:] + private lazy var locationMap: [IndexPath:Google_Protobuf_SourceCodeInfo.Location] = { + var result: [IndexPath:Google_Protobuf_SourceCodeInfo.Location] = [:] for loc in self.proto.sourceCodeInfo.location { - result[HashableArray(loc.path)] = loc + let intList = loc.path.map { return Int($0) } + result[IndexPath(indexes: intList)] = loc } return result }() diff --git a/Sources/PluginLibrary/FieldNumbers.swift b/Sources/PluginLibrary/FieldNumbers.swift index 61a467719..66cd0dcbe 100644 --- a/Sources/PluginLibrary/FieldNumbers.swift +++ b/Sources/PluginLibrary/FieldNumbers.swift @@ -16,31 +16,31 @@ import Foundation extension Google_Protobuf_FileDescriptorProto { struct FieldNumbers { - static let messageType: Int32 = 4 - static let enumType: Int32 = 5 - static let service: Int32 = 6 - static let `extension`: Int32 = 7 + static let messageType: Int = 4 + static let enumType: Int = 5 + static let service: Int = 6 + static let `extension`: Int = 7 } } extension Google_Protobuf_DescriptorProto { struct FieldNumbers { - static let field: Int32 = 2 - static let nestedType: Int32 = 3 - static let enumType: Int32 = 4 - static let `extension`: Int32 = 6 - static let oneofDecl: Int32 = 8 + static let field: Int = 2 + static let nestedType: Int = 3 + static let enumType: Int = 4 + static let `extension`: Int = 6 + static let oneofDecl: Int = 8 } } extension Google_Protobuf_EnumDescriptorProto { struct FieldNumbers { - static let value: Int32 = 2 + static let value: Int = 2 } } extension Google_Protobuf_ServiceDescriptorProto { struct FieldNumbers { - static let method: Int32 = 2 + static let method: Int = 2 } } diff --git a/Sources/PluginLibrary/HashableArray.swift b/Sources/PluginLibrary/HashableArray.swift deleted file mode 100644 index 6b85c298b..000000000 --- a/Sources/PluginLibrary/HashableArray.swift +++ /dev/null @@ -1,27 +0,0 @@ -// Sources/PluginLibrary/HashableArray.swift - Wrapper array to support hashing -// -// Copyright (c) 2014 - 2017 Apple Inc. and the project authors -// Licensed under Apache License v2.0 with Runtime Library Exception -// -// See LICENSE.txt for license information: -// https://github.com/apple/swift-protobuf/blob/master/LICENSE.txt -// -// ----------------------------------------------------------------------------- - -/// Helper type to use an array as a dictionary key. -struct HashableArray: Hashable { - let array: [T] - let hashValue: Int - - init(_ array: [T]) { - self.array = array - var hash = Int(bitPattern: 2166136261) - for i in array { - hash = (hash &* Int(16777619)) ^ i.hashValue - } - hashValue = hash - } - static func ==(lhs: HashableArray, rhs: HashableArray) -> Bool { - return lhs.hashValue == rhs.hashValue && lhs.array == rhs.array - } -} diff --git a/Sources/PluginLibrary/ProvidesLocationPath.swift b/Sources/PluginLibrary/ProvidesLocationPath.swift index 55d9a0bcc..05e313664 100644 --- a/Sources/PluginLibrary/ProvidesLocationPath.swift +++ b/Sources/PluginLibrary/ProvidesLocationPath.swift @@ -11,6 +11,6 @@ import Foundation public protocol ProvidesLocationPath { - func getLocationPath(path: inout [Int32]) + func getLocationPath(path: inout IndexPath) weak var file: FileDescriptor! { get } } diff --git a/Sources/PluginLibrary/ProvidesSourceCodeLocation.swift b/Sources/PluginLibrary/ProvidesSourceCodeLocation.swift index 4689d2f14..c97d30590 100644 --- a/Sources/PluginLibrary/ProvidesSourceCodeLocation.swift +++ b/Sources/PluginLibrary/ProvidesSourceCodeLocation.swift @@ -17,7 +17,7 @@ public protocol ProvidesSourceCodeLocation { // Default implementation for things that support ProvidesLocationPath. extension ProvidesSourceCodeLocation where Self: ProvidesLocationPath { public var sourceCodeInfoLocation: Google_Protobuf_SourceCodeInfo.Location? { - var path = [Int32]() + var path = IndexPath() getLocationPath(path: &path) return file.sourceCodeInfoLocation(path: path) } diff --git a/Sources/protoc-gen-swift/FileGenerator.swift b/Sources/protoc-gen-swift/FileGenerator.swift index 61ae4dcc7..34386308c 100644 --- a/Sources/protoc-gen-swift/FileGenerator.swift +++ b/Sources/protoc-gen-swift/FileGenerator.swift @@ -206,7 +206,7 @@ class FileGenerator { // the file is an empty path. That never seems to have comments on it. // https://github.com/google/protobuf/issues/2249 opened to figure out // the right way to do this since the syntax entry is optional. - let syntaxPath = [Google_Protobuf_FileDescriptorProto.FieldNumbers.syntax] + let syntaxPath = IndexPath(index: Google_Protobuf_FileDescriptorProto.FieldNumbers.syntax) if let syntaxLocation = fileDescriptor.sourceCodeInfoLocation(path: syntaxPath) { let comments = syntaxLocation.asSourceComment(commentPrefix: "///", leadingDetachedPrefix: "//") diff --git a/Sources/protoc-gen-swift/Google_Protobuf_FileDescriptorProto+Extensions.swift b/Sources/protoc-gen-swift/Google_Protobuf_FileDescriptorProto+Extensions.swift index 73e02bc76..b122a9dd8 100644 --- a/Sources/protoc-gen-swift/Google_Protobuf_FileDescriptorProto+Extensions.swift +++ b/Sources/protoc-gen-swift/Google_Protobuf_FileDescriptorProto+Extensions.swift @@ -19,6 +19,6 @@ import SwiftProtobuf extension Google_Protobuf_FileDescriptorProto { // Field numbers used to collect .proto file comments. struct FieldNumbers { - static let syntax: Int32 = 12 + static let syntax: Int = 12 } } From bb55257e1683eb3e5e0804a1249b4776e3e03e08 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Mon, 1 May 2017 13:37:17 -0400 Subject: [PATCH 16/16] Add note about performance as to why enumerated().map() wasn't used. --- Sources/PluginLibrary/Array+Extensions.swift | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Sources/PluginLibrary/Array+Extensions.swift b/Sources/PluginLibrary/Array+Extensions.swift index bc3486d0e..bec466309 100644 --- a/Sources/PluginLibrary/Array+Extensions.swift +++ b/Sources/PluginLibrary/Array+Extensions.swift @@ -11,7 +11,22 @@ import Foundation extension Array { + /// Like map, but calls the transform with the index and value. + /// + /// NOTE: It would seem like doing: + /// return self.enumerated().map { + /// return try transform($0.index, $0.element) + /// } + /// would seem like a simple thing to avoid extension. However as of Xcode 8.3.2 + /// (Swift 3.1), building/running 5000000 interation test (macOS) of the differences + /// are rather large - + /// Release build: + /// Using enumerated: 3.694987967 + /// Using enumeratedMap: 0.961241992 + /// Debug build: + /// Using enumerated: 20.038512905 + /// Using enumeratedMap: 8.521299144 func enumeratedMap(_ transform: (Int, Element) throws -> T) rethrows -> [T] { var i: Int = -1 return try map {