Make FieldResults in field resolvers optional :D

Author: mhallin

README.md

@@ -132,7 +132,7 @@ as well.
     * [X] `graphql_input_object!` helper completely missing
     * [X] Add support for deprecating things
     * [X] Custom enum values and descriptions
-    * [ ] Improved syntax for fields that can't fail resolution - make
+    * [X] Improved syntax for fields that can't fail resolution - make
       `FieldResult<T>` optional maybe?
 * [ ] Investigate asynchronous execution - implementing it is not necessary, but
   at least look at what API changes will be needed for us to hook into

src/executor.rs

@@ -61,6 +61,23 @@ pub type FieldResult<T> = Result<T, String>;
 /// The result of resolving an unspecified field
 pub type ExecutionResult = Result<Value, String>;
 
+/// Convert a value into a successful field result
+///
+/// Used by the helper macros to support both returning a naked value
+/// *and* a `FieldResult` from a field.
+pub trait IntoFieldResult<T>: Sized {
+    /// Wrap `self` in a `Result`
+    ///
+    /// The implementation of this should always be `Ok(self)`.
+    fn into(self) -> FieldResult<T>;
+}
+
+impl<T> IntoFieldResult<T> for FieldResult<T> {
+    fn into(self) -> FieldResult<T> {
+        self
+    }
+}
+
 impl<'a, CtxT> Executor<'a, CtxT> {
     /// Resolve a single arbitrary value into an `ExecutionResult`
     pub fn resolve<T: GraphQLType<CtxT>>(&mut self, value: &T) -> ExecutionResult {
@@ -293,6 +310,17 @@ impl<CtxT> Registry<CtxT> {
         }
     }
 
+    #[doc(hidden)]
+    pub fn field_convert<T: IntoFieldResult<I>, I>(&mut self, name: &str) -> Field where I: GraphQLType<CtxT> {
+        Field {
+            name: name.to_owned(),
+            description: None,
+            arguments: None,
+            field_type: self.get_type::<I>(),
+            deprecation_reason: None,
+        }
+    }
+
     #[doc(hidden)]
     pub fn field_inside_result<T>(&mut self, name: &str, _: FieldResult<T>) -> Field where T: GraphQLType<CtxT> {
         Field {

src/executor_tests/introspection.rs

@@ -1,6 +1,5 @@
 use std::collections::HashMap;
 
-use executor::FieldResult;
 use value::Value;
 use schema::model::RootNode;
 
@@ -32,9 +31,9 @@ graphql_enum!(Sample as "SampleEnum" {
 
 graphql_interface!(Interface: () as "SampleInterface" |&self| {
     description: "A sample interface"
-    
-    field sample_enum() -> FieldResult<Sample> as "A sample field in the interface" {
-        Ok(Sample::One)
+
+    field sample_enum() -> Sample as "A sample field in the interface" {
+        Sample::One
     }
 
     instance_resolvers: |&_| [
@@ -47,15 +46,15 @@ graphql_object!(Root: () as "Root" |&self| {
 
     interfaces: [Interface]
 
-    field sample_enum() -> FieldResult<Sample> {
-        Ok(Sample::One)
+    field sample_enum() -> Sample {
+        Sample::One
     }
 
     field sample_scalar(
-        first: i64 as "The first number", 
+        first: i64 as "The first number",
         second = 123: i64 as "The second number"
-    ) -> FieldResult<Scalar> as "A sample scalar field on the object" {
-        Ok(Scalar(first + second))
+    ) -> Scalar as "A sample scalar field on the object" {
+        Scalar(first + second)
     }
 });
 
@@ -121,7 +120,7 @@ fn enum_introspection() {
 
     assert_eq!(type_info.get("name"), Some(&Value::string("SampleEnum")));
     assert_eq!(type_info.get("kind"), Some(&Value::string("ENUM")));
-    assert_eq!(type_info.get("description"), Some(&Value::null())); 
+    assert_eq!(type_info.get("description"), Some(&Value::null()));
     assert_eq!(type_info.get("interfaces"), Some(&Value::null()));
     assert_eq!(type_info.get("possibleTypes"), Some(&Value::null()));
     assert_eq!(type_info.get("inputFields"), Some(&Value::null()));

src/lib.rs

@@ -28,8 +28,8 @@ existing object types as GraphQL objects:
 
 ```rust
 #[macro_use] extern crate juniper;
-use juniper::FieldResult;
 # use std::collections::HashMap;
+use juniper::FieldResult;
 
 struct User { id: String, name: String, friend_ids: Vec<String>  }
 struct QueryRoot;
@@ -42,15 +42,19 @@ struct Database { users: HashMap<String, User> }
 graphql_object!(User: Database as "User" |&self| {
 
     // Expose a simple field as a GraphQL string.
+    field id() -> &String {
+        &self.id
+    }
+
+    field name() -> &String {
+        &self.name
+    }
+
     // FieldResult<T> is an alias for Result<T, String> - simply return
     // a string from this method and it will be correctly inserted into
     // the execution response.
-    field id() -> FieldResult<&String> {
-        Ok(&self.id)
-    }
-
-    field name() -> FieldResult<&String> {
-        Ok(&self.name)
+    field secret() -> FieldResult<&String> {
+        Err("Can't touch this".to_owned())
     }
 
     // Field accessors can optionally take an "executor" as their first
@@ -59,10 +63,10 @@ graphql_object!(User: Database as "User" |&self| {
     //
     // In this example, the context is used to convert the friend_ids array
     // into actual User objects.
-    field friends(&mut executor) -> FieldResult<Vec<&User>> {
-        Ok(self.friend_ids.iter()
+    field friends(&mut executor) -> Vec<&User> {
+        self.friend_ids.iter()
             .filter_map(|id| executor.context().users.get(id))
-            .collect())
+            .collect()
     }
 });
 
@@ -71,8 +75,8 @@ graphql_object!(User: Database as "User" |&self| {
 graphql_object!(QueryRoot: Database as "Query" |&self| {
 
     // Arguments work just like they do on functions.
-    field user(&mut executor, id: String) -> FieldResult<Option<&User>> {
-        Ok(executor.context().users.get(&id))
+    field user(&mut executor, id: String) -> Option<&User> {
+        executor.context().users.get(&id)
     }
 });
 
@@ -197,13 +201,13 @@ use rustc_serialize::json::{ToJson, Json};
 
 use parser::{parse_document_source, ParseError, Spanning, SourcePosition};
 use validation::{RuleError, ValidatorContext, visit_all_rules};
+use executor::execute_validated_query;
 
 pub use ast::{ToInputValue, FromInputValue, InputValue, Type, Selection};
 pub use value::Value;
 pub use types::base::{Arguments, GraphQLType, TypeKind};
 pub use executor::{
-    Executor, Registry, ExecutionResult, ExecutionError, FieldResult,
-    execute_validated_query,
+    Executor, Registry, ExecutionResult, ExecutionError, FieldResult, IntoFieldResult,
 };
 pub use types::scalars::ID;
 pub use schema::model::RootNode;

src/macros/enums.rs

@@ -110,6 +110,12 @@ macro_rules! graphql_enum {
                 }
             }
         }
+
+        impl $crate::IntoFieldResult<$name> for $name {
+            fn into(self) -> $crate::FieldResult<$name> {
+                Ok(self)
+            }
+        }
     };
 
     // No more items to parse

src/macros/field.rs

@@ -77,7 +77,7 @@ macro_rules! __graphql__build_field_matches {
                     $body
                 };
 
-                return result.and_then(|r| $executorvar.resolve(&r))
+                return ($crate::IntoFieldResult::into(result)).and_then(|r| $executorvar.resolve(&r))
             }
         )*
         panic!("Field {} not found on type {}", $fieldvar, $outname);

src/macros/interface.rs

@@ -38,7 +38,6 @@ shared context to implement downcasts.
 
 ```rust
 # #[macro_use] extern crate juniper;
-# use juniper::FieldResult;
 # use std::collections::HashMap;
 struct Human { id: String }
 struct Droid { id: String }
@@ -60,16 +59,16 @@ impl Character for Droid {
 }
 
 graphql_object!(Human: Database as "Human" |&self| {
-    field id() -> FieldResult<&str> { Ok(&self.id) }
+    field id() -> &str { &self.id }
 });
 
 graphql_object!(Droid: Database as "Droid" |&self| {
-    field id() -> FieldResult<&str> { Ok(&self.id) }
+    field id() -> &str { &self.id }
 });
 
 // You can introduce lifetimes or generic parameters by < > before the name.
 graphql_interface!(<'a> &'a Character: Database as "Character" |&self| {
-    field id() -> FieldResult<&str> { Ok(self.id()) }
+    field id() -> &str { self.id() }
 
     instance_resolvers: |&context| [
         context.humans.get(self.id()),
@@ -97,9 +96,8 @@ macro_rules! graphql_interface {
         $acc.push(__graphql__args!(
             @apply_args,
             $reg,
-            $reg.field_inside_result(
-                &$crate::to_snake_case(stringify!($name)),
-                Err("dummy".to_owned()) as $t)
+            $reg.field_convert::<$t, _>(
+                &$crate::to_snake_case(stringify!($name)))
                 .description($desc)
                 .deprecated($reason),
             $args));
@@ -116,9 +114,8 @@ macro_rules! graphql_interface {
         $acc.push(__graphql__args!(
             @apply_args,
             $reg,
-            $reg.field_inside_result(
-                &$crate::to_snake_case(stringify!($name)),
-                Err("dummy".to_owned()) as $t)
+            $reg.field_convert::<$t, _>(
+                &$crate::to_snake_case(stringify!($name)))
                 .deprecated($reason),
             $args));
 
@@ -134,9 +131,8 @@ macro_rules! graphql_interface {
         $acc.push(__graphql__args!(
             @apply_args,
             $reg,
-            $reg.field_inside_result(
-                &$crate::to_snake_case(stringify!($name)),
-                Err("dummy".to_owned()) as $t)
+            $reg.field_convert::<$t, _>(
+                &$crate::to_snake_case(stringify!($name)))
                 .description($desc),
             $args));
 
@@ -152,9 +148,8 @@ macro_rules! graphql_interface {
         $acc.push(__graphql__args!(
             @apply_args,
             $reg,
-            $reg.field_inside_result(
-                &$crate::to_snake_case(stringify!($name)),
-                Err("dummy".to_owned()) as $t),
+            $reg.field_convert::<$t, _>(
+                &$crate::to_snake_case(stringify!($name))),
             $args));
 
         graphql_interface!(@gather_meta, $reg, $acc, $descr, $( $rest )*);