Grpc.Tools: support generated filename corner cases #18470
Conversation
protoc and gRPC codegens differently treat non-ASCII letter characters and symbols other than underscores when constructing their respective output filenames for generated .cs files. This change reproduces their respective behaviors exactly. Fixes grpc#17661
jtattermusch
added
lang/C#
release notes: yes
| @@ -109,6 +108,26 @@ string LowerUnderscoreToUpperCamel(string str) | |||
| } | |||
| return result.ToString(); | |||
| } | |||
|
|
|||
| // This is how the protoc codegen constructs its output filename. | |||
| // See protobuf/compiler/csharp/csharp_helpers.cc:356. | |||
There was a problem hiding this comment.
for comparison, the logic is here:
https://github.com/protocolbuffers/protobuf/blob/3a3956e8a258784461270961c6577341356bce52/src/google/protobuf/compiler/csharp/csharp_helpers.cc#L137 with cap_next_letter=true and preserve_period=false
There was a problem hiding this comment.
Yup, this is where I got it from. Looks like I gave a wrong line number, will fix in a moment.
I do not understand what is this last # check at the very end of the function, but it does not seem to be important?
| { | ||
| result.Append(cap ? upperC : c); | ||
| } | ||
| cap = !isAsciiLetter; |
There was a problem hiding this comment.
I believe there's still a subtle problem - after a digit, protoc will still capitalize the next letter:
I tested and one123two.cs generates One123Two.cs and One123twoGrpc.cs.
There was a problem hiding this comment.
Uhhhh, thanks! I'll take care of this.
There was a problem hiding this comment.
This passes, I added a test and verified a real build, too. cap = !isAsciiLetter; will be true after a digit, as well as after any (non-emitted) other character. I'll push the test case tho, so that we have it, just in case.
There was a problem hiding this comment.
ah, thanks for clarification!
jtattermusch
changed the title
jtattermusch
changed the title
protoc and gRPC codegens differently treat non-ASCII letter characters and symbols other than underscores when constructing their respective output filenames for generated .cs files. This change reproduces their respective behaviors exactly.
I added test cases similar to my hello.world.proto flop, and verified the built dll by dropping it into the .nuget directory on a Linux machine. Build failed with a repro of #17661 before, succeeded after.
Closes #17661