Empty exception messages when command has missing required options (#115)

Matthiee · web-flow · commit b3a65e806bc9 · 2021-01-04T17:57:33.000+01:00
* Write exception message correctly in case of CommandParseException. Fixes #114 * Update exception message to use reason instead of because * Extend interfaces so there is a difference between a unprocessed and unused argument. * Add tests for #114 * Improve when printing errors and usage occures Actually fixes #114 * Codefactor: The code must not contain multiple blank lines in a row.
diff --git a/CommandLineParser.Tests/Exceptions/ExceptionsTest.cs b/CommandLineParser.Tests/Exceptions/ExceptionsTest.cs
@@ -1,6 +1,11 @@
 ﻿using MatthiWare.CommandLine.Abstractions.Command;
+using MatthiWare.CommandLine.Abstractions.Usage;
 using MatthiWare.CommandLine.Core.Attributes;
 using MatthiWare.CommandLine.Core.Exceptions;
+using Microsoft.Extensions.DependencyInjection;
+using Moq;
+using System;
+using System.Collections.Generic;
 using System.Linq;
 using System.Threading.Tasks;
 using Xunit;
@@ -130,6 +135,67 @@ public async Task CommandParseExceptionTestAsync()
             Assert.Same(parser.Commands.First(), result.Errors.Cast<CommandParseException>().First().Command);
         }
 
+        [Fact]
+        public async Task CommandParseException_Prints_Errors()
+        {
+            var printerMock = new Mock<IUsagePrinter>();
+
+            Services.AddSingleton(printerMock.Object);
+
+            var parser = new CommandLineParser<OtherOptions>(Services);
+
+            parser.AddCommand<Options>()
+                .Name("missing")
+                .Required()
+                .Configure(opt => opt.MissingOption)
+                .Name("o")
+                .Required();
+
+            var result = await parser.ParseAsync(new string[] { "-a", "1", "-b", "2", "-a", "10" ,"20" ,"30", "missing" });
+
+            printerMock.Verify(_ => _.PrintErrors(It.IsAny<IReadOnlyCollection<Exception>>()));
+        }
+
+        [Fact]
+        public void CommandParseException_Should_Contain_Correct_Message_Single()
+        {
+            var cmdMock = new Mock<ICommandLineCommand>();
+            cmdMock.SetupGet(_ => _.Name).Returns("test");
+
+            var exceptionList = new List<Exception>
+            {
+                new Exception("msg1")
+            };
+
+            var parseException = new CommandParseException(cmdMock.Object, exceptionList.AsReadOnly());
+            var msg = parseException.Message;
+            var expected = @"Unable to parse command 'test' reason: msg1";
+
+            Assert.Equal(expected, msg);
+        }
+
+        [Fact]
+        public void CommandParseException_Should_Contain_Correct_Message_Multiple()
+        {
+            var cmdMock = new Mock<ICommandLineCommand>();
+            cmdMock.SetupGet(_ => _.Name).Returns("test");
+
+            var exceptionList = new List<Exception>
+            {
+                new Exception("msg1"),
+                new Exception("msg2")
+            };
+
+            var parseException = new CommandParseException(cmdMock.Object, exceptionList.AsReadOnly());
+            var msg = parseException.Message;
+            var expected = @"Unable to parse command 'test' because 2 errors occured
+ - msg1
+ - msg2
+";
+
+            Assert.Equal(expected, msg);
+        }
+
         [Fact]
         public void OptionParseExceptionTest()
         {
@@ -158,6 +224,15 @@ public async Task OptionParseExceptionTestAsync()
             Assert.Same(parser.Options.First(), result.Errors.Cast<OptionParseException>().First().Option);
         }
 
