New validation rule: VariablesInAllowedPosition (#31)

Author: dotansimha

README.md

@@ -65,6 +65,6 @@ cargo add graphql-tools
 - [x] UniqueArgumentNames
 - [ ] ValuesOfCorrectType
 - [x] ProvidedRequiredArguments
-- [ ] VariablesInAllowedPosition
+- [x] VariablesInAllowedPosition
 - [x] OverlappingFieldsCanBeMerged
 - [ ] UniqueInputFieldNames (blocked by https://github.com/graphql-rust/graphql-parser/issues/59)

src/ast/ext.rs

@@ -86,6 +86,13 @@ pub trait SchemaDocumentExtension {
     fn query_type(&self) -> ObjectType;
     fn mutation_type(&self) -> Option<ObjectType>;
     fn subscription_type(&self) -> Option<ObjectType>;
+    fn is_subtype(&self, sub_type: &Type, super_type: &Type) -> bool;
+    fn is_named_subtype(&self, sub_type_name: &String, super_type_name: &String) -> bool;
+    fn is_possible_type(
+        &self,
+        abstract_type: &TypeDefinition,
+        possible_type: &TypeDefinition,
+    ) -> bool;
 }
 
 impl SchemaDocumentExtension for schema::Document {
@@ -170,10 +177,92 @@ impl SchemaDocumentExtension for schema::Document {
 
         type_map
     }
+
+    fn is_named_subtype(&self, sub_type_name: &String, super_type_name: &String) -> bool {
+        if sub_type_name == super_type_name {
+            true
+        } else if let (Some(sub_type), Some(super_type)) = (
+            self.type_by_name(sub_type_name),
+            self.type_by_name(super_type_name),
+        ) {
+            super_type.is_abstract_type() && self.is_possible_type(&super_type, &sub_type)
+        } else {
+            false
+        }
+    }
+
+    fn is_possible_type(
+        &self,
+        abstract_type: &TypeDefinition,
+        possible_type: &TypeDefinition,
+    ) -> bool {
+        match abstract_type {
+            TypeDefinition::Union(union_typedef) => {
+                return union_typedef.types.contains(&possible_type.name());
+            }
+            TypeDefinition::Interface(interface_typedef) => {
+                let implementes_interfaces = possible_type.interfaces();
+
+                return implementes_interfaces.contains(&interface_typedef.name);
+            }
+            _ => false,
+        }
+    }
+
+    fn is_subtype(&self, sub_type: &Type, super_type: &Type) -> bool {
+        // Equivalent type is a valid subtype
+        if sub_type == super_type {
+            return true;
+        }
+
+        // If superType is non-null, maybeSubType must also be non-null.
+        if super_type.is_non_null() {
+            if sub_type.is_non_null() {
+                return self.is_subtype(sub_type.of_type(), super_type.of_type());
+            }
+            return false;
+        }
+
+        if sub_type.is_non_null() {
+            // If superType is nullable, maybeSubType may be non-null or nullable.
+            return self.is_subtype(sub_type.of_type(), super_type);
+        }
+
+        // If superType type is a list, maybeSubType type must also be a list.
+        if super_type.is_list_type() {
+            if sub_type.is_list_type() {
+                return self.is_subtype(sub_type.of_type(), super_type.of_type());
+            }
+
+            return false;
+        }
+
+        if sub_type.is_list_type() {
+            // If superType is nullable, maybeSubType may be non-null or nullable.
+            return false;
+        }
+
+        // If superType type is an abstract type, check if it is super type of maybeSubType.
+        // Otherwise, the child type is not a valid subtype of the parent type.
+        if let (Some(sub_type), Some(super_type)) = (
+            self.type_by_name(&sub_type.inner_type()),
+            self.type_by_name(&super_type.inner_type()),
+        ) {
+            return super_type.is_abstract_type()
+                && (sub_type.is_interface_type() || sub_type.is_object_type())
+                && self.is_possible_type(&super_type, &sub_type);
+        }
+
+        false
+    }
 }
 
 pub trait TypeExtension {
     fn inner_type(&self) -> String;
+    fn is_non_null(&self) -> bool;
+    fn is_list_type(&self) -> bool;
+    fn is_named_type(&self) -> bool;
+    fn of_type(&self) -> &Type;
 }
 
 impl TypeExtension for Type {
@@ -184,6 +273,35 @@ impl TypeExtension for Type {
             Type::NonNullType(child) => child.inner_type(),
         }
     }
+
+    fn of_type(&self) -> &Type {
+        match self {
+            Type::ListType(child) => child,
+            Type::NonNullType(child) => child,
+            Type::NamedType(_) => self,
+        }
+    }
+
+    fn is_non_null(&self) -> bool {
+        match self {
+            Type::NonNullType(_) => true,
+            _ => false,
+        }
+    }
+
+    fn is_list_type(&self) -> bool {
+        match self {
+            Type::ListType(_) => true,
+            _ => false,
+        }
+    }
+
+    fn is_named_type(&self) -> bool {
+        match self {
+            Type::NamedType(_) => true,
+            _ => false,
+        }
+    }
 }
 
 pub trait ValueExtension {
@@ -247,6 +365,7 @@ pub trait TypeDefinitionExtension {
     fn is_input_type(&self) -> bool;
     fn is_object_type(&self) -> bool;
     fn is_union_type(&self) -> bool;
+    fn is_interface_type(&self) -> bool;
     fn is_enum_type(&self) -> bool;
     fn is_scalar_type(&self) -> bool;
     fn is_abstract_type(&self) -> bool;
@@ -381,6 +500,13 @@ impl TypeDefinitionExtension for Option<schema::TypeDefinition> {
         }
     }
 
+    fn is_interface_type(&self) -> bool {
+        match self {
+            Some(t) => t.is_interface_type(),
+            _ => false,
+        }
+    }
+
     fn is_object_type(&self) -> bool {
         match self {
             Some(t) => t.is_object_type(),
@@ -444,6 +570,13 @@ impl TypeDefinitionExtension for schema::TypeDefinition {
         }
     }
 
+    fn is_interface_type(&self) -> bool {
+        match self {
+            schema::TypeDefinition::Interface(_i) => true,
+            _ => false,
+        }
+    }
+
     fn is_leaf_type(&self) -> bool {
         match self {
             schema::TypeDefinition::Scalar(_u) => true,

src/lib.rs

@@ -8,7 +8,7 @@
 pub mod ast;
 
 pub mod static_graphql {
-  macro_rules! static_graphql {
+    macro_rules! static_graphql {
     ($m:ident, $m2:ident, {$($n:ident,)*}) => {
         pub mod $m {
             use graphql_parser::$m2 as $m;
@@ -20,16 +20,16 @@ pub mod static_graphql {
     };
   }
 
-  static_graphql!(query, query, {
-    Document, Value, OperationDefinition, InlineFragment, TypeCondition,
-    FragmentSpread, Field, Selection, SelectionSet, FragmentDefinition,
-    Directive, VariableDefinition, Type, Query, Definition, Subscription, Mutation,
-  });
-  static_graphql!(schema, schema, {
-    Field, Directive, InterfaceType, ObjectType, Value, TypeDefinition,
-    EnumType, Type, Document, ScalarType, InputValue, DirectiveDefinition,
-    UnionType, InputObjectType, EnumValue, SchemaDefinition,
-  });
+    static_graphql!(query, query, {
+      Document, Value, OperationDefinition, InlineFragment, TypeCondition,
+      FragmentSpread, Field, Selection, SelectionSet, FragmentDefinition,
+      Directive, VariableDefinition, Type, Query, Definition, Subscription, Mutation,
+    });
+    static_graphql!(schema, schema, {
+      Field, Directive, InterfaceType, ObjectType, Value, TypeDefinition,
+      EnumType, Type, Document, ScalarType, InputValue, DirectiveDefinition,
+      UnionType, InputObjectType, EnumValue, SchemaDefinition,
+    });
 }
 
 pub mod validation;

src/validation/rules/defaults.rs

@@ -6,7 +6,7 @@ use super::{
     NoFragmentsCycle, NoUndefinedVariables, NoUnusedFragments, NoUnusedVariables,
     OverlappingFieldsCanBeMerged, PossibleFragmentSpreads, ProvidedRequiredArguments,
     SingleFieldSubscriptions, UniqueArgumentNames, UniqueFragmentNames, UniqueOperationNames,
-    UniqueVariableNames, VariablesAreInputTypes,
+    UniqueVariableNames, VariablesAreInputTypes, VariablesInAllowedPosition,
 };
 
 pub fn default_rules_validation_plan() -> ValidationPlan {
@@ -33,6 +33,7 @@ pub fn default_rules_validation_plan() -> ValidationPlan {
     plan.add_rule(Box::new(UniqueVariableNames::new()));
     plan.add_rule(Box::new(ProvidedRequiredArguments::new()));
     plan.add_rule(Box::new(KnownDirectives::new()));
+    plan.add_rule(Box::new(VariablesInAllowedPosition::new()));
 
     plan
 }

src/validation/rules/fragments_on_composite_types.rs

@@ -16,7 +16,7 @@ use crate::validation::utils::{ValidationError, ValidationErrorContext};
 pub struct FragmentsOnCompositeTypes;
 
 impl FragmentsOnCompositeTypes {
-  pub fn new() -> Self {
+    pub fn new() -> Self {
         FragmentsOnCompositeTypes
     }
 }

src/validation/rules/known_directives.rs

@@ -316,6 +316,5 @@ fn misplaced_directives() {
         &mut plan,
     );
 
-    println!("{:?}", get_messages(&errors));
     assert_eq!(get_messages(&errors).len(), 11);
 }

src/validation/rules/mod.rs

@@ -22,6 +22,7 @@ pub mod unique_fragment_names;
 pub mod unique_operation_names;
 pub mod unique_variable_names;
 pub mod variables_are_input_types;
+pub mod variables_in_allowed_position;
 
 pub use self::defaults::*;
 pub use self::rule::*;
@@ -47,3 +48,4 @@ pub use self::unique_fragment_names::*;
 pub use self::unique_operation_names::*;
 pub use self::unique_variable_names::*;
 pub use self::variables_are_input_types::*;
+pub use self::variables_in_allowed_position::*;

src/validation/rules/no_unused_variables.rs

@@ -357,7 +357,6 @@ fn multiple_variables_not_used() {
     let messages = get_messages(&errors);
 
     assert_eq!(messages.len(), 2);
-    println!("messages: {:?}", messages);
     assert!(messages.contains(&&"Variable \"$a\" is never used in operation \"Foo\".".to_owned()));
     assert!(messages.contains(&&"Variable \"$c\" is never used in operation \"Foo\".".to_owned()));
 }

src/validation/rules/overlapping_fields_can_be_merged.rs

@@ -1798,7 +1798,6 @@ fn finds_invalid_case_even_with_immediately_recursive_fragment() {
     );
 
     let messages = get_messages(&errors);
-    println!("{:?}", messages);
     assert_eq!(messages.len(), 1);
     assert_eq!(messages, vec![
       "Fields \"fido\" conflict because \"name\" and \"nickname\" are different fields. Use different aliases on the fields to fetch both if this was intentional."