Skip to content

Commit a32b806

Browse files
authored
Rework of all value nodes + tune error messages (#166)
1 parent 241e1fd commit a32b806

22 files changed

+625
-285
lines changed

src/GraphQLParser.ApiTests/GraphQLParser.approved.txt

Lines changed: 56 additions & 26 deletions
Large diffs are not rendered by default.

src/GraphQLParser.Tests/Issue82.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public void Parse_Named_And_Literal_Variables(IgnoreOptions options)
3030
var selection = def.SelectionSet.Selections[0].ShouldBeAssignableTo<GraphQLField>();
3131
selection.Arguments.Count.ShouldBe(2);
3232
selection.Arguments[0].Value.ShouldBeAssignableTo<GraphQLVariable>().Name.Value.ShouldBe("username");
33-
selection.Arguments[1].Value.ShouldBeAssignableTo<GraphQLScalarValue>().Value.ShouldBe("Pete");
33+
selection.Arguments[1].Value.ShouldBeAssignableTo<GraphQLStringValue>().Value.ShouldBe("Pete");
3434
}
3535
}
3636
}

src/GraphQLParser.Tests/ParserTests.cs

Lines changed: 36 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -208,13 +208,13 @@ public void Comments_on_Values_Should_Read_Correctly(IgnoreOptions options)
208208
field.SelectionSet.Selections.Count.ShouldBe(1);
209209
field.Arguments.Count.ShouldBe(9);
210210

211-
var boolValue = field.Arguments[0].Value.ShouldBeAssignableTo<GraphQLScalarValue>();
211+
var boolValue = field.Arguments[0].Value.ShouldBeAssignableTo<GraphQLBooleanValue>();
212212
boolValue.Comment.ShouldNotBeNull().Text.ShouldBe("comment for bool");
213213

214-
var nullValue = field.Arguments[1].Value.ShouldBeAssignableTo<GraphQLScalarValue>();
214+
var nullValue = field.Arguments[1].Value.ShouldBeAssignableTo<GraphQLNullValue>();
215215
nullValue.Comment.ShouldNotBeNull().Text.ShouldBe("comment for null");
216216

217-
var enumValue = field.Arguments[2].Value.ShouldBeAssignableTo<GraphQLScalarValue>();
217+
var enumValue = field.Arguments[2].Value.ShouldBeAssignableTo<GraphQLEnumValue>();
218218
enumValue.Comment.ShouldNotBeNull().Text.ShouldBe("comment for enum");
219219

220220
var listValue = field.Arguments[3].Value.ShouldBeAssignableTo<GraphQLListValue>();
@@ -223,13 +223,13 @@ public void Comments_on_Values_Should_Read_Correctly(IgnoreOptions options)
223223
var objValue = field.Arguments[4].Value.ShouldBeAssignableTo<GraphQLObjectValue>();
224224
objValue.Comment.ShouldNotBeNull().Text.ShouldBe("comment for object");
225225

226-
var intValue = field.Arguments[5].Value.ShouldBeAssignableTo<GraphQLScalarValue>();
226+
var intValue = field.Arguments[5].Value.ShouldBeAssignableTo<GraphQLIntValue>();
227227
intValue.Comment.ShouldNotBeNull().Text.ShouldBe("comment for int");
228228

229-
var floatValue = field.Arguments[6].Value.ShouldBeAssignableTo<GraphQLScalarValue>();
229+
var floatValue = field.Arguments[6].Value.ShouldBeAssignableTo<GraphQLFloatValue>();
230230
floatValue.Comment.ShouldNotBeNull().Text.ShouldBe("comment for float");
231231

232-
var stringValue = field.Arguments[7].Value.ShouldBeAssignableTo<GraphQLScalarValue>();
232+
var stringValue = field.Arguments[7].Value.ShouldBeAssignableTo<GraphQLStringValue>();
233233
stringValue.Comment.ShouldNotBeNull().Text.ShouldBe("comment for string");
234234

235235
var varValue = field.Arguments[8].Value.ShouldBeAssignableTo<GraphQLVariable>();
@@ -527,6 +527,18 @@ public void Should_Throw_On_Unknown_OperationType()
527527
ex.Column.ShouldBe(1);
528528
}
529529

