Repository: tokio-rs/prost (language: rust)

Issue:
Tuple structs with enumerations don't work
It seems tuple structs are broken when they contain an enumeration. I hit it when implementing tests for https://github.com/danburkert/prost/pull/54, but I can reproduce on master:

```rust
/// A special case with a tuple struct
#[test]
fn tuple_struct() {
    #[derive(Clone, PartialEq, Message, Debug)]
    struct NewType(
        #[prost(int32, tag="1")]
        pub i32,
        #[prost(enumeration="BasicEnumeration", tag="5")]
        pub i32,
    );
}
```

The error is:

```
   Compiling conformance v0.0.0 (file:///home/vorner/prog/prost/conformance)
error: expected identifier, found `1`
   --> tests/src/lib.rs:152:32
    |
152 |     #[derive(Clone, PartialEq, Message, Debug)]
```

If I comment the second field out,it compiles. I had no success yet pinpointing where exactly it happens, because cargo expand gives me the same error instead of the (broken) source code. 

As the prost-derive code contains code to handle tuple structs, I guess they are supposed to work.


Discussion / hints:
Yeah, since the code generator doesn't output tuple structs they are not well tested (if at all).  Let's leave this open as documentation of the bug.  I don't use hand-written structs except for tests, so it won't be at the top of my to-do list to fix.

