From a684e1d91cea0a33ef1a9c76a5f7e236d149ea3e Mon Sep 17 00:00:00 2001 From: Kai Ren Date: Wed, 2 Sep 2020 22:48:54 +0300 Subject: [PATCH] Re-enable marks-based static checks in code generated by macros (#751) - add associated type to IntoResolvable and IntoFieldResult traits allowing to name the GraphQLType being resolved - relax Sized requirement on some IsInputType and IsOutputType impls --- .../derive_incompatible_object.stderr | 9 ++ .../object/impl_argument_no_object.stderr | 9 ++ .../src/codegen/derive_input_object.rs | 6 +- .../juniper_tests/src/codegen/impl_object.rs | 15 +++- juniper/src/executor/mod.rs | 14 +++ juniper/src/lib.rs | 2 +- juniper/src/macros/interface.rs | 4 + juniper/src/macros/subscription_helpers.rs | 21 +++-- juniper/src/types/marker.rs | 7 +- juniper_codegen/src/derive_scalar_value.rs | 3 + juniper_codegen/src/graphql_union/mod.rs | 2 +- juniper_codegen/src/util/mod.rs | 88 ++++++++++++------- 12 files changed, 136 insertions(+), 44 deletions(-) diff --git a/integration_tests/codegen_fail/fail/input-object/derive_incompatible_object.stderr b/integration_tests/codegen_fail/fail/input-object/derive_incompatible_object.stderr index 69932a98..92eeb518 100644 --- a/integration_tests/codegen_fail/fail/input-object/derive_incompatible_object.stderr +++ b/integration_tests/codegen_fail/fail/input-object/derive_incompatible_object.stderr @@ -1,3 +1,12 @@ +error[E0277]: the trait bound `ObjectA: juniper::marker::IsInputType<__S>` is not satisfied + --> $DIR/derive_incompatible_object.rs:6:10 + | +6 | #[derive(juniper::GraphQLInputObject)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `juniper::marker::IsInputType<__S>` is not implemented for `ObjectA` + | + = note: required by `juniper::marker::IsInputType::mark` + = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info) + error[E0277]: the trait bound `ObjectA: juniper::FromInputValue<__S>` is not satisfied --> $DIR/derive_incompatible_object.rs:6:10 | diff --git a/integration_tests/codegen_fail/fail/object/impl_argument_no_object.stderr b/integration_tests/codegen_fail/fail/object/impl_argument_no_object.stderr index 19ead7c4..946ea4fd 100644 --- a/integration_tests/codegen_fail/fail/object/impl_argument_no_object.stderr +++ b/integration_tests/codegen_fail/fail/object/impl_argument_no_object.stderr @@ -1,3 +1,12 @@ +error[E0277]: the trait bound `Obj: juniper::marker::IsInputType` is not satisfied + --> $DIR/impl_argument_no_object.rs:8:1 + | +8 | #[juniper::graphql_object] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `juniper::marker::IsInputType` is not implemented for `Obj` + | + = note: required by `juniper::marker::IsInputType::mark` + = note: this error originates in an attribute macro (in Nightly builds, run with -Z macro-backtrace for more info) + error[E0277]: the trait bound `Obj: juniper::FromInputValue` is not satisfied --> $DIR/impl_argument_no_object.rs:8:1 | diff --git a/integration_tests/juniper_tests/src/codegen/derive_input_object.rs b/integration_tests/juniper_tests/src/codegen/derive_input_object.rs index 85604a84..d018ad8b 100644 --- a/integration_tests/juniper_tests/src/codegen/derive_input_object.rs +++ b/integration_tests/juniper_tests/src/codegen/derive_input_object.rs @@ -1,8 +1,8 @@ use fnv::FnvHashMap; use juniper::{ - DefaultScalarValue, FromInputValue, GraphQLInputObject, GraphQLType, GraphQLValue, InputValue, - ToInputValue, + marker, DefaultScalarValue, FromInputValue, GraphQLInputObject, GraphQLType, GraphQLValue, + InputValue, ToInputValue, }; #[derive(GraphQLInputObject, Debug, PartialEq)] @@ -50,6 +50,8 @@ struct OverrideDocComment { #[derive(Debug, PartialEq)] struct Fake; +impl<'a> marker::IsInputType for &'a Fake {} + impl<'a> FromInputValue for &'a Fake { fn from_input_value(_v: &InputValue) -> Option<&'a Fake> { None diff --git a/integration_tests/juniper_tests/src/codegen/impl_object.rs b/integration_tests/juniper_tests/src/codegen/impl_object.rs index 5f676086..463be9b7 100644 --- a/integration_tests/juniper_tests/src/codegen/impl_object.rs +++ b/integration_tests/juniper_tests/src/codegen/impl_object.rs @@ -3,7 +3,7 @@ use juniper::DefaultScalarValue; use juniper::Object; #[cfg(test)] -use juniper::{self, execute, EmptyMutation, EmptySubscription, RootNode, Value, Variables}; +use juniper::{execute, EmptyMutation, EmptySubscription, FieldError, RootNode, Value, Variables}; pub struct MyObject; @@ -84,3 +84,16 @@ where f((type_info, fields)); } + +mod fallible { + use super::*; + + struct Obj; + + #[juniper::graphql_object] + impl Obj { + fn test(&self, arg: String) -> Result { + Ok(arg) + } + } +} diff --git a/juniper/src/executor/mod.rs b/juniper/src/executor/mod.rs index f58aef1c..07be7ac9 100644 --- a/juniper/src/executor/mod.rs +++ b/juniper/src/executor/mod.rs @@ -253,6 +253,8 @@ where T: GraphQLValue, S: ScalarValue, { + type Type; + #[doc(hidden)] fn into(self, ctx: &'a C) -> FieldResult, S>; } @@ -263,6 +265,8 @@ where S: ScalarValue, T::Context: FromContext, { + type Type = T; + fn into(self, ctx: &'a C) -> FieldResult, S> { Ok(Some((FromContext::from(ctx), self))) } @@ -274,6 +278,8 @@ where T: GraphQLValue, T::Context: FromContext, { + type Type = T; + fn into(self, ctx: &'a C) -> FieldResult, S> { self.map(|v: T| Some((>::from(ctx), v))) .map_err(IntoFieldError::into_field_error) @@ -285,6 +291,8 @@ where S: ScalarValue, T: GraphQLValue, { + type Type = T; + fn into(self, _: &'a C) -> FieldResult, S> { Ok(Some(self)) } @@ -295,6 +303,8 @@ where S: ScalarValue, T: GraphQLValue, { + type Type = T; + fn into(self, _: &'a C) -> FieldResult)>, S> { Ok(self.map(|(ctx, v)| (ctx, Some(v)))) } @@ -305,6 +315,8 @@ where S: ScalarValue, T: GraphQLValue, { + type Type = T; + fn into(self, _: &'a C) -> FieldResult, S> { self.map(Some) } @@ -316,6 +328,8 @@ where S: ScalarValue, T: GraphQLValue, { + type Type = T; + fn into(self, _: &'a C) -> FieldResult)>, S> { self.map(|o| o.map(|(ctx, v)| (ctx, Some(v)))) } diff --git a/juniper/src/lib.rs b/juniper/src/lib.rs index cf8bac69..154ab67c 100644 --- a/juniper/src/lib.rs +++ b/juniper/src/lib.rs @@ -183,7 +183,7 @@ pub use crate::{ types::{ async_await::{GraphQLTypeAsync, GraphQLValueAsync}, base::{Arguments, GraphQLType, GraphQLValue, TypeKind}, - marker::{self, GraphQLUnion}, + marker::{self, GraphQLUnion, IsOutputType}, scalars::{EmptyMutation, EmptySubscription, ID}, subscriptions::{ ExecutionOutput, GraphQLSubscriptionType, GraphQLSubscriptionValue, diff --git a/juniper/src/macros/interface.rs b/juniper/src/macros/interface.rs index d23c63cd..bca44882 100644 --- a/juniper/src/macros/interface.rs +++ b/juniper/src/macros/interface.rs @@ -178,6 +178,10 @@ macro_rules! graphql_interface { } ); + $crate::__juniper_impl_trait!( + impl<$($scalar)* $(, $lifetimes)* > IsOutputType for $name { } + ); + $crate::__juniper_impl_trait!( impl<$($scalar)* $(, $lifetimes)* > GraphQLValue for $name { type Context = $ctx; diff --git a/juniper/src/macros/subscription_helpers.rs b/juniper/src/macros/subscription_helpers.rs index 406cc3ec..9d533185 100644 --- a/juniper/src/macros/subscription_helpers.rs +++ b/juniper/src/macros/subscription_helpers.rs @@ -7,27 +7,36 @@ use futures::Stream; use crate::{FieldError, GraphQLValue, ScalarValue}; -/// Trait for converting `T` to `Ok(T)` if T is not Result. -/// This is useful in subscription macros when user can provide type alias for -/// Stream or Result and then a function on Stream should be called. +/// Trait for wrapping [`Stream`] into [`Ok`] if it's not [`Result`]. +/// +/// Used in subscription macros when user can provide type alias for [`Stream`] or +/// `Result` and then a function on [`Stream`] should be called. pub trait IntoFieldResult { - /// Turn current type into a generic result + /// Type of items yielded by this [`Stream`]. + type Item; + + /// Turns current [`Stream`] type into a generic [`Result`]. fn into_result(self) -> Result>; } impl IntoFieldResult for Result where + T: IntoFieldResult, E: Into>, { + type Item = T::Item; + fn into_result(self) -> Result> { self.map_err(|e| e.into()) } } -impl IntoFieldResult for T +impl IntoFieldResult for T where - T: Stream, + T: Stream, { + type Item = T::Item; + fn into_result(self) -> Result> { Ok(self) } diff --git a/juniper/src/types/marker.rs b/juniper/src/types/marker.rs index 24d4b58f..79237741 100644 --- a/juniper/src/types/marker.rs +++ b/juniper/src/types/marker.rs @@ -53,7 +53,8 @@ pub trait GraphQLUnion: GraphQLType { /// types. Each type which can be used as an output type should /// implement this trait. The specification defines enum, scalar, /// object, union, and interface as output types. -pub trait IsOutputType: GraphQLType { +// TODO: Re-enable GraphQLType requirement in #682 +pub trait IsOutputType /*: GraphQLType*/ { /// An arbitrary function without meaning. /// /// May contain compile timed check logic which ensures that types @@ -132,13 +133,13 @@ where impl IsInputType for Box where - T: IsInputType, + T: IsInputType + ?Sized, S: ScalarValue, { } impl IsOutputType for Box where - T: IsOutputType, + T: IsOutputType + ?Sized, S: ScalarValue, { } diff --git a/juniper_codegen/src/derive_scalar_value.rs b/juniper_codegen/src/derive_scalar_value.rs index 254fb93e..98129a62 100644 --- a/juniper_codegen/src/derive_scalar_value.rs +++ b/juniper_codegen/src/derive_scalar_value.rs @@ -195,6 +195,9 @@ fn impl_scalar_struct( <#inner_ty as ::juniper::ParseScalarValue>::from_str(value) } } + + impl ::juniper::marker::IsOutputType for #ident { } + impl ::juniper::marker::IsInputType for #ident { } ); Ok(content) diff --git a/juniper_codegen/src/graphql_union/mod.rs b/juniper_codegen/src/graphql_union/mod.rs index 12904452..6ed0a3b5 100644 --- a/juniper_codegen/src/graphql_union/mod.rs +++ b/juniper_codegen/src/graphql_union/mod.rs @@ -611,7 +611,7 @@ impl ToTokens for UnionDefinition { #where_clause { fn mark() { - #( <#var_types as ::juniper::marker::GraphQLObjectType<#scalar>>::mark(); )* + #( <#var_types as ::juniper::marker::IsOutputType<#scalar>>::mark(); )* } } }; diff --git a/juniper_codegen/src/util/mod.rs b/juniper_codegen/src/util/mod.rs index f91696c9..ac6e5a5e 100644 --- a/juniper_codegen/src/util/mod.rs +++ b/juniper_codegen/src/util/mod.rs @@ -831,12 +831,9 @@ impl GraphQLTypeDefiniton { if self.scalar.is_none() && self.generic_scalar { // No custom scalar specified, but always generic specified. // Therefore we inject the generic scalar. - generics.params.push(parse_quote!(__S)); - - let where_clause = generics.where_clause.get_or_insert(parse_quote!(where)); - // Insert ScalarValue constraint. - where_clause + generics + .make_where_clause() .predicates .push(parse_quote!(__S: ::juniper::ScalarValue)); } @@ -962,26 +959,29 @@ impl GraphQLTypeDefiniton { ) }; - // FIXME: enable this if interfaces are supported - // let marks = self.fields.iter().map(|field| { - // let field_ty = &field._type; + let marks = self.fields.iter().map(|field| { + let field_marks = field.args.iter().map(|arg| { + let arg_ty = &arg._type; + quote! { <#arg_ty as ::juniper::marker::IsInputType<#scalar>>::mark(); } + }); - // let field_marks = field.args.iter().map(|arg| { - // let arg_ty = &arg._type; - // quote!(<#arg_ty as ::juniper::marker::IsInputType<#scalar>>::mark();) - // }); + let field_ty = &field._type; + let resolved_ty = quote! { + <#field_ty as ::juniper::IntoResolvable< + '_, #scalar, _, >::Context, + >>::Type + }; - // quote!( - // #( #field_marks)* - // <#field_ty as ::juniper::marker::IsOutputType<#scalar>>::mark(); - // ) - // }); + quote! { + #( #field_marks )* + <#resolved_ty as ::juniper::marker::IsOutputType<#scalar>>::mark(); + } + }); let output = quote!( impl#impl_generics ::juniper::marker::IsOutputType<#scalar> for #ty #type_generics_tokens #where_clause { fn mark() { - // FIXME: enable this if interfaces are supported - // #( #marks )* + #( #marks )* } } @@ -1001,10 +1001,10 @@ impl GraphQLTypeDefiniton { ) -> ::juniper::meta::MetaType<'r, #scalar> where #scalar : 'r, { - let fields = vec![ + let fields = [ #( #field_definitions ),* ]; - let meta = registry.build_object_type::<#ty>( info, &fields ) + let meta = registry.build_object_type::<#ty>(info, &fields) #description #interfaces; meta.into_meta() @@ -1226,7 +1226,37 @@ impl GraphQLTypeDefiniton { }, ); + let marks = self.fields.iter().map(|field| { + let field_marks = field.args.iter().map(|arg| { + let arg_ty = &arg._type; + quote! { <#arg_ty as ::juniper::marker::IsInputType<#scalar>>::mark(); } + }); + + let field_ty = &field._type; + let stream_item_ty = quote! { + <#field_ty as ::juniper::IntoFieldResult::<_, #scalar>>::Item + }; + let resolved_ty = quote! { + <#stream_item_ty as ::juniper::IntoResolvable< + '_, #scalar, _, >::Context, + >>::Type + }; + + quote! { + #( #field_marks )* + <#resolved_ty as ::juniper::marker::IsOutputType<#scalar>>::mark(); + } + }); + let graphql_implementation = quote!( + impl#impl_generics ::juniper::marker::IsOutputType<#scalar> for #ty #type_generics_tokens + #where_clause + { + fn mark() { + #( #marks )* + } + } + impl#impl_generics ::juniper::GraphQLType<#scalar> for #ty #type_generics_tokens #where_clause { @@ -1240,10 +1270,10 @@ impl GraphQLTypeDefiniton { ) -> ::juniper::meta::MetaType<'r, #scalar> where #scalar : 'r, { - let fields = vec![ + let fields = [ #( #field_definitions ),* ]; - let meta = registry.build_object_type::<#ty>( info, &fields ) + let meta = registry.build_object_type::<#ty>(info, &fields) #description #interfaces; meta.into_meta() @@ -1690,18 +1720,16 @@ impl GraphQLTypeDefiniton { {} ); - // FIXME: enable this if interfaces are supported - // let marks = self.fields.iter().map(|field| { - // let _ty = &field._type; - // quote!(<#_ty as ::juniper::marker::IsInputType<#scalar>>::mark();) - // }); + let marks = self.fields.iter().map(|field| { + let field_ty = &field._type; + quote! { <#field_ty as ::juniper::marker::IsInputType<#scalar>>::mark(); } + }); let mut body = quote!( impl#impl_generics ::juniper::marker::IsInputType<#scalar> for #ty #type_generics_tokens #where_clause { fn mark() { - // FIXME: enable this if interfaces are supported - // #( #marks )* + #( #marks )* } }