530+
[Theory]
531+
[InlineData("enum E { true A }", "Unexpected Name \"true\"; enum values are represented as unquoted names but not 'true' or 'false' or 'null'.", 1, 10)]
532+
[InlineData("enum E { B false }", "Unexpected Name \"false\"; enum values are represented as unquoted names but not 'true' or 'false' or 'null'.", 1, 12)]
533+
[InlineData("enum E { A null B }", "Unexpected Name \"null\"; enum values are represented as unquoted names but not 'true' or 'false' or 'null'.", 1, 12)]
534+
public void Should_Throw_On_Invalid_EnumValue(string query, string description, int line, int column)
535+
{
536+
var ex = Should.Throw<GraphQLSyntaxErrorException>(() => query.Parse());
537+
ex.Description.ShouldBe(description);
538+
ex.Line.ShouldBe(line);
539+
ex.Column.ShouldBe(column);
540+
}
541+
530542
[Theory]
531543
[InlineData(IgnoreOptions.None)]
532544
[InlineData(IgnoreOptions.Comments)]
@@ -825,7 +837,7 @@ public void Should_Parse_SpecifiedBy()
825837
def.Directives[0].Name.Value.ShouldBe("specifiedBy");
826838
def.Directives[0].Arguments.Count.ShouldBe(1);
827839
def.Directives[0].Arguments[0].Name.Value.ShouldBe("url");
828-
var value = def.Directives[0].Arguments[0].Value.ShouldBeAssignableTo<GraphQLScalarValue>();
840+
var value = def.Directives[0].Arguments[0].Value.ShouldBeAssignableTo<GraphQLStringValue>();
829841
value.Value.ShouldBe("https://tools.ietf.org/html/rfc4122");
830842
}
831843

@@ -936,23 +948,23 @@ public void Should_Parse_Extensions(string text, ASTNodeKind kind)
936948
}
937949

