Skip to content

Redacted Kotlin scalars respect nullability#1977

Merged
oldergod merged 3 commits into
square:masterfrom
dhenry-stripe:redacted-kotlin-scalars-fix
Jun 2, 2021
Merged

Redacted Kotlin scalars respect nullability#1977
oldergod merged 3 commits into
square:masterfrom
dhenry-stripe:redacted-kotlin-scalars-fix

Conversation

@dhenry-stripe
Copy link
Copy Markdown
Contributor

Prior to this PR KotlinGenerator was generating invalid code for redacted proto3 scalars. For example:

public override fun redact(value: MyExample): MyExample = value.copy(
    my_string = null, // build fails because type is String not String?
    my_int32 = null, // build fails because type is Int and not Int?
    unknownFields = ByteString.EMPTY
)

With this change, non-nullable Kotlin scalar values are redacted as the identity value for the given type.

return when {
isRepeated -> CodeBlock.of("emptyList()")
isMap -> CodeBlock.of("emptyMap()")
isOptional || isOneOf -> CodeBlock.of("null")
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.

Is there an existing test case for one ofs?

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.

Also could we look at Field.EncodeMode.NULL_IF_ABSENT instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hrmm not for this use case. I've updated the included test to cover redacted one ofs and switched over to Field.EncodeMode.NULL_IF_ABSENT as well, thanks.

Copy link
Copy Markdown
Member

@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.

Sweet, thank you

@oldergod oldergod force-pushed the redacted-kotlin-scalars-fix branch from 0c2ccee to 3cf2743 Compare June 2, 2021 11:33
@oldergod oldergod merged commit 2b43858 into square:master Jun 2, 2021
@dhenry-stripe dhenry-stripe deleted the redacted-kotlin-scalars-fix branch June 2, 2021 13:19
MehdiAghajani pushed a commit to MehdiAghajani/wire that referenced this pull request Dec 26, 2021
* kotlin redacted scalars fix

* Field.EncodeMode used to determine redacted field value

* Tests for message

Co-authored-by: Benoit Quenaudon <bquenaudon@squareup.com>
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