Skip to content
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

Fix some issues with indentation #313

Merged
merged 1 commit into from
Apr 21, 2023
Merged

Fix some issues with indentation #313

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
43 changes: 43 additions & 0 deletions src/GraphQLParser.Tests/Visitors/SDLPrinterFromParsedTextTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -790,6 +790,49 @@ query Q {
}
}
}
""")]
[InlineData(55,
"""
fragment f
#comment
on Person { name }
""",
"""
fragment f
#comment
on Person {
name
}
""")]
[InlineData(56,
"""
type Person
#comment
implements Entity { name: String }
""",
"""
type Person
#comment
implements Entity {
name: String
}
""")]
[InlineData(57,
"""
type Person
#comment
implements Entity &
#comment
Entity2 { name: String }
""",
"""
type Person
#comment
implements Entity &
#comment
Entity2 {
name: String
}
""")]
public async Task SDLPrinter_Should_Print_Document(
int number,
Expand Down
44 changes: 19 additions & 25 deletions src/GraphQLParser/Visitors/SDLPrinter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ protected override async ValueTask VisitFragmentDefinitionAsync(GraphQLFragmentD
await VisitAsync(fragmentDefinition.Comments, context).ConfigureAwait(false);
await VisitAsync(LiteralNode.Wrap("fragment "), context).ConfigureAwait(false);
await VisitAsync(fragmentDefinition.FragmentName, context).ConfigureAwait(false);
await context.WriteAsync(" ").ConfigureAwait(false);
await VisitAsync(fragmentDefinition.TypeCondition, context).ConfigureAwait(false);
await VisitAsync(fragmentDefinition.Directives, context).ConfigureAwait(false);
await VisitAsync(fragmentDefinition.SelectionSet, context).ConfigureAwait(false);
Expand All @@ -177,7 +176,7 @@ protected override async ValueTask VisitFragmentSpreadAsync(GraphQLFragmentSprea
protected override async ValueTask VisitInlineFragmentAsync(GraphQLInlineFragment inlineFragment, TContext context)
{
await VisitAsync(inlineFragment.Comments, context).ConfigureAwait(false);
await VisitAsync(LiteralNode.Wrap("... "), context).ConfigureAwait(false);
await VisitAsync(LiteralNode.Wrap("..."), context).ConfigureAwait(false);
await VisitAsync(inlineFragment.TypeCondition, context).ConfigureAwait(false);
await VisitAsync(inlineFragment.Directives, context).ConfigureAwait(false);
await VisitAsync(inlineFragment.SelectionSet, context).ConfigureAwait(false);
Expand All @@ -187,21 +186,21 @@ protected override async ValueTask VisitInlineFragmentAsync(GraphQLInlineFragmen
protected override async ValueTask VisitTypeConditionAsync(GraphQLTypeCondition typeCondition, TContext context)
{
await VisitAsync(typeCondition.Comments, context).ConfigureAwait(false);
await context.WriteAsync("on ").ConfigureAwait(false);
await VisitAsync(LiteralNode.Wrap(HasPrintableComments(typeCondition) ? "on " : " on "), context).ConfigureAwait(false);
await VisitAsync(typeCondition.Type, context).ConfigureAwait(false);
}

/// <inheritdoc/>
protected override async ValueTask VisitImplementsInterfacesAsync(GraphQLImplementsInterfaces implementsInterfaces, TContext context)
{
await VisitAsync(implementsInterfaces.Comments, context).ConfigureAwait(false);
await context.WriteAsync(" implements ").ConfigureAwait(false);
await VisitAsync(LiteralNode.Wrap(HasPrintableComments(implementsInterfaces) ? "implements " : " implements "), context).ConfigureAwait(false);

for (int i = 0; i < implementsInterfaces.Items.Count; ++i)
{
await VisitAsync(implementsInterfaces.Items[i], context).ConfigureAwait(false);
if (i < implementsInterfaces.Items.Count - 1)
await context.WriteAsync(" & ").ConfigureAwait(false);
await context.WriteAsync(HasPrintableComments(implementsInterfaces[i + 1]) ? " &" : " & ").ConfigureAwait(false);
}
}

Expand All @@ -210,9 +209,8 @@ protected override async ValueTask VisitSelectionSetAsync(GraphQLSelectionSet se
{
await VisitAsync(selectionSet.Comments, context).ConfigureAwait(false);

bool freshLine = selectionSet.Comments != null && Options.PrintComments;
bool hasParent = TryPeekParent(context, out var parent);
if (!freshLine && hasParent && (parent is GraphQLOperationDefinition op && op.Name is not null || parent is not GraphQLOperationDefinition))
if (!HasPrintableComments(selectionSet) && hasParent && (parent is GraphQLOperationDefinition op && op.Name is not null || parent is not GraphQLOperationDefinition))
{
await context.WriteAsync(" {").ConfigureAwait(false);
}
Expand Down Expand Up @@ -385,8 +383,7 @@ protected override async ValueTask VisitSchemaExtensionAsync(GraphQLSchemaExtens
//TODO: https://github.com/graphql/graphql-spec/issues/921
if (schemaExtension.OperationTypes?.Count > 0)
{
//bool freshLine = schemaExtension.Comments != null && Options.PrintComments; always false
await context.WriteAsync(/*freshLine ? "{" :*/" {").ConfigureAwait(false);
await context.WriteAsync(/*HasPrintableComments(schemaExtension) ? "{" :*/" {").ConfigureAwait(false); // always false
await context.WriteLineAsync().ConfigureAwait(false);

for (int i = 0; i < schemaExtension.OperationTypes.Count; ++i)
Expand Down Expand Up @@ -442,9 +439,7 @@ protected override async ValueTask VisitEnumValueDefinitionAsync(GraphQLEnumValu
protected override async ValueTask VisitEnumValuesDefinitionAsync(GraphQLEnumValuesDefinition enumValuesDefinition, TContext context)
{
await VisitAsync(enumValuesDefinition.Comments, context).ConfigureAwait(false);

bool freshLine = enumValuesDefinition.Comments != null && Options.PrintComments;
await VisitAsync(LiteralNode.Wrap(freshLine ? "{" : " {"), context).ConfigureAwait(false);
await VisitAsync(LiteralNode.Wrap(HasPrintableComments(enumValuesDefinition) ? "{" : " {"), context).ConfigureAwait(false);
await context.WriteLineAsync().ConfigureAwait(false);

if (enumValuesDefinition.Items?.Count > 0) // should always be true but may be negligently uninitialized
Expand Down Expand Up @@ -506,9 +501,7 @@ protected override async ValueTask VisitInputValueDefinitionAsync(GraphQLInputVa
protected override async ValueTask VisitInputFieldsDefinitionAsync(GraphQLInputFieldsDefinition inputFieldsDefinition, TContext context)
{
await VisitAsync(inputFieldsDefinition.Comments, context).ConfigureAwait(false);

bool freshLine = inputFieldsDefinition.Comments != null && Options.PrintComments;
await VisitAsync(LiteralNode.Wrap(freshLine ? "{" : " {"), context).ConfigureAwait(false);
await VisitAsync(LiteralNode.Wrap(HasPrintableComments(inputFieldsDefinition) ? "{" : " {"), context).ConfigureAwait(false);
await context.WriteLineAsync().ConfigureAwait(false);

if (inputFieldsDefinition.Items?.Count > 0) // should always be true but may be negligently uninitialized
Expand Down Expand Up @@ -585,9 +578,7 @@ protected override async ValueTask VisitFieldDefinitionAsync(GraphQLFieldDefinit
protected override async ValueTask VisitFieldsDefinitionAsync(GraphQLFieldsDefinition fieldsDefinition, TContext context)
{
await VisitAsync(fieldsDefinition.Comments, context).ConfigureAwait(false);

bool freshLine = fieldsDefinition.Comments != null && Options.PrintComments;
await VisitAsync(LiteralNode.Wrap(freshLine ? "{" : " {"), context).ConfigureAwait(false);
await VisitAsync(LiteralNode.Wrap(HasPrintableComments(fieldsDefinition) ? "{" : " {"), context).ConfigureAwait(false);
await context.WriteLineAsync().ConfigureAwait(false);

if (fieldsDefinition.Items?.Count > 0) // should always be true but may be negligently uninitialized
Expand All @@ -609,9 +600,7 @@ protected override async ValueTask VisitSchemaDefinitionAsync(GraphQLSchemaDefin
await VisitAsync(schemaDefinition.Description, context).ConfigureAwait(false);
await VisitAsync(LiteralNode.Wrap("schema"), context).ConfigureAwait(false);
await VisitAsync(schemaDefinition.Directives, context).ConfigureAwait(false);

//bool freshLine = schemaDefinition.Comments != null && Options.PrintComments; always false
await context.WriteAsync(/*freshLine? "{" : */" {").ConfigureAwait(false);
await context.WriteAsync(/*HasPrintableComments(schemaDefinition) ? "{" : */" {").ConfigureAwait(false); // always false
await context.WriteLineAsync().ConfigureAwait(false);

if (schemaDefinition.OperationTypes.Count > 0)
Expand Down Expand Up @@ -839,9 +828,10 @@ protected override async ValueTask VisitObjectFieldAsync(GraphQLObjectField obje
}

/// <inheritdoc/>
protected override ValueTask VisitNamedTypeAsync(GraphQLNamedType namedType, TContext context)
protected override async ValueTask VisitNamedTypeAsync(GraphQLNamedType namedType, TContext context)
{
return base.VisitNamedTypeAsync(namedType, context);
await VisitAsync(namedType.Comments, context).ConfigureAwait(false);
await VisitAsync(namedType.Name, context).ConfigureAwait(false);
}

/// <summary>
Expand Down Expand Up @@ -878,7 +868,7 @@ node is GraphQLEnumValueDefinition ||
node is GraphQLRootOperationTypeDefinition ||
node is GraphQLDirectiveLocations && Options.EachDirectiveLocationOnNewLine ||
node is GraphQLUnionMemberTypes && Options.EachUnionMemberOnNewLine ||
node is GraphQLArgumentsDefinition && node.Comments?.Count > 0
node is GraphQLArgumentsDefinition && HasPrintableComments(node)
)
context.IndentLevel = 1; // fixed indentation of 1
else if (
Expand Down Expand Up @@ -974,6 +964,8 @@ private async ValueTask WriteIndentAsync(TContext context)
context.IndentPrinted = true;
}

private bool HasPrintableComments(ASTNode node) => node.Comments?.Count > 0 && Options.PrintComments;

// Returns parent if called inside VisitXXX i.e. after context.Parents.Push(node);
// Returns grand-parent if called inside VisitAsync i.e. before context.Parents.Push(node);
private static bool TryPeekParent(TContext context, [NotNullWhen(true)] out ASTNode? node)
Expand Down Expand Up @@ -1139,7 +1131,9 @@ public class SDLPrinterOptions
}

/// <summary>
/// Preudo AST node to allow calls to <see cref="SDLPrinter{TContext}.VisitAsync(ASTNode?, TContext)"/> for indentation purposes.
/// Preudo AST node to allow calls to <see cref="SDLPrinter{TContext}.VisitAsync(ASTNode?, TContext)"/>
/// for indentation purposes. Any literal printed first after optional comment or description nodes in
/// any VisitXXX method should be wrapped into <see cref="LiteralNode"/> for proper indentation.
/// </summary>
internal class LiteralNode : ASTNode
{
Expand Down