Swift experimental-module-manifest cannot reference enums from another module

[jordcarfetch]

We've found that swift modules do not work when referencing enums.
Here's a repo that reproduces the issue.

TL;DR

proto/foo/v1/foo.proto

syntax = "proto3";

package foo.v1;

import "bar/v1/bar.proto";

// Foo represents a basic entity with a single Bar field
message Foo {
  bar.v1.Bar bar = 1;
}

proto/bar/v1/bar.proto

syntax = "proto3";

package bar.v1;

// an example enum
enum Bar {
  BAR_UNSPECIFIED = 0;
  BAR_A = 1;
  BAR_B = 2;
}

wire.yaml

Foo:
  roots:
    - foo.v1.*
  dependencies:
    - Bar

Bar:
  roots:
    - bar.v1.*

❯ rm -rf swift/modules && java -jar wire-compiler-5.3.3.jar \
    --proto_path=proto/ \
    --swift_out=swift/modules \
    --experimental-module-manifest=wire.yaml
Writing .Bar declared in bar/v1/bar.proto to swift/modules/Bar (target=Swift)
Exception in thread "main" java.util.NoSuchElementException: Missing a zero value for Bar
        at com.squareup.wire.swift.SwiftGenerator.validateProto3DefaultsExist(SwiftGenerator.kt:312)
        at com.squareup.wire.swift.SwiftGenerator.generateMessage(SwiftGenerator.kt:335)
        at com.squareup.wire.swift.SwiftGenerator.generateType(SwiftGenerator.kt:269)
        at com.squareup.wire.swift.SwiftGenerator.generateTypeTo(SwiftGenerator.kt:256)
        at com.squareup.wire.swift.SwiftSchemaHandler.handle(SwiftSchemaHandler.kt:47)
        at com.squareup.wire.schema.SchemaHandler.handle(SchemaHandler.kt:152)
        at com.squareup.wire.schema.SchemaHandler.handle(SchemaHandler.kt:43)
        at com.squareup.wire.swift.SwiftSchemaHandler.handle(SwiftSchemaHandler.kt:34)
        at com.squareup.wire.schema.WireRun.execute$wire_schema(WireRun.kt:321)
        at com.squareup.wire.schema.WireRun.execute(WireRun.kt:240)
        at com.squareup.wire.WireCompiler.compile(WireCompiler.kt:228)
        at com.squareup.wire.WireCompiler$Companion.main(WireCompiler.kt:302)
        at com.squareup.wire.WireCompiler.main(WireCompiler.kt)

The same proto files generate without experimental-module-manifest.

[oldergod]

Thanks for reporting. We probably not use Proto3 enough in Swift to have reproduced that ourselves. If you feel like debugging it, or even fixing it, you're much welcome to do so (until we do it eventually)

[berbs-fetch]

When generating the model,SwiftGenerator::validateProto3DefaultsExist(type:) is getting hit from the enum not having a default / zero value registered. Since the enum is a dependency, it looks like it is getting "stubbed" in Utils.kt-Type::asStub()

SwiftGenerator.kt

  @Throws(NoSuchElementException::class)
  private fun validateProto3DefaultsExist(type: MessageType) {
    // TODO: Remove when we support unknown cases
    if (type.syntax == PROTO_2) { return }

    // validate each enum field
    type
      .fields
      .mapNotNull { schema.getType(it.type!!) as? EnumType }
      .forEach { enum ->
        // ensure that a protoDefaultedName case exists
        if (enum.protoDefaultedName == null) {
          throw NoSuchElementException("Missing a zero value for ${enum.name}")
        }
      }
  }

SwiftGenerator.kt

  private val EnumType.protoDefaultedName: String?
    get() = constants.takeIf {
      // Proto2 allows for non-zero default values
      // We can only handle them if the enum has no pruned values
      syntax == PROTO_3
    }?.firstOrNull { it.tag == 0 }?.name

Utils.kt

  this is EnumType -> copy(
    constants = emptyList(),  << where the `default / zero` case is getting dropped.
    options = Options(Options.ENUM_OPTIONS, emptyList()),
  )

Couple questions:

• Why are stubs being created? What is their purpose? Is it to mark a code path that should not be "traversed"
• Why are the constants being cleared? I feel like an enum should hold onto that? They shouldn't be holding a reference to any types right? They are "terminal" nodes in the syntax tree. Are there complications with oneOf

Would an acceptable fix to
a) Include only the default constant to allow Swift to code generate?
b) Allow all constants? Is clearing them out really needed.
c) Make a fix at the call of validateProto3DefaultsExist, should it just be removed? When looking at the KotlinGenerator::generateMessage(type:) I'm not initially seeing the same check. Can Field.default just be leveraged? Is there some code around Codable that needs this default constant?

Happy to make a PR.

[oldergod]

A PR would be most welcome 👍