Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Undo breaking changes in SwiftProtobufPluginLibrary #1666

Merged
merged 4 commits into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading