Fix KnownDirectives validator to support all directives

src/Validator/DocumentValidator.php

@@ -266,9 +266,9 @@ class DocumentValidator
                     }
                 }
                 return $errors;
-            } else {
-                return static::isValidLiteralValue($itemType, $valueNode);
             }
+
+            return static::isValidLiteralValue($itemType, $valueNode);
         }
 
         // Input objects check each defined field and look for undefined fields.
@@ -278,6 +278,7 @@ class DocumentValidator
             }
 
             $fields = $type->getFields();
+
             $errors = [];
 
             // Ensure every provided field is defined.

src/Validator/Rules/KnownDirectives.php

@@ -1,10 +1,9 @@
 <?php
 namespace GraphQL\Validator\Rules;
 
-
 use GraphQL\Error\Error;
 use GraphQL\Language\AST\DirectiveNode;
-use GraphQL\Language\AST\Node;
+use GraphQL\Language\AST\InputObjectTypeDefinitionNode;
 use GraphQL\Language\AST\NodeKind;
 use GraphQL\Validator\ValidationContext;
 use GraphQL\Type\Definition\DirectiveLocation;
@@ -40,8 +39,7 @@ class KnownDirectives extends AbstractValidationRule
                     ));
                     return ;
                 }
-                $appliedTo = $ancestors[count($ancestors) - 1];
+                $candidateLocation = $this->getDirectiveLocationForASTPath($ancestors);
-                $candidateLocation = $this->getLocationForAppliedNode($appliedTo);
 
                 if (!$candidateLocation) {
                     $context->reportError(new Error(
@@ -58,8 +56,9 @@ class KnownDirectives extends AbstractValidationRule
         ];
     }
 
-    private function getLocationForAppliedNode(Node $appliedTo)
+    private function getDirectiveLocationForASTPath(array $ancestors)
     {
+        $appliedTo = $ancestors[count($ancestors) - 1];
         switch ($appliedTo->kind) {
             case NodeKind::OPERATION_DEFINITION:
                 switch ($appliedTo->operation) {
@@ -72,6 +71,20 @@ class KnownDirectives extends AbstractValidationRule
             case NodeKind::FRAGMENT_SPREAD: return DirectiveLocation::FRAGMENT_SPREAD;
             case NodeKind::INLINE_FRAGMENT: return DirectiveLocation::INLINE_FRAGMENT;
             case NodeKind::FRAGMENT_DEFINITION: return DirectiveLocation::FRAGMENT_DEFINITION;
+            case NodeKind::SCHEMA_DEFINITION: return DirectiveLocation::SCHEMA;
+            case NodeKind::SCALAR_TYPE_DEFINITION: return DirectiveLocation::SCALAR;
+            case NodeKind::OBJECT_TYPE_DEFINITION: return DirectiveLocation::OBJECT;
+            case NodeKind::FIELD_DEFINITION: return DirectiveLocation::FIELD_DEFINITION;
+            case NodeKind::INTERFACE_TYPE_DEFINITION: return DirectiveLocation::IFACE;
+            case NodeKind::UNION_TYPE_DEFINITION: return DirectiveLocation::UNION;
+            case NodeKind::ENUM_TYPE_DEFINITION: return DirectiveLocation::ENUM;
+            case NodeKind::ENUM_VALUE_DEFINITION: return DirectiveLocation::ENUM_VALUE;
+            case NodeKind::INPUT_OBJECT_TYPE_DEFINITION: return DirectiveLocation::INPUT_OBJECT;
+            case NodeKind::INPUT_VALUE_DEFINITION:
+                $parentNode = $ancestors[count($ancestors) - 3];
+                return $parentNode instanceof InputObjectTypeDefinitionNode
+                    ? DirectiveLocation::INPUT_FIELD_DEFINITION
+                    : DirectiveLocation::ARGUMENT_DEFINITION;
         }
     }
 }

tests/Language/VisitorTest.php

@@ -1131,7 +1131,7 @@ class VisitorTest extends \PHPUnit_Framework_TestCase
     {
         $visited = [];
 
-        $typeInfo = new TypeInfo(TestCase::getDefaultSchema());
+        $typeInfo = new TypeInfo(TestCase::getTestSchema());
 
         $ast = Parser::parse('{ human(id: 4) { name, pets { ... { name } }, unknown } }');
         Visitor::visit($ast, Visitor::visitWithTypeInfo($typeInfo, [
@@ -1213,7 +1213,7 @@ class VisitorTest extends \PHPUnit_Framework_TestCase
     public function testMaintainsTypeInfoDuringEdit()
     {
         $visited = [];
-        $typeInfo = new TypeInfo(TestCase::getDefaultSchema());
+        $typeInfo = new TypeInfo(TestCase::getTestSchema());
 
         $ast = Parser::parse(
             '{ human(id: 4) { name, pets }, alien }'

tests/Validator/ArgumentsOfCorrectTypeTest.php

@@ -615,7 +615,7 @@ class ArgumentsOfCorrectTypeTest extends TestCase
         $this->expectPassesRule(new ArgumentsOfCorrectType(), '
         {
           complicatedArgs {
-            stringListArgField(stringListArg: ["one", "two"])
+            stringListArgField(stringListArg: ["one", null, "two"])
           }
         }
         ');

tests/Validator/KnownDirectivesTest.php

@@ -89,12 +89,16 @@ class KnownDirectivesTest extends TestCase
     public function testWithWellPlacedDirectives()
     {
         $this->expectPassesRule(new KnownDirectives, '
-      query Foo {
+      query Foo @onQuery {
         name @include(if: true)
         ...Frag @include(if: true)
         skippedField @skip(if: true)
         ...SkippedFrag @skip(if: true)
       }
+      
+      mutation Bar @onMutation {
+        someField
+      }
         ');
     }
 
@@ -105,16 +109,103 @@ class KnownDirectivesTest extends TestCase
     {
         $this->expectFailsRule(new KnownDirectives, '
       query Foo @include(if: true) {
-        name @operationOnly
+        name @onQuery
-        ...Frag @operationOnly
+        ...Frag @onQuery
+      }
+
+      mutation Bar @onQuery {
+        someField
       }
         ', [
             $this->misplacedDirective('include', 'QUERY', 2, 17),
-            $this->misplacedDirective('operationOnly', 'FIELD', 3, 14),
+            $this->misplacedDirective('onQuery', 'FIELD', 3, 14),
-            $this->misplacedDirective('operationOnly', 'FRAGMENT_SPREAD', 4, 17),
+            $this->misplacedDirective('onQuery', 'FRAGMENT_SPREAD', 4, 17),
+            $this->misplacedDirective('onQuery', 'MUTATION', 7, 20),
         ]);
     }
 
+    // within schema language
+
+    /**
+     * @it with well placed directives
+     */
+    public function testWSLWithWellPlacedDirectives()
+    {
+        $this->expectPassesRule(new KnownDirectives, '
+        type MyObj implements MyInterface @onObject {
+          myField(myArg: Int @onArgumentDefinition): String @onFieldDefinition
+        }
+
+        scalar MyScalar @onScalar
+
+        interface MyInterface @onInterface {
+          myField(myArg: Int @onArgumentDefinition): String @onFieldDefinition
+        }
+
+        union MyUnion @onUnion = MyObj | Other
+
+        enum MyEnum @onEnum {
+          MY_VALUE @onEnumValue
+        }
+
+        input MyInput @onInputObject {
+          myField: Int @onInputFieldDefinition
+        }
+
+        schema @onSchema {
+          query: MyQuery
+        }
+        ');
+    }
+
+    /**
+     * @it with misplaced directives
+     */
+    public function testWSLWithMisplacedDirectives()
+    {
+        $this->expectFailsRule(new KnownDirectives, '
+        type MyObj implements MyInterface @onInterface {
+          myField(myArg: Int @onInputFieldDefinition): String @onInputFieldDefinition
+        }
+
+        scalar MyScalar @onEnum
+
+        interface MyInterface @onObject {
+          myField(myArg: Int @onInputFieldDefinition): String @onInputFieldDefinition
+        }
+
+        union MyUnion @onEnumValue = MyObj | Other
+
+        enum MyEnum @onScalar {
+          MY_VALUE @onUnion
+        }
+
+        input MyInput @onEnum {
+          myField: Int @onArgumentDefinition
+        }
+
+        schema @onObject {
+          query: MyQuery
+        }
+        ',
+            [
+                $this->misplacedDirective('onInterface', 'OBJECT', 2, 43),
+                $this->misplacedDirective('onInputFieldDefinition', 'ARGUMENT_DEFINITION', 3, 30),
+                $this->misplacedDirective('onInputFieldDefinition', 'FIELD_DEFINITION', 3, 63),
+                $this->misplacedDirective('onEnum', 'SCALAR', 6, 25),
+                $this->misplacedDirective('onObject', 'INTERFACE', 8, 31),
+                $this->misplacedDirective('onInputFieldDefinition', 'ARGUMENT_DEFINITION', 9, 30),
+                $this->misplacedDirective('onInputFieldDefinition', 'FIELD_DEFINITION', 9, 63),
+                $this->misplacedDirective('onEnumValue', 'UNION', 12, 23),
+                $this->misplacedDirective('onScalar', 'ENUM', 14, 21),
+                $this->misplacedDirective('onUnion', 'ENUM_VALUE', 15, 20),
+                $this->misplacedDirective('onEnum', 'INPUT_OBJECT', 18, 23),
+                $this->misplacedDirective('onArgumentDefinition', 'INPUT_FIELD_DEFINITION', 19, 24),
+                $this->misplacedDirective('onObject', 'SCHEMA', 22, 16),
+            ]
+        );
+    }
+
     private function unknownDirective($directiveName, $line, $column)
     {
         return FormattedError::create(

tests/Validator/OverlappingFieldsCanBeMergedTest.php

@@ -2,13 +2,11 @@
 namespace GraphQL\Tests\Validator;
 
 use GraphQL\Error\FormattedError;
-use GraphQL\Language\Source;
 use GraphQL\Language\SourceLocation;
-use GraphQL\Schema;
+use GraphQL\Type\Schema;
 use GraphQL\Type\Definition\InterfaceType;
 use GraphQL\Type\Definition\ObjectType;
 use GraphQL\Type\Definition\Type;
-use GraphQL\Type\Definition\UnionType;
 use GraphQL\Validator\Rules\OverlappingFieldsCanBeMerged;
 
 class OverlappingFieldsCanBeMergedTest extends TestCase
@@ -445,7 +443,7 @@ class OverlappingFieldsCanBeMergedTest extends TestCase
         // type IntBox and the interface type NonNullStringBox1. While that
         // condition does not exist in the current schema, the schema could
         // expand in the future to allow this. Thus it is invalid.
-        $this->expectFailsRuleWithSchema($this->getTestSchema(), new OverlappingFieldsCanBeMerged, '
+        $this->expectFailsRuleWithSchema($this->getSchema(), new OverlappingFieldsCanBeMerged, '
         {
           someBox {
             ...on IntBox {
@@ -476,7 +474,7 @@ class OverlappingFieldsCanBeMergedTest extends TestCase
         // In this case `deepBox` returns `SomeBox` in the first usage, and
         // `StringBox` in the second usage. These return types are not the same!
         // however this is valid because the return *shapes* are compatible.
-        $this->expectPassesRuleWithSchema($this->getTestSchema(), new OverlappingFieldsCanBeMerged, '
+        $this->expectPassesRuleWithSchema($this->getSchema(), new OverlappingFieldsCanBeMerged, '
       {
         someBox {
           ... on SomeBox {
@@ -499,7 +497,7 @@ class OverlappingFieldsCanBeMergedTest extends TestCase
      */
     public function testDisallowsDifferingReturnTypesDespiteNoOverlap()
     {
-        $this->expectFailsRuleWithSchema($this->getTestSchema(), new OverlappingFieldsCanBeMerged, '
+        $this->expectFailsRuleWithSchema($this->getSchema(), new OverlappingFieldsCanBeMerged, '
         {
           someBox {
             ... on IntBox {
@@ -527,7 +525,7 @@ class OverlappingFieldsCanBeMergedTest extends TestCase
      */
     public function testDisallowsDifferingReturnTypeNullabilityDespiteNoOverlap()
     {
-        $this->expectFailsRuleWithSchema($this->getTestSchema(), new OverlappingFieldsCanBeMerged, '
+        $this->expectFailsRuleWithSchema($this->getSchema(), new OverlappingFieldsCanBeMerged, '
         {
           someBox {
             ... on NonNullStringBox1 {
@@ -555,7 +553,7 @@ class OverlappingFieldsCanBeMergedTest extends TestCase
      */
     public function testDisallowsDifferingReturnTypeListDespiteNoOverlap()
     {
-        $this->expectFailsRuleWithSchema($this->getTestSchema(), new OverlappingFieldsCanBeMerged, '
+        $this->expectFailsRuleWithSchema($this->getSchema(), new OverlappingFieldsCanBeMerged, '
         {
           someBox {
             ... on IntBox {
@@ -582,7 +580,7 @@ class OverlappingFieldsCanBeMergedTest extends TestCase
         ]);
 
 
-        $this->expectFailsRuleWithSchema($this->getTestSchema(), new OverlappingFieldsCanBeMerged, '
+        $this->expectFailsRuleWithSchema($this->getSchema(), new OverlappingFieldsCanBeMerged, '
         {
           someBox {
             ... on IntBox {
@@ -611,7 +609,7 @@ class OverlappingFieldsCanBeMergedTest extends TestCase
 
     public function testDisallowsDifferingSubfields()
     {
-        $this->expectFailsRuleWithSchema($this->getTestSchema(), new OverlappingFieldsCanBeMerged, '
+        $this->expectFailsRuleWithSchema($this->getSchema(), new OverlappingFieldsCanBeMerged, '
         {
           someBox {
             ... on IntBox {
@@ -645,7 +643,7 @@ class OverlappingFieldsCanBeMergedTest extends TestCase
      */
     public function testDisallowsDifferingDeepReturnTypesDespiteNoOverlap()
     {
-        $this->expectFailsRuleWithSchema($this->getTestSchema(), new OverlappingFieldsCanBeMerged, '
+        $this->expectFailsRuleWithSchema($this->getSchema(), new OverlappingFieldsCanBeMerged, '
         {
           someBox {
             ... on IntBox {
@@ -681,7 +679,7 @@ class OverlappingFieldsCanBeMergedTest extends TestCase
      */
     public function testAllowsNonConflictingOverlapingTypes()
     {
-        $this->expectPassesRuleWithSchema($this->getTestSchema(), new OverlappingFieldsCanBeMerged, '
+        $this->expectPassesRuleWithSchema($this->getSchema(), new OverlappingFieldsCanBeMerged, '
         {
           someBox {
             ... on IntBox {
@@ -700,7 +698,7 @@ class OverlappingFieldsCanBeMergedTest extends TestCase
      */
     public function testSameWrappedScalarReturnTypes()
     {
-        $this->expectPassesRuleWithSchema($this->getTestSchema(), new OverlappingFieldsCanBeMerged, '
+        $this->expectPassesRuleWithSchema($this->getSchema(), new OverlappingFieldsCanBeMerged, '
         {
           someBox {
             ...on NonNullStringBox1 {
@@ -719,7 +717,7 @@ class OverlappingFieldsCanBeMergedTest extends TestCase
      */
     public function testAllowsInlineTypelessFragments()
     {
-        $this->expectPassesRuleWithSchema($this->getTestSchema(), new OverlappingFieldsCanBeMerged, '
+        $this->expectPassesRuleWithSchema($this->getSchema(), new OverlappingFieldsCanBeMerged, '
         {
           a
           ... {
@@ -734,7 +732,7 @@ class OverlappingFieldsCanBeMergedTest extends TestCase
      */
     public function testComparesDeepTypesIncludingList()
     {
-        $this->expectFailsRuleWithSchema($this->getTestSchema(), new OverlappingFieldsCanBeMerged, '
+        $this->expectFailsRuleWithSchema($this->getSchema(), new OverlappingFieldsCanBeMerged, '
         {
           connection {
             ...edgeID
@@ -773,7 +771,7 @@ class OverlappingFieldsCanBeMergedTest extends TestCase
      */
     public function testIgnoresUnknownTypes()
     {
-        $this->expectPassesRuleWithSchema($this->getTestSchema(), new OverlappingFieldsCanBeMerged, '
+        $this->expectPassesRuleWithSchema($this->getSchema(), new OverlappingFieldsCanBeMerged, '
         {
           someBox {
             ...on UnknownType {
@@ -787,7 +785,7 @@ class OverlappingFieldsCanBeMergedTest extends TestCase
         ');
     }
 
-    private function getTestSchema()
+    private function getSchema()
     {
         $StringBox = null;
         $IntBox = null;

tests/Validator/QuerySecuritySchema.php

@@ -1,7 +1,7 @@
 <?php
 namespace GraphQL\Tests\Validator;
 
-use GraphQL\Schema;
+use GraphQL\Type\Schema;
 use GraphQL\Type\Definition\ObjectType;
 use GraphQL\Type\Definition\Type;
 

tests/Validator/TestCase.php

@@ -1,15 +1,12 @@
 <?php
 namespace GraphQL\Tests\Validator;
 
-use GraphQL\GraphQL;
-use GraphQL\Language\Lexer;
 use GraphQL\Language\Parser;
-use GraphQL\Schema;
+use GraphQL\Type\Schema;
 use GraphQL\Type\Definition\Directive;
 use GraphQL\Type\Definition\EnumType;
 use GraphQL\Type\Definition\InputObjectType;
 use GraphQL\Type\Definition\InterfaceType;
-use GraphQL\Type\Definition\ListOfType;
 use GraphQL\Type\Definition\ObjectType;
 use GraphQL\Type\Definition\Type;
 use GraphQL\Type\Definition\UnionType;
@@ -20,7 +17,7 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase
     /**
      * @return Schema
      */
-    public static function getDefaultSchema()
+    public static function getTestSchema()
     {
         $FurColor = null;
 
@@ -276,20 +273,90 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase
                 'catOrDog' => ['type' => $CatOrDog],
                 'dogOrHuman' => ['type' => $DogOrHuman],
                 'humanOrAlien' => ['type' => $HumanOrAlien],
-                'complicatedArgs' => ['type' => $ComplicatedArgs]
+                'complicatedArgs' => ['type' => $ComplicatedArgs],
             ]
         ]);
 
-        $defaultSchema = new Schema([
+        $testSchema = new Schema([
             'query' => $queryRoot,
-            'directives' => array_merge(GraphQL::getInternalDirectives(), [
+            'directives' => [
+                Directive::includeDirective(),
+                Directive::skipDirective(),
                 new Directive([
-                    'name' => 'operationOnly',
+                    'name' => 'onQuery',
-                    'locations' => [ 'QUERY' ],
+                    'locations' => ['QUERY'],
-                ])
+                ]),
-            ])
+                new Directive([
+                    'name' => 'onMutation',
+                    'locations' => ['MUTATION'],
+                ]),
+                new Directive([
+                    'name' => 'onSubscription',
+                    'locations' => ['SUBSCRIPTION'],
+                ]),
+                new Directive([
+                    'name' => 'onField',
+                    'locations' => ['FIELD'],
+                ]),
+                new Directive([
+                    'name' => 'onFragmentDefinition',
+                    'locations' => ['FRAGMENT_DEFINITION'],
+                ]),
+                new Directive([
+                    'name' => 'onFragmentSpread',
+                    'locations' => ['FRAGMENT_SPREAD'],
+                ]),
+                new Directive([
+                    'name' => 'onInlineFragment',
+                    'locations' => ['INLINE_FRAGMENT'],
+                ]),
+                new Directive([
+                    'name' => 'onSchema',
+                    'locations' => ['SCHEMA'],
+                ]),
+                new Directive([
+                    'name' => 'onScalar',
+                    'locations' => ['SCALAR'],
+                ]),
+                new Directive([
+                    'name' => 'onObject',
+                    'locations' => ['OBJECT'],
+                ]),
+                new Directive([
+                    'name' => 'onFieldDefinition',
+                    'locations' => ['FIELD_DEFINITION'],
+                ]),
+                new Directive([
+                    'name' => 'onArgumentDefinition',
+                    'locations' => ['ARGUMENT_DEFINITION'],
+                ]),
+                new Directive([
+                    'name' => 'onInterface',
+                    'locations' => ['INTERFACE'],
+                ]),
+                new Directive([
+                    'name' => 'onUnion',
+                    'locations' => ['UNION'],
+                ]),
+                new Directive([
+                    'name' => 'onEnum',
+                    'locations' => ['ENUM'],
+                ]),
+                new Directive([
+                    'name' => 'onEnumValue',
+                    'locations' => ['ENUM_VALUE'],
+                ]),
+                new Directive([
+                    'name' => 'onInputObject',
+                    'locations' => ['INPUT_OBJECT'],
+                ]),
+                new Directive([
+                    'name' => 'onInputFieldDefinition',
+                    'locations' => ['INPUT_FIELD_DEFINITION'],
+                ]),
+            ],
         ]);
-        return $defaultSchema;
+        return $testSchema;
     }
 
     function expectValid($schema, $rules, $queryString)
@@ -313,12 +380,12 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase
 
     function expectPassesRule($rule, $queryString)
     {
-        $this->expectValid($this->getDefaultSchema(), [$rule], $queryString);
+        $this->expectValid($this->getTestSchema(), [$rule], $queryString);
     }
 
     function expectFailsRule($rule, $queryString, $errors)
     {
-        return $this->expectInvalid($this->getDefaultSchema(), [$rule], $queryString, $errors);
+        return $this->expectInvalid($this->getTestSchema(), [$rule], $queryString, $errors);
     }
 
     function expectPassesRuleWithSchema($schema, $rule, $queryString)
@@ -333,11 +400,11 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase
 
     function expectPassesCompleteValidation($queryString)
     {
-        $this->expectValid($this->getDefaultSchema(), DocumentValidator::allRules(), $queryString);
+        $this->expectValid($this->getTestSchema(), DocumentValidator::allRules(), $queryString);
     }
 
     function expectFailsCompleteValidation($queryString, $errors)
     {
-        $this->expectInvalid($this->getDefaultSchema(), DocumentValidator::allRules(), $queryString, $errors);
+        $this->expectInvalid($this->getTestSchema(), DocumentValidator::allRules(), $queryString, $errors);
     }
 }

tests/Validator/ValidationTest.php

@@ -45,6 +45,6 @@ class ValidationTest extends TestCase
             'locations' => [ ['line' => 1, 'column' => 2] ]
         ];
         $this->expectFailsCompleteValidation($query, [$expectedError]);
-        $this->expectValid($this->getDefaultSchema(), [], $query);
+        $this->expectValid($this->getTestSchema(), [], $query);
     }
 }