Fix patch:
diff --git a/prost-build/src/ident.rs b/prost-build/src/ident.rs
index 5c48d0f60..55e8e48a1 100644
--- a/prost-build/src/ident.rs
+++ b/prost-build/src/ident.rs
@@ -77,8 +77,7 @@ mod tests {
         assert_eq!("field_name9", &to_snake("field_Name9"));
         assert_eq!("field_name10", &to_snake("Field_Name10"));
 
-        // TODO(withoutboats/heck#3)
-        //assert_eq!("field_name11", &to_snake("FIELD_NAME11"));
+        assert_eq!("field_name11", &to_snake("FIELD_NAME11"));
         assert_eq!("field_name12", &to_snake("FIELD_name12"));
         assert_eq!("field_name13", &to_snake("__field_name13"));
         assert_eq!("field_name14", &to_snake("__Field_name14"));
diff --git a/prost-derive/src/field/map.rs b/prost-derive/src/field/map.rs
index 829962e47..d67d8e6c2 100644
--- a/prost-derive/src/field/map.rs
+++ b/prost-derive/src/field/map.rs
@@ -257,7 +257,7 @@ impl Field {
     }
 
     /// Returns methods to embed in the message.
-    pub fn methods(&self, ident: &Ident) -> Option<TokenStream> {
+    pub fn methods(&self, ident: &TokenStream) -> Option<TokenStream> {
         if let ValueTy::Scalar(scalar::Ty::Enumeration(ty)) = &self.value_ty {
             let key_ty = self.key_ty.rust_type();
             let key_ref_ty = self.key_ty.rust_ref_type();
diff --git a/prost-derive/src/field/mod.rs b/prost-derive/src/field/mod.rs
index 09fef830e..fcd8adaa0 100644
--- a/prost-derive/src/field/mod.rs
+++ b/prost-derive/src/field/mod.rs
@@ -10,7 +10,7 @@ use std::slice;
 use anyhow::{bail, Error};
 use proc_macro2::TokenStream;
 use quote::quote;
-use syn::{Attribute, Ident, Lit, LitBool, Meta, MetaList, MetaNameValue, NestedMeta};
+use syn::{Attribute, Lit, LitBool, Meta, MetaList, MetaNameValue, NestedMeta};
 
 #[derive(Clone)]
 pub enum Field {
@@ -164,7 +164,7 @@ impl Field {
         }
     }
 
-    pub fn methods(&self, ident: &Ident) -> Option<TokenStream> {
+    pub fn methods(&self, ident: &TokenStream) -> Option<TokenStream> {
         match *self {
             Field::Scalar(ref scalar) => scalar.methods(ident),
             Field::Map(ref map) => map.methods(ident),
diff --git a/prost-derive/src/field/scalar.rs b/prost-derive/src/field/scalar.rs
index e088dbab6..6a2fb96d6 100644
--- a/prost-derive/src/field/scalar.rs
+++ b/prost-derive/src/field/scalar.rs
@@ -4,7 +4,9 @@ use std::fmt;
 use anyhow::{anyhow, bail, Error};
 use proc_macro2::{Span, TokenStream};
 use quote::{quote, ToTokens, TokenStreamExt};
-use syn::{parse_str, Ident, Lit, LitByteStr, Meta, MetaList, MetaNameValue, NestedMeta, Path};
+use syn::{
+    parse_str, Ident, Index, Lit, LitByteStr, Meta, MetaList, MetaNameValue, NestedMeta, Path,
+};
 
 use crate::field::{bool_attr, set_option, tag_attr, Label};
 
@@ -267,12 +269,21 @@ impl Field {
     }
 
     /// Returns methods to embed in the message.
-    pub fn methods(&self, ident: &Ident) -> Option<TokenStream> {
+    pub fn methods(&self, ident: &TokenStream) -> Option<TokenStream> {
         let mut ident_str = ident.to_string();
         if ident_str.starts_with("r#") {
             ident_str = ident_str[2..].to_owned();
         }
 
+        // Prepend `get_` for getter methods of tuple structs.
+        let get = match syn::parse_str::<Index>(&ident_str) {
+            Ok(index) => {
+                let get = Ident::new(&format!("get_{}", index.index), Span::call_site());
+                quote!(#get)
+            }
+            Err(_) => quote!(#ident),
+        };
+
         if let Ty::Enumeration(ref ty) = self.ty {
             let set = Ident::new(&format!("set_{}", ident_str), Span::call_site());
             let set_doc = format!("Sets `{}` to the provided enum value.", ident_str);
@@ -285,7 +296,7 @@ impl Field {
                     );
                     quote! {
                         #[doc=#get_doc]
-                        pub fn #ident(&self) -> #ty {
+                        pub fn #get(&self) -> #ty {
                             #ty::from_i32(self.#ident).unwrap_or(#default)
                         }
 
@@ -303,7 +314,7 @@ impl Field {
                     );
                     quote! {
                         #[doc=#get_doc]
-                        pub fn #ident(&self) -> #ty {
+                        pub fn #get(&self) -> #ty {
                             self.#ident.and_then(#ty::from_i32).unwrap_or(#default)
                         }
 
@@ -322,7 +333,7 @@ impl Field {
                     let push_doc = format!("Appends the provided enum value to `{}`.", ident_str);
                     quote! {
                         #[doc=#iter_doc]
-                        pub fn #ident(&self) -> ::core::iter::FilterMap<
+                        pub fn #get(&self) -> ::core::iter::FilterMap<
                             ::core::iter::Cloned<::core::slice::Iter<i32>>,
                             fn(i32) -> ::core::option::Option<#ty>,
                         > {
@@ -351,7 +362,7 @@ impl Field {
 
             Some(quote! {
                 #[doc=#get_doc]
-                pub fn #ident(&self) -> #ty {
+                pub fn #get(&self) -> #ty {
                     match self.#ident {
                         #match_some
                         ::core::option::Option::None => #default,
diff --git a/prost-derive/src/lib.rs b/prost-derive/src/lib.rs
index 71497be57..9a77a7a3b 100644
--- a/prost-derive/src/lib.rs
+++ b/prost-derive/src/lib.rs
@@ -12,7 +12,7 @@ use proc_macro2::Span;
 use quote::quote;
 use syn::{
     punctuated::Punctuated, Data, DataEnum, DataStruct, DeriveInput, Expr, Fields, FieldsNamed,
-    FieldsUnnamed, Ident, Variant,
+    FieldsUnnamed, Ident, Index, Variant,
 };
 
 mod field;
@@ -32,32 +32,36 @@ fn try_message(input: TokenStream) -> Result<TokenStream, Error> {
     let generics = &input.generics;
     let (impl_generics, ty_generics, where_clause) = generics.split_for_impl();
 
-    let fields = match variant_data {
+    let (is_struct, fields) = match variant_data {
         DataStruct {
             fields: Fields::Named(FieldsNamed { named: fields, .. }),
             ..
-        }
-        | DataStruct {
+        } => (true, fields.into_iter().collect()),
+        DataStruct {
             fields:
                 Fields::Unnamed(FieldsUnnamed {
                     unnamed: fields, ..
                 }),
             ..
-        } => fields.into_iter().collect(),
+        } => (false, fields.into_iter().collect()),
         DataStruct {
             fields: Fields::Unit,
             ..
-        } => Vec::new(),
+        } => (false, Vec::new()),
     };
 
     let mut next_tag: u32 = 1;
     let mut fields = fields
         .into_iter()
         .enumerate()
-        .flat_map(|(idx, field)| {
-            let field_ident = field
-                .ident
-                .unwrap_or_else(|| Ident::new(&idx.to_string(), Span::call_site()));
+        .flat_map(|(i, field)| {
+            let field_ident = field.ident.map(|x| quote!(#x)).unwrap_or_else(|| {
+                let index = Index {
+                    index: i as u32,
+                    span: Span::call_site(),
+                };
+                quote!(#index)
+            });
             match Field::new(field.attrs, Some(next_tag)) {
                 Ok(Some(field)) => {
                     next_tag = field.tags().iter().max().map(|t| t + 1).unwrap_or(next_tag);
@@ -124,17 +128,27 @@ fn try_message(input: TokenStream) -> Result<TokenStream, Error> {
         )
     };
 
-    // TODO
-    let is_struct = true;
-
     let clear = fields
         .iter()
         .map(|&(ref field_ident, ref field)| field.clear(quote!(self.#field_ident)));
 
-    let default = fields.iter().map(|&(ref field_ident, ref field)| {
-        let value = field.default();
-        quote!(#field_ident: #value,)
-    });
+    let default = if is_struct {
+        let default = fields.iter().map(|(field_ident, field)| {
+            let value = field.default();
+            quote!(#field_ident: #value,)
+        });
+        quote! {#ident {
+            #(#default)*
+        }}
+    } else {
+        let default = fields.iter().map(|(_, field)| {
+            let value = field.default();
+            quote!(#value,)
+        });
+        quote! {#ident (
+            #(#default)*
+        )}
+    };
 
     let methods = fields
         .iter()
@@ -206,9 +220,7 @@ fn try_message(input: TokenStream) -> Result<TokenStream, Error> {
 
         impl #impl_generics ::core::default::Default for #ident #ty_generics #where_clause {
             fn default() -> Self {
-                #ident {
-                    #(#default)*
-                }
+                #default
             }
         }
 
diff --git a/src/encoding.rs b/src/encoding.rs
index 7674581d5..60b55204a 100644
--- a/src/encoding.rs
+++ b/src/encoding.rs
@@ -1603,9 +1603,6 @@ mod test {
     #[test]
     fn varint() {
         fn check(value: u64, mut encoded: &[u8]) {
-            // TODO(rust-lang/rust-clippy#5494)
-            #![allow(clippy::clone_double_ref)]
-
             // Small buffer.
             let mut buf = Vec::with_capacity(1);
             encode_varint(value, &mut buf);


Test patch:
diff --git a/tests/src/debug.rs b/tests/src/debug.rs
index 23031e434..bfcc29569 100644
--- a/tests/src/debug.rs
+++ b/tests/src/debug.rs
@@ -56,21 +56,17 @@ fn basic() {
     );
 }
 
-/*
-TODO(tokio-rs/prost#56):
-
 /// A special case with a tuple struct
 #[test]
 fn tuple_struct() {
-    #[derive(Clone, PartialEq, Message)]
-    struct NewType(
-        #[prost(enumeration="BasicEnumeration", tag="5")]
-        i32,
+    #[derive(Clone, PartialEq, prost::Message)]
+    struct NewType(#[prost(enumeration = "BasicEnumeration", tag = "5")] i32);
+    assert_eq!(
+        format!("{:?}", NewType(BasicEnumeration::TWO as i32)),
+        "NewType(TWO)"
     );
-    assert_eq!(format!("{:?}", NewType(BasicEnumeration::TWO as i32)), "NewType(TWO)");
     assert_eq!(format!("{:?}", NewType(42)), "NewType(42)");
 }
-*/
 
 #[derive(Clone, PartialEq, prost::Oneof)]
 pub enum OneofWithEnum {