Prevent conflicts from same name options of different types #3005
Conversation
oldergod
force-pushed
the
bquenaudon.2024-06-24.optionconflict
branch
2 times, most recently
from
6142266
to
3cf6efb
Compare
| ClassName annotationName = | ||
| annotationName(protoFile, field, new ClassNameFactory(), "Option"); | ||
| if (memberToJavaName.containsValue(annotationName)) { | ||
| // To avoid conflicts for same named options on different types, we generate a more |
There was a problem hiding this comment.
nit:
| // To avoid conflicts for same named options on different types, we generate a more | |
| // To avoid conflicts for same named options of different types, we generate a more |
| memberToKotlinName[protoMember] = annotationName(protoFile, field, ClassNameFactory()) | ||
| val annotationName = annotationName(protoFile, field, ClassNameFactory()) | ||
| if (memberToKotlinName.containsValue(annotationName)) { | ||
| // To avoid conflicts for same named options on different types, we generate a more |
There was a problem hiding this comment.
nit:
| // To avoid conflicts for same named options on different types, we generate a more | |
| // To avoid conflicts for same named options of different types, we generate a more |
| ClassName annotationName = | ||
| annotationName(protoFile, field, new ClassNameFactory(), "Option"); | ||
| if (memberToJavaName.containsValue(annotationName)) { | ||
| // To avoid conflicts for same named options on different types, we generate a more |
There was a problem hiding this comment.
Is it possible for us to always pick suffix based on the option type, so that all message options will be named XxxMessageOption and all field names - XxxFieldOption?
There was a problem hiding this comment.
That'd be the ideal solution but the feature has already shipped and we'll be breaking all clients with this change 😢
So we decided to do that only when conflicts arise.
| protoFile, | ||
| field, | ||
| new ClassNameFactory(), | ||
| extendSimpleName.substring(0, extendSimpleName.length() - 1))); |
There was a problem hiding this comment.
Will extendSimpleName here always have the value FieldOptions, or it can also have the value MessageOptions?
There was a problem hiding this comment.
Can be any option type, FieldOption, MessageOption, EnumValueOption, etc. This is how we avoid the conflict
oldergod
force-pushed
the
bquenaudon.2024-06-24.optionconflict
branch
from
3cf6efb
to
046a14e
Compare
No description provided.