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, {legacySDL: true})`
  • Loading branch information
leebyron committed Dec 20, 2017
1 parent 8173c24 commit a930eec
Show file tree
Hide file tree
Showing 8 changed files with 119 additions and 26 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
76 changes: 69 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 @@ -708,4 +737,37 @@ input Hello {
{ line: 2, column: 33 },
);
});

describe('Option: legacySDL', () => {
it('Supports type inheriting multiple interfaces with no ampersand', () => {
const body = 'type Hello implements Wo rld { field: String }';
expect(() => parse(body)).to.throw('Syntax Error: Unexpected Name "rld"');
const doc = parse(body, { legacySDL: true });
expect(doc).to.containSubset({
definitions: [
{
interfaces: [
typeNode('Wo', { start: 22, end: 24 }),
typeNode('rld', { start: 25, end: 28 }),
],
},
],
});
});

it('Supports type with empty fields', () => {
const body = 'type Hello { }';
expect(() => parse(body)).to.throw(
'Syntax Error: Expected Name, found }',
);
const doc = parse(body, { legacySDL: true });
expect(doc).to.containSubset({
definitions: [
{
fields: [],
},
],
});
});
});
});
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
7 changes: 6 additions & 1 deletion 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 @@ -160,7 +162,7 @@ const slice = String.prototype.slice;
* Helper function for constructing the Token object.
*/
function Tok(
kind,
kind: $Values<typeof TokenKind>,
start: number,
end: number,
line: number,
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
49 changes: 37 additions & 12 deletions src/language/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,15 @@ export type ParseOptions = {
*/
noLocation?: boolean,

/**
* If enabled, the parser will parse legacy Schema Definition Language.
* Otherwise, the parser will follow the current specification.
*
* Note: this option is provided to ease adoption and may be removed in a
* future release.
*/
legacySDL?: boolean,

/**
* EXPERIMENTAL:
*
Expand Down Expand Up @@ -912,15 +921,23 @@ 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.legacySDL && peek(lexer, TokenKind.NAME))
);
}
return types;
}
Expand All @@ -929,6 +946,16 @@ function parseImplementsInterfaces(lexer: Lexer<*>): Array<NamedTypeNode> {
* FieldsDefinition : { FieldDefinition+ }
*/
function parseFieldsDefinition(lexer: Lexer<*>): Array<FieldDefinitionNode> {
// Legacy support for the SDL?
if (
lexer.options.legacySDL &&
peek(lexer, TokenKind.BRACE_L) &&
lexer.lookahead().kind === TokenKind.BRACE_R
) {
lexer.advance();
lexer.advance();
return [];
}
return peek(lexer, TokenKind.BRACE_L)
? many(lexer, TokenKind.BRACE_L, parseFieldDefinition, TokenKind.BRACE_R)
: [];
Expand Down Expand Up @@ -1018,15 +1045,15 @@ function parseInterfaceTypeDefinition(

/**
* UnionTypeDefinition :
* - Description? union Name Directives[Const]? MemberTypesDefinition?
* - Description? union Name Directives[Const]? UnionMemberTypes?
*/
function parseUnionTypeDefinition(lexer: Lexer<*>): UnionTypeDefinitionNode {
const start = lexer.token;
const description = parseDescription(lexer);
expectKeyword(lexer, 'union');
const name = parseName(lexer);
const directives = parseDirectives(lexer, true);
const types = parseMemberTypesDefinition(lexer);
const types = parseUnionMemberTypes(lexer);
return {
kind: UNION_TYPE_DEFINITION,
description,
Expand All @@ -1038,13 +1065,11 @@ function parseUnionTypeDefinition(lexer: Lexer<*>): UnionTypeDefinitionNode {
}

/**
* MemberTypesDefinition : = MemberTypes
*
* MemberTypes :
* - `|`? NamedType
* - MemberTypes | NamedType
* UnionMemberTypes :
* - = `|`? NamedType
* - UnionMemberTypes | NamedType
*/
function parseMemberTypesDefinition(lexer: Lexer<*>): Array<NamedTypeNode> {
function parseUnionMemberTypes(lexer: Lexer<*>): Array<NamedTypeNode> {
const types = [];
if (skip(lexer, TokenKind.EQUALS)) {
// Optional leading pipe
Expand Down Expand Up @@ -1258,7 +1283,7 @@ function parseInterfaceTypeExtension(

/**
* UnionTypeExtension :
* - extend union Name Directives[Const]? MemberTypesDefinition
* - extend union Name Directives[Const]? UnionMemberTypes
* - extend union Name Directives[Const]
*/
function parseUnionTypeExtension(lexer: Lexer<*>): UnionTypeExtensionNode {
Expand All @@ -1267,7 +1292,7 @@ function parseUnionTypeExtension(lexer: Lexer<*>): UnionTypeExtensionNode {
expectKeyword(lexer, 'union');
const name = parseName(lexer);
const directives = parseDirectives(lexer, true);
const types = parseMemberTypesDefinition(lexer);
const types = parseUnionMemberTypes(lexer);
if (directives.length === 0 && types.length === 0) {
throw unexpected(lexer);
}
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 a930eec

Please sign in to comment.