Skip to content

Commit

Permalink
RFC: SDL - Separate multiple inherited interfaces with &
Browse files Browse the repository at this point in the history
  • Loading branch information
@vladar
vladar committed Aug 7, 2018
1 parent 8e02fdc commit a19fc3d
Show file tree
Hide file tree
Showing 9 changed files with 113 additions and 16 deletions.
16 changes: 16 additions & 0 deletions UPGRADE.md
View file
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/Language/Lexer.php
View file
Expand Up @@ -147,6 +147,8 @@ private function readToken(Token $prev)
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: // )
Expand Down
21 changes: 19 additions & 2 deletions src/Language/Parser.php
View file
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -1072,16 +1079,26 @@ function parseObjectTypeDefinition()
}

/**
* ImplementsInterfaces :
* - implements `&`? NamedType
* - ImplementsInterfaces & NamedType
*
* @return NamedTypeNode[]
*/
function parseImplementsInterfaces()
{
$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;
}
Expand Down
4 changes: 2 additions & 2 deletions src/Language/Printer.php
View file
Expand Up @@ -212,7 +212,7 @@ public function printAST($ast)
$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)
], ' ')
Expand Down Expand Up @@ -300,7 +300,7 @@ public function printAST($ast)
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),
], ' ');
Expand Down
1 change: 1 addition & 0 deletions src/Language/Token.php
View file
Expand Up @@ -12,6 +12,7 @@ class Token
const EOF = '<EOF>';
const BANG = '!';
const DOLLAR = '$';
const AMP = '&';
const PAREN_L = '(';
const PAREN_R = ')';
const SPREAD = '...';
Expand Down
77 changes: 69 additions & 8 deletions tests/Language/SchemaParserTest.php
View file
Expand Up @@ -330,7 +330,7 @@ public function testSimpleTypeInheritingInterface()
*/
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);

Expand All @@ -341,27 +341,62 @@ public function testSimpleTypeInheritingMultipleInterfaces()
'kind' => NodeKind::OBJECT_TYPE_DEFINITION,
'name' => $this->nameNode('Hello', $loc(5, 10)),
'interfaces' => [
$this->typeNode('Wo', $loc(22,24)),
$this->typeNode('rld', $loc(26,29))
$this->typeNode('Wo', $loc(22, 24)),
$this->typeNode('rld', $loc(27, 30))
],
'directives' => [],
'fields' => [
$this->fieldNode(
$this->nameNode('field', $loc(32, 37)),
$this->typeNode('String', $loc(39, 45)),
$loc(32, 45)
$this->nameNode('field', $loc(33, 38)),
$this->typeNode('String', $loc(40, 46)),
$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
*/
Expand Down Expand Up @@ -884,6 +919,32 @@ public function testAllowLegacySDLEmptyFieldsOption()
$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 [
Expand Down
2 changes: 1 addition & 1 deletion tests/Language/SchemaPrinterTest.php
View file
Expand Up @@ -64,7 +64,7 @@ public function testPrintsKitchenSink()
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
Expand Down
2 changes: 1 addition & 1 deletion tests/Language/schema-kitchen-sink.graphql
View file
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions tests/Type/ValidationTest.php
View file
Expand Up @@ -1015,15 +1015,15 @@ interface AnotherInterface {
field: String
}
type AnotherObject implements AnotherInterface, AnotherInterface {
type AnotherObject implements AnotherInterface & AnotherInterface {
field: String
}
');
$this->assertContainsValidationMessage(
$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]],
]]
);
}
Expand Down

0 comments on commit a19fc3d

Please sign in to comment.