Skip to content

Commit fa07cc6

Browse files
committed
This allows empty types but NOT empty type extensions
1 parent f188b24 commit fa07cc6

File tree

3 files changed

+141
-55
lines changed

3 files changed

+141
-55
lines changed

src/main/antlr/GraphqlSDL.g4

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,18 +38,24 @@ typeExtension :
3838

3939
scalarTypeDefinition : description? SCALAR name directives?;
4040

41-
scalarTypeExtensionDefinition : EXTEND SCALAR name directives?;
41+
scalarTypeExtensionDefinition : EXTEND SCALAR name directives;
4242

4343
objectTypeDefinition : description? TYPE name implementsInterfaces? directives? fieldsDefinition?;
4444

45-
objectTypeExtensionDefinition : EXTEND TYPE name implementsInterfaces? directives? fieldsDefinition?;
45+
objectTypeExtensionDefinition :
46+
EXTEND TYPE name implementsInterfaces? directives? extensionFieldsDefinition |
47+
EXTEND TYPE name implementsInterfaces? directives |
48+
EXTEND TYPE name implementsInterfaces
49+
;
4650

4751
implementsInterfaces :
4852
IMPLEMENTS '&'? typeName+ |
4953
implementsInterfaces '&' typeName ;
5054

5155
fieldsDefinition : '{' fieldDefinition* '}';
5256

57+
extensionFieldsDefinition : '{' fieldDefinition+ '}';
58+
5359
fieldDefinition : description? name argumentsDefinition? ':' type directives?;
5460

5561
argumentsDefinition : '(' inputValueDefinition+ ')';
@@ -58,12 +64,18 @@ inputValueDefinition : description? name ':' type defaultValue? directives?;
5864

5965
interfaceTypeDefinition : description? INTERFACE name directives? fieldsDefinition?;
6066

61-
interfaceTypeExtensionDefinition : EXTEND INTERFACE name directives? fieldsDefinition?;
67+
interfaceTypeExtensionDefinition :
68+
EXTEND INTERFACE name directives? extensionFieldsDefinition |
69+
EXTEND INTERFACE name directives
70+
;
6271

6372

64-
unionTypeDefinition : description? UNION name directives? unionMembership;
73+
unionTypeDefinition : description? UNION name directives? unionMembership?;
6574

66-
unionTypeExtensionDefinition : EXTEND UNION name directives? unionMembership?;
75+
unionTypeExtensionDefinition :
76+
EXTEND UNION name directives? unionMembership |
77+
EXTEND UNION name directives
78+
;
6779

6880
unionMembership : '=' unionMembers;
6981

@@ -74,19 +86,29 @@ unionMembers '|' typeName
7486

7587
enumTypeDefinition : description? ENUM name directives? enumValueDefinitions?;
7688

77-
enumTypeExtensionDefinition : EXTEND ENUM name directives? enumValueDefinitions?;
89+
enumTypeExtensionDefinition :
90+
EXTEND ENUM name directives? extensionEnumValueDefinitions |
91+
EXTEND ENUM name directives
92+
;
93+
94+
enumValueDefinitions : '{' enumValueDefinition* '}';
7895

79-
enumValueDefinitions : '{' enumValueDefinition+ '}';
96+
extensionEnumValueDefinitions : '{' enumValueDefinition+ '}';
8097

8198
enumValueDefinition : description? enumValue directives?;
8299

83100

84101
inputObjectTypeDefinition : description? INPUT name directives? inputObjectValueDefinitions?;
85102

86-
inputObjectTypeExtensionDefinition : EXTEND INPUT name directives? inputObjectValueDefinitions?;
103+
inputObjectTypeExtensionDefinition :
104+
EXTEND INPUT name directives? extensionInputObjectValueDefinitions |
105+
EXTEND INPUT name directives
106+
;
87107

88108
inputObjectValueDefinitions : '{' inputValueDefinition* '}';
89109

110+
extensionInputObjectValueDefinitions : '{' inputValueDefinition+ '}';
111+
90112

91113
directiveDefinition : description? DIRECTIVE '@' name argumentsDefinition? 'on' directiveLocations;
92114

