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.

Sign up for GitHub

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

Fix bug on codegen in ::prost::Enumeration #955

Closed
wants to merge 1 commit into from
Closed

Fix bug on codegen in ::prost::Enumeration #955

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,22 +163,21 @@ The `#[derive(::prost::Enumeration)]` annotation added to the generated
```rust,ignore
impl PhoneType {
pub fn is_valid(value: i32) -> bool { ... }
#[deprecated]
pub fn from_i32(value: i32) -> Option<PhoneType> { ... }
}
```

It also adds an `impl TryFrom<i32> for PhoneType`, so you can convert an `i32` to its corresponding `PhoneType` value by doing,
so you can convert an `i32` to its corresponding `PhoneType` value by doing,
for example:

```rust,ignore
let phone_type = 2i32;

match PhoneType::try_from(phone_type) {
Ok(PhoneType::Mobile) => ...,
Ok(PhoneType::Home) => ...,
Ok(PhoneType::Work) => ...,
Err(_) => ...,
match PhoneType::from_i32(phone_type) {
Some(PhoneType::Mobile) => ...,
Some(PhoneType::Home) => ...,
Some(PhoneType::Work) => ...,
None => ...,
}
```

Expand Down
10 changes: 2 additions & 8 deletions prost-derive/src/field/map.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,17 +275,11 @@ impl Field {
Some(quote! {
#[doc=#get_doc]
pub fn #get(&self, key: #key_ref_ty) -> ::core::option::Option<#ty> {
self.#ident.get(#take_ref key).cloned().and_then(|x| {
let result: ::core::result::Result<#ty, _> = ::core::convert::TryFrom::try_from(x);
result.ok()
})
self.#ident.get(#take_ref key).cloned().and_then(#ty::from_i32)
}
#[doc=#insert_doc]
pub fn #insert(&mut self, key: #key_ty, value: #ty) -> ::core::option::Option<#ty> {
self.#ident.insert(key, value as i32).and_then(|x| {
let result: ::core::result::Result<#ty, _> = ::core::convert::TryFrom::try_from(x);
result.ok()
})
self.#ident.insert(key, value as i32).and_then(#ty::from_i32)
}
})
} else {
Expand Down
19 changes: 6 additions & 13 deletions prost-derive/src/field/scalar.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,10 +219,9 @@ impl Field {
struct #wrap_name<'a>(&'a i32);
impl<'a> ::core::fmt::Debug for #wrap_name<'a> {
fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result {
let res: ::core::result::Result<#ty, _> = ::core::convert::TryFrom::try_from(*self.0);
match res {
Err(_) => ::core::fmt::Debug::fmt(&self.0, f),
Ok(en) => ::core::fmt::Debug::fmt(&en, f),
match #ty::from_i32(*self.0) {
None => ::core::fmt::Debug::fmt(&self.0, f),
Some(en) => ::core::fmt::Debug::fmt(&en, f),
}
}
}
Expand Down Expand Up @@ -297,7 +296,7 @@ impl Field {
quote! {
#[doc=#get_doc]
pub fn #get(&self) -> #ty {
::core::convert::TryFrom::try_from(self.#ident).unwrap_or(#default)
#ty::from_i32(self.#ident).unwrap_or(#default)
}

#[doc=#set_doc]
Expand All @@ -315,10 +314,7 @@ impl Field {
quote! {
#[doc=#get_doc]
pub fn #get(&self) -> #ty {
self.#ident.and_then(|x| {
let result: ::core::result::Result<#ty, _> = ::core::convert::TryFrom::try_from(x);
result.ok()
}).unwrap_or(#default)
self.#ident.and_then(#ty::from_i32).unwrap_or(#default)
}

#[doc=#set_doc]
Expand All @@ -340,10 +336,7 @@ impl Field {
::core::iter::Cloned<::core::slice::Iter<i32>>,
fn(i32) -> ::core::option::Option<#ty>,
> {
self.#ident.iter().cloned().filter_map(|x| {
let result: ::core::result::Result<#ty, _> = ::core::convert::TryFrom::try_from(x);
result.ok()
})
self.#ident.iter().cloned().filter_map(#ty::from_i32)
}
#[doc=#push_doc]
pub fn #push(&mut self, value: #ty) {
Expand Down
16 changes: 0 additions & 16 deletions prost-derive/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,10 +307,6 @@ fn try_enumeration(input: TokenStream) -> Result<TokenStream, Error> {
|&(ref variant, ref value)| quote!(#value => ::core::option::Option::Some(#ident::#variant)),
);

let try_from = variants.iter().map(
|&(ref variant, ref value)| quote!(#value => ::core::result::Result::Ok(#ident::#variant)),
);

let is_valid_doc = format!("Returns `true` if `value` is a variant of `{}`.", ident);
let from_i32_doc = format!(
"Converts an `i32` to a `{}`, or `None` if `value` is not a valid variant.",
Expand All @@ -327,7 +323,6 @@ fn try_enumeration(input: TokenStream) -> Result<TokenStream, Error> {
}
}

#[deprecated = "Use the TryFrom<i32> implementation instead"]
#[doc=#from_i32_doc]
pub fn from_i32(value: i32) -> ::core::option::Option<#ident> {
match value {
Expand All @@ -348,17 +343,6 @@ fn try_enumeration(input: TokenStream) -> Result<TokenStream, Error> {
value as i32
}
}

impl #impl_generics ::core::convert::TryFrom::<i32> for #ident #ty_generics #where_clause {
type Error = ::prost::DecodeError;

fn try_from(value: i32) -> ::core::result::Result<#ident, ::prost::DecodeError> {
match value {
#(#try_from,)*
_ => ::core::result::Result::Err(::prost::DecodeError::new("invalid enumeration value")),
}
}
}
};

Ok(expanded.into())
Expand Down
26 changes: 0 additions & 26 deletions tests/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -608,32 +608,6 @@ mod tests {
);
}

#[test]
fn test_enum_try_from_i32() {
use core::convert::TryFrom;
use default_enum_value::{ERemoteClientBroadcastMsg, PrivacyLevel};

assert_eq!(Ok(PrivacyLevel::One), PrivacyLevel::try_from(1));
assert_eq!(Ok(PrivacyLevel::Two), PrivacyLevel::try_from(2));
assert_eq!(
Ok(PrivacyLevel::PrivacyLevelThree),
PrivacyLevel::try_from(3)
);
assert_eq!(
Ok(PrivacyLevel::PrivacyLevelprivacyLevelFour),
PrivacyLevel::try_from(4)
);
assert_eq!(
Err(prost::DecodeError::new("invalid enumeration value")),
PrivacyLevel::try_from(5)
);

assert_eq!(
Ok(ERemoteClientBroadcastMsg::KERemoteClientBroadcastMsgDiscovery),
ERemoteClientBroadcastMsg::try_from(0)
);
}

#[test]
fn test_default_string_escape() {
let msg = default_string_escape::Person::default();
Expand Down