From f317a655c6af000dc3e865f0e9dfcf6fc7b5b471 Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Tue, 19 Dec 2017 16:08:34 -0800 Subject: [PATCH] RFC: SDL - Separate multiple inherited interfaces with `&` 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})` --- .../__tests__/schema-kitchen-sink.graphql | 2 +- src/language/__tests__/schema-parser-test.js | 59 ++++++++++++++++--- src/language/__tests__/schema-printer-test.js | 2 +- src/language/ast.js | 1 + src/language/lexer.js | 5 ++ src/language/parser.js | 23 +++++++- src/language/printer.js | 4 +- src/type/__tests__/validation-test.js | 4 +- 8 files changed, 85 insertions(+), 15 deletions(-) diff --git a/src/language/__tests__/schema-kitchen-sink.graphql b/src/language/__tests__/schema-kitchen-sink.graphql index f18b09968b..f94f47c8e5 100644 --- a/src/language/__tests__/schema-kitchen-sink.graphql +++ b/src/language/__tests__/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 diff --git a/src/language/__tests__/schema-parser-test.js b/src/language/__tests__/schema-parser-test.js index e150a3f594..a483599103 100644 --- a/src/language/__tests__/schema-parser-test.js +++ b/src/language/__tests__/schema-parser-test.js @@ -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', @@ -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)); }); @@ -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 }), + ], + }, + ], + }); + }); }); diff --git a/src/language/__tests__/schema-printer-test.js b/src/language/__tests__/schema-printer-test.js index f57c824fb7..1bacb8e586 100644 --- a/src/language/__tests__/schema-printer-test.js +++ b/src/language/__tests__/schema-printer-test.js @@ -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 diff --git a/src/language/ast.js b/src/language/ast.js index 01d17cdbe8..bec5ae3d57 100644 --- a/src/language/ast.js +++ b/src/language/ast.js @@ -50,6 +50,7 @@ type TokenKind = | '' | '!' | '$' + | '&' | '(' | ')' | '...' diff --git a/src/language/lexer.js b/src/language/lexer.js index 8b2bd99f44..9edc753c9d 100644 --- a/src/language/lexer.js +++ b/src/language/lexer.js @@ -99,6 +99,7 @@ const SOF = ''; const EOF = ''; const BANG = '!'; const DOLLAR = '$'; +const AMP = '&'; const PAREN_L = '('; const PAREN_R = ')'; const SPREAD = '...'; @@ -126,6 +127,7 @@ export const TokenKind = { EOF, BANG, DOLLAR, + AMP, PAREN_L, PAREN_R, SPREAD, @@ -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); diff --git a/src/language/parser.js b/src/language/parser.js index e14161e4e2..afa5d6ecab 100644 --- a/src/language/parser.js +++ b/src/language/parser.js @@ -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: * @@ -922,15 +932,24 @@ function parseObjectTypeDefinition(lexer: Lexer<*>): ObjectTypeDefinitionNode { } /** - * ImplementsInterfaces : implements NamedType+ + * ImplementsInterfaces : + * - implements `&`? NamedType + * - ImplementsInterfaces & NamedType */ function parseImplementsInterfaces(lexer: Lexer<*>): Array { 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; } diff --git a/src/language/printer.js b/src/language/printer.js index 8dcf1bba67..10bb63595a 100644 --- a/src/language/printer.js +++ b/src/language/printer.js @@ -130,7 +130,7 @@ const printDocASTReducer = { [ 'type', name, - wrap('implements ', join(interfaces, ', ')), + wrap('implements ', join(interfaces, ' & ')), join(directives, ' '), block(fields), ], @@ -226,7 +226,7 @@ const printDocASTReducer = { [ 'extend type', name, - wrap('implements ', join(interfaces, ', ')), + wrap('implements ', join(interfaces, ' & ')), join(directives, ' '), block(fields), ], diff --git a/src/type/__tests__/validation-test.js b/src/type/__tests__/validation-test.js index 107e94e331..5e0572d4e0 100644 --- a/src/type/__tests__/validation-test.js +++ b/src/type/__tests__/validation-test.js @@ -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 }], }, ]); });