From d6d9cc32c0c13421273f8931f9511f9cd21c1573 Mon Sep 17 00:00:00 2001 From: ilslv Date: Tue, 14 Dec 2021 16:35:24 +0300 Subject: [PATCH 1/3] Support directives on variable definitions --- juniper/CHANGELOG.md | 1 + juniper/src/ast.rs | 1 + juniper/src/parser/document.rs | 3 + juniper/src/schema/model.rs | 3 + juniper/src/tests/schema_introspection.rs | 11 ++++ .../src/validation/rules/known_directives.rs | 60 ++++++++++++++++++- juniper/src/validation/visitor.rs | 13 ++++ 7 files changed, 91 insertions(+), 1 deletion(-) diff --git a/juniper/CHANGELOG.md b/juniper/CHANGELOG.md index 914f4b911..586f0e9b9 100644 --- a/juniper/CHANGELOG.md +++ b/juniper/CHANGELOG.md @@ -17,6 +17,7 @@ - Use `null` in addition to `None` to create `Value::Null` in `graphql_value!` macro to mirror `serde_json::json!`. ([#996](https://github.com/graphql-rust/juniper/pull/996)) - Add `From` impls to `InputValue` mirroring the ones for `Value` and provide better support for `Option` handling. ([#996](https://github.com/graphql-rust/juniper/pull/996)) - Implement `graphql_input_value!` and `graphql_vars!` macros. ([#996](https://github.com/graphql-rust/juniper/pull/996)) +- Add support for directives on variables definitions. ([#1005](https://github.com/graphql-rust/juniper/pull/1005)) ## Fixes diff --git a/juniper/src/ast.rs b/juniper/src/ast.rs index b0f856579..b4b535158 100644 --- a/juniper/src/ast.rs +++ b/juniper/src/ast.rs @@ -49,6 +49,7 @@ pub enum InputValue { pub struct VariableDefinition<'a, S> { pub var_type: Spanning>, pub default_value: Option>>, + pub directives: Option>>>, } #[derive(Clone, PartialEq, Debug)] diff --git a/juniper/src/parser/document.rs b/juniper/src/parser/document.rs index 23063ce38..cd891883c 100644 --- a/juniper/src/parser/document.rs +++ b/juniper/src/parser/document.rs @@ -451,6 +451,8 @@ where None }; + let directives = parse_directives(parser, schema)?; + Ok(Spanning::start_end( &start_pos, &default_value @@ -462,6 +464,7 @@ where VariableDefinition { var_type, default_value, + directives: directives.map(|s| s.item), }, ), )) diff --git a/juniper/src/schema/model.rs b/juniper/src/schema/model.rs index 953904cb0..43d6c9b4f 100644 --- a/juniper/src/schema/model.rs +++ b/juniper/src/schema/model.rs @@ -86,6 +86,8 @@ pub enum DirectiveLocation { FragmentSpread, #[graphql(name = "INLINE_FRAGMENT")] InlineFragment, + #[graphql(name = "VARIABLE_DEFINITION")] + VariableDefinition, } impl<'a, QueryT, MutationT, SubscriptionT> @@ -544,6 +546,7 @@ impl fmt::Display for DirectiveLocation { DirectiveLocation::FragmentDefinition => "fragment definition", DirectiveLocation::FragmentSpread => "fragment spread", DirectiveLocation::InlineFragment => "inline fragment", + DirectiveLocation::VariableDefinition => "variable definition", }) } } diff --git a/juniper/src/tests/schema_introspection.rs b/juniper/src/tests/schema_introspection.rs index 5926235b0..7b72730c4 100644 --- a/juniper/src/tests/schema_introspection.rs +++ b/juniper/src/tests/schema_introspection.rs @@ -1023,6 +1023,12 @@ pub(crate) fn schema_introspection_result() -> Value { "description": null, "isDeprecated": false, "deprecationReason": null + }, + { + "name": "VARIABLE_DEFINITION", + "description": null, + "isDeprecated": false, + "deprecationReason": null } ], "possibleTypes": null @@ -2231,6 +2237,11 @@ pub(crate) fn schema_introspection_result_without_descriptions() -> Value { "name": "INLINE_FRAGMENT", "isDeprecated": false, "deprecationReason": null + }, + { + "name": "VARIABLE_DEFINITION", + "isDeprecated": false, + "deprecationReason": null } ], "possibleTypes": null diff --git a/juniper/src/validation/rules/known_directives.rs b/juniper/src/validation/rules/known_directives.rs index 012ae1c74..38333841b 100644 --- a/juniper/src/validation/rules/known_directives.rs +++ b/juniper/src/validation/rules/known_directives.rs @@ -1,5 +1,8 @@ use crate::{ - ast::{Directive, Field, Fragment, FragmentSpread, InlineFragment, Operation, OperationType}, + ast::{ + Directive, Field, Fragment, FragmentSpread, InlineFragment, Operation, OperationType, + VariableDefinition, + }, parser::Spanning, schema::model::DirectiveLocation, validation::{ValidatorContext, Visitor}, @@ -106,6 +109,24 @@ where assert_eq!(top, Some(DirectiveLocation::InlineFragment)); } + fn enter_variable_definition( + &mut self, + _: &mut ValidatorContext<'a, S>, + _: &'a (Spanning<&'a str>, VariableDefinition), + ) { + self.location_stack + .push(DirectiveLocation::VariableDefinition); + } + + fn exit_variable_definition( + &mut self, + _: &mut ValidatorContext<'a, S>, + _: &'a (Spanning<&'a str>, VariableDefinition), + ) { + let top = self.location_stack.pop(); + assert_eq!(top, Some(DirectiveLocation::VariableDefinition)); + } + fn enter_directive( &mut self, ctx: &mut ValidatorContext<'a, S>, @@ -206,6 +227,43 @@ mod tests { ); } + #[test] + fn with_unknown_directive_on_var_definition() { + expect_fails_rule::<_, _, DefaultScalarValue>( + factory, + r#" + query Foo( + $var1: Int = 1 @skip(if: true) @any @directive, + $var2: String @is, @unsupported + ) { + name + } + "#, + &[ + RuleError::new( + &misplaced_error_message("skip", &DirectiveLocation::VariableDefinition), + &[SourcePosition::new(49, 2, 27)], + ), + RuleError::new( + &unknown_error_message("any"), + &[SourcePosition::new(65, 2, 43)], + ), + RuleError::new( + &unknown_error_message("directive"), + &[SourcePosition::new(70, 2, 48)], + ), + RuleError::new( + &unknown_error_message("is"), + &[SourcePosition::new(109, 3, 26)], + ), + RuleError::new( + &unknown_error_message("unsupported"), + &[SourcePosition::new(114, 3, 31)], + ), + ], + ); + } + #[test] fn with_many_unknown_directives() { expect_fails_rule::<_, _, DefaultScalarValue>( diff --git a/juniper/src/validation/visitor.rs b/juniper/src/validation/visitor.rs index 21c31112b..488e65abb 100644 --- a/juniper/src/validation/visitor.rs +++ b/juniper/src/validation/visitor.rs @@ -141,6 +141,19 @@ fn visit_variable_definitions<'a, S, V>( visit_input_value(v, ctx, default_value); } + if let Some(dirs) = &def.1.directives { + for directive in dirs { + let directive_arguments = ctx + .schema + .directive_by_name(directive.item.name.item) + .map(|d| &d.arguments); + + v.enter_directive(ctx, directive); + visit_arguments(v, ctx, directive_arguments, &directive.item.arguments); + v.exit_directive(ctx, directive); + } + } + v.exit_variable_definition(ctx, def); }) } From 0e522a68f37be9e5ea8eed634a7dc5a6b5166ec3 Mon Sep 17 00:00:00 2001 From: ilslv Date: Tue, 21 Dec 2021 12:54:19 +0300 Subject: [PATCH 2/3] Correction --- juniper/src/tests/schema_introspection.rs | 12 ++++++------ .../src/validation/rules/known_directives.rs | 18 +++++------------- 2 files changed, 11 insertions(+), 19 deletions(-) diff --git a/juniper/src/tests/schema_introspection.rs b/juniper/src/tests/schema_introspection.rs index 2ee18d452..1e1b03e0a 100644 --- a/juniper/src/tests/schema_introspection.rs +++ b/juniper/src/tests/schema_introspection.rs @@ -1057,19 +1057,19 @@ pub(crate) fn schema_introspection_result() -> Value { "deprecationReason": null }, { - "name": "FRAGMENT_SPREAD", + "name": "VARIABLE_DEFINITION", "description": null, "isDeprecated": false, "deprecationReason": null }, { - "name": "INLINE_FRAGMENT", + "name": "FRAGMENT_SPREAD", "description": null, "isDeprecated": false, "deprecationReason": null }, { - "name": "VARIABLE_DEFINITION", + "name": "INLINE_FRAGMENT", "description": null, "isDeprecated": false, "deprecationReason": null @@ -2394,17 +2394,17 @@ pub(crate) fn schema_introspection_result_without_descriptions() -> Value { "deprecationReason": null }, { - "name": "FRAGMENT_SPREAD", + "name": "VARIABLE_DEFINITION", "isDeprecated": false, "deprecationReason": null }, { - "name": "INLINE_FRAGMENT", + "name": "FRAGMENT_SPREAD", "isDeprecated": false, "deprecationReason": null }, { - "name": "VARIABLE_DEFINITION", + "name": "INLINE_FRAGMENT", "isDeprecated": false, "deprecationReason": null }, diff --git a/juniper/src/validation/rules/known_directives.rs b/juniper/src/validation/rules/known_directives.rs index 38333841b..0aedd932c 100644 --- a/juniper/src/validation/rules/known_directives.rs +++ b/juniper/src/validation/rules/known_directives.rs @@ -233,8 +233,8 @@ mod tests { factory, r#" query Foo( - $var1: Int = 1 @skip(if: true) @any @directive, - $var2: String @is, @unsupported + $var1: Int = 1 @skip(if: true) @unknown, + $var2: String @deprecated ) { name } @@ -245,20 +245,12 @@ mod tests { &[SourcePosition::new(49, 2, 27)], ), RuleError::new( - &unknown_error_message("any"), + &unknown_error_message("unknown"), &[SourcePosition::new(65, 2, 43)], ), RuleError::new( - &unknown_error_message("directive"), - &[SourcePosition::new(70, 2, 48)], - ), - RuleError::new( - &unknown_error_message("is"), - &[SourcePosition::new(109, 3, 26)], - ), - RuleError::new( - &unknown_error_message("unsupported"), - &[SourcePosition::new(114, 3, 31)], + &misplaced_error_message("deprecated", &DirectiveLocation::VariableDefinition), + &[SourcePosition::new(102, 3, 26)], ), ], ); From 51a0b6154f8d5c09ddc69b61d7f998a9799497da Mon Sep 17 00:00:00 2001 From: tyranron Date: Tue, 21 Dec 2021 17:35:13 +0100 Subject: [PATCH 3/3] Minor corrections --- juniper/CHANGELOG.md | 2 +- juniper/src/schema/model.rs | 32 +++++++++---------- .../src/validation/rules/known_directives.rs | 20 ++++++------ 3 files changed, 26 insertions(+), 28 deletions(-) diff --git a/juniper/CHANGELOG.md b/juniper/CHANGELOG.md index ed0ce499f..836472fb0 100644 --- a/juniper/CHANGELOG.md +++ b/juniper/CHANGELOG.md @@ -23,7 +23,7 @@ - Add `specified_by_url` attribute argument to `#[derive(GraphQLScalarValue)]` and `#[graphql_scalar]` macros. ([#1003](https://github.com/graphql-rust/juniper/pull/1003), [#1000](https://github.com/graphql-rust/juniper/pull/1000)) - Support `isRepeatable` field on directives. ([#1003](https://github.com/graphql-rust/juniper/pull/1003), [#1000](https://github.com/graphql-rust/juniper/pull/1000)) - Support `__Schema.description`, `__Type.specifiedByURL` and `__Directive.isRepeatable` fields in introspection. ([#1003](https://github.com/graphql-rust/juniper/pull/1003), [#1000](https://github.com/graphql-rust/juniper/pull/1000)) -- Add support for directives on variables definitions. ([#1005](https://github.com/graphql-rust/juniper/pull/1005)) +- Support directives on variables definitions. ([#1005](https://github.com/graphql-rust/juniper/pull/1005)) ## Fixes diff --git a/juniper/src/schema/model.rs b/juniper/src/schema/model.rs index 9dd7d307f..0576174e5 100644 --- a/juniper/src/schema/model.rs +++ b/juniper/src/schema/model.rs @@ -590,28 +590,28 @@ where impl fmt::Display for DirectiveLocation { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.write_str(match *self { - DirectiveLocation::Query => "query", - DirectiveLocation::Mutation => "mutation", - DirectiveLocation::Subscription => "subscription", - DirectiveLocation::Field => "field", - DirectiveLocation::FieldDefinition => "field definition", - DirectiveLocation::FragmentDefinition => "fragment definition", - DirectiveLocation::FragmentSpread => "fragment spread", - DirectiveLocation::InlineFragment => "inline fragment", - DirectiveLocation::VariableDefinition => "variable definition", - DirectiveLocation::Scalar => "scalar", - DirectiveLocation::EnumValue => "enum value", + f.write_str(match self { + Self::Query => "query", + Self::Mutation => "mutation", + Self::Subscription => "subscription", + Self::Field => "field", + Self::FieldDefinition => "field definition", + Self::FragmentDefinition => "fragment definition", + Self::FragmentSpread => "fragment spread", + Self::InlineFragment => "inline fragment", + Self::VariableDefinition => "variable definition", + Self::Scalar => "scalar", + Self::EnumValue => "enum value", }) } } impl<'a, S> fmt::Display for TypeType<'a, S> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match *self { - TypeType::Concrete(t) => f.write_str(t.name().unwrap()), - TypeType::List(ref i, _) => write!(f, "[{}]", i), - TypeType::NonNull(ref i) => write!(f, "{}!", i), + match self { + Self::Concrete(t) => f.write_str(t.name().unwrap()), + Self::List(i, _) => write!(f, "[{}]", i), + Self::NonNull(i) => write!(f, "{}!", i), } } } diff --git a/juniper/src/validation/rules/known_directives.rs b/juniper/src/validation/rules/known_directives.rs index 0aedd932c..291e17dbf 100644 --- a/juniper/src/validation/rules/known_directives.rs +++ b/juniper/src/validation/rules/known_directives.rs @@ -231,26 +231,24 @@ mod tests { fn with_unknown_directive_on_var_definition() { expect_fails_rule::<_, _, DefaultScalarValue>( factory, - r#" - query Foo( - $var1: Int = 1 @skip(if: true) @unknown, - $var2: String @deprecated - ) { - name - } - "#, + r#"query Foo( + $var1: Int = 1 @skip(if: true) @unknown, + $var2: String @deprecated + ) { + name + }"#, &[ RuleError::new( &misplaced_error_message("skip", &DirectiveLocation::VariableDefinition), - &[SourcePosition::new(49, 2, 27)], + &[SourcePosition::new(42, 1, 31)], ), RuleError::new( &unknown_error_message("unknown"), - &[SourcePosition::new(65, 2, 43)], + &[SourcePosition::new(58, 1, 47)], ), RuleError::new( &misplaced_error_message("deprecated", &DirectiveLocation::VariableDefinition), - &[SourcePosition::new(102, 3, 26)], + &[SourcePosition::new(98, 2, 30)], ), ], );