+        private class OtherOptions
+        {
+            [Required, Name("a")]
+            public int A { get; set; }
+
+            [Required, Name("b")]
+            public int B { get; set; }
+        }
+
         private class Options
         {
             [Required, Name("m", "missing")]
diff --git a/CommandLineParser.Tests/Usage/DamerauLevenshteinTests.cs b/CommandLineParser.Tests/Usage/DamerauLevenshteinTests.cs
@@ -1,6 +1,8 @@
 ﻿using MatthiWare.CommandLine.Abstractions;
 using MatthiWare.CommandLine.Abstractions.Command;
+using MatthiWare.CommandLine.Abstractions.Usage;
 using MatthiWare.CommandLine.Core.Usage;
+using Microsoft.Extensions.DependencyInjection;
 using Moq;
 using System.Collections.Generic;
 using Xunit;
@@ -107,15 +109,35 @@ public void NoSuggestionsReturnsEmpty()
         }
 
         [Fact]
-        public void Test()
+        public void ShouldPrintSuggestion()
         {
+            var builderMock = new Mock<IUsageBuilder>();
+            Services.AddSingleton(builderMock.Object);
+
             var parser = new CommandLineParser(Services);
 
             parser.AddCommand().Name("cmd");
 
             var result = parser.Parse(new[] { "cmdd" });
 
             result.AssertNoErrors();
+            builderMock.Verify(_ => _.AddSuggestion("cmd"), Times.Once());
+        }
+
+        [Fact]
+        public void ShouldNotPrintSuggestionForSkippedArguments()
+        {
+            var builderMock = new Mock<IUsageBuilder>();
+            Services.AddSingleton(builderMock.Object);
+
+            var parser = new CommandLineParser(new CommandLineParserOptions { StopParsingAfter = "--" }, Services);
+
+            parser.AddCommand().Name("cmd");
+
+            var result = parser.Parse(new[] { "--", "cmdd" });
+
+            result.AssertNoErrors();
+            builderMock.Verify(_ => _.AddSuggestion("cmd"), Times.Never());
         }
     }
 }
diff --git a/CommandLineParser.Tests/Usage/UsagePrinterTests.cs b/CommandLineParser.Tests/Usage/UsagePrinterTests.cs
@@ -9,6 +9,7 @@
 using Microsoft.Extensions.DependencyInjection;
 using Moq;
 using System;
+using System.Threading.Tasks;
 using Xunit;
 using Xunit.Abstractions;
 
@@ -67,19 +68,39 @@ private class UsagePrinterCommandOptions
         [InlineData(new string[] { }, true)]
         [InlineData(new string[] { "-o", "bla" }, false)]
         [InlineData(new string[] { "-xd", "bla" }, true)]
+        [InlineData(new string[] { "--", "test" }, true)]
         public void UsagePrintGetsCalledInCorrectCases(string[] args, bool called)
         {
             var printerMock = new Mock<IUsagePrinter>();
 
             Services.AddSingleton(printerMock.Object);
 
-            var parser = new CommandLineParser<UsagePrinterGetsCalledOptions>(Services);
+            var parser = new CommandLineParser<UsagePrinterGetsCalledOptions>(new CommandLineParserOptions { StopParsingAfter = "--" }, Services);
 
             parser.Parse(args);
 
             printerMock.Verify(mock => mock.PrintUsage(), called ? Times.Once() : Times.Never());
         }
 
