Merge pull request #12 from tomhoule/fix-non-nullability-on-field-type

Fix nullability bug on From impls of FieldType

Author: tomhoule

graphql_query_derive/src/field_type.rs

@@ -1,5 +1,5 @@
 use enums::ENUMS_PREFIX;
-use graphql_parser::schema;
+use graphql_parser;
 use introspection_response;
 use proc_macro2::{Ident, Span, TokenStream};
 use query::QueryContext;
@@ -56,62 +56,68 @@ impl FieldType {
             FieldType::Vector(inner) => inner.inner_name_string(),
         }
     }
-
-    pub fn to_string(&self) -> String {
-        match &self {
-            FieldType::Named(name) => name.to_string(),
-            FieldType::Optional(inner) => format!("Option<{}>", inner.to_string()),
-            FieldType::Vector(inner) => format!("Vec<{}>", inner.to_string()),
-        }
-    }
 }
 
-impl ::std::convert::From<schema::Type> for FieldType {
-    fn from(schema_type: schema::Type) -> FieldType {
+impl ::std::convert::From<graphql_parser::schema::Type> for FieldType {
+    fn from(schema_type: graphql_parser::schema::Type) -> FieldType {
         from_schema_type_inner(schema_type, false)
     }
 }
 
-fn from_schema_type_inner(inner: schema::Type, non_null: bool) -> FieldType {
-    let inner_field_type = match inner {
-        schema::Type::ListType(inner) => {
+fn from_schema_type_inner(inner: graphql_parser::schema::Type, non_null: bool) -> FieldType {
+    match inner {
+        graphql_parser::schema::Type::ListType(inner) => {
             let inner = from_schema_type_inner(*inner, false);
-            FieldType::Vector(Box::new(inner))
+            let f = FieldType::Vector(Box::new(inner));
+            if non_null {
+                f
+            } else {
+                FieldType::Optional(Box::new(f))
+            }
         }
-        schema::Type::NamedType(name) => FieldType::Named(Ident::new(&name, Span::call_site())),
-        schema::Type::NonNullType(inner) => from_schema_type_inner(*inner, true),
-    };
-
-    if non_null {
-        inner_field_type
-    } else {
-        FieldType::Optional(Box::new(inner_field_type))
+        graphql_parser::schema::Type::NamedType(name) => {
+            let f = FieldType::Named(Ident::new(&name, Span::call_site()));
+            if non_null {
+                f
+            } else {
+                FieldType::Optional(Box::new(f))
+            }
+        }
+        graphql_parser::schema::Type::NonNullType(inner) => from_schema_type_inner(*inner, true),
     }
 }
 
 fn from_json_type_inner(inner: &introspection_response::TypeRef, non_null: bool) -> FieldType {
     use introspection_response::*;
     let inner = inner.clone();
 
-    let inner_field_type = match inner.kind {
+    match inner.kind {
         Some(__TypeKind::NON_NULL) => {
             from_json_type_inner(&inner.of_type.expect("inner type is missing"), true)
         }
-        Some(__TypeKind::LIST) => FieldType::Vector(Box::new(from_json_type_inner(
-            &inner.of_type.expect("inner type is missing"),
-            false,
-        ))),
-        Some(_) => FieldType::Named(Ident::new(
-            &inner.name.expect("type name"),
-            Span::call_site(),
-        )),
+        Some(__TypeKind::LIST) => {
+            let f = FieldType::Vector(Box::new(from_json_type_inner(
+                &inner.of_type.expect("inner type is missing"),
+                false,
+            )));
+            if non_null {
+                f
+            } else {
+                FieldType::Optional(Box::new(f))
+            }
+        }
+        Some(_) => {
+            let f = FieldType::Named(Ident::new(
+                &inner.name.expect("type name"),
+                Span::call_site(),
+            ));
+            if non_null {
+                f
+            } else {
+                FieldType::Optional(Box::new(f))
+            }
+        }
         None => unreachable!("non-convertible type"),
-    };
-
-    if non_null {
-        inner_field_type
-    } else {
-        FieldType::Optional(Box::new(inner_field_type))
     }
 }
 
@@ -120,3 +126,63 @@ impl ::std::convert::From<introspection_response::FullTypeFieldsType> for FieldT
         from_json_type_inner(&schema_type.type_ref, false)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use graphql_parser::schema::Type as GqlParserType;
+    use introspection_response::{FullTypeFieldsType, TypeRef, __TypeKind};
+
+    #[test]
+    fn field_type_from_graphql_parser_schema_type_works() {
+        let ty = GqlParserType::NamedType("Cat".to_string());
+        assert_eq!(
+            FieldType::from(ty),
+            FieldType::Optional(Box::new(FieldType::Named(Ident::new(
+                "Cat",
+                Span::call_site()
+            ))))
+        );
+
+        let ty = GqlParserType::NonNullType(Box::new(GqlParserType::NamedType("Cat".to_string())));
+
+        assert_eq!(
+            FieldType::from(ty),
+            FieldType::Named(Ident::new("Cat", Span::call_site()))
+        );
+    }
+
+    #[test]
+    fn field_type_from_introspection_response_works() {
+        let ty = FullTypeFieldsType {
+            type_ref: TypeRef {
+                kind: Some(__TypeKind::OBJECT),
+                name: Some("Cat".to_string()),
+                of_type: None,
+            },
+        };
+        assert_eq!(
+            FieldType::from(ty),
+            FieldType::Optional(Box::new(FieldType::Named(Ident::new(
+                "Cat",
+                Span::call_site()
+            ))))
+        );
+
+        let ty = FullTypeFieldsType {
+            type_ref: TypeRef {
+                kind: Some(__TypeKind::NON_NULL),
+                name: None,
+                of_type: Some(Box::new(TypeRef {
+                    kind: Some(__TypeKind::OBJECT),
+                    name: Some("Cat".to_string()),
+                    of_type: None,
+                })),
+            },
+        };
+        assert_eq!(
+            FieldType::from(ty),
+            FieldType::Named(Ident::new("Cat", Span::call_site()))
+        );
+    }
+}

graphql_query_derive/src/schema.rs

@@ -369,17 +369,11 @@ mod tests {
                     },
                     GqlObjectField {
                         name: "id".to_string(),
-                        type_: FieldType::Optional(Box::new(FieldType::Named(Ident::new(
-                            "ID",
-                            Span::call_site(),
-                        )))),
+                        type_: FieldType::Named(Ident::new("ID", Span::call_site())),
                     },
                     GqlObjectField {
                         name: "name".to_string(),
-                        type_: FieldType::Optional(Box::new(FieldType::Named(Ident::new(
-                            "String",
-                            Span::call_site(),
-                        )))),
+                        type_: FieldType::Named(Ident::new("String", Span::call_site())),
                     },
                     GqlObjectField {
                         name: "friends".to_string(),
@@ -392,18 +386,12 @@ mod tests {
                     },
                     GqlObjectField {
                         name: "friendsConnection".to_string(),
-                        type_: FieldType::Optional(Box::new(FieldType::Named(Ident::new(
-                            "FriendsConnection",
-                            Span::call_site(),
-                        )))),
+                        type_: FieldType::Named(Ident::new("FriendsConnection", Span::call_site())),
                     },
                     GqlObjectField {
                         name: "appearsIn".to_string(),
-                        type_: FieldType::Optional(Box::new(FieldType::Vector(Box::new(
-                            FieldType::Optional(Box::new(FieldType::Named(Ident::new(
-                                "Episode",
-                                Span::call_site(),
-                            )))),
+                        type_: FieldType::Vector(Box::new(FieldType::Optional(Box::new(
+                            FieldType::Named(Ident::new("Episode", Span::call_site())),
                         )))),
                     },
                     GqlObjectField {

graphql_query_derive/src/unions.rs

@@ -1,6 +1,5 @@
 use constants::*;
 use failure;
-use heck::SnakeCase;
 use proc_macro2::{Ident, Span, TokenStream};
 use query::QueryContext;
 use selection::{Selection, SelectionItem};
@@ -15,7 +14,7 @@ enum UnionError {
     #[fail(display = "Unknown type: {}", ty)]
     UnknownType { ty: String },
     #[fail(display = "Missing __typename in selection for {}", union_name)]
-    MissingTypename { union_name: String }
+    MissingTypename { union_name: String },
 }
 
 impl GqlUnion {
@@ -37,7 +36,9 @@ impl GqlUnion {
         });
 
         if typename_field.is_none() {
-            Err(UnionError::MissingTypename { union_name: prefix.into() })?;
+            Err(UnionError::MissingTypename {
+                union_name: prefix.into(),
+            })?;
         }
 
         let variants: Result<Vec<TokenStream>, failure::Error> = selection
@@ -53,7 +54,7 @@ impl GqlUnion {
             })
             .map(|item| {
                 match item {
-                    SelectionItem::Field(f) => panic!("field selection on union"),
+                    SelectionItem::Field(_) => panic!("field selection on union"),
                     SelectionItem::FragmentSpread(_) => panic!("fragment spread on union"),
                     SelectionItem::InlineFragment(frag) => {
                         let variant_name = Ident::new(&frag.on, Span::call_site());
@@ -63,23 +64,23 @@ impl GqlUnion {
                         let variant_type = Ident::new(&new_prefix, Span::call_site());
 
                         let field_object_type =
-                            query_context.schema.objects.get(&frag.on).map(|f| {
+                            query_context.schema.objects.get(&frag.on).map(|_f| {
                                 query_context.maybe_expand_field(
                                     &frag.on,
                                     &frag.fields,
                                     &new_prefix,
                                 )
                             });
                         let field_interface =
-                            query_context.schema.interfaces.get(&frag.on).map(|f| {
+                            query_context.schema.interfaces.get(&frag.on).map(|_f| {
                                 query_context.maybe_expand_field(
                                     &frag.on,
                                     &frag.fields,
                                     &new_prefix,
                                 )
                             });
                         // nested unions, is that even a thing?
-                        let field_union_type = query_context.schema.unions.get(&frag.on).map(|f| {
+                        let field_union_type = query_context.schema.unions.get(&frag.on).map(|_f| {
                             query_context.maybe_expand_field(&frag.on, &frag.fields, &new_prefix)
                         });
 
@@ -184,7 +185,10 @@ mod tests {
 
         assert!(result.is_err());
 
-        assert_eq!(format!("{}", result.unwrap_err()), "Missing __typename in selection for Meow");
+        assert_eq!(
+            format!("{}", result.unwrap_err()),
+            "Missing __typename in selection for Meow"
+        );
     }
 
     #[test]
@@ -196,21 +200,17 @@ mod tests {
             }),
             SelectionItem::InlineFragment(SelectionInlineFragment {
                 on: "User".to_string(),
-                fields: Selection(vec![
-                    SelectionItem::Field(SelectionField {
-                        name: "first_name".to_string(),
-                        fields: Selection(vec![]),
-                    }),
-                ]),
+                fields: Selection(vec![SelectionItem::Field(SelectionField {
+                    name: "first_name".to_string(),
+                    fields: Selection(vec![]),
+                })]),
             }),
             SelectionItem::InlineFragment(SelectionInlineFragment {
                 on: "Organization".to_string(),
-                fields: Selection(vec![
-                    SelectionItem::Field(SelectionField {
-                        name: "title".to_string(),
-                        fields: Selection(vec![]),
-                    }),
-                ]),
+                fields: Selection(vec![SelectionItem::Field(SelectionField {
+                    name: "title".to_string(),
+                    fields: Selection(vec![]),
+                })]),
             }),
         ];
         let mut context = QueryContext::new_empty();

tests/union_query.rs

@@ -9,9 +9,9 @@ const RESPONSE: &'static str = include_str!("union_query_response.json");
 
 #[derive(GraphQLQuery)]
 #[GraphQLQuery(
-    query_path = "tests/union_query.graphql",
-    schema_path = "tests/union_schema.graphql",
+    query_path = "tests/union_query.graphql", schema_path = "tests/union_schema.graphql"
 )]
+#[allow(dead_code)]
 struct UnionQuery;
 
 #[test]
@@ -20,10 +20,9 @@ fn union_query_deserialization() {
 
     println!("{:?}", response_data);
 
-    let expected = r##"ResponseData { names: Some([Some(Person(RustMyQueryNamesOnPerson { firstName: Some("Audrey"), lastName: Some("Lorde") })), Some(Dog(RustMyQueryNamesOnDog { name: Some("Laïka") })), Some(Organization(RustMyQueryNamesOnOrganization { title: Some("Mozilla") })), Some(Dog(RustMyQueryNamesOnDog { name: Some("Norbert") }))]) }"##;
+    let expected = r##"ResponseData { names: Some([Person(RustMyQueryNamesOnPerson { firstName: "Audrey", lastName: Some("Lorde") }), Dog(RustMyQueryNamesOnDog { name: "Laïka" }), Organization(RustMyQueryNamesOnOrganization { title: "Mozilla" }), Dog(RustMyQueryNamesOnDog { name: "Norbert" })]) }"##;
 
     assert_eq!(format!("{:?}", response_data), expected);
 
     assert_eq!(response_data.names.map(|names| names.len()), Some(4));
-
 }