Schema Parsing: allow leading pipe for union type definitions

Author: vladar

src/Language/Parser.php

@@ -1080,14 +1080,17 @@ function parseUnionTypeDefinition()
 
     /**
      * UnionMembers :
-     *   - NamedType
+     *   - `|`? NamedType
      *   - UnionMembers | NamedType
      *
      * @return NamedTypeNode[]
      */
     function parseUnionMembers()
     {
+        // Optional leading pipe
+        $this->skip(Token::PIPE);
         $members = [];
+
         do {
             $members[] = $this->parseNamedType();
         } while ($this->skip(Token::PIPE));
@@ -1213,6 +1216,8 @@ function parseDirectiveDefinition()
      */
     function parseDirectiveLocations()
     {
+        // Optional leading pipe
+        $this->skip(Token::PIPE);
         $locations = [];
         do {
             $locations[] = $this->parseName();

tests/Language/SchemaParserTest.php

@@ -1,27 +1,9 @@
 <?php
 namespace GraphQL\Tests\Language;
 
-use GraphQL\Language\AST\BooleanValueNode;
-use GraphQL\Language\AST\DocumentNode;
-use GraphQL\Language\AST\EnumTypeDefinitionNode;
-use GraphQL\Language\AST\EnumValueDefinitionNode;
-use GraphQL\Language\AST\FieldDefinitionNode;
-use GraphQL\Language\AST\InputObjectTypeDefinitionNode;
-use GraphQL\Language\AST\InputValueDefinitionNode;
-use GraphQL\Language\AST\InterfaceTypeDefinitionNode;
-use GraphQL\Language\AST\ListTypeNode;
-use GraphQL\Language\AST\Location;
-use GraphQL\Language\AST\NameNode;
-use GraphQL\Language\AST\NamedTypeNode;
-use GraphQL\Language\AST\Node;
+use GraphQL\Error\SyntaxError;
 use GraphQL\Language\AST\NodeKind;
-use GraphQL\Language\AST\NonNullTypeNode;
-use GraphQL\Language\AST\ObjectTypeDefinitionNode;
-use GraphQL\Language\AST\ScalarTypeDefinitionNode;
-use GraphQL\Language\AST\TypeExtensionDefinitionNode;
-use GraphQL\Language\AST\UnionTypeDefinitionNode;
 use GraphQL\Language\Parser;
-use GraphQL\Language\Source;
 
 class SchemaParserTest extends \PHPUnit_Framework_TestCase
 {
@@ -541,6 +523,86 @@ public function testUnionWithTwoTypes()
         $this->assertEquals($expected, TestUtils::nodeToArray($doc));
     }
 
+
+    /**
+     * @it Union with two types and leading pipe
+     */
+    public function testUnionWithTwoTypesAndLeadingPipe()
+    {
+        $body = 'union Hello = | Wo | Rld';
+        $doc = Parser::parse($body);
+        $expected = [
+            'kind' => 'Document',
+            'definitions' => [
+                [
+                    'kind' => 'UnionTypeDefinition',
+                    'name' => $this->nameNode('Hello', ['start' => 6, 'end' => 11]),
+                    'directives' => [],
+                    'types' => [
+                        $this->typeNode('Wo', ['start' => 16, 'end' => 18]),
+                        $this->typeNode('Rld', ['start' => 21, 'end' => 24]),
+                    ],
+                    'loc' => ['start' => 0, 'end' => 24],
+                    'description' => null
+                ]
+            ],
+            'loc' => ['start' => 0, 'end' => 24],
+        ];
+        $this->assertEquals($expected, TestUtils::nodeToArray($doc));
+    }
+
+    /**
+     * @it Union fails with no types
+     */
+    public function testUnionFailsWithNoTypes()
+    {
+        $body = 'union Hello = |';
+        try {
+            Parser::parse($body);
+        } catch (SyntaxError $e) {
+            $this->assertContains('Syntax Error GraphQL (1:16) Expected Name, found <EOF>', $e->getMessage());
+        }
+    }
+
+    /**
+     * @it Union fails with leading douple pipe
+     */
+    public function testUnionFailsWithLeadingDoublePipe()
+    {
+        $body = 'union Hello = || Wo | Rld';
+        try {
+            Parser::parse($body);
+        } catch (SyntaxError $e) {
+            $this->assertContains('Syntax Error GraphQL (1:16) Expected Name, found |', $e->getMessage());
+        }
+    }
+
+    /**
+     * @it Union fails with double pipe
+     */
+    public function testUnionFailsWithDoublePipe()
+    {
+        $body = 'union Hello = Wo || Rld';
+        try {
+            Parser::parse($body);
+        } catch (SyntaxError $e) {
+            $this->assertContains('Syntax Error GraphQL (1:19) Expected Name, found |', $e->getMessage());
+        }
+    }
+
+    /**
+     * @it Union fails with trailing pipe
+     */
+    public function testUnionFailsWithTrailingPipe()
+    {
+        $body = 'union Hello = | Wo | Rld |';
+        try {
+            Parser::parse($body);
+        } catch (SyntaxError $e) {
+            $this->assertContains('Syntax Error GraphQL (1:27) Expected Name, found <EOF>', $e->getMessage());
+        }
+    }
+
     /**
      * @it Scalar
      */

tests/Language/SchemaPrinterTest.php

@@ -83,6 +83,8 @@ interface AnnotatedInterface @onInterface {
 
 union AnnotatedUnion @onUnion = A | B
 
+union AnnotatedUnionTwo @onUnion = A | B
+
 scalar CustomScalar
 
 scalar AnnotatedScalar @onScalar
@@ -117,6 +119,8 @@ enum AnnotatedEnum @onEnum {
 directive @skip(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
 
 directive @include(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
+
+directive @include2(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
 ';
         $this->assertEquals($expected, $printed);
     }

tests/Language/schema-kitchen-sink.graphql

@@ -37,6 +37,8 @@ union Feed = Story | Article | Advert
 
 union AnnotatedUnion @onUnion = A | B
 
+union AnnotatedUnionTwo @onUnion = | A | B
+
 scalar CustomScalar
 
 scalar AnnotatedScalar @onScalar
@@ -74,3 +76,8 @@ directive @include(if: Boolean!)
   on FIELD
    | FRAGMENT_SPREAD
    | INLINE_FRAGMENT
+
+directive @include2(if: Boolean!) on
+  | FIELD
+  | FRAGMENT_SPREAD
+  | INLINE_FRAGMENT