prost icon indicating copy to clipboard operation
prost copied to clipboard

Prefix idents that don't start with an alphabetic character

Open LegNeato opened this issue 4 years ago • 8 comments

LegNeato avatar Apr 26 '21 19:04 LegNeato

Hi @LegNeato do you know of any precedent on how other protobuf implementations handle this?

LucioFranco avatar Jul 06 '21 16:07 LucioFranco

I do not, I can look into it though. I think if the find & replace infra was a bit more robust this wouldn't be necessary. I'll look into a larger patch as well as what other impls do

LegNeato avatar Jul 14 '21 06:07 LegNeato

Ok, I looked into how protoc handles this. It is a mess! Every language does different things and there is a bunch of undocumented behavior.

This is a problem because prost strips enum prefixes, which turns a valid identifier field name to an invalid identifier. But, similar issues can arise with rust restricted keywords (there are a ton of protoc issues where people have valid idents in one language that are reserved in another, chaos ensues).

LegNeato avatar Aug 03 '21 09:08 LegNeato

Please also add a test to fn test_ident_conversions/ident_conversion.proto's StrawberryRhubarbPIE

danburkert avatar Aug 03 '21 16:08 danburkert

I plan to get to this in the next couple of days, thanks @danburkert for the review!

LegNeato avatar Sep 09 '21 20:09 LegNeato

@danburkert Sorry this took so long, thank you again for the review.

LegNeato avatar Oct 01 '21 01:10 LegNeato

@LegNeato seems need to fix the rustfmt ci stage

LucioFranco avatar Oct 06 '21 20:10 LucioFranco

@LucioFranco whoops, done.

LegNeato avatar Dec 02 '21 03:12 LegNeato

This has recently been fixed by https://github.com/tokio-rs/prost/pull/998

caspermeijn avatar Apr 12 '24 14:04 caspermeijn