src/main/java/graphql/parser/GraphqlAntlrToLanguage.java

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ public GraphqlAntlrToLanguage(CommonTokenStream tokens) {
9393
public Document createDocument(GraphqlParser.DocumentContext ctx) {
9494
Document.Builder document = Document.newDocument();
9595
addCommonData(document, ctx);
96-
document.definitions(ctx.definition().stream().map(definition -> createDefinition(definition))
96+
document.definitions(ctx.definition().stream().map(this::createDefinition)
9797
.collect(toList()));
9898
return document.build();
9999
}
@@ -129,14 +129,15 @@ protected OperationDefinition createOperationDefinition(GraphqlParser.OperationD
129129
}
130130

131131
protected OperationDefinition.Operation parseOperation(GraphqlParser.OperationTypeContext operationTypeContext) {
132-
if (operationTypeContext.getText().equals("query")) {
133-
return OperationDefinition.Operation.QUERY;
134-
} else if (operationTypeContext.getText().equals("mutation")) {
135-
return OperationDefinition.Operation.MUTATION;
136-
} else if (operationTypeContext.getText().equals("subscription")) {
137-
return OperationDefinition.Operation.SUBSCRIPTION;
138-
} else {
139-
return assertShouldNeverHappen("InternalError: unknown operationTypeContext=%s", operationTypeContext.getText());
132+
switch (operationTypeContext.getText()) {
133+
case "query":
134+
return OperationDefinition.Operation.QUERY;
135+
case "mutation":
136+
return OperationDefinition.Operation.MUTATION;
137+
case "subscription":
138+
return OperationDefinition.Operation.SUBSCRIPTION;
139+
default:
140+
return assertShouldNeverHappen("InternalError: unknown operationTypeContext=%s", operationTypeContext.getText());
140141
}
141142
}
142143

@@ -429,8 +430,8 @@ protected ObjectTypeExtensionDefinition createObjectTypeExtensionDefinition(Grap
429430
implementsInterfacesContext = implementsInterfacesContext.implementsInterfaces();
430431
}
431432
def.implementz(implementz);
432-
if (ctx.fieldsDefinition() != null) {
433-
def.fieldDefinitions(createFieldDefinitions(ctx.fieldsDefinition()));
433+
if (ctx.extensionFieldsDefinition() != null) {
434+
def.fieldDefinitions(createFieldDefinitions(ctx.extensionFieldsDefinition()));
434435
}
435436
return def.build();
436437
}
@@ -442,6 +443,14 @@ protected List<FieldDefinition> createFieldDefinitions(GraphqlParser.FieldsDefin
442443
return ctx.fieldDefinition().stream().map(this::createFieldDefinition).collect(toList());
443444
}
444445

446+
protected List<FieldDefinition> createFieldDefinitions(GraphqlParser.ExtensionFieldsDefinitionContext ctx) {
447+
if (ctx == null) {
448+
return new ArrayList<>();
449+
}
450+
return ctx.fieldDefinition().stream().map(this::createFieldDefinition).collect(toList());
451+
}
452+
453+
445454
protected FieldDefinition createFieldDefinition(GraphqlParser.FieldDefinitionContext ctx) {
446455
FieldDefinition.Builder def = FieldDefinition.newFieldDefinition();
447456
def.name(ctx.name().getText());
@@ -488,7 +497,7 @@ protected InterfaceTypeExtensionDefinition createInterfaceTypeExtensionDefinitio
488497
def.name(ctx.name().getText());
489498
addCommonData(def, ctx);
490499
def.directives(createDirectives(ctx.directives()));
491-
def.definitions(createFieldDefinitions(ctx.fieldsDefinition()));
500+
def.definitions(createFieldDefinitions(ctx.extensionFieldsDefinition()));
492501
return def.build();
493502
}
494503

@@ -543,9 +552,9 @@ protected EnumTypeExtensionDefinition createEnumTypeExtensionDefinition(GraphqlP
543552
def.name(ctx.name().getText());
544553
addCommonData(def, ctx);
545554
def.directives(createDirectives(ctx.directives()));
546-
if (ctx.enumValueDefinitions() != null) {
555+
if (ctx.extensionEnumValueDefinitions() != null) {
547556
def.enumValueDefinitions(
548-
ctx.enumValueDefinitions().enumValueDefinition().stream().map(this::createEnumValueDefinition).collect(toList()));
557+
ctx.extensionEnumValueDefinitions().enumValueDefinition().stream().map(this::createEnumValueDefinition).collect(toList()));
549558
}
550559
return def.build();
551560
}
@@ -576,8 +585,8 @@ protected InputObjectTypeExtensionDefinition createInputObjectTypeExtensionDefin
576585
def.name(ctx.name().getText());
577586
addCommonData(def, ctx);
578587
def.directives(createDirectives(ctx.directives()));
579-
if (ctx.inputObjectValueDefinitions() != null) {
580-
def.inputValueDefinitions(createInputValueDefinitions(ctx.inputObjectValueDefinitions().inputValueDefinition()));
588+
if (ctx.extensionInputObjectValueDefinitions() != null) {
589+
def.inputValueDefinitions(createInputValueDefinitions(ctx.extensionInputObjectValueDefinitions().inputValueDefinition()));
581590
}
582591
return def.build();
583592
}

src/test/groovy/graphql/schema/idl/SchemaParserTest.groovy

Lines changed: 86 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -164,46 +164,101 @@ class SchemaParserTest extends Specification {
164164
165165
"""
166166
when:
167-
TypeDefinitionRegistry typeRegistry = new SchemaParser().parse(schema)
167+
TypeDefinitionRegistry typeRegistry = read(schema)
168168
then:
169169
typeRegistry.types().size() == 4
170170
}
171171

172-
def "empty types and extensions are allowed"() {
173-
def schema = '''
174-
type Foo {
175-
}
172+
def assertSchemaProblem(String s) {
173+
try {
174+
read(s)
175+
assert false, "Expected a a schema problem for : " + s
176+
} catch (SchemaProblem ignored) {
177+
true
178+
}
179+
}
176180

177-
extend type Foo {
178-
}
179-
180-
interface InterfaceFoo {
181-
}
181+
def assertNoSchemaProblem(String s) {
182+
try {
183+
read(s)
184+
true
185+
} catch (SchemaProblem problem) {
186+
assert false, "Did not expected a schema problem for : " + s + " of : " + problem
187+
}
188+
}
182189

183-
extend interface InterfaceFoo {
184-
}
185-
186-
input InputFoo {
187-
}
188190

189-
extend input InputFoo {
190-
}
191-
192-
enum EnumFoo {
193-
}
191+
def "empty types (with and without parentheses) are allowed"() {
192+
//
193+
// empty parentheses are not quite allowed by the spec but in the name of backwards compatibility
194+
// AND general usefulness we are going to allow them. So in the list below the last two of each section
195+
// is not technically allowed by the latest spec
196+
//
197+
expect:
198+
assertNoSchemaProblem(schema)
194199
195-
extend enum EnumFoo {
196-
}
197-
198-
'''
200+
where:
201+
schema | _
202+
''' type Foo ''' | _
203+
''' type Foo @directive ''' | _
204+
''' type Foo { } ''' | _
205+
''' type Foo @directive { } ''' | _
199206
200-
when:
201-
TypeDefinitionRegistry typeRegistry = new SchemaParser().parse(schema)
202-
then:
203-
typeRegistry.types().size() == 3
204-
typeRegistry.interfaceTypeExtensions().size() == 1
205-
typeRegistry.objectTypeExtensions().size() == 1
206-
typeRegistry.inputObjectTypeExtensions().size() == 1
207+
''' interface Foo ''' | _
208+
''' interface Foo @directive ''' | _
209+
''' interface Foo { } ''' | _
210+
''' interface Foo @directive { } ''' | _
211+
212+
''' input Foo ''' | _
213+
''' input Foo @directive ''' | _
214+
''' input Foo { } ''' | _
215+
''' input Foo @directive { } ''' | _
216+
217+
''' enum Foo ''' | _
218+
''' enum Foo @directive ''' | _
219+
''' enum Foo { } ''' | _
220+
''' enum Foo @directive { } ''' | _
221+
}
222+
223+
224+
def "extensions are not allowed to be empty"() {
225+
226+
expect:
227+
assertSchemaProblem(schema)
228+
229+
where:
230+
schema | _
231+
''' extend type Foo''' | _
232+
''' extend type Foo {}''' | _
233+
''' extend interface Foo ''' | _
234+
''' extend interface Foo {}''' | _
235+
''' extend input Foo ''' | _
236+
''' extend input Foo {}''' | _
237+
''' extend enum Foo ''' | _
238+
''' extend enum Foo {}''' | _
239+
}
240+
241+
def "extensions must extend with fields or directives"() {
242+
243+
expect:
244+
assertNoSchemaProblem(schema)
245+
246+
where:
247+
schema | _
248+
''' extend type Foo @directive''' | _
249+
''' extend type Foo { f : Int }''' | _
250+
''' extend type Foo @directive { f : Int }''' | _
251+
252+
''' extend interface Foo @directive ''' | _
253+
''' extend interface Foo { f : Int }''' | _
254+
''' extend interface Foo { f : Int }''' | _
255+
256+
''' extend input Foo @directive ''' | _
257+
''' extend input Foo { f : Int }''' | _
258+
''' extend input Foo { f : Int }''' | _
207259
260+
''' extend enum Foo @directive ''' | _
261+
''' extend enum Foo { a,b,c }''' | _
262+
''' extend enum Foo @directive { a,b,c }''' | _
208263
}
209264
}

0 commit comments

Comments
 (0)