RFC: SDL - Separate multiple inherited interfaces with &

UPGRADE.md

@@ -3,6 +3,22 @@
 ### Breaking: minimum supported version of PHP
 New minimum required version of PHP is **7.1+**
 
+### Breaking: multiple interfaces separated with & in SDL
+Before the change:
+```graphql
+type Foo implements Bar, Baz { field: Type }
+```
+
+After the change:
+```graphql
+type Foo implements Bar & Baz { field: Type }
+```
+
+To allow for an adaptive migration, use `allowLegacySDLImplementsInterfaces` option of parser:
+```php
+Parser::parse($source, [ 'allowLegacySDLImplementsInterfaces' => true])
+```
+
 ### Breaking: errors formatting changed according to spec 
 Extensions assigned to errors are shown under `extensions` key
 ```php

src/Language/Lexer.php

@@ -147,6 +147,8 @@ class Lexer
                 return $this->readComment($line, $col, $prev);
             case 36: // $
                 return new Token(Token::DOLLAR, $position, $position + 1, $line, $col, $prev);
+            case 38: // &
+                return new Token(Token::AMP, $position, $position + 1, $line, $col, $prev);
             case 40: // (
                 return new Token(Token::PAREN_L, $position, $position + 1, $line, $col, $prev);
             case 41: // )

src/Language/Parser.php

@@ -66,7 +66,6 @@ class Parser
      *   in the source that they correspond to. This configuration flag
      *   disables that behavior for performance or testing.)
      *
-     *
      * allowLegacySDLEmptyFields: boolean
      *   If enabled, the parser will parse empty fields sets in the Schema
      *   Definition Language. Otherwise, the parser will follow the current
@@ -75,6 +74,14 @@ class Parser
      *   This option is provided to ease adoption of the final SDL specification
      *   and will be removed in a future major release.
      *
+     * allowLegacySDLImplementsInterfaces: boolean
+     *   If enabled, the parser will parse implemented interfaces with no `&`
+     *   character between each interface. Otherwise, the parser will follow the
+     *   current specification.
+     *
+     *   This option is provided to ease adoption of the final SDL specification
+     *   and will be removed in a future major release.
+     *
      * experimentalFragmentVariables: boolean,
      *   (If enabled, the parser will understand and parse variable definitions
      *   contained in a fragment definition. They'll be represented in the
@@ -1072,6 +1079,10 @@ class Parser
     }
 
     /**
+     * ImplementsInterfaces :
+     *   - implements `&`? NamedType
+     *   - ImplementsInterfaces & NamedType
+     *
      * @return NamedTypeNode[]
      */
     function parseImplementsInterfaces()
@@ -1079,9 +1090,15 @@ class Parser
         $types = [];
         if ($this->lexer->token->value === 'implements') {
             $this->lexer->advance();
+            // Optional leading ampersand
+            $this->skip(Token::AMP);
             do {
                 $types[] = $this->parseNamedType();
-            } while ($this->peek(Token::NAME));
+            } while (
+                $this->skip(Token::AMP) ||
+                // Legacy support for the SDL?
+                (!empty($this->lexer->options['allowLegacySDLImplementsInterfaces']) && $this->peek(Token::NAME))
+            );
         }
         return $types;
     }

src/Language/Printer.php

@@ -212,7 +212,7 @@ class Printer
                         $this->join([
                             'type',
                             $def->name,
-                            $this->wrap('implements ', $this->join($def->interfaces, ', ')),
+                            $this->wrap('implements ', $this->join($def->interfaces, ' & ')),
                             $this->join($def->directives, ' '),
                             $this->block($def->fields)
                         ], ' ')
@@ -300,7 +300,7 @@ class Printer
                     return $this->join([
                         'extend type',
                         $def->name,
-                        $this->wrap('implements ', $this->join($def->interfaces, ', ')),
+                        $this->wrap('implements ', $this->join($def->interfaces, ' & ')),
                         $this->join($def->directives, ' '),
                         $this->block($def->fields),
                     ], ' ');

src/Language/Token.php

@@ -12,6 +12,7 @@ class Token
     const EOF = '<EOF>';
     const BANG = '!';
     const DOLLAR = '$';
+    const AMP = '&';
     const PAREN_L = '(';
     const PAREN_R = ')';
     const SPREAD = '...';

tests/Language/SchemaParserTest.php

@@ -330,7 +330,7 @@ type Hello {
      */
     public function testSimpleTypeInheritingMultipleInterfaces()
     {
-        $body = 'type Hello implements Wo, rld { field: String }';
+        $body = 'type Hello implements Wo & rld { field: String }';
         $loc = function($start, $end) {return TestUtils::locArray($start, $end);};
         $doc = Parser::parse($body);
 
@@ -342,26 +342,61 @@ type Hello {
                     'name' => $this->nameNode('Hello', $loc(5, 10)),
                     'interfaces' => [
                         $this->typeNode('Wo', $loc(22, 24)),
-                        $this->typeNode('rld', $loc(26,29))
+                        $this->typeNode('rld', $loc(27, 30))
                     ],
                     'directives' => [],
                     'fields' => [
                         $this->fieldNode(
-                            $this->nameNode('field', $loc(32, 37)),
+                            $this->nameNode('field', $loc(33, 38)),
-                            $this->typeNode('String', $loc(39, 45)),
+                            $this->typeNode('String', $loc(40, 46)),
-                            $loc(32, 45)
+                            $loc(33, 46)
                         )
                     ],
-                    'loc' => $loc(0, 47),
+                    'loc' => $loc(0, 48),
                     'description' => null
                 ]
             ],
-            'loc' => $loc(0, 47)
+            'loc' => $loc(0, 48)
         ];
 
         $this->assertEquals($expected, TestUtils::nodeToArray($doc));
     }
 
+    /**
+     * @it Simple type inheriting multiple interfaces with leading ampersand
+     */
+    public function testSimpleTypeInheritingMultipleInterfacesWithLeadingAmpersand()
+    {
+        $body = 'type Hello implements & Wo & rld { field: String }';
+        $loc = function($start, $end) {return TestUtils::locArray($start, $end);};
+        $doc = Parser::parse($body);
+        $expected = [
+            'kind' => 'Document',
+            'definitions' => [
+                [
+                    'kind' => 'ObjectTypeDefinition',
+                    'name' => $this->nameNode('Hello', $loc(5, 10)),
+                    'interfaces' => [
+                        $this->typeNode('Wo', $loc(24, 26)),
+                        $this->typeNode('rld', $loc(29, 32)),
+                    ],
+                    'directives' => [],
+                    'fields' => [
+                        $this->fieldNode(
+                            $this->nameNode('field', $loc(35, 40)),
+                            $this->typeNode('String', $loc(42, 48)),
+                            $loc(35, 48)
+                        ),
+                    ],
+                    'loc' => $loc(0, 50),
+                    'description' => null,
+                ],
+            ],
+            'loc' => $loc(0, 50),
+        ];
+        $this->assertEquals($expected, TestUtils::nodeToArray($doc));
+    }
+
     /**
      * @it Single value enum
      */
@@ -884,6 +919,32 @@ input Hello {
         $this->assertArraySubset($expected, $doc->toArray(true));
     }
 
+    public function testDoesntAllowLegacySDLImplementsInterfacesByDefault()
+    {
+        $body = 'type Hello implements Wo rld { field: String }';
+        $this->expectSyntaxError($body, 'Syntax Error: Unexpected Name "rld"', new SourceLocation(1, 26));
+    }
+
+    /**
+     * @it Option: allowLegacySDLImplementsInterfaces
+     */
+    public function testDefaultSDLImplementsInterfaces()
+    {
+        $body = 'type Hello implements Wo rld { field: String }';
+        $doc = Parser::parse($body, ['allowLegacySDLImplementsInterfaces' => true]);
+        $expected = [
+            'definitions' => [
+                [
+                    'interfaces' => [
+                        $this->typeNode('Wo', ['start' => 22, 'end' => 24]),
+                        $this->typeNode('rld', ['start' => 25, 'end' => 28]),
+                    ],
+                ],
+            ],
+        ];
+        $this->assertArraySubset($expected, $doc->toArray(true));
+    }
+
     private function typeNode($name, $loc)
     {
         return [

tests/Language/SchemaPrinterTest.php

@@ -64,7 +64,7 @@ class SchemaPrinterTest extends TestCase
 This is a description
 of the `Foo` type.
 """
-type Foo implements Bar {
+type Foo implements Bar & Baz {
   one: Type
   two(argument: InputType!): Type
   three(argument: InputType, other: String): Int

tests/Language/schema-kitchen-sink.graphql

@@ -12,7 +12,7 @@ schema {
 This is a description
 of the `Foo` type.
 """
-type Foo implements Bar {
+type Foo implements Bar & Baz {
 one: Type
 two(argument: InputType!): Type
 three(argument: InputType, other: String): Int

tests/Type/ValidationTest.php

@@ -1015,7 +1015,7 @@ class ValidationTest extends TestCase
         field: String
       }
       
-      type AnotherObject implements AnotherInterface, AnotherInterface {
+      type AnotherObject implements AnotherInterface & AnotherInterface {
         field: String
       }
         ');
@@ -1023,7 +1023,7 @@ class ValidationTest extends TestCase
             $schema->validate(),
             [[
                 'message' => 'Type AnotherObject can only implement AnotherInterface once.',
-                'locations' => [['line' => 10, 'column' => 37], ['line' => 10, 'column' => 55]],
+                'locations' => [['line' => 10, 'column' => 37], ['line' => 10, 'column' => 56]],
             ]]
         );
     }