Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Kotlin] Why does the fieldOrNull extension property only support message fields? #12935

Closed
be-hase opened this issue May 31, 2023 · 5 comments
Closed
Assignees
Labels

Comments

@be-hase
Copy link
Contributor

be-hase commented May 31, 2023

protobuf-kotlin introduces a useful extension property called fieldOrNull for message fields.

optional was introduced in proto3.

optional string hoge = 1;.

It would be more useful if fieldOrNull was introduced for optional fields as well. Is there any reason why this has not been introduced?

val BlahBlah.hogeOrNull: kotlin.String?
    get() = if (hasHoge()) hoge else null

related code:
https://github.com/protocolbuffers/protobuf/blob/b30f3de12f946b6d610c21bc605726bf56ea889f/src/google/protobuf/compiler/java/message.cc#L1352C33-L1370

@be-hase be-hase added the untriaged auto added to all issues by default when created. label May 31, 2023
@anandolee anandolee added kotlin and removed untriaged auto added to all issues by default when created. labels Jun 1, 2023
@deannagarcia
Copy link
Member

We had long internal discussions about this issue. It came down to 2 main issues:

  1. Messages have a logical default to return when null: an empty message. Scalars don't have logical null values.
  2. While this can be worked around in Kotlin, other languages that protobuf supports (most notably Java) have a much more difficult time working with nulls. We strive to keep our language support as consistent as possible and given that Java could never implement something like OrNull with strings we decided to not implement it for Kotlin either.

There are still some internal discussions around this issue and we see the convenience it would add, but we would need to solve the issue on a bigger (language neutral) scale and that isn't something we are actively prioritizing now.

@be-hase
Copy link
Contributor Author

be-hase commented Jun 10, 2023

Thanks for answering my question. I understand your situation.

In my opinion, it would be great if you could support this feature.
In the actual code, it is inconvenient because it is full of boilerplate code like the following

val hoge = if (hasHoge()) hoge else null

In some cases, the default value was referenced without realizing it was optional, causing a bug.

In the end I can't stand it and am using kapt to automatically generate the extended function.... haha.
It would be nice to see official support for this.

thanks.

@dhoepelman
Copy link

Thanks for the answer.

I'm not too familiar with all other languages supported by Protobuf, but wouldn't this be possible to support in Java as Optional<Integer>? Or Integer + getXOrNull() but that has a higher risk of NPEs.

I've also found this as a footgun, with developers expect a nullable type for optional but then unintentionally letting the default value pass through.

@be-hase
Copy link
Contributor Author

be-hase commented Sep 29, 2023

I've also found this as a footgun, with developers expect a nullable type for optional but then unintentionally letting the default value pass through.

Yes, I totally agree with you.
In fact, I have seen cases where this has actually triggered a bug.

I'm not too familiar with all other languages supported by Protobuf, but wouldn't this be possible to support in Java as Optional? Or Integer + getXOrNull() but that has a higher risk of NPEs.

It seems that java is not supported because Optional is not available for older versions.
#2553 (comment)
( However, this was in 2017, and now versions older than java8 are end-of-life. )

It may be difficult due to compatibility issues, but it is not difficult to add kotlin extension functions.

@be-hase
Copy link
Contributor Author

be-hase commented May 21, 2024

FYI:
I have implemented a protoc compiler plugin that meets my needs.
https://github.com/be-hase/protoc-gen-kotlin-ext

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants