Skip to content

Commit d25220c

Browse files
tomhouletomhoule
authored
Merge pull request #12 from tomhoule/fix-non-nullability-on-field-type
Fix nullability bug on From impls of FieldType
2 parents c9d30d7 + 851f356 commit d25220c

File tree

4 files changed

+132
-79
lines changed

4 files changed

+132
-79
lines changed

‎graphql_query_derive/src/field_type.rs

Lines changed: 104 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use enums::ENUMS_PREFIX;
2-
use graphql_parser::schema;
2+
use graphql_parser;
33
use introspection_response;
44
use proc_macro2::{Ident, Span, TokenStream};
55
use query::QueryContext;
@@ -56,62 +56,68 @@ impl FieldType {
5656
FieldType::Vector(inner) => inner.inner_name_string(),
5757
}
5858
}
59-
60-
pub fn to_string(&self) -> String {
61-
match &self {
62-
FieldType::Named(name) => name.to_string(),
63-
FieldType::Optional(inner) => format!("Option<{}>", inner.to_string()),
64-
FieldType::Vector(inner) => format!("Vec<{}>", inner.to_string()),
65-
}
66-
}
6759
}
6860

69-
impl ::std::convert::From<schema::Type> for FieldType {
70-
fn from(schema_type: schema::Type) -> FieldType {
61+
impl ::std::convert::From<graphql_parser::schema::Type> for FieldType {
62+
fn from(schema_type: graphql_parser::schema::Type) -> FieldType {
7163
from_schema_type_inner(schema_type, false)
7264
}
7365
}
7466

75-
fn from_schema_type_inner(inner: schema::Type, non_null: bool) -> FieldType {
76-
let inner_field_type = match inner {
77-
schema::Type::ListType(inner) => {
67+
fn from_schema_type_inner(inner: graphql_parser::schema::Type, non_null: bool) -> FieldType {
68+
match inner {
69+
graphql_parser::schema::Type::ListType(inner) => {
7870
let inner = from_schema_type_inner(*inner, false);
79-
FieldType::Vector(Box::new(inner))
71+
let f = FieldType::Vector(Box::new(inner));
72+
if non_null {
73+
f
74+
} else {
75+
FieldType::Optional(Box::new(f))
76+
}
8077
}
81-
schema::Type::NamedType(name) => FieldType::Named(Ident::new(&name, Span::call_site())),
82-
schema::Type::NonNullType(inner) => from_schema_type_inner(*inner, true),
83-
};
84-
85-
if non_null {
86-
inner_field_type
87-
} else {
88-
FieldType::Optional(Box::new(inner_field_type))
78+
graphql_parser::schema::Type::NamedType(name) => {
79+
let f = FieldType::Named(Ident::new(&name, Span::call_site()));
80+
if non_null {
81+
f
82+
} else {
83+
FieldType::Optional(Box::new(f))
84+
}
85+
}
86+
graphql_parser::schema::Type::NonNullType(inner) => from_schema_type_inner(*inner, true),
8987
}
9088
}
9189

9290
fn from_json_type_inner(inner: &introspection_response::TypeRef, non_null: bool) -> FieldType {
9391
use introspection_response::*;
9492
let inner = inner.clone();
9593

96-
let inner_field_type = match inner.kind {
94+
match inner.kind {
9795
Some(__TypeKind::NON_NULL) => {
9896
from_json_type_inner(&inner.of_type.expect("inner type is missing"), true)
9997
}
100-
Some(__TypeKind::LIST) => FieldType::Vector(Box::new(from_json_type_inner(
101-
&inner.of_type.expect("inner type is missing"),
102-
false,
103-
))),
104-
Some(_) => FieldType::Named(Ident::new(
105-
&inner.name.expect("type name"),
106-
Span::call_site(),
107-
)),
98+
Some(__TypeKind::LIST) => {
99+
let f = FieldType::Vector(Box::new(from_json_type_inner(
100+
&inner.of_type.expect("inner type is missing"),
101+
false,
102+
)));
103+
if non_null {
104+
f
105+
} else {
106+
FieldType::Optional(Box::new(f))
107+
}
108+
}
109+
Some(_) => {
110+
let f = FieldType::Named(Ident::new(
111+
&inner.name.expect("type name"),
112+
Span::call_site(),
113+
));
114+
if non_null {
115+
f
116+
} else {
117+
FieldType::Optional(Box::new(f))
118+
}
119+
}
108120
None => unreachable!("non-convertible type"),
109-
};
110-
111-
if non_null {
112-
inner_field_type
113-
} else {
114-
FieldType::Optional(Box::new(inner_field_type))
115121
}
116122
}
117123

@@ -120,3 +126,63 @@ impl ::std::convert::From<introspection_response::FullTypeFieldsType> for FieldT
120126
from_json_type_inner(&schema_type.type_ref, false)
121127
}
122128
}
129+
130+
#[cfg(test)]
131+
mod tests {
132+
use super::*;
133+
use graphql_parser::schema::Type as GqlParserType;
134+
use introspection_response::{FullTypeFieldsType, TypeRef, __TypeKind};
135+
136+
#[test]
137+
fn field_type_from_graphql_parser_schema_type_works() {
138+
let ty = GqlParserType::NamedType("Cat".to_string());
139+
assert_eq!(
140+
FieldType::from(ty),
141+
FieldType::Optional(Box::new(FieldType::Named(Ident::new(
142+
"Cat",
143+
Span::call_site()
144+
))))
145+
);
146+
147+
let ty = GqlParserType::NonNullType(Box::new(GqlParserType::NamedType("Cat".to_string())));
148+
149+
assert_eq!(
150+
FieldType::from(ty),
151+
FieldType::Named(Ident::new("Cat", Span::call_site()))
152+
);
153+
}
154+
155+
#[test]
156+
fn field_type_from_introspection_response_works() {
157+
let ty = FullTypeFieldsType {
158+
type_ref: TypeRef {
159+
kind: Some(__TypeKind::OBJECT),
160+
name: Some("Cat".to_string()),
161+
of_type: None,
162+
},
163+
};
164+
assert_eq!(
165+
FieldType::from(ty),
166+
FieldType::Optional(Box::new(FieldType::Named(Ident::new(
167+
"Cat",
168+
Span::call_site()
169+
))))
170+
);
171+
172+
let ty = FullTypeFieldsType {
173+
type_ref: TypeRef {
174+
kind: Some(__TypeKind::NON_NULL),
175+
name: None,
176+
of_type: Some(Box::new(TypeRef {
177+
kind: Some(__TypeKind::OBJECT),
178+
name: Some("Cat".to_string()),
179+
of_type: None,
180+
})),
181+
},
182+
};
183+
assert_eq!(
184+
FieldType::from(ty),
185+
FieldType::Named(Ident::new("Cat", Span::call_site()))
186+
);
187+
}
188+
}

‎graphql_query_derive/src/schema.rs

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -369,17 +369,11 @@ mod tests {
369369
},
370370
GqlObjectField {
371371
name: "id".to_string(),
372-
type_: FieldType::Optional(Box::new(FieldType::Named(Ident::new(
373-
"ID",
374-
Span::call_site(),
375-
)))),
372+
type_: FieldType::Named(Ident::new("ID", Span::call_site())),
376373
},
377374
GqlObjectField {
378375
name: "name".to_string(),
379-
type_: FieldType::Optional(Box::new(FieldType::Named(Ident::new(
380-
"String",
381-
Span::call_site(),
382-
)))),
376+
type_: FieldType::Named(Ident::new("String", Span::call_site())),
383377
},
384378
GqlObjectField {
385379
name: "friends".to_string(),
@@ -392,18 +386,12 @@ mod tests {
392386
},
393387
GqlObjectField {
394388
name: "friendsConnection".to_string(),
395-
type_: FieldType::Optional(Box::new(FieldType::Named(Ident::new(
396-
"FriendsConnection",
397-
Span::call_site(),
398-
)))),
389+
type_: FieldType::Named(Ident::new("FriendsConnection", Span::call_site())),
399390
},
400391
GqlObjectField {
401392
name: "appearsIn".to_string(),
402-
type_: FieldType::Optional(Box::new(FieldType::Vector(Box::new(
403-
FieldType::Optional(Box::new(FieldType::Named(Ident::new(
404-
"Episode",
405-
Span::call_site(),
406-
)))),
393+
type_: FieldType::Vector(Box::new(FieldType::Optional(Box::new(
394+
FieldType::Named(Ident::new("Episode", Span::call_site())),
407395
)))),
408396
},
409397
GqlObjectField {

‎graphql_query_derive/src/unions.rs

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
use constants::*;
22
use failure;
3-
use heck::SnakeCase;
43
use proc_macro2::{Ident, Span, TokenStream};
54
use query::QueryContext;
65
use selection::{Selection, SelectionItem};
@@ -15,7 +14,7 @@ enum UnionError {
1514
#[fail(display = "Unknown type: {}", ty)]
1615
UnknownType { ty: String },
1716
#[fail(display = "Missing __typename in selection for {}", union_name)]
18-
MissingTypename { union_name: String }
17+
MissingTypename { union_name: String },
1918
}
2019

2120
impl GqlUnion {
@@ -37,7 +36,9 @@ impl GqlUnion {
3736
});
3837

3938
if typename_field.is_none() {
40-
Err(UnionError::MissingTypename { union_name: prefix.into() })?;
39+
Err(UnionError::MissingTypename {
40+
union_name: prefix.into(),
41+
})?;
4142
}
4243

4344
let variants: Result<Vec<TokenStream>, failure::Error> = selection
@@ -53,7 +54,7 @@ impl GqlUnion {
5354
})
5455
.map(|item| {
5556
match item {
56-
SelectionItem::Field(f) => panic!("field selection on union"),
57+
SelectionItem::Field(_) => panic!("field selection on union"),
5758
SelectionItem::FragmentSpread(_) => panic!("fragment spread on union"),
5859
SelectionItem::InlineFragment(frag) => {
5960
let variant_name = Ident::new(&frag.on, Span::call_site());
@@ -63,23 +64,23 @@ impl GqlUnion {
6364
let variant_type = Ident::new(&new_prefix, Span::call_site());
6465

6566
let field_object_type =
66-
query_context.schema.objects.get(&frag.on).map(|f| {
67+
query_context.schema.objects.get(&frag.on).map(|_f| {
6768
query_context.maybe_expand_field(
6869
&frag.on,
6970
&frag.fields,
7071
&new_prefix,
7172
)
7273
});
7374
let field_interface =
74-
query_context.schema.interfaces.get(&frag.on).map(|f| {
75+
query_context.schema.interfaces.get(&frag.on).map(|_f| {
7576
query_context.maybe_expand_field(
7677
&frag.on,
7778
&frag.fields,
7879
&new_prefix,
7980
)
8081
});
8182
// nested unions, is that even a thing?
82-
let field_union_type = query_context.schema.unions.get(&frag.on).map(|f| {
83+
let field_union_type = query_context.schema.unions.get(&frag.on).map(|_f| {
8384
query_context.maybe_expand_field(&frag.on, &frag.fields, &new_prefix)
8485
});
8586

@@ -184,7 +185,10 @@ mod tests {
184185

185186
assert!(result.is_err());
186187

187-
assert_eq!(format!("{}", result.unwrap_err()), "Missing __typename in selection for Meow");
188+
assert_eq!(
189+
format!("{}", result.unwrap_err()),
190+
"Missing __typename in selection for Meow"
191+
);
188192
}
189193

190194
#[test]
@@ -196,21 +200,17 @@ mod tests {
196200
}),
197201
SelectionItem::InlineFragment(SelectionInlineFragment {
198202
on: "User".to_string(),
199-
fields: Selection(vec![
200-
SelectionItem::Field(SelectionField {
201-
name: "first_name".to_string(),
202-
fields: Selection(vec![]),
203-
}),
204-
]),
203+
fields: Selection(vec![SelectionItem::Field(SelectionField {
204+
name: "first_name".to_string(),
205+
fields: Selection(vec![]),
206+
})]),
205207
}),
206208
SelectionItem::InlineFragment(SelectionInlineFragment {
207209
on: "Organization".to_string(),
208-
fields: Selection(vec![
209-
SelectionItem::Field(SelectionField {
210-
name: "title".to_string(),
211-
fields: Selection(vec![]),
212-
}),
213-
]),
210+
fields: Selection(vec![SelectionItem::Field(SelectionField {
211+
name: "title".to_string(),
212+
fields: Selection(vec![]),
213+
})]),
214214
}),
215215
];
216216
let mut context = QueryContext::new_empty();

‎tests/union_query.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ const RESPONSE: &'static str = include_str!("union_query_response.json");
99

1010
#[derive(GraphQLQuery)]
1111
#[GraphQLQuery(
12-
query_path = "tests/union_query.graphql",
13-
schema_path = "tests/union_schema.graphql",
12+
query_path = "tests/union_query.graphql", schema_path = "tests/union_schema.graphql"
1413
)]
14+
#[allow(dead_code)]
1515
struct UnionQuery;
1616

1717
#[test]
@@ -20,10 +20,9 @@ fn union_query_deserialization() {
2020

2121
println!("{:?}", response_data);
2222

23-
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") }))]) }"##;
23+
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" })]) }"##;
2424

2525
assert_eq!(format!("{:?}", response_data), expected);
2626

2727
assert_eq!(response_data.names.map(|names| names.len()), Some(4));
28-
2928
}

0 commit comments

Comments
 (0)