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 {

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));
-
 }