Skip to content

Commit

Permalink
Merge pull request #1666 from gjcairo/breaking-changes-plugin-lib
Browse files Browse the repository at this point in the history
Undo breaking changes in SwiftProtobufPluginLibrary
  • Loading branch information
thomasvl committed Jun 26, 2024
2 parents de2ea22 + ac31b5c commit 7201e81
Show file tree
Hide file tree
Showing 13 changed files with 447 additions and 160 deletions.
8 changes: 7 additions & 1 deletion Sources/SwiftProtobufPluginLibrary/CodePrinter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ public struct CodePrinter {
/// print apis.
private let newlines: Bool

public init(indent: String.UnicodeScalarView = " ".unicodeScalars) {
contentScalars.reserveCapacity(CodePrinter.initialBufferSize)
singleIndent = indent
newlines = false
}

/// Initialize the printer for use.
///
/// - Parameters:
Expand All @@ -56,7 +62,7 @@ public struct CodePrinter {
/// should automatically add newlines to the end of the strings.
public init(
indent: String.UnicodeScalarView = " ".unicodeScalars,
addNewlines newlines: Bool = false
addNewlines newlines: Bool
) {
contentScalars.reserveCapacity(CodePrinter.initialBufferSize)
singleIndent = indent
Expand Down
310 changes: 274 additions & 36 deletions Sources/SwiftProtobufPluginLibrary/Descriptor.swift

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public protocol TypeOrFileProvidesDeprecationComment: ProvidesDeprecationComment
/// If the type is deprecated.
var isDeprecated: Bool { get }
/// Returns the File this conforming object is in.
var file: FileDescriptor { get }
var file: FileDescriptor! { get }
}

extension TypeOrFileProvidesDeprecationComment {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@ public protocol ProvidesLocationPath {
/// `GetSourceLocation()` in the C++ Descriptor apis.
func getLocationPath(path: inout IndexPath)
/// Returns the File this conforming object is in.
var file: FileDescriptor { get }
var file: FileDescriptor! { get }
}
38 changes: 33 additions & 5 deletions Sources/SwiftProtobufPluginLibrary/SwiftProtobufNamer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ public final class SwiftProtobufNamer {
for (camelCased, enumValues) in candidates {
// If there is only one, sanitize and cache it.
guard enumValues.count > 1 else {
enumValueRelativeNameCache[enumValues.first!.fullName] =
NamingUtils.sanitize(enumCaseName: camelCased)
let fullName = enumValues.first!.fullName
enumValueRelativeNameCache[fullName] = NamingUtils.sanitize(enumCaseName: camelCased)
continue
}

Expand All @@ -135,7 +135,8 @@ public final class SwiftProtobufNamer {
// to the same name.
let name = NamingUtils.sanitize(enumCaseName: camelCased)
for e in enumValues {
enumValueRelativeNameCache[e.fullName] = name
let fullName = e.fullName
enumValueRelativeNameCache[fullName] = name
}
continue
}
Expand All @@ -144,8 +145,8 @@ public final class SwiftProtobufNamer {
// Can't put a negative size, so use "n" and make the number
// positive.
let suffix = e.number >= 0 ? "_\(e.number)" : "_n\(-e.number)"
enumValueRelativeNameCache[e.fullName] =
NamingUtils.sanitize(enumCaseName: camelCased + suffix)
let fullName = e.fullName
enumValueRelativeNameCache[fullName] = NamingUtils.sanitize(enumCaseName: camelCased + suffix)
}
}
}
Expand All @@ -171,6 +172,33 @@ public final class SwiftProtobufNamer {
return "." + NamingUtils.trimBackticks(relativeName)
}

/// Filters the Enum's values to those that will have unique Swift
/// names. Only poorly named proto enum alias values get filtered
/// away, so the assumption is they aren't really needed from an
/// api pov.
@available(*, deprecated, message: "Please open a GitHub issue if you think functionality is missing.")
public func uniquelyNamedValues(enum e: EnumDescriptor) -> [EnumValueDescriptor] {
return e.values.filter {
// Original are kept as is. The computations for relative
// name already adds values for collisions with different
// values.
guard let aliasOf = $0.aliasOf else { return true }
let relativeName = self.relativeName(enumValue: $0)
let aliasOfRelativeName = self.relativeName(enumValue: aliasOf)
// If the relative name matches for the alias and original, drop
// the alias.
guard relativeName != aliasOfRelativeName else { return false }
// Only include this alias if it is the first one with this name.
// (handles alias with different cases in their names that get
// mangled to a single Swift name.)
let firstAlias = aliasOf.aliases.firstIndex {
let otherRelativeName = self.relativeName(enumValue: $0)
return relativeName == otherRelativeName
}
return aliasOf.aliases[firstAlias!] === $0
}
}

/// Calculate the relative name for the given oneof.
public func relativeName(oneof: OneofDescriptor) -> String {
let camelCase = NamingUtils.toUpperCamelCase(oneof.name)
Expand Down
10 changes: 5 additions & 5 deletions Sources/protoc-gen-swift/Descriptor+Extensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ extension Descriptor {

// If it can support extensions, then return true as an extension could
// have a required field.
if !descriptor.extensionRanges.isEmpty {
if !descriptor.messageExtensionRanges.isEmpty {
return true
}

Expand All @@ -209,8 +209,8 @@ extension Descriptor {
///
/// This also uses Range<> since the options that could be on
/// `extensionRanges` no longer can apply as the things have been merged.
var normalizedExtensionRanges: [Range<Int32>] {
var ordered: [Range<Int32>] = self.extensionRanges.sorted(by: {
var _normalizedExtensionRanges: [Range<Int32>] {
var ordered: [Range<Int32>] = self.messageExtensionRanges.sorted(by: {
return $0.start < $1.start }).map { return $0.start ..< $0.end
}
if ordered.count > 1 {
Expand All @@ -231,8 +231,8 @@ extension Descriptor {
///
/// This also uses Range<> since the options that could be on
/// `extensionRanges` no longer can apply as the things have been merged.
var ambitiousExtensionRanges: [Range<Int32>] {
var merged = self.normalizedExtensionRanges
var _ambitiousExtensionRanges: [Range<Int32>] {
var merged = self._normalizedExtensionRanges
if merged.count > 1 {
var fieldNumbersReversedIterator =
self.fields.map({ Int($0.number) }).sorted(by: { $0 > $1 }).makeIterator()
Expand Down
4 changes: 2 additions & 2 deletions Sources/protoc-gen-swift/FieldGenerator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class FieldGeneratorBase {
result = ".standard(proto: \"\(protoName)\")"
} else {
/// The library's generation didn't match, so specify this explicitly.
result = ".unique(proto: \"\(protoName)\", json: \"\(jsonName)\")"
result = ".unique(proto: \"\(protoName)\", json: \"\(jsonName ?? "")\")"
}
}

Expand All @@ -105,7 +105,7 @@ class FieldGeneratorBase {
if nameLowercase == jsonName {
return [".same(proto: \"\(nameLowercase)\")", result]
} else {
return [".unique(proto: \"\(nameLowercase)\", json: \"\(jsonName)\")", result]
return [".unique(proto: \"\(nameLowercase)\", json: \"\(jsonName ?? "")\")", result]
}
} else {
return [result]
Expand Down
6 changes: 3 additions & 3 deletions Sources/protoc-gen-swift/MessageGenerator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class MessageGenerator {
self.namer = namer

visibility = generatorOptions.visibilitySourceSnippet
isExtensible = !descriptor.extensionRanges.isEmpty
isExtensible = !descriptor.messageExtensionRanges.isEmpty
swiftRelativeName = namer.relativeName(message: descriptor)
swiftFullName = namer.fullName(message: descriptor)

Expand Down Expand Up @@ -284,7 +284,7 @@ class MessageGenerator {
// code. This also avoids typechecking performance issues if there are
// dozens of ranges because we aren't constructing a single large
// expression containing untyped integer literals.
let normalizedExtensionRanges = descriptor.normalizedExtensionRanges
let normalizedExtensionRanges = descriptor._normalizedExtensionRanges
if !fields.isEmpty || normalizedExtensionRanges.count > 3 {
p.print("""
// The use of inline closures is to circumvent an issue where the compiler
Expand Down Expand Up @@ -346,7 +346,7 @@ class MessageGenerator {
// Use the "ambitious" ranges because for visit because subranges with no
// intermixed fields can be merged to reduce the number of calls for
// extension visitation.
var ranges = descriptor.ambitiousExtensionRanges.makeIterator()
var ranges = descriptor._ambitiousExtensionRanges.makeIterator()
var nextRange = ranges.next()
for f in fieldsSortedByNumber {
while nextRange != nil && Int(nextRange!.lowerBound) < f.number {
Expand Down
2 changes: 1 addition & 1 deletion Sources/protoc-gen-swift/OneofGenerator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ class OneofGenerator {
// from each extension range as an easy way to check for them being
// mixed in between the fields.
var parentNumbers = descriptor.containingType.fields.map { Int($0.number) }
parentNumbers.append(contentsOf: descriptor.containingType.normalizedExtensionRanges.map { Int($0.lowerBound) })
parentNumbers.append(contentsOf: descriptor.containingType._normalizedExtensionRanges.map { Int($0.lowerBound) })
var parentNumbersIterator = parentNumbers.sorted(by: { $0 < $1 }).makeIterator()
var nextParentFieldNumber = parentNumbersIterator.next()
var grouped = [[MemberFieldGenerator]]()
Expand Down
64 changes: 32 additions & 32 deletions Tests/SwiftProtobufPluginLibraryTests/Test_Descriptor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -213,22 +213,22 @@ final class Test_Descriptor: XCTestCase {
XCTAssertEqual(proto2ForPresence.fields[14].name, "oneof_enum_field")
XCTAssertEqual(proto2ForPresence.fields[15].name, "oneof_message_field")

XCTAssertFalse(proto2ForPresence.fields[0].hasOptionalKeyword)
XCTAssertFalse(proto2ForPresence.fields[1].hasOptionalKeyword)
XCTAssertFalse(proto2ForPresence.fields[2].hasOptionalKeyword)
XCTAssertFalse(proto2ForPresence.fields[3].hasOptionalKeyword)
XCTAssertTrue(proto2ForPresence.fields[4].hasOptionalKeyword)
XCTAssertTrue(proto2ForPresence.fields[5].hasOptionalKeyword)
XCTAssertTrue(proto2ForPresence.fields[6].hasOptionalKeyword)
XCTAssertTrue(proto2ForPresence.fields[7].hasOptionalKeyword)
XCTAssertFalse(proto2ForPresence.fields[8].hasOptionalKeyword)
XCTAssertFalse(proto2ForPresence.fields[9].hasOptionalKeyword)
XCTAssertFalse(proto2ForPresence.fields[10].hasOptionalKeyword)
XCTAssertFalse(proto2ForPresence.fields[11].hasOptionalKeyword)
XCTAssertFalse(proto2ForPresence.fields[12].hasOptionalKeyword)
XCTAssertFalse(proto2ForPresence.fields[13].hasOptionalKeyword)
XCTAssertFalse(proto2ForPresence.fields[14].hasOptionalKeyword)
XCTAssertFalse(proto2ForPresence.fields[15].hasOptionalKeyword)
XCTAssertFalse(proto2ForPresence.fields[0]._hasOptionalKeyword)
XCTAssertFalse(proto2ForPresence.fields[1]._hasOptionalKeyword)
XCTAssertFalse(proto2ForPresence.fields[2]._hasOptionalKeyword)
XCTAssertFalse(proto2ForPresence.fields[3]._hasOptionalKeyword)
XCTAssertTrue(proto2ForPresence.fields[4]._hasOptionalKeyword)
XCTAssertTrue(proto2ForPresence.fields[5]._hasOptionalKeyword)
XCTAssertTrue(proto2ForPresence.fields[6]._hasOptionalKeyword)
XCTAssertTrue(proto2ForPresence.fields[7]._hasOptionalKeyword)
XCTAssertFalse(proto2ForPresence.fields[8]._hasOptionalKeyword)
XCTAssertFalse(proto2ForPresence.fields[9]._hasOptionalKeyword)
XCTAssertFalse(proto2ForPresence.fields[10]._hasOptionalKeyword)
XCTAssertFalse(proto2ForPresence.fields[11]._hasOptionalKeyword)
XCTAssertFalse(proto2ForPresence.fields[12]._hasOptionalKeyword)
XCTAssertFalse(proto2ForPresence.fields[13]._hasOptionalKeyword)
XCTAssertFalse(proto2ForPresence.fields[14]._hasOptionalKeyword)
XCTAssertFalse(proto2ForPresence.fields[15]._hasOptionalKeyword)

XCTAssertTrue(proto2ForPresence.fields[0].hasPresence)
XCTAssertTrue(proto2ForPresence.fields[1].hasPresence)
Expand Down Expand Up @@ -274,22 +274,22 @@ final class Test_Descriptor: XCTestCase {
XCTAssertEqual(proto3ForPresence.fields[14].name, "oneof_enum_field")
XCTAssertEqual(proto3ForPresence.fields[15].name, "oneof_message_field")

XCTAssertFalse(proto3ForPresence.fields[0].hasOptionalKeyword)
XCTAssertFalse(proto3ForPresence.fields[1].hasOptionalKeyword)
XCTAssertFalse(proto3ForPresence.fields[2].hasOptionalKeyword)
XCTAssertFalse(proto3ForPresence.fields[3].hasOptionalKeyword)
XCTAssertTrue(proto3ForPresence.fields[4].hasOptionalKeyword)
XCTAssertTrue(proto3ForPresence.fields[5].hasOptionalKeyword)
XCTAssertTrue(proto3ForPresence.fields[6].hasOptionalKeyword)
XCTAssertTrue(proto3ForPresence.fields[7].hasOptionalKeyword)
XCTAssertFalse(proto3ForPresence.fields[8].hasOptionalKeyword)
XCTAssertFalse(proto3ForPresence.fields[9].hasOptionalKeyword)
XCTAssertFalse(proto3ForPresence.fields[10].hasOptionalKeyword)
XCTAssertFalse(proto3ForPresence.fields[11].hasOptionalKeyword)
XCTAssertFalse(proto3ForPresence.fields[12].hasOptionalKeyword)
XCTAssertFalse(proto3ForPresence.fields[13].hasOptionalKeyword)
XCTAssertFalse(proto3ForPresence.fields[14].hasOptionalKeyword)
XCTAssertFalse(proto3ForPresence.fields[15].hasOptionalKeyword)
XCTAssertFalse(proto3ForPresence.fields[0]._hasOptionalKeyword)
XCTAssertFalse(proto3ForPresence.fields[1]._hasOptionalKeyword)
XCTAssertFalse(proto3ForPresence.fields[2]._hasOptionalKeyword)
XCTAssertFalse(proto3ForPresence.fields[3]._hasOptionalKeyword)
XCTAssertTrue(proto3ForPresence.fields[4]._hasOptionalKeyword)
XCTAssertTrue(proto3ForPresence.fields[5]._hasOptionalKeyword)
XCTAssertTrue(proto3ForPresence.fields[6]._hasOptionalKeyword)
XCTAssertTrue(proto3ForPresence.fields[7]._hasOptionalKeyword)
XCTAssertFalse(proto3ForPresence.fields[8]._hasOptionalKeyword)
XCTAssertFalse(proto3ForPresence.fields[9]._hasOptionalKeyword)
XCTAssertFalse(proto3ForPresence.fields[10]._hasOptionalKeyword)
XCTAssertFalse(proto3ForPresence.fields[11]._hasOptionalKeyword)
XCTAssertFalse(proto3ForPresence.fields[12]._hasOptionalKeyword)
XCTAssertFalse(proto3ForPresence.fields[13]._hasOptionalKeyword)
XCTAssertFalse(proto3ForPresence.fields[14]._hasOptionalKeyword)
XCTAssertFalse(proto3ForPresence.fields[15]._hasOptionalKeyword)

XCTAssertFalse(proto3ForPresence.fields[0].hasPresence)
XCTAssertFalse(proto3ForPresence.fields[1].hasPresence)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ final class Test_Descriptor_FeatureResolution: XCTestCase {
}
""")

let features = context.file.messages.first!.extensionRanges.first!.features
let features = context.file.messages.first!.messageExtensionRanges.first!.features
XCTAssertTrue(features.hasSwiftFeatureTest_test)
XCTAssertEqual(features.SwiftFeatureTest_test.feature1, .value1)
XCTAssertEqual(features.SwiftFeatureTest_test.feature2, .value1)
Expand Down Expand Up @@ -300,7 +300,7 @@ final class Test_Descriptor_FeatureResolution: XCTestCase {
}
""")

let features = context.file.messages.first!.extensionRanges.first!.features
let features = context.file.messages.first!.messageExtensionRanges.first!.features
XCTAssertTrue(features.hasSwiftFeatureTest_test)
XCTAssertEqual(features.SwiftFeatureTest_test.feature1, .value2) // File override
XCTAssertEqual(features.SwiftFeatureTest_test.feature2, .value3) // Message override
Expand Down
Loading

0 comments on commit 7201e81

Please sign in to comment.