Fix some issues with indentation (#313)

Author: sungam3r

src/GraphQLParser.Tests/Visitors/SDLPrinterFromParsedTextTests.cs

@@ -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,

src/GraphQLParser/Visitors/SDLPrinter.cs

@@ -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);
@@ -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);
@@ -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);
         }
     }
 
@@ -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);
         }
@@ -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)
@@ -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
@@ -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
@@ -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
@@ -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)
@@ -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>
@@ -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 (
@@ -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)
@@ -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
 {