flipt-io / flipt

Enterprise-ready, GitOps enabled, CloudNative feature management solution
https://flipt.io
GNU General Public License v3.0
3.85k stars 218 forks source link

[Bug]: an error compiling "EvaluationServiceCoroutineGrpc.kt: (130, 13): 'boolean' overrides nothing" #2351

Closed vishaloures closed 1 year ago

vishaloures commented 1 year ago

Bug Description

Hi, When I try use new API https://github.com/flipt-io/flipt/blob/3504651b34893321b1aefbe5fe97b27e3ebe432c/rpc/flipt/evaluation/evaluation.proto#L82-L86 , I get an error when compiling "EvaluationServiceCoroutineGrpc.kt: (130, 13): 'boolean' overrides nothing" , because "Boolean" keyword in many programming languages, it is advisable not to use it explicitly. Is it possible to rename a method like GetBoolean ?

Version Info

v.1.27.2

Search

Steps to Reproduce

Try compiling API https://github.com/flipt-io/flipt/blob/3504651b34893321b1aefbe5fe97b27e3ebe432c/rpc/flipt/evaluation/evaluation.proto#L82-L86 on Kotlin

Result : "EvaluationServiceCoroutineGrpc.kt: (130, 13): 'boolean' overrides nothing"

Expected Behavior

No response

Additional Context

No response

markphelps commented 1 year ago

👋🏻 Hey @vishaloures ! Thank you for filing this bug! As @GeorgeMac mentioned in the other issue: https://github.com/flipt-io/flipt/issues/2329#issuecomment-1798603965 , we've been putting our heads together to try to figure out a way around this.

It is definitely unfortunate that we chose to name the GRPC method Boolean as you mentioned that is a reserved word in many programming languages, including Java and Kotlin.

One thing I wondered about was why users of Java GRPC-generated SDK do not run into this same problem.. so I did some digging in the Java GRPC Generator Plugin codebase and found this:

https://github.com/grpc/grpc-java/blob/master/compiler/src/java_plugin/cpp/java_generator.cpp#L90-L166

It seems that the Java GRPC generator looks for any methods that would match a reserved word, such as boolean and if it finds one, it appends an _ to it, so the GRPC generated method will become boolean_.

While this is still not ideal from a developer experience case, it does work around the issue of it not compiling.

I then looked at the Kotlin GRPC plugin and searched and did not see any code that does something similar to what the Java GRPC plugin does (looks for and handles reserved words).

I also didn't see any previous issues about this in the Kotlin GRPC repo.

So, in order to fix this issue for your usecase, I was wondering what you thought about these approaches:

  1. We could create an issue in the Kotlin GRPC repo and see if they will fix this issue in the generator to look for and handle reserved method names as the Java generator does
  2. Would it be possible for us (Flipt) to publish and maintain our own version of the Kotlin GRPC SDK for Flipt that has this method renamed? Not sure if this is possible yet to be honest.
  3. Alternatively, is there a way to wrap the Java GRPC SDK in a thin Kotlin wrapper since they are both JVM languages? (I havently actually used Kotlin personally, so wondering if you have any insight here?)

Ultimately we want to fix this issue at our API level too, by renaming the Boolean GRPC and REST endpoints to something that is less likely to class with various programming languages reserved words. However, this would be a breaking change for our users, so we have to think about this much more in-depth.

We're also looking into what a v2 of our entire API would/could look like, in which we would also fix this issue.

Sorry for the long response, but I want to figure out how we can fix this issue for you and your team! Do any of the above options work for you?

ruslansennov commented 1 year ago

Hi @markphelps @vishaloures and I have made fixes to the grpc plugins we use. I don't think you should change anything because it's not your problem :)

markphelps commented 1 year ago

Thanks @ruslansennov is there anything you can share here on what updates you made? Would help us guide others who are using Kotlin and GRPC maybe?

ruslansennov commented 1 year ago

At the moment we have only patched the rxgrpc plugin locally and plan to make the PR to the salesforce/reactive-grpc repo. The kroto plugin has the same problem, but we're already going to replace it with grpckt, which doesn't seem to have such problems.

markphelps commented 1 year ago

Thanks! Closing this then. Let us know if there is anything else we can help with!!