search

streem / pbandk

Kotlin Code Generator and Runtime for Protocol Buffers
MIT License
271 stars 37 forks source link

Update enum naming and validation #245

Closed fluxxion82 closed 2 months ago

fluxxion82 commented 1 year ago

Enum namings could end up being named as an invalid kotlin identifier, so updated the enum name generation to avoid invalid names.

Fixes https://github.com/streem/pbandk/issues/226

fluxxion82 commented 1 year ago

@garyp hey gary, how does this look?

fluxxion82 commented 1 year ago

@garyp is this project still alive?

fluxxion82 commented 7 months ago

@garyp you're alive! welcome back :) I did switch things up but I'd still be interested in seeing this pr through. I'll need to find some time to get back in this and remember what I was doing but I'll try to push an update in the not too distant future.

garyp commented 2 months ago

@fluxxion82 I just ran into this bug myself because the newest version of descriptor.proto includes an Edition.EDITION_2023 enum value that was being turned into an invalid Kotlin identifier. Since this was blocking me now, I went ahead and finished this PR myself. Thanks for the initial version! It saved me a bunch of time. I'll leave some comments about the changes I made to your PR. Please reply if you have any questions and I can address them in a follow-up PR.

garyp commented 2 months ago

Your original PR had this change (I can't comment on it anymore since I removed it):

-                    disallowedValueTypeNames.contains(name) ||
+                    disallowedValueTypeNames.any { it.equals(name, true) } ||

I found that this was being too aggressive in renaming identifiers in the existing generated code even though they weren't causing any conflicts (e.g. an enum value named STRING was being turned into STRING_ because of the case-insensitive matching you added). So I changed the code back to its original behavior of doing case-sensitive matching.