938950
[Theory]
939-
[InlineData("extend", "Unexpected EOF")]
940-
[InlineData("extend scalar", "Expected Name, found EOF")]
941-
[InlineData("extend scalar A", "Unexpected EOF")]
942-
[InlineData("extend scalar A B", "Unexpected Name \"B\"")]
943-
[InlineData("extend type", "Expected Name, found EOF")]
944-
[InlineData("extend type A", "Unexpected EOF")]
945-
[InlineData("extend type A B", "Unexpected Name \"B\"")]
946-
[InlineData("extend interface", "Expected Name, found EOF")]
947-
[InlineData("extend interface A", "Unexpected EOF")]
948-
[InlineData("extend interface A B", "Unexpected Name \"B\"")]
949-
[InlineData("extend union", "Expected Name, found EOF")]
950-
[InlineData("extend union A", "Unexpected EOF")]
951-
[InlineData("extend enum", "Expected Name, found EOF")]
952-
[InlineData("extend enum A", "Unexpected EOF")]
953-
[InlineData("extend input", "Expected Name, found EOF")]
954-
[InlineData("extend input A", "Unexpected EOF")]
955-
[InlineData("extend variable", "Unexpected Name \"variable\"")]
951+
[InlineData("extend", "Unexpected EOF; for more information see http://spec.graphql.org/October2021/#TypeExtension")]
952+
[InlineData("extend scalar", "Expected Name, found EOF; for more information see http://spec.graphql.org/October2021/#ScalarTypeExtension")]
953+
[InlineData("extend scalar A", "Unexpected EOF; for more information see http://spec.graphql.org/October2021/#ScalarTypeExtension")]
954+
[InlineData("extend scalar A B", "Unexpected Name \"B\"; for more information see http://spec.graphql.org/October2021/#ScalarTypeExtension")]
955+
[InlineData("extend type", "Expected Name, found EOF; for more information see http://spec.graphql.org/October2021/#ObjectTypeExtension")]
956+
[InlineData("extend type A", "Unexpected EOF; for more information see http://spec.graphql.org/October2021/#ObjectTypeExtension")]
957+
[InlineData("extend type A B", "Unexpected Name \"B\"; for more information see http://spec.graphql.org/October2021/#ObjectTypeExtension")]
958+
[InlineData("extend interface", "Expected Name, found EOF; for more information see http://spec.graphql.org/October2021/#InterfaceTypeExtension")]
959+
[InlineData("extend interface A", "Unexpected EOF; for more information see http://spec.graphql.org/October2021/#InterfaceTypeExtension")]
960+
[InlineData("extend interface A B", "Unexpected Name \"B\"; for more information see http://spec.graphql.org/October2021/#InterfaceTypeExtension")]
961+
[InlineData("extend union", "Expected Name, found EOF; for more information see http://spec.graphql.org/October2021/#UnionTypeExtension")]
962+
[InlineData("extend union A", "Unexpected EOF; for more information see http://spec.graphql.org/October2021/#UnionTypeExtension")]
963+
[InlineData("extend enum", "Expected Name, found EOF; for more information see http://spec.graphql.org/October2021/#EnumTypeExtension")]
964+
[InlineData("extend enum A", "Unexpected EOF; for more information see http://spec.graphql.org/October2021/#EnumTypeExtension")]
965+
[InlineData("extend input", "Expected Name, found EOF; for more information see http://spec.graphql.org/October2021/#InputObjectTypeExtension")]
966+
[InlineData("extend input A", "Unexpected EOF; for more information see http://spec.graphql.org/October2021/#InputObjectTypeExtension")]
967+
[InlineData("extend variable", "Unexpected Name \"variable\"; for more information see http://spec.graphql.org/October2021/#TypeExtension")]
956968
public void Should_Throw_Extensions(string text, string description)
957969
{
958970
var ex = Should.Throw<GraphQLSyntaxErrorException>(() => text.Parse());

src/GraphQLParser.Tests/Validation/ParserValidationTests.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@ public void Parse_FragmentInvalidOnName_ThrowsExceptionWithCorrectMessage(Ignore
1616
var exception = Should.Throw<GraphQLSyntaxErrorException>(() => "fragment on on on { on }".Parse(new ParserOptions { Ignore = options }));
1717

1818
exception.Message.ShouldBe(
19-
"Syntax Error GraphQL (1:10) Unexpected Name \"on\"\n" +
19+
"Syntax Error GraphQL (1:10) Unexpected Name \"on\"; fragment name can not be 'on'.\n" +
2020
"1: fragment on on on { on }\n" +
2121
" ^\n");
22-
exception.Description.ShouldBe("Unexpected Name " + "\"on\"");
22+
exception.Description.ShouldBe("Unexpected Name " + "\"on\"; fragment name can not be 'on'.");
2323
exception.Line.ShouldBe(1);
2424
exception.Column.ShouldBe(10);
2525
}
@@ -52,10 +52,10 @@ public void Parse_InvalidFragmentNameInSpread_ThrowsExceptionWithCorrectMessage(
5252
var exception = Should.Throw<GraphQLSyntaxErrorException>(() => "{ ...on }".Parse(new ParserOptions { Ignore = options }));
5353

5454
exception.Message.ShouldBe(
55-
"Syntax Error GraphQL (1:9) Expected Name, found }\n" +
55+
"Syntax Error GraphQL (1:9) Expected Name, found }; for more information see http://spec.graphql.org/October2021/#NamedType\n" +
5656
"1: { ...on }\n" +
5757
" ^\n");
58-
exception.Description.ShouldBe("Expected Name, found }");
58+
exception.Description.ShouldBe("Expected Name, found }; for more information see http://spec.graphql.org/October2021/#NamedType");
5959
exception.Line.ShouldBe(1);
6060
exception.Column.ShouldBe(9);
6161
}
@@ -88,10 +88,10 @@ public void Parse_MissingEndingBrace_ThrowsExceptionWithCorrectMessage(IgnoreOpt
8888
var exception = Should.Throw<GraphQLSyntaxErrorException>(() => "{".Parse(new ParserOptions { Ignore = options }));
8989

9090
exception.Message.ShouldBe(
91-
"Syntax Error GraphQL (1:2) Expected Name, found EOF\n" +
91+
"Syntax Error GraphQL (1:2) Expected Name, found EOF; for more information see http://spec.graphql.org/October2021/#Field\n" +
9292
"1: {\n" +
9393
" ^\n");
94-
exception.Description.ShouldBe("Expected Name, found EOF");
94+
exception.Description.ShouldBe("Expected Name, found EOF; for more information see http://spec.graphql.org/October2021/#Field");
9595
exception.Line.ShouldBe(1);
9696
exception.Column.ShouldBe(2);
9797
}
@@ -106,10 +106,10 @@ public void Parse_MissingFieldNameWhenAliasing_ThrowsExceptionWithCorrectMessage
106106
var exception = Should.Throw<GraphQLSyntaxErrorException>(() => "{ field: {} }".Parse(new ParserOptions { Ignore = options }));
107107

108108
exception.Message.ShouldBe(
109-
"Syntax Error GraphQL (1:10) Expected Name, found {\n" +
109+
"Syntax Error GraphQL (1:10) Expected Name, found {; for more information see http://spec.graphql.org/October2021/#Field\n" +
110110
"1: { field: {} }\n" +
111111
" ^\n");
112-
exception.Description.ShouldBe("Expected Name, found {");
112+
exception.Description.ShouldBe("Expected Name, found {; for more information see http://spec.graphql.org/October2021/#Field");
113113
exception.Line.ShouldBe(1);
114114
exception.Column.ShouldBe(10);
115115
}

src/GraphQLParser.Tests/Visitors/GraphQLAstVisitorTests.cs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,13 @@ public class GraphQLAstVisitorTests
1313
{
1414
public class CountVisitor : DefaultNodeVisitor<CountContext>
1515
{
16-
public override async ValueTask VisitBooleanValue(GraphQLScalarValue booleanValue, CountContext context)
16+
public override async ValueTask VisitBooleanValue(GraphQLBooleanValue booleanValue, CountContext context)
1717
{
1818
context.VisitedBooleanValues.Add(booleanValue);
1919
await base.VisitBooleanValue(booleanValue, context);
2020
}
2121

22-
public override async ValueTask VisitIntValue(GraphQLScalarValue intValue, CountContext context)
22+
public override async ValueTask VisitIntValue(GraphQLIntValue intValue, CountContext context)
2323
{
2424
context.VisitedIntValues.Add(intValue);
2525
await base.VisitIntValue(intValue, context);
@@ -55,13 +55,13 @@ public override async ValueTask VisitDirective(GraphQLDirective directive, Count
5555
await base.VisitDirective(directive, context);
5656
}
5757

58-
public override async ValueTask VisitEnumValue(GraphQLScalarValue enumValue, CountContext context)
58+
public override async ValueTask VisitEnumValue(GraphQLEnumValue enumValue, CountContext context)
5959
{
6060
context.VisitedEnumValues.Add(enumValue);
6161
await base.VisitEnumValue(enumValue, context);
6262
}
6363

64-
public override async ValueTask VisitStringValue(GraphQLScalarValue stringValue, CountContext context)
64+
public override async ValueTask VisitStringValue(GraphQLStringValue stringValue, CountContext context)
6565
{
6666
context.VisitedStringValues.Add(stringValue);
6767
await base.VisitStringValue(stringValue, context);
@@ -81,7 +81,7 @@ public override async ValueTask VisitField(GraphQLField field, CountContext cont
8181
await base.VisitField(field, context);
8282
}
8383

84-
public override async ValueTask VisitFloatValue(GraphQLScalarValue floatValue, CountContext context)
84+
public override async ValueTask VisitFloatValue(GraphQLFloatValue floatValue, CountContext context)
8585
{
8686
context.VisitedFloatValues.Add(floatValue);
8787
await base.VisitFloatValue(floatValue, context);
@@ -192,19 +192,19 @@ public class CountContext : INodeVisitorContext
192192
public List<GraphQLArgument> VisitedArguments = new();
193193
public List<ASTNode> VisitedDefinitions = new();
194194
public List<GraphQLDirective> VisitedDirectives = new();
195-
public List<GraphQLScalarValue> VisitedEnumValues = new();
195+
public List<GraphQLEnumValue> VisitedEnumValues = new();
196196
public List<GraphQLField> VisitedFields = new();
197-
public List<GraphQLScalarValue> VisitedFloatValues = new();
197+
public List<GraphQLFloatValue> VisitedFloatValues = new();
198198
public List<GraphQLFragmentDefinition> VisitedFragmentDefinitions = new();
199199
public List<GraphQLFragmentSpread> VisitedFragmentSpreads = new();
200200
public List<GraphQLTypeCondition> VisitedFragmentTypeConditions = new();
201201
public List<GraphQLInlineFragment> VisitedInlineFragments = new();
202-
public List<GraphQLScalarValue> VisitedIntValues = new();
202+
public List<GraphQLIntValue> VisitedIntValues = new();
203203
public List<GraphQLName> VisitedNames = new();
204204
public List<GraphQLSelectionSet> VisitedSelectionSets = new();
205-
public List<GraphQLScalarValue> VisitedStringValues = new();
205+
public List<GraphQLStringValue> VisitedStringValues = new();
206206
public List<GraphQLVariable> VisitedVariables = new();
207-
public List<GraphQLScalarValue> VisitedBooleanValues = new();
207+
public List<GraphQLBooleanValue> VisitedBooleanValues = new();
208208

209209
public CancellationToken CancellationToken { get; set; }
210210
}
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
namespace GraphQLParser.AST
2+
{
3+
/// <summary>
4+
/// AST node for <see cref="ASTNodeKind.BooleanValue"/>.
5+
/// </summary>
6+
public class GraphQLBooleanValue : GraphQLValue
7+
{
8+
/// <inheritdoc/>
9+
public override ASTNodeKind Kind => ASTNodeKind.BooleanValue;
10+
11+
/// <summary>
12+
/// Value represented as <see cref="ROM"/>.
13+
/// </summary>
14+
public ROM Value { get; set; }
15+
}
16+
17+
internal sealed class GraphQLBooleanValueWithLocation : GraphQLBooleanValue
18+
{
19+
private GraphQLLocation _location;
20+
21+
public override GraphQLLocation Location
22+
{
23+
get => _location;
24+
set => _location = value;
25+
}
26+
}
27+
28+
internal sealed class GraphQLBooleanValueWithComment : GraphQLBooleanValue
29+
{
30+
private GraphQLComment? _comment;
31+
32+
public override GraphQLComment? Comment
33+
{
34+
get => _comment;
35+
set => _comment = value;
36+
}
37+
}
38+
39+
internal sealed class GraphQLBooleanValueFull : GraphQLBooleanValue
40+
{
41+
private GraphQLLocation _location;
42+
private GraphQLComment? _comment;
43+
44+
public override GraphQLLocation Location
45+
{
46+
get => _location;
47+
set => _location = value;
48+
}
49+
50+
public override GraphQLComment? Comment
51+
{
52+
get => _comment;
53+
set => _comment = value;
54+
}
55+
}
56+
}
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
namespace GraphQLParser.AST
2+
{
3+
/// <summary>
4+
/// AST node for <see cref="ASTNodeKind.EnumValue"/>.
5+
/// </summary>
6+
public class GraphQLEnumValue : GraphQLValue, INamedNode
7+
{
8+
/// <inheritdoc/>
9+
public override ASTNodeKind Kind => ASTNodeKind.EnumValue;
10+
11+
/// <inheritdoc/>
12+
public GraphQLName Name { get; set; } = null!;
13+
}
14+
15+
internal sealed class GraphQLEnumValueWithLocation : GraphQLEnumValue
16+
{
17+
private GraphQLLocation _location;
18+
19+
public override GraphQLLocation Location
20+
{
21+
get => _location;
22+
set => _location = value;
23+
}
24+
}
25+
26+
internal sealed class GraphQLEnumValueWithComment : GraphQLEnumValue
27+
{
28+
private GraphQLComment? _comment;
29+
30+
public override GraphQLComment? Comment
31+
{
32+
get => _comment;
33+
set => _comment = value;
34+
}
35+
}
36+
37+
internal sealed class GraphQLEnumValueFull : GraphQLEnumValue
38+
{
39+
private GraphQLLocation _location;
40+
private GraphQLComment? _comment;
41+
42+
public override GraphQLLocation Location
43+
{
44+
get => _location;
45+
set => _location = value;
46+
}
47+
48+
public override GraphQLComment? Comment
49+
{
50+
get => _comment;
51+
set => _comment = value;
52+
}
53+
}
54+
}

src/GraphQLParser/AST/GraphQLEnumValueDefinition.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@ public class GraphQLEnumValueDefinition : GraphQLTypeDefinition, IHasDirectivesN
88
/// <inheritdoc/>
99
public override ASTNodeKind Kind => ASTNodeKind.EnumValueDefinition;
1010

11+
/// <summary>
12+
/// Enum value represented as a nested AST node. Alas, inherited
13+
/// <see cref="GraphQLTypeDefinition.Name"/> property holds almost
14+
/// the same data and should be set as well.
15+
/// </summary>
16+
public GraphQLEnumValue EnumValue { get; set; } = null!;
17+
1118
/// <inheritdoc/>
1219
public GraphQLDirectives? Directives { get; set; }
1320
}

src/GraphQLParser/AST/GraphQLExecutableDefinition.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ namespace GraphQLParser.AST
22
{
33
/// <summary>
44
/// Base AST node for <see cref="GraphQLOperationDefinition"/> and <see cref="GraphQLFragmentDefinition"/>.
5+
/// <br/>
56
/// http://spec.graphql.org/October2021/#ExecutableDefinition
67
/// </summary>
78
public abstract class GraphQLExecutableDefinition : ASTNode, IHasDirectivesNode

0 commit comments

Comments
 (0)