Skip to content

OneofMode to generate SealedClass for oneofs#3581

Open
oldergod wants to merge 5 commits intomasterfrom
bquenaudon.2026-04-27.sealedclass
Open

OneofMode to generate SealedClass for oneofs#3581
oldergod wants to merge 5 commits intomasterfrom
bquenaudon.2026-04-27.sealedclass

Conversation

@oldergod
Copy link
Copy Markdown
Member

@oldergod oldergod commented Apr 27, 2026 •
edited
Loading

Apologies for the long PR. I wanted to make everything sweet, and it's split into 4 commits.
Golden files + the interop test is the meat of the end result.

Fixes #2749

@oldergod oldergod force-pushed the bquenaudon.2026-04-27.sealedclass branch from 31952a4 to 03e7232 Compare April 27, 2026 15:43
scmorse
scmorse reviewed Apr 27, 2026
View reviewed changes
Copy link
Copy Markdown

@scmorse scmorse left a comment

Choose a reason for hiding this comment

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

I know you said this is still a WIP, but I noticed some other files that probably need some updates:

  • wire-compiler/src/main/java/com/squareup/wire/WireCompiler.kt
  • wire-gradle-plugin/src/main/kotlin/com/squareup/wire/gradle/WireOutput.kt
  • wire-gradle-plugin/api/wire-gradle-plugin.api
  • wire-compiler/src/test/java/com/squareup/wire/WireCompilerTest.kt
  • docs/wire_compiler.md

Comment thread wire-kotlin-generator/src/main/java/com/squareup/wire/kotlin/KotlinGenerator.kt
Comment thread wire-kotlin-generator/src/main/java/com/squareup/wire/kotlin/KotlinGenerator.kt
Comment thread wire-kotlin-generator/src/main/java/com/squareup/wire/kotlin/KotlinGenerator.kt
@oldergod oldergod force-pushed the bquenaudon.2026-04-27.sealedclass branch 2 times, most recently from 55ce945 to 7c70231 Compare April 27, 2026 21:06
@oldergod oldergod force-pushed the bquenaudon.2026-04-27.sealedclass branch 2 times, most recently from aa613d7 to bb132be Compare April 28, 2026 13:34
@oldergod oldergod force-pushed the bquenaudon.2026-04-27.sealedclass branch from bb132be to b95ff69 Compare April 28, 2026 13:56
@oldergod oldergod force-pushed the bquenaudon.2026-04-27.sealedclass branch 2 times, most recently from 8688c43 to f8a20c8 Compare April 28, 2026 20:12
@oldergod oldergod changed the title WIP SealedClass for oneofs OneofMode to generate SealedClass for oneofs Apr 28, 2026
@oldergod oldergod marked this pull request as ready for review April 28, 2026 20:13
@oldergod oldergod requested review from JakeWharton, scmorse and swankjesse and removed request for scmorse April 28, 2026 20:13
@oldergod oldergod force-pushed the bquenaudon.2026-04-27.sealedclass branch 3 times, most recently from 64741ee to 1af547d Compare April 29, 2026 08:25
scmorse
scmorse reviewed Apr 29, 2026
View reviewed changes
Copy link
Copy Markdown

@scmorse scmorse left a comment •
edited
Loading

Choose a reason for hiding this comment

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

  • Looks like docs/wire_compiler.md could use an update

Comment thread wire-runtime/src/jvmMain/kotlin/com/squareup/wire/internal/SealedOneOfBinding.kt
) : FieldOrOneOfBinding<M, B>() {

private val builderField: Field =
builderType.getDeclaredField(messageField.name).also { it.isAccessible = true }
Copy link
Copy Markdown

@scmorse scmorse Apr 29, 2026

Choose a reason for hiding this comment

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

Should this use by lazy {}, like valueField does, so that it doesn't fail if builder types aren't enabled?

Copy link
Copy Markdown
Member Author

@oldergod oldergod Apr 29, 2026

Choose a reason for hiding this comment

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

Code has been updated here

Comment thread wire-kotlin-generator/src/main/java/com/squareup/wire/kotlin/KotlinGenerator.kt
buildersOnly: Boolean = false,
escapeKotlinKeywords: Boolean = false,
enumMode: EnumMode = ENUM_CLASS,
oneofMode: OneofMode = OneofMode.FLAT,
Copy link
Copy Markdown

@scmorse scmorse Apr 29, 2026

Choose a reason for hiding this comment

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

I hit a binary compatibility issue testing this against cash-server. Adding oneofMode to KotlinGenerator.Companion.get changes the JVM signature of Kotlin’s generated get$default method, so existing compiled schema handlers/extensions that call KotlinGenerator.get(...) fail at runtime with NoSuchMethodError.

So I think that Cash's existing compiled extensions can’t run with the new Wire jars unless they are recompiled, is that okay? Maybe cash's extensions can just be recompiled and then updated simultaneously with the wire version upgrade.

Alternatively, maybe we could preserve the old overload and have it delegate to the new one with oneofMode = OneofMode.FLAT?

Copy link
Copy Markdown
Member Author

@oldergod oldergod Apr 29, 2026

Choose a reason for hiding this comment

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

Can do it!

Copy link
Copy Markdown
Member Author

@oldergod oldergod Apr 29, 2026

Choose a reason for hiding this comment

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

Addressed by adding a hidden constructor

@oldergod oldergod force-pushed the bquenaudon.2026-04-27.sealedclass branch from 1af547d to 8e123e0 Compare April 29, 2026 14:15
oldergod
oldergod commented Apr 29, 2026
View reviewed changes
Copy link
Copy Markdown
Member Author

@oldergod oldergod left a comment

Choose a reason for hiding this comment

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

@scmorse WireCompiler.md is updated and fixed some old things in it

Comment thread wire-kotlin-generator/src/main/java/com/squareup/wire/kotlin/KotlinGenerator.kt
Comment thread wire-runtime/src/jvmMain/kotlin/com/squareup/wire/internal/SealedOneOfBinding.kt
) : FieldOrOneOfBinding<M, B>() {

private val builderField: Field =
builderType.getDeclaredField(messageField.name).also { it.isAccessible = true }
Copy link
Copy Markdown
Member Author

@oldergod oldergod Apr 29, 2026

Choose a reason for hiding this comment

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

Code has been updated here

Comment thread wire-kotlin-generator/src/main/java/com/squareup/wire/kotlin/KotlinGenerator.kt
buildersOnly: Boolean = false,
escapeKotlinKeywords: Boolean = false,
enumMode: EnumMode = ENUM_CLASS,
oneofMode: OneofMode = OneofMode.FLAT,
Copy link
Copy Markdown
Member Author

@oldergod oldergod Apr 29, 2026

Choose a reason for hiding this comment

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

Can do it!

Comment thread wire-kotlin-generator/src/main/java/com/squareup/wire/kotlin/KotlinGenerator.kt
buildersOnly: Boolean = false,
escapeKotlinKeywords: Boolean = false,
enumMode: EnumMode = ENUM_CLASS,
oneofMode: OneofMode = OneofMode.FLAT,
Copy link
Copy Markdown
Member Author

@oldergod oldergod Apr 29, 2026

Choose a reason for hiding this comment

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

Addressed by adding a hidden constructor

@oldergod oldergod requested a review from staktrace April 30, 2026 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JakeWharton JakeWharton Awaiting requested review from JakeWharton

@swankjesse swankjesse Awaiting requested review from swankjesse

@staktrace staktrace Awaiting requested review from staktrace

1 more reviewer

@scmorse scmorse scmorse approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Exhaustive checks in oneof fields

2 participants

@oldergod @scmorse