merge ParseDirectiveAction and ParseResultExtensions into one internal type (#2125)

adamsitnik · web-flow · commit 4ba4a2829e84 · 2023-03-29T17:01:50.000+02:00
make ParseDirectiveAction.InvokeAsync actually async: use the new WriteLine(StringBuilder, CancellationToken) overload
diff --git a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt
@@ -258,8 +258,6 @@ System.CommandLine.Parsing
     public static System.CommandLine.ParseResult Parse(System.CommandLine.Command command, System.Collections.Generic.IReadOnlyList<System.String> args, System.CommandLine.CommandLineConfiguration configuration = null)
     public static System.CommandLine.ParseResult Parse(System.CommandLine.Command command, System.String commandLine, System.CommandLine.CommandLineConfiguration configuration = null)
     public static System.Collections.Generic.IEnumerable<System.String> SplitCommandLine(System.String commandLine)
-  public static class ParseResultExtensions
-    public static System.String Diagram(this System.CommandLine.ParseResult parseResult)
   public abstract class SymbolResult
     public SymbolResult Parent { get; }
     public System.Collections.Generic.IReadOnlyList<Token> Tokens { get; }
diff --git a/src/System.CommandLine.Benchmarks/CommandLine/Perf_Parser_ParseResult.cs b/src/System.CommandLine.Benchmarks/CommandLine/Perf_Parser_ParseResult.cs
@@ -4,7 +4,10 @@
 using System.Collections.Generic;
 using System.CommandLine.Benchmarks.Helpers;
 using System.CommandLine.Parsing;
+using System.IO;
 using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
 using BenchmarkDotNet.Attributes;
 
 namespace System.CommandLine.Benchmarks.CommandLine
@@ -16,14 +19,17 @@ namespace System.CommandLine.Benchmarks.CommandLine
     public class Perf_Parser_ParseResult
     {
         private readonly CommandLineConfiguration _configuration;
+        private readonly StringWriter _output;
 
         public Perf_Parser_ParseResult()
         {
+            _output = new StringWriter();
             var option = new Option<bool>("-opt");
 
             _configuration = new CommandLineConfiguration(new RootCommand { option })
             {
-                Directives = { new ParseDirective() }
+                Directives = { new ParseDirective() },
+                Output = _output
             };
         }
 
@@ -49,6 +55,29 @@ public ParseResult ParseResult_Directives(string input)
         [Benchmark]
         [ArgumentsSource(nameof(GenerateTestParseResults))]
         public string ParseResult_Diagram(BdnParam<ParseResult> parseResult)
-            => parseResult.Value.Diagram();
+        {
+            StringBuilder stringBuilder = _output.GetStringBuilder();
+
+            // clear the contents, so each benchmark has the same starting state
+            stringBuilder.Clear();
+
+            parseResult.Value.Action!.Invoke(parseResult.Value);
+
+            return stringBuilder.ToString();
+        }
+
+        [Benchmark]
+        [ArgumentsSource(nameof(GenerateTestParseResults))]
+        public async Task<string> ParseResult_DiagramAsync(BdnParam<ParseResult> parseResult)
+        {
+            StringBuilder stringBuilder = _output.GetStringBuilder();
+
+            // clear the contents, so each benchmark has the same starting state
+            stringBuilder.Clear();
+
+            await parseResult.Value.Action!.InvokeAsync(parseResult.Value);
+
+            return stringBuilder.ToString();
+        }
     }
 }
diff --git a/src/System.CommandLine.Rendering.Tests/ViewRenderingTests.cs b/src/System.CommandLine.Rendering.Tests/ViewRenderingTests.cs
@@ -1,8 +1,6 @@
 ﻿// Copyright (c) .NET Foundation and contributors. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
-using System.CommandLine.IO;
-using System.CommandLine.Parsing;
 using System.CommandLine.Rendering.Views;
 using System.CommandLine.Tests.Utility;
 using System.Drawing;
@@ -35,7 +33,7 @@ public void Views_can_be_used_for_specific_types()
 
             config.Invoke("");
 
-            terminal.Out.ToString().Should().Contain(parseResult.Diagram());
+            terminal.Out.ToString().Should().Contain(parseResult.ToString());
         }
 
         [Theory]
diff --git a/src/System.CommandLine.Tests/Utility/ParseResultExtensions.cs b/src/System.CommandLine.Tests/Utility/ParseResultExtensions.cs
@@ -0,0 +1,25 @@
+﻿using System.IO;
+
+namespace System.CommandLine.Tests
+{
+    internal static class ParseResultExtensions
+    {
+        internal static string Diagram(this ParseResult parseResult)
+        {
+            TextWriter outputBefore = parseResult.Configuration.Output;
+
+            try
+            {
+                parseResult.Configuration.Output = new StringWriter();
+                new ParseDirective().Action.Invoke(parseResult);
+                return parseResult.Configuration.Output.ToString()
+                    .TrimEnd(); // the directive adds a new line, tests that used to rely on Diagram extension method don't expect it
+            }
+            finally
+            {
+                // some of the tests check the Output after getting the Diagram
+                parseResult.Configuration.Output = outputBefore;
+            }
+        }
+    }
+}
diff --git a/src/System.CommandLine/ParseDirective.cs b/src/System.CommandLine/ParseDirective.cs
@@ -1,11 +1,10 @@
 ﻿using System.CommandLine.Parsing;
-using System.Threading;
-using System.Threading.Tasks;
 
 namespace System.CommandLine
 {
     /// <summary>
-    /// Enables the use of the <c>[parse]</c> directive, which when specified on the command line will short circuit normal command handling and display a diagram explaining the parse result for the command line input.
+    /// Enables the use of the <c>[parse]</c> directive, which when specified on the command line will short 
+    /// circuit normal command handling and display a diagram explaining the parse result for the command line input.
     /// </summary>
     public sealed class ParseDirective : Directive
     {
@@ -27,23 +26,5 @@ public override CliAction? Action
             get => _action ??= new ParseDirectiveAction(DefaultErrorExitCode);
             set => _action = value ?? throw new ArgumentNullException(nameof(value));
         }
-
-        private sealed class ParseDirectiveAction : CliAction
-        {
-            private readonly int _errorExitCode;
-
-            internal ParseDirectiveAction(int errorExitCode) => _errorExitCode = errorExitCode;
-
-            public override int Invoke(ParseResult parseResult)
-            {
-                parseResult.Configuration.Output.WriteLine(parseResult.Diagram());
-                return parseResult.Errors.Count == 0 ? 0 : _errorExitCode;
-            }
-
-            public override Task<int> InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default)
-                => cancellationToken.IsCancellationRequested
-                    ? Task.FromCanceled<int>(cancellationToken)
-                    : Task.FromResult(Invoke(parseResult));
-        }
     }
 }
diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs
@@ -194,7 +194,7 @@ void Populate<TSymbol>(Dictionary<string, SymbolResult?> cache, IList<TSymbol> s
         }
 
         /// <inheritdoc />
-        public override string ToString() => $"{nameof(ParseResult)}: {this.Diagram()}";
+        public override string ToString() => ParseDirectiveAction.Diagram(this).ToString();
 
         /// <summary>
         /// Gets the result, if any, for the specified argument.
diff --git a/src/System.CommandLine/Parsing/ParseDirectiveAction.cs b/src/System.CommandLine/Parsing/ParseDirectiveAction.cs
@@ -5,24 +5,51 @@
 using System.CommandLine.Binding;
 using System.Linq;
 using System.Text;
+using System.Threading.Tasks;
+using System.Threading;
 
 namespace System.CommandLine.Parsing
 {
     /// <summary>
-    /// Provides extension methods for parse results.
+    /// Implements the <c>[parse]</c> directive action, which when specified on the command line 
+    /// will short circuit normal command handling and display a diagram explaining the parse result for the command line input.
     /// </summary>
-    public static class ParseResultExtensions
+    internal sealed class ParseDirectiveAction : CliAction
     {
+        private readonly int _errorExitCode;
+
+        internal ParseDirectiveAction(int errorExitCode) => _errorExitCode = errorExitCode;
+
+        public override int Invoke(ParseResult parseResult)
+        {
+            parseResult.Configuration.Output.WriteLine(Diagram(parseResult));
+            return parseResult.Errors.Count == 0 ? 0 : _errorExitCode;
+        }
+
+        public override async Task<int> InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default)
+        {
+            cancellationToken.ThrowIfCancellationRequested();
+
+            StringBuilder diagram = Diagram(parseResult);
+
+#if NET7_0_OR_GREATER
+            await parseResult.Configuration.Output.WriteLineAsync(diagram, cancellationToken);
+#else
+            await parseResult.Configuration.Output.WriteLineAsync(diagram.ToString());
+#endif
+            return parseResult.Errors.Count == 0 ? 0 : _errorExitCode;
+        }
+
         /// <summary>
         /// Formats a string explaining a parse result.
         /// </summary>
         /// <param name="parseResult">The parse result to be diagrammed.</param>
         /// <returns>A string containing a diagram of the parse result.</returns>
-        public static string Diagram(this ParseResult parseResult)
+        internal static StringBuilder Diagram(ParseResult parseResult)
         {
             var builder = new StringBuilder(100);
 
-            builder.Diagram(parseResult.RootCommandResult, parseResult);
+            Diagram(builder, parseResult.RootCommandResult, parseResult);
 
             var unmatchedTokens = parseResult.UnmatchedTokens;
             if (unmatchedTokens.Length > 0)
@@ -37,11 +64,11 @@ public static string Diagram(this ParseResult parseResult)
                 }
             }
 
-            return builder.ToString();
+            return builder;
         }
 
         private static void Diagram(
-            this StringBuilder builder,
+            StringBuilder builder,
             SymbolResult symbolResult,
             ParseResult parseResult)
         {
@@ -134,7 +161,6 @@ private static void Diagram(
                     {
                         builder.Append(((CommandResult)symbolResult).IdentifierToken.Value);
                     }
-                    
 
                     foreach (SymbolResult child in symbolResult.SymbolResultTree.GetChildren(symbolResult))
                     {
@@ -147,7 +173,7 @@ private static void Diagram(
 
                         builder.Append(" ");
 
-                        builder.Diagram(child, parseResult);
+                        Diagram(builder, child, parseResult);
                     }
 
                     builder.Append(" ]");

Original file line number	Diff line number	Diff line change
`@@ -194,7 +194,7 @@ void Populate<TSymbol>(Dictionary<string, SymbolResult?> cache, IList<TSymbol> s`
`194`	`194`	`}`
`195`	`195`
`196`	`196`	`/// <inheritdoc />`
`197`		`- public override string ToString() => $"{nameof(ParseResult)}: {this.Diagram()}";`
	`197`	`+ public override string ToString() => ParseDirectiveAction.Diagram(this).ToString();`
`198`	`198`
`199`	`199`	`/// <summary>`
`200`	`200`	`/// Gets the result, if any, for the specified argument.`