Skip to content

Add accessors for oneof's generated data classes#3594

Open
oldergod wants to merge 1 commit into
masterfrom
bquenaudon.2026-05-11.readings
Open

Add accessors for oneof's generated data classes#3594
oldergod wants to merge 1 commit into
masterfrom
bquenaudon.2026-05-11.readings

Conversation

@oldergod
Copy link
Copy Markdown
Member

@oldergod oldergod commented May 11, 2026

Created the extension on the sealed class, not the outer class, to make it clear it's going through a oneof

@oldergod oldergod force-pushed the bquenaudon.2026-05-11.readings branch from 95108e0 to 1c7fe5a Compare May 11, 2026 20:58
@oldergod oldergod marked this pull request as ready for review May 11, 2026 20:58
@oldergod oldergod force-pushed the bquenaudon.2026-05-11.readings branch from 1c7fe5a to 620e7cf Compare May 11, 2026 21:34
accessorNameAllocator.newName(field.name, field)
}
val subclassName = sealedClassName.nestedClass(subclassNameAllocator[field])
accessors += PropertySpec.builder(fieldName, field.type!!.typeName.copy(nullable = true))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do we need the if (field.isDeprecated) { and for (annotation in optionAnnotations(field.options)) { bits from sealedOneOfClass below, in order to propagate metadata / annotations on the extension fields too?

private fun MessageType.flatOneOfs(): List<OneOf> = when (oneofMode) {
OneofMode.FLAT -> oneOfs.filter { it.fields.size < boxOneOfsMinSize }
else -> emptyList()
else -> listOf()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No problem, I'm just curious to understand why switching from emptyList() to listOf(), is that important?

return when (type) {
is MessageType -> type.sealedOneOfAccessors()
is EnclosingType -> type.nestedTypes.flatMap(::generateSealedOneOfAccessors)
else -> listOf()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nit, would prefer to see this last case listed out for exhaustivity and future-proofing.

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.

2 participants