+        [Theory]
+        [InlineData(new string[] { "--", "get-al" }, false)]
+        [InlineData(new string[] { "--", "get-all" }, false)]
+        public async Task PrintUsage_ShouldBeCalled_When_Command_Is_Defined_After_StopParsingFlag(string[] args, bool _)
+        {
+            var printerMock = new Mock<IUsagePrinter>();
+
+            Services.AddSingleton(printerMock.Object);
+
+            var parser = new CommandLineParser(new CommandLineParserOptions { StopParsingAfter = "--" }, Services);
+
+            parser.AddCommand().Name("get-all");
+
+            await parser.ParseAsync(args);
+
+            printerMock.Verify(mock => mock.PrintUsage(), Times.Once());
+            printerMock.Verify(mock => mock.PrintSuggestion(It.IsAny<UnusedArgumentModel>()), Times.Never());
+        }
+
         [Fact]
         public void UsagePrinterPrintsOptionCorrectly()
         {
diff --git a/CommandLineParser/Abstractions/Parsing/IArgumentManager.cs b/CommandLineParser/Abstractions/Parsing/IArgumentManager.cs
@@ -1,4 +1,5 @@
 ﻿using MatthiWare.CommandLine.Abstractions.Models;
+using MatthiWare.CommandLine.Abstractions.Usage;
 using System;
 using System.Collections.Generic;
 
@@ -10,10 +11,22 @@ namespace MatthiWare.CommandLine.Abstractions.Parsing
     public interface IArgumentManager
     {
         /// <summary>
-        /// Returns a read-only list of unused arguments
+        /// Returns a read-only list of arguments that never got processed because they appeared after the <see cref="CommandLineParserOptions.StopParsingAfter"/> flag.
+        /// </summary>
+        IReadOnlyList<UnusedArgumentModel> UnprocessedArguments { get; }
+
+        /// <summary>
+        /// Returns a read-only list of unused arguments. 
+        /// In most cases this will be mistyped arguments that are not mapped to the actual option/command names.
+        /// You can pass these arguments inside the <see cref="IUsagePrinter.PrintSuggestion(UnusedArgumentModel)"/> to get a suggestion of what could be the correct argument.
         /// </summary>
         IReadOnlyList<UnusedArgumentModel> UnusedArguments { get; }
 
+        /// <summary>
+        /// Returns if the <see cref="CommandLineParserOptions.StopParsingAfter"/> flag was found. 
+        /// </summary>
+        bool StopParsingFlagSpecified { get; }
+
         /// <summary>
         /// Tries to get the arguments associated to the current option
         /// </summary>
diff --git a/CommandLineParser/Abstractions/Usage/IUsagePrinter.cs b/CommandLineParser/Abstractions/Usage/IUsagePrinter.cs
@@ -25,8 +25,8 @@ public interface IUsagePrinter
         /// Print an argument
         /// </summary>
         /// <param name="argument">The given argument</param>
-        [EditorBrowsable(EditorBrowsableState.Never)]
         [Obsolete("Use PrintCommandUsage or PrintOptionUsage instead")]
+        [EditorBrowsable(EditorBrowsableState.Never)]
         void PrintUsage(IArgument argument);
 
         /// <summary>
@@ -63,6 +63,11 @@ public interface IUsagePrinter
         /// <param name="errors">list of errors</param>
         void PrintErrors(IReadOnlyCollection<Exception> errors);
 
-        void PrintSuggestion(UnusedArgumentModel model);
+        /// <summary>
+        /// Prints suggestions based on the input arguments
+        /// </summary>
+        /// <param name="model">Input model</param>
+        /// <returns>True if a suggestion was found and printed, otherwise false</returns>
+        bool PrintSuggestion(UnusedArgumentModel model);
     }
 }
diff --git a/CommandLineParser/CommandLineParser.xml b/CommandLineParser/CommandLineParser.xml
diff --git a/CommandLineParser/CommandLineParser`TOption.cs b/CommandLineParser/CommandLineParser`TOption.cs
@@ -214,11 +214,24 @@ public async Task<IParserResult<TOption>> ParseAsync(string[] args, Cancellation
 
             await AutoExecuteCommandsAsync(result, cancellationToken);
 
-            AutoPrintUsageAndErrors(result, args.Length == 0);
+            AutoPrintUsageAndErrors(result, NoActualArgsSupplied(args.Length));
 
             return result;
         }
 
+        private bool NoActualArgsSupplied(int argsCount)
+        {
+            int leftOverAmountOfArguments = argsCount;
+
+            if (argumentManager.StopParsingFlagSpecified)
+            {
+                leftOverAmountOfArguments--; // the flag itself
+                leftOverAmountOfArguments -= argumentManager.UnprocessedArguments.Count;
+            }
+
+            return leftOverAmountOfArguments == 0;
+        }
+
         private async Task ValidateAsync<T>(T @object, ParseResult<TOption> parseResult, List<Exception> errors, CancellationToken token)
         {
             if (parseResult.HelpRequested)
@@ -268,6 +281,10 @@ private void AutoPrintUsageAndErrors(ParseResult<TOption> result, bool noArgsSup
             {
                 PrintHelpRequestedForArgument(result.HelpRequestedFor);
             }
+            else if (!noArgsSupplied && result.HasErrors && result.Errors.Any(e => e is CommandParseException || e is OptionParseException))
+            {
+                PrintErrors(result.Errors);
+            }
             else if (!noArgsSupplied && argumentManager.UnusedArguments.Count > 0)
             {
                 PrintHelp();
@@ -303,7 +320,16 @@ private void PrintErrors(IReadOnlyCollection<Exception> errors)
 
         private void PrintHelp() => Printer.PrintUsage();
 
-        private void PrintSuggestionsIfAny() => Printer.PrintSuggestion(argumentManager.UnusedArguments.First());
+        private void PrintSuggestionsIfAny()
+        {
+            foreach (var argument in argumentManager.UnusedArguments)
+            {
+                if (Printer.PrintSuggestion(argument))
+                {
+                    return;
+                }
+            }
+        }
 
         private async Task AutoExecuteCommandsAsync(ParseResult<TOption> result, CancellationToken cancellationToken)
         {
diff --git a/CommandLineParser/Core/Exceptions/CommandParseException.cs b/CommandLineParser/Core/Exceptions/CommandParseException.cs
@@ -1,5 +1,7 @@
 ﻿using System;
 using System.Collections.Generic;
+using System.Linq;
+using System.Text;
 using MatthiWare.CommandLine.Abstractions.Command;
 
 namespace MatthiWare.CommandLine.Core.Exceptions
@@ -20,7 +22,35 @@ public class CommandParseException : BaseParserException
         /// <param name="command">the failed command</param>
         /// <param name="innerExceptions">collection of inner exception</param>
         public CommandParseException(ICommandLineCommand command, IReadOnlyCollection<Exception> innerExceptions)
-            : base(command, "", new AggregateException(innerExceptions))
+            : base(command, CreateMessage(command, innerExceptions), new AggregateException(innerExceptions))
         { }
+
+        private static string CreateMessage(ICommandLineCommand command, IReadOnlyCollection<Exception> exceptions)
+        {
+            if (exceptions.Count > 1)
+            {
+                return CreateMultipleExceptionsMessage(command, exceptions);
+            }
+            else
+            {
+                return CreateSingleExceptionMessage(command, exceptions.First());
+            }
+        }
+
+        private static string CreateSingleExceptionMessage(ICommandLineCommand command, Exception exception)
+            => $"Unable to parse command '{command.Name}' reason: {exception.Message}";
+
+        private static string CreateMultipleExceptionsMessage(ICommandLineCommand command, IReadOnlyCollection<Exception> exceptions)
+        {
+            var message = new StringBuilder();
+            message.AppendLine($"Unable to parse command '{command.Name}' because {exceptions.Count} errors occured");
+
+            foreach (var exception in exceptions)
+            {
+                message.AppendLine($" - {exception.Message}");
+            }
+
+            return message.ToString();
+        }
     }
 }
diff --git a/CommandLineParser/Core/Parsing/ArgumentManager.cs b/CommandLineParser/Core/Parsing/ArgumentManager.cs
diff --git a/CommandLineParser/Core/Usage/UsagePrinter.cs b/CommandLineParser/Core/Usage/UsagePrinter.cs
