Skip to content

Skip assigning unrecognized enum values in generated Swift decode - #3725

Merged
oldergod merged 3 commits into
square:masterfrom
bek-kah:bek-kah/swift-skip-unknown-singular-enum-assignment
Sep 23, 2026
Merged

oldergod merged 3 commits into
square:masterfrom
bek-kah:bek-kah/swift-skip-unknown-singular-enum-assignment

Conversation

@bek-kah

@bek-kah bek-kah commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up to #3708. Fixes the generator edge disclosed there and tracked in #3710.

What goes wrong today

A protobuf message can carry the same field twice, and the last occurrence is meant to win. Since #3708, when generated Swift reads an enum value it does not recognize, the runtime keeps the raw bytes in unknownFields and hands back nil. The generated decode loop then stored that nil in the field.

So if a message arrives as "field 2 = HOME, field 2 = some value this app doesn't know yet", Swift ends up with type == nil (or the zero value in proto3) even though it just read HOME. Generated Kotlin keeps HOME, and so does Google's protobuf runtime.

The same thing happens inside a oneof. An unrecognized enum value also marked the oneof as "this enum case is set", which threw away a message case that had already been decoded a few bytes earlier.

The fix

The generator now wraps each enum read in if let, so the field (and the oneof marker) only change when the value was recognized. The unknown bytes still end up in unknownFields and are written back out unchanged.

Before, for Person.PhoneNumber (PhoneType type = 2) and OneOfs (oneof choice { NestedEnum enum_option = 1; … }):

case 2: type = try protoReader.decode(Person.PhoneType.self)
case 1: choiceProtoTag = 1; choice = (try protoReader.decode(OneOfs.NestedEnum.self)).flatMap { .enum_option($0) }

After:

case 2: if let value = try protoReader.decode(Person.PhoneType.self) { type = value }
case 1: if let value = try protoReader.decode(OneOfs.NestedEnum.self) { choiceProtoTag = 1; choice = .enum_option(value) }

If a message already has a field named value, the binding is named _value instead (see the FooBar.swift golden). Repeated and map enum fields are untouched; they already skip unrecognized values in ProtoReader.decode(into:).

This is the shape generated Kotlin has always used: it wraps the read in try and only records the unknown field in catch (e: EnumConstantNotFoundException) (KotlinGenerator.kt L2099–L2110). Encoding is not affected and already matched Kotlin: known fields first, retained unknown bytes last.

All golden files were regenerated with ./gradlew generateSwiftTests. Every changed line in them is one of the two rewrites above.

How it is verified

New tests, all failing before the fix and passing after:

  • SwiftGeneratorTest.skipsAssignmentOfUnrecognizedEnumValues checks the emitted Swift for a plain enum field, a field named value, a repeated enum (unchanged), and a oneof enum with and without a message sibling.
  • Four RoundTripTests feed "known value, then unknown value" bytes for a proto2 field, a oneof enum, a oneof next to a message case, and a proto3 field. Each checks that the known value survives, the unknown bytes land in unknownFields, and re-encoding gives back the exact input bytes. The oneof-next-to-a-message case needed a new message_option field in the test-only oneofs.proto.

Two deliberate breakages to confirm the tests bite:

  • Restoring the old generator: all four round-trip tests and the generator test fail (nil or MOBILE where the known value was expected).
  • Moving the oneof marker back outside the if let: only the "oneof next to a message" test fails, which is exactly what it exists to catch.

Ran locally on a clean tree: ./gradlew :wire-runtime-swift:build -x xcTest generateSwiftTests :wire-swift-generator:test spotlessCheck and swift test (303 tests, 0 failures).

bek-kah and others added 2 commits September 21, 2026 16:23
Since square#3708, ProtoReader.decode(_:) under .returnNil retains an
unrecognized singular enum value in unknownFields and returns nil.
Generated Swift assigned that nil unconditionally, so when the same tag
appeared twice as [recognized][unknown] the later occurrence erased the
recognized value. For oneofs it also claimed the oneof's tag, dropping
an already decoded sibling message case after the field loop.

Generated Kotlin catches EnumConstantNotFoundException around the
assignment and only records the unknown field, and protobuf runtimes
only set recognized values. Emit `if let` around the enum decode so the
assignment (and the oneof tag) happen only for a recognized value.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@bek-kah bek-kah changed the title Skip assigning unrecognized enum values in generated Swift decode CATCL-3557: Skip assigning unrecognized enum values in generated Swift decode Sep 21, 2026
@bek-kah
bek-kah marked this pull request as ready for review September 22, 2026 17:45
@loganblevins

Copy link
Copy Markdown
Member

@codex review

@loganblevins loganblevins left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice work!

@bek-kah bek-kah changed the title CATCL-3557: Skip assigning unrecognized enum values in generated Swift decode Skip assigning unrecognized enum values in generated Swift decode Sep 22, 2026
@loganblevins

Copy link
Copy Markdown
Member

@oldergod fyi

@oldergod
oldergod merged commit 68c14d6 into square:master Sep 23, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants