Skip to content

Rework of all value nodes + tune error messages #166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 7, 2022
Merged

Rework of all value nodes + tune error messages #166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 56 additions & 26 deletions src/GraphQLParser.ApiTests/GraphQLParser.approved.txt
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/GraphQLParser.Tests/Issue82.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public void Parse_Named_And_Literal_Variables(IgnoreOptions options)
var selection = def.SelectionSet.Selections[0].ShouldBeAssignableTo<GraphQLField>();
selection.Arguments.Count.ShouldBe(2);
selection.Arguments[0].Value.ShouldBeAssignableTo<GraphQLVariable>().Name.Value.ShouldBe("username");
selection.Arguments[1].Value.ShouldBeAssignableTo<GraphQLScalarValue>().Value.ShouldBe("Pete");
selection.Arguments[1].Value.ShouldBeAssignableTo<GraphQLStringValue>().Value.ShouldBe("Pete");
}
}
}
60 changes: 36 additions & 24 deletions src/GraphQLParser.Tests/ParserTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,13 +208,13 @@ public void Comments_on_Values_Should_Read_Correctly(IgnoreOptions options)
field.SelectionSet.Selections.Count.ShouldBe(1);
field.Arguments.Count.ShouldBe(9);

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

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

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

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

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

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

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

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

[Theory]
[InlineData("enum E { true A }", "Unexpected Name \"true\"; enum values are represented as unquoted names but not 'true' or 'false' or 'null'.", 1, 10)]
[InlineData("enum E { B false }", "Unexpected Name \"false\"; enum values are represented as unquoted names but not 'true' or 'false' or 'null'.", 1, 12)]
[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)]
public void Should_Throw_On_Invalid_EnumValue(string query, string description, int line, int column)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test failed before changes since EnumValueDefinition accepts true/false/null that contradicts spec.

{
var ex = Should.Throw<GraphQLSyntaxErrorException>(() => query.Parse());
ex.Description.ShouldBe(description);
ex.Line.ShouldBe(line);
ex.Column.ShouldBe(column);
}

[Theory]
[InlineData(IgnoreOptions.None)]
[InlineData(IgnoreOptions.Comments)]
Expand Down Expand Up @@ -825,7 +837,7 @@ public void Should_Parse_SpecifiedBy()
def.Directives[0].Name.Value.ShouldBe("specifiedBy");
def.Directives[0].Arguments.Count.ShouldBe(1);
def.Directives[0].Arguments[0].Name.Value.ShouldBe("url");
var value = def.Directives[0].Arguments[0].Value.ShouldBeAssignableTo<GraphQLScalarValue>();
var value = def.Directives[0].Arguments[0].Value.ShouldBeAssignableTo<GraphQLStringValue>();
value.Value.ShouldBe("https://tools.ietf.org/html/rfc4122");
}

Expand Down Expand Up @@ -936,23 +948,23 @@ public void Should_Parse_Extensions(string text, ASTNodeKind kind)
}

[Theory]
[InlineData("extend", "Unexpected EOF")]
[InlineData("extend scalar", "Expected Name, found EOF")]
[InlineData("extend scalar A", "Unexpected EOF")]
[InlineData("extend scalar A B", "Unexpected Name \"B\"")]
[InlineData("extend type", "Expected Name, found EOF")]
[InlineData("extend type A", "Unexpected EOF")]
[InlineData("extend type A B", "Unexpected Name \"B\"")]
[InlineData("extend interface", "Expected Name, found EOF")]
[InlineData("extend interface A", "Unexpected EOF")]
[InlineData("extend interface A B", "Unexpected Name \"B\"")]
[InlineData("extend union", "Expected Name, found EOF")]
[InlineData("extend union A", "Unexpected EOF")]
[InlineData("extend enum", "Expected Name, found EOF")]
[InlineData("extend enum A", "Unexpected EOF")]
[InlineData("extend input", "Expected Name, found EOF")]
[InlineData("extend input A", "Unexpected EOF")]
[InlineData("extend variable", "Unexpected Name \"variable\"")]
[InlineData("extend", "Unexpected EOF; for more information see http://spec.graphql.org/October2021/#TypeExtension")]
[InlineData("extend scalar", "Expected Name, found EOF; for more information see http://spec.graphql.org/October2021/#ScalarTypeExtension")]
[InlineData("extend scalar A", "Unexpected EOF; for more information see http://spec.graphql.org/October2021/#ScalarTypeExtension")]
[InlineData("extend scalar A B", "Unexpected Name \"B\"; for more information see http://spec.graphql.org/October2021/#ScalarTypeExtension")]
[InlineData("extend type", "Expected Name, found EOF; for more information see http://spec.graphql.org/October2021/#ObjectTypeExtension")]
[InlineData("extend type A", "Unexpected EOF; for more information see http://spec.graphql.org/October2021/#ObjectTypeExtension")]
[InlineData("extend type A B", "Unexpected Name \"B\"; for more information see http://spec.graphql.org/October2021/#ObjectTypeExtension")]
[InlineData("extend interface", "Expected Name, found EOF; for more information see http://spec.graphql.org/October2021/#InterfaceTypeExtension")]
[InlineData("extend interface A", "Unexpected EOF; for more information see http://spec.graphql.org/October2021/#InterfaceTypeExtension")]
[InlineData("extend interface A B", "Unexpected Name \"B\"; for more information see http://spec.graphql.org/October2021/#InterfaceTypeExtension")]
[InlineData("extend union", "Expected Name, found EOF; for more information see http://spec.graphql.org/October2021/#UnionTypeExtension")]
[InlineData("extend union A", "Unexpected EOF; for more information see http://spec.graphql.org/October2021/#UnionTypeExtension")]
[InlineData("extend enum", "Expected Name, found EOF; for more information see http://spec.graphql.org/October2021/#EnumTypeExtension")]
[InlineData("extend enum A", "Unexpected EOF; for more information see http://spec.graphql.org/October2021/#EnumTypeExtension")]
[InlineData("extend input", "Expected Name, found EOF; for more information see http://spec.graphql.org/October2021/#InputObjectTypeExtension")]
[InlineData("extend input A", "Unexpected EOF; for more information see http://spec.graphql.org/October2021/#InputObjectTypeExtension")]
[InlineData("extend variable", "Unexpected Name \"variable\"; for more information see http://spec.graphql.org/October2021/#TypeExtension")]
public void Should_Throw_Extensions(string text, string description)
{
var ex = Should.Throw<GraphQLSyntaxErrorException>(() => text.Parse());
Expand Down
16 changes: 8 additions & 8 deletions src/GraphQLParser.Tests/Validation/ParserValidationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ public void Parse_FragmentInvalidOnName_ThrowsExceptionWithCorrectMessage(Ignore
var exception = Should.Throw<GraphQLSyntaxErrorException>(() => "fragment on on on { on }".Parse(new ParserOptions { Ignore = options }));

exception.Message.ShouldBe(
"Syntax Error GraphQL (1:10) Unexpected Name \"on\"\n" +
"Syntax Error GraphQL (1:10) Unexpected Name \"on\"; fragment name can not be 'on'.\n" +
"1: fragment on on on { on }\n" +
" ^\n");
exception.Description.ShouldBe("Unexpected Name " + "\"on\"");
exception.Description.ShouldBe("Unexpected Name " + "\"on\"; fragment name can not be 'on'.");
exception.Line.ShouldBe(1);
exception.Column.ShouldBe(10);
}
Expand Down Expand Up @@ -52,10 +52,10 @@ public void Parse_InvalidFragmentNameInSpread_ThrowsExceptionWithCorrectMessage(
var exception = Should.Throw<GraphQLSyntaxErrorException>(() => "{ ...on }".Parse(new ParserOptions { Ignore = options }));

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

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

exception.Message.ShouldBe(
"Syntax Error GraphQL (1:10) Expected Name, found {\n" +
"Syntax Error GraphQL (1:10) Expected Name, found {; for more information see http://spec.graphql.org/October2021/#Field\n" +
"1: { field: {} }\n" +
" ^\n");
exception.Description.ShouldBe("Expected Name, found {");
exception.Description.ShouldBe("Expected Name, found {; for more information see http://spec.graphql.org/October2021/#Field");
exception.Line.ShouldBe(1);
exception.Column.ShouldBe(10);
}
Expand Down
20 changes: 10 additions & 10 deletions src/GraphQLParser.Tests/Visitors/GraphQLAstVisitorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ public class GraphQLAstVisitorTests
{
public class CountVisitor : DefaultNodeVisitor<CountContext>
{
public override async ValueTask VisitBooleanValue(GraphQLScalarValue booleanValue, CountContext context)
public override async ValueTask VisitBooleanValue(GraphQLBooleanValue booleanValue, CountContext context)
{
context.VisitedBooleanValues.Add(booleanValue);
await base.VisitBooleanValue(booleanValue, context);
}

public override async ValueTask VisitIntValue(GraphQLScalarValue intValue, CountContext context)
public override async ValueTask VisitIntValue(GraphQLIntValue intValue, CountContext context)
{
context.VisitedIntValues.Add(intValue);
await base.VisitIntValue(intValue, context);
Expand Down Expand Up @@ -55,13 +55,13 @@ public override async ValueTask VisitDirective(GraphQLDirective directive, Count
await base.VisitDirective(directive, context);
}

public override async ValueTask VisitEnumValue(GraphQLScalarValue enumValue, CountContext context)
public override async ValueTask VisitEnumValue(GraphQLEnumValue enumValue, CountContext context)
{
context.VisitedEnumValues.Add(enumValue);
await base.VisitEnumValue(enumValue, context);
}

public override async ValueTask VisitStringValue(GraphQLScalarValue stringValue, CountContext context)
public override async ValueTask VisitStringValue(GraphQLStringValue stringValue, CountContext context)
{
context.VisitedStringValues.Add(stringValue);
await base.VisitStringValue(stringValue, context);
Expand All @@ -81,7 +81,7 @@ public override async ValueTask VisitField(GraphQLField field, CountContext cont
await base.VisitField(field, context);
}

public override async ValueTask VisitFloatValue(GraphQLScalarValue floatValue, CountContext context)
public override async ValueTask VisitFloatValue(GraphQLFloatValue floatValue, CountContext context)
{
context.VisitedFloatValues.Add(floatValue);
await base.VisitFloatValue(floatValue, context);
Expand Down Expand Up @@ -192,19 +192,19 @@ public class CountContext : INodeVisitorContext
public List<GraphQLArgument> VisitedArguments = new();
public List<ASTNode> VisitedDefinitions = new();
public List<GraphQLDirective> VisitedDirectives = new();
public List<GraphQLScalarValue> VisitedEnumValues = new();
public List<GraphQLEnumValue> VisitedEnumValues = new();
public List<GraphQLField> VisitedFields = new();
public List<GraphQLScalarValue> VisitedFloatValues = new();
public List<GraphQLFloatValue> VisitedFloatValues = new();
public List<GraphQLFragmentDefinition> VisitedFragmentDefinitions = new();
public List<GraphQLFragmentSpread> VisitedFragmentSpreads = new();
public List<GraphQLTypeCondition> VisitedFragmentTypeConditions = new();
public List<GraphQLInlineFragment> VisitedInlineFragments = new();
public List<GraphQLScalarValue> VisitedIntValues = new();
public List<GraphQLIntValue> VisitedIntValues = new();
public List<GraphQLName> VisitedNames = new();
public List<GraphQLSelectionSet> VisitedSelectionSets = new();
public List<GraphQLScalarValue> VisitedStringValues = new();
public List<GraphQLStringValue> VisitedStringValues = new();
public List<GraphQLVariable> VisitedVariables = new();
public List<GraphQLScalarValue> VisitedBooleanValues = new();
public List<GraphQLBooleanValue> VisitedBooleanValues = new();

public CancellationToken CancellationToken { get; set; }
}
Expand Down
56 changes: 56 additions & 0 deletions src/GraphQLParser/AST/GraphQLBooleanValue.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
namespace GraphQLParser.AST
{
/// <summary>
/// AST node for <see cref="ASTNodeKind.BooleanValue"/>.
/// </summary>
public class GraphQLBooleanValue : GraphQLValue
{
/// <inheritdoc/>
public override ASTNodeKind Kind => ASTNodeKind.BooleanValue;

/// <summary>
/// Value represented as <see cref="ROM"/>.
/// </summary>
public ROM Value { get; set; }
}

internal sealed class GraphQLBooleanValueWithLocation : GraphQLBooleanValue
{
private GraphQLLocation _location;

public override GraphQLLocation Location
{
get => _location;
set => _location = value;
}
}

internal sealed class GraphQLBooleanValueWithComment : GraphQLBooleanValue
{
private GraphQLComment? _comment;

public override GraphQLComment? Comment
{
get => _comment;
set => _comment = value;
}
}

internal sealed class GraphQLBooleanValueFull : GraphQLBooleanValue
{
private GraphQLLocation _location;
private GraphQLComment? _comment;

public override GraphQLLocation Location
{
get => _location;
set => _location = value;
}

public override GraphQLComment? Comment
{
get => _comment;
set => _comment = value;
}
}
}
54 changes: 54 additions & 0 deletions src/GraphQLParser/AST/GraphQLEnumValue.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
namespace GraphQLParser.AST
{
/// <summary>
/// AST node for <see cref="ASTNodeKind.EnumValue"/>.
/// </summary>
public class GraphQLEnumValue : GraphQLValue, INamedNode
{
/// <inheritdoc/>
public override ASTNodeKind Kind => ASTNodeKind.EnumValue;

/// <inheritdoc/>
public GraphQLName Name { get; set; } = null!;
}

internal sealed class GraphQLEnumValueWithLocation : GraphQLEnumValue
{
private GraphQLLocation _location;

public override GraphQLLocation Location
{
get => _location;
set => _location = value;
}
}

internal sealed class GraphQLEnumValueWithComment : GraphQLEnumValue
{
private GraphQLComment? _comment;

public override GraphQLComment? Comment
{
get => _comment;
set => _comment = value;
}
}

internal sealed class GraphQLEnumValueFull : GraphQLEnumValue
{
private GraphQLLocation _location;
private GraphQLComment? _comment;

public override GraphQLLocation Location
{
get => _location;
set => _location = value;
}

public override GraphQLComment? Comment
{
get => _comment;
set => _comment = value;
}
}
}
7 changes: 7 additions & 0 deletions src/GraphQLParser/AST/GraphQLEnumValueDefinition.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ public class GraphQLEnumValueDefinition : GraphQLTypeDefinition, IHasDirectivesN
/// <inheritdoc/>
public override ASTNodeKind Kind => ASTNodeKind.EnumValueDefinition;

/// <summary>
/// Enum value represented as a nested AST node. Alas, inherited
/// <see cref="GraphQLTypeDefinition.Name"/> property holds almost
/// the same data and should be set as well.
/// </summary>
public GraphQLEnumValue EnumValue { get; set; } = null!;

/// <inheritdoc/>
public GraphQLDirectives? Directives { get; set; }
}
Expand Down
1 change: 1 addition & 0 deletions src/GraphQLParser/AST/GraphQLExecutableDefinition.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ namespace GraphQLParser.AST
{
/// <summary>
/// Base AST node for <see cref="GraphQLOperationDefinition"/> and <see cref="GraphQLFragmentDefinition"/>.
/// <br/>
/// http://spec.graphql.org/October2021/#ExecutableDefinition
/// </summary>
public abstract class GraphQLExecutableDefinition : ASTNode, IHasDirectivesNode
Expand Down
Loading