Skip to content

Commit

Permalink
RFC: SDL - Separate multiple inherited interfaces with &
Browse files Browse the repository at this point in the history
This replaces:

```graphql
type Foo implements Bar, Baz { field: Type }
```

With:

```graphql
type Foo implements Bar & Baz { field: Type }
```

With no changes to the common case of implementing a single interface.

This is more consistent with other trailing lists of values which either have an explicit separator (union members) or are prefixed with a sigil (directives). This avoids parse ambiguity in the case of an omitted field set, illustrated by #1166

This is a breaking change for existing uses of multiple inheritence. To allow for an adaptive migration, this adds a parse option to continue to support the existing experimental SDL: `parse(source, {allowLegacySDLImplementsInterfaces: true})`
  • Loading branch information
leebyron committed Dec 20, 2017
1 parent b03b19c commit f317a65
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 15 deletions.
2 changes: 1 addition & 1 deletion src/language/__tests__/schema-kitchen-sink.graphql
Original file line number Diff line number Diff line change
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
59 changes: 52 additions & 7 deletions src/language/__tests__/schema-parser-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ type Hello {
});

it('Simple type inheriting multiple interfaces', () => {
const body = 'type Hello implements Wo, rld { field: String }';
const body = 'type Hello implements Wo & rld { field: String }';
const doc = parse(body);
const expected = {
kind: 'Document',
Expand All @@ -301,20 +301,49 @@ type Hello {
name: nameNode('Hello', { start: 5, end: 10 }),
interfaces: [
typeNode('Wo', { start: 22, end: 24 }),
typeNode('rld', { start: 26, end: 29 }),
typeNode('rld', { start: 27, end: 30 }),
],
directives: [],
fields: [
fieldNode(
nameNode('field', { start: 32, end: 37 }),
typeNode('String', { start: 39, end: 45 }),
{ start: 32, end: 45 },
nameNode('field', { start: 33, end: 38 }),
typeNode('String', { start: 40, end: 46 }),
{ start: 33, end: 46 },
),
],
loc: { start: 0, end: 47 },
loc: { start: 0, end: 48 },
},
],
loc: { start: 0, end: 47 },
loc: { start: 0, end: 48 },
};
expect(printJson(doc)).to.equal(printJson(expected));
});

it('Simple type inheriting multiple interfaces with leading ampersand', () => {
const body = 'type Hello implements & Wo & rld { field: String }';
const doc = parse(body);
const expected = {
kind: 'Document',
definitions: [
{
kind: 'ObjectTypeDefinition',
name: nameNode('Hello', { start: 5, end: 10 }),
interfaces: [
typeNode('Wo', { start: 24, end: 26 }),
typeNode('rld', { start: 29, end: 32 }),
],
directives: [],
fields: [
fieldNode(
nameNode('field', { start: 35, end: 40 }),
typeNode('String', { start: 42, end: 48 }),
{ start: 35, end: 48 },
),
],
loc: { start: 0, end: 50 },
},
],
loc: { start: 0, end: 50 },
};
expect(printJson(doc)).to.equal(printJson(expected));
});
Expand Down Expand Up @@ -721,4 +750,20 @@ input Hello {
],
});
});

it('Option: allowLegacySDLImplementsInterfaces', () => {
const body = 'type Hello implements Wo rld { field: String }';
expect(() => parse(body)).to.throw('Syntax Error: Unexpected Name "rld"');
const doc = parse(body, { allowLegacySDLImplementsInterfaces: true });
expect(doc).to.containSubset({
definitions: [
{
interfaces: [
typeNode('Wo', { start: 22, end: 24 }),
typeNode('rld', { start: 25, end: 28 }),
],
},
],
});
});
});
2 changes: 1 addition & 1 deletion src/language/__tests__/schema-printer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ describe('Printer', () => {
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
1 change: 1 addition & 0 deletions src/language/ast.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ type TokenKind =
| '<EOF>'
| '!'
| '$'
| '&'
| '('
| ')'
| '...'
Expand Down
5 changes: 5 additions & 0 deletions src/language/lexer.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ const SOF = '<SOF>';
const EOF = '<EOF>';
const BANG = '!';
const DOLLAR = '$';
const AMP = '&';
const PAREN_L = '(';
const PAREN_R = ')';
const SPREAD = '...';
Expand Down Expand Up @@ -126,6 +127,7 @@ export const TokenKind = {
EOF,
BANG,
DOLLAR,
AMP,
PAREN_L,
PAREN_R,
SPREAD,
Expand Down Expand Up @@ -242,6 +244,9 @@ function readToken(lexer: Lexer<*>, prev: Token): Token {
// $
case 36:
return new Tok(DOLLAR, position, position + 1, line, col, prev);
// &
case 38:
return new Tok(AMP, position, position + 1, line, col, prev);
// (
case 40:
return new Tok(PAREN_L, position, position + 1, line, col, prev);
Expand Down
23 changes: 21 additions & 2 deletions src/language/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,16 @@ export type ParseOptions = {
*/
allowLegacySDLEmptyFields?: 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.
*/
allowLegacySDLImplementsInterfaces?: boolean,

/**
* EXPERIMENTAL:
*
Expand Down Expand Up @@ -922,15 +932,24 @@ function parseObjectTypeDefinition(lexer: Lexer<*>): ObjectTypeDefinitionNode {
}

/**
* ImplementsInterfaces : implements NamedType+
* ImplementsInterfaces :
* - implements `&`? NamedType
* - ImplementsInterfaces & NamedType
*/
function parseImplementsInterfaces(lexer: Lexer<*>): Array<NamedTypeNode> {
const types = [];
if (lexer.token.value === 'implements') {
lexer.advance();
// Optional leading ampersand
skip(lexer, TokenKind.AMP);
do {
types.push(parseNamedType(lexer));
} while (peek(lexer, TokenKind.NAME));
} while (
skip(lexer, TokenKind.AMP) ||
// Legacy support for the SDL?
(lexer.options.allowLegacySDLImplementsInterfaces &&
peek(lexer, TokenKind.NAME))
);
}
return types;
}
Expand Down
4 changes: 2 additions & 2 deletions src/language/printer.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ const printDocASTReducer = {
[
'type',
name,
wrap('implements ', join(interfaces, ', ')),
wrap('implements ', join(interfaces, ' & ')),
join(directives, ' '),
block(fields),
],
Expand Down Expand Up @@ -226,7 +226,7 @@ const printDocASTReducer = {
[
'extend type',
name,
wrap('implements ', join(interfaces, ', ')),
wrap('implements ', join(interfaces, ' & ')),
join(directives, ' '),
block(fields),
],
Expand Down
4 changes: 2 additions & 2 deletions src/type/__tests__/validation-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -822,14 +822,14 @@ describe('Type System: Objects can only implement unique interfaces', () => {
field: String
}
type AnotherObject implements AnotherInterface, AnotherInterface {
type AnotherObject implements AnotherInterface & AnotherInterface {
field: String
}
`);
expect(validateSchema(schema)).to.containSubset([
{
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 f317a65

Please sign in to comment.