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

Proto Builder Generator uses wrong java builder function #115

Open
hpuac opened this issue Jul 30, 2020 · 2 comments
Open

Proto Builder Generator uses wrong java builder function #115

hpuac opened this issue Jul 30, 2020 · 2 comments

Comments

@hpuac
Copy link

hpuac commented Jul 30, 2020

Hey folks, I'm having an issue with the generated builders.
I'm using the following versions in my project

  • com.google.protobuf:3.10.0
  • com.google.protobuf:protobuf-java-util:3.10.0
  • com.github.marcoferrer.krotoplus:protoc-gen-kroto-plus:0.6.1

With the following demo proto:

syntax = "proto3";

package somePackage;

option java_multiple_files = true;
option java_outer_classname = "ProtoMyTestOuter";
option java_package = "com.example";

message ProtoMyAwesomeMessage {
    repeated ProtoOtherMessage some_b2b_books = 1;
}

message ProtoOtherMessage {
    string demo = 1;
}

When building the java class from it the builder for the field looks like the following:
public Builder addSomeB2BBooks(com.example.ProtoOtherMessage value) {
But the generated kroto+ builders try to call:
this.addSomeB2bBooks(ProtoOtherMessage.newBuilder().apply(block).build()).
So BB vs bB.
Full generated kroto+ code:

inline fun ProtoMyAwesomeMessage.Builder.addSomeB2bBooks(block: ProtoOtherMessage.Builder.() ->
        Unit): ProtoMyAwesomeMessage.Builder =
        this.addSomeB2bBooks(ProtoOtherMessage.newBuilder().apply(block).build())

So this references itself which makes my builds break.

Is this a bug or do I need to adjust the generated casing on my side?

Just in case someone asks my kroto+ settings look like the following:

---
protoBuilders:
  - unwrapBuilders: false
    useDslMarkers: true

grpcCoroutines: [{}]
@hpuac
Copy link
Author

hpuac commented Jul 31, 2020

I did some more tests to see what rules the casing for the generated Java classes follows.
Here you see the Proto field name, then the Java method name and the generated Kroto+ name:

some_b2b_books
addSomeB2BBooks
addSomeB2bBooks

some_a2bc_books
addSomeA2BcBooks
addSomeA2bcBooks

some_de2f_books
addSomeDe2FBooks
addSomeDe2fBooks

some_gh2jk_books
addSomeGh2JkBooks
addSomeGh2jkBooks

Looks like the first letter after a number is always uppercase in the generated Java code.
Same for non repeated objects, just that it's then a set instead of an add.

@hpuac
Copy link
Author

hpuac commented Dec 17, 2020

Looks like this is the issue:
https://github.com/marcoferrer/kroto-plus/blob/master/protoc-gen-kroto-plus/src/main/kotlin/com/github/marcoferrer/krotoplus/utils/StringCaseExts.kt#L25

The way the java plugin does it is capitalizing the next character after an integer:
https://github.com/protocolbuffers/protobuf/blob/master/src/google/protobuf/compiler/java/java_helpers.cc#L191-L193

It's quite funny that even the google documentation suggests to look at the logic in that file:

The logic for determining output file names in the Java code generator is fairly complicated. You should probably look at the protoc source code, particularly java_headers.cc, to make sure you have covered all cases.

Source: https://developers.google.com/protocol-buffers/docs/reference/java-generated#plugins

I guess it would make the most sense to port the logic from the UnderscoresToCamelCase function.
Maybe I will give it a try and provide a PR if I find the time.

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

No branches or pull requests

1 participant