Fix optional bool option with 'null' value and DefaultValue configured is not parsed correctly (#39)

* #35 'null' value not respected when DefaultValue is configured.

* Fix string resolver always being able to parse

* Better exception message

* Only found options can throw OptionParseException

* Fix codefactor issue

* Split Parsing into seperate methods

* Extend parsing with defaults test

Author: Matthiee

CommandLineParser.Tests/CommandLineParserTests.cs

@@ -194,29 +194,72 @@ public void ParseEnumInArguments(string[] args, bool hasErrors, EnumOption enumO
         }
 
         [Theory]
-        [InlineData(new[] { "app.exe", "-1", "message1", "-2", "-3" }, "message1", "message2", "message3")]
-        [InlineData(new[] { "app.exe", "-1", "-2", "message2", "-3" }, "message1", "message2", "message3")]
-        [InlineData(new[] { "app.exe", "-1", "-2", "-3" }, "message1", "message2", "message3")]
-        [InlineData(new[] { "-1", "message1", "-2", "-3" }, "message1", "message2", "message3")]
-        [InlineData(new[] { "-1", "-2", "message2", "-3" }, "message1", "message2", "message3")]
-        [InlineData(new[] { "-1", "-2", "-3" }, "message1", "message2", "message3")]
-        public void ParseWithDefaults(string[] args, string result1, string result2, string result3)
+        // string
+        [InlineData(typeof(string), new[] { "-1", "message1", "-2", "-3" }, "default", "message1", "default", "default")]
+        [InlineData(typeof(string), new[] { "-1", "-2", "message2", "-3" }, "default", "default", "message2", "default")]
+        [InlineData(typeof(string), new[] { "-1", "-2", "-3" }, "default", "default", "default", "default")]
+        // bool
+        [InlineData(typeof(bool), new[] { "-1", "false", "-2", "-3" }, false, false, true, true)]
+        [InlineData(typeof(bool), new[] { "-1", "-2", "false", "-3" }, false, true, false, true)]
+        [InlineData(typeof(bool), new[] { "-1", "-2", "-3" }, false, true, true, true)]
+        //// int
+        [InlineData(typeof(int), new[] { "-1", "5", "-2", "-3" }, 0, 5, 0, 0)]
+        [InlineData(typeof(int), new[] { "-1", "-2", "5", "-3" }, 0, 0, 5, 0)]
+        [InlineData(typeof(int), new[] { "-1", "-2", "-3" }, 0, 0, 0, 0)]
+        // double
+        [InlineData(typeof(double), new[] { "-1", "0.5", "-2", "-3" }, 0.1, 0.5, 0.1, 0.1)]
+        [InlineData(typeof(double), new[] { "-1", "-2", "0.5", "-3" }, 0.1, 0.1, 0.5, 0.1)]
+        [InlineData(typeof(double), new[] { "-1", "-2", "-3" }, 0.1, 0.1, 0.1, 0.1)]
+        //// enum
+        [InlineData(typeof(EnumOption), new[] { "-1", "Opt1", "-2", "-3" }, EnumOption.Opt1, EnumOption.Opt1, "message2", "message3")]
+        [InlineData(typeof(EnumOption), new[] { "-1", "-2", "Opt1", "-3" }, EnumOption.Opt1, "message1", EnumOption.Opt1, "message3")]
+        [InlineData(typeof(EnumOption), new[] { "-1", "-2", "-3" }, EnumOption.Opt1, "message1", "message2", "message3")]
+        public void ParseWithDefaults(Type type, string[] args, object defaultValue, object result1, object result2, object result3)
         {
-            var parser = new CommandLineParser<OptionsWithThreeParams>();
+            if (type == typeof(bool))
+            {
+                Test<bool>();
+            }
+            else if (type == typeof(string))
+            {
+                Test<string>();
+            }
+            else if (type == typeof(int))
+            {
+                Test<int>();
+            }
+            else if (type == typeof(double))
+            {
+                Test<double>();
+            }
+            else if (type == typeof(Enum))
+            {
+                Test<Enum>();
+            }
+
+            void Test<T>()
+            {
+                TestParsingWithDefaults<T>(args, (T)defaultValue, (T)result1, (T)result2, (T)result3);
+            }
+        }
+
+        private void TestParsingWithDefaults<T>(string[] args, T defaultValue, T result1, T result2, T result3)
+        {
+            var parser = new CommandLineParser<OptionsWithThreeParams<T>>();
 
             parser.Configure(opt => opt.Option1)
                 .Name("1")
-                .Default(result1)
+                .Default(defaultValue)
                 .Required();
 
             parser.Configure(opt => opt.Option2)
                 .Name("2")
-                .Default(result2)
+                .Default(defaultValue)
                 .Required();
 
             parser.Configure(opt => opt.Option3)
                 .Name("3")
-                .Default(result3)
+                .Default(defaultValue)
                 .Required();
 
             var parsed = parser.Parse(args);
@@ -341,6 +384,32 @@ public void BoolResolverSpecialCaseParsesCorrectly(string[] args, bool expected)
             Assert.Equal(expected, result.Result.Option2);
         }
 
+        #region Issue_35_Bool_Option_Not_Parsed_Correctly
+
+        [Theory]
+        [InlineData(new string[] { "-v", "command" }, true)]
+        [InlineData(new string[] { "command", "-v" }, true)]
+        public void BoolResolverSpecialCaseParsesCorrectlyWithDefaultValueAndNotBeingSpecified(string[] args, bool expected)
+        {
+            var parser = new CommandLineParser<Model_Issue_35>();
+
+            parser.AddCommand().Name("command");
+
+            var result = parser.Parse(args);
+
+            result.AssertNoErrors();
+
+            Assert.Equal(expected, result.Result.VerbA);
+        }
+
+        private class Model_Issue_35
+        {
+            [Name("v", "verb"), Description("Print usage"), DefaultValue(false)]
+            public bool VerbA { get; set; }
+        }
+
+        #endregion
+
         [Fact]
         public void ConfigureTests()
         {
@@ -400,11 +469,11 @@ public class EnumOptions
             public EnumOption EnumOption { get; set; }
         }
 
-        private class OptionsWithThreeParams
+        private class OptionsWithThreeParams<T>
         {
-            public string Option1 { get; set; }
-            public string Option2 { get; set; }
-            public string Option3 { get; set; }
+            public T Option1 { get; set; }
+            public T Option2 { get; set; }
+            public T Option3 { get; set; }
         }
     }
 }

CommandLineParser/CommandLineParser`TOption.cs

@@ -330,13 +330,13 @@ private void ParseCommands(IList<Exception> errors, ParseResult<TOption> result,
 
                 var cmdParseResult = cmd.Parse(argumentManager);
 
+                if (result.HelpRequested)
+                    break;
+
                 if (cmdParseResult.HasErrors)
                     errors.Add(new CommandParseException(cmd, cmdParseResult.Errors));
 
                 result.MergeResult(cmdParseResult);
-
-                if (result.HelpRequested)
-                    break;
             }
         }
 
@@ -357,13 +357,14 @@ private void ParseOptions(IList<Exception> errors, ParseResult<TOption> result,
 
                     continue;
                 }
-                else if (!model.HasValue && option.HasDefault)
+                else if ((!found && !model.HasValue && option.HasDefault) ||
+                    (found && !option.CanParse(model) && option.HasDefault))
                 {
                     option.UseDefault();
 
                     continue;
                 }
-                else if (!option.CanParse(model))
+                else if (found && !option.CanParse(model))
                 {
                     errors.Add(new OptionParseException(option, model));
 

CommandLineParser/Core/Command/CommandLineCommand`TOption`TCommandOption.cs

@@ -98,29 +98,17 @@ public override ICommandParserResult Parse(IArgumentManager argumentManager)
             var result = new CommandParserResult(this);
             var errors = new List<Exception>();
 
-            foreach (var cmd in m_commands)
-            {
-                if (!argumentManager.TryGetValue(cmd, out ArgumentModel model))
-                {
-                    if (cmd.IsRequired)
-                        errors.Add(new CommandNotFoundException(cmd));
-
-                    result.MergeResult(new CommandNotFoundParserResult(cmd));
-
-                    continue;
-                }
-
-                var cmdParseResult = cmd.Parse(argumentManager);
+            ParseCommands(errors, result, argumentManager);
 
-                if (cmdParseResult.HelpRequested)
-                    return result;
+            ParseOptions(errors, result, argumentManager);
 
-                if (cmdParseResult.HasErrors)
-                    errors.Add(new CommandParseException(cmd, cmdParseResult.Errors));
+            result.MergeResult(errors);
 
-                result.MergeResult(cmdParseResult);
-            }
+            return result;
+        }
 
+        private void ParseOptions(IList<Exception> errors, CommandParserResult result, IArgumentManager argumentManager)
+        {
             foreach (var o in m_options)
             {
                 var option = o.Value;
@@ -136,13 +124,14 @@ public override ICommandParserResult Parse(IArgumentManager argumentManager)
 
                     continue;
                 }
-                else if (!model.HasValue && option.HasDefault)
+                else if ((!found && !model.HasValue && option.HasDefault) ||
+                    (found && !option.CanParse(model) && option.HasDefault))
                 {
                     option.UseDefault();
 
                     continue;
                 }
-                else if (!option.CanParse(model))
+                else if (found && !option.CanParse(model))
                 {
                     errors.Add(new OptionParseException(option, model));
 
@@ -151,10 +140,32 @@ public override ICommandParserResult Parse(IArgumentManager argumentManager)
 
                 option.Parse(model);
             }
+        }
 
-            result.MergeResult(errors);
+        private void ParseCommands(IList<Exception> errors, CommandParserResult result, IArgumentManager argumentManager)
+        {
+            foreach (var cmd in m_commands)
+            {
+                if (!argumentManager.TryGetValue(cmd, out ArgumentModel model))
+                {
+                    if (cmd.IsRequired)
+                        errors.Add(new CommandNotFoundException(cmd));
 
-            return result;
+                    result.MergeResult(new CommandNotFoundParserResult(cmd));
+
+                    continue;
+                }
+
+                var cmdParseResult = cmd.Parse(argumentManager);
+
+                if (cmdParseResult.HelpRequested)
+                    break;
+
+                if (cmdParseResult.HasErrors)
+                    errors.Add(new CommandParseException(cmd, cmdParseResult.Errors));
+
+                result.MergeResult(cmdParseResult);
+            }
         }
 
         private bool HelpRequested(CommandParserResult result, CommandLineOptionBase option, ArgumentModel model)

CommandLineParser/Core/Exceptions/OptionParseException.cs

@@ -31,7 +31,7 @@ private static string CreateMessage(ICommandLineOption option, ArgumentModel arg
             string shortName = hasShort ? option.ShortName : string.Empty;
             string longName = hasLong ? option.LongName : string.Empty;
 
-            return $"Unable to parse option {shortName}{hasBothSeperator}{longName} value '{argModel.Value}' is invalid!";
+            return $"Unable to parse option '{shortName}{hasBothSeperator}{longName}' value '{argModel.Value}' is invalid!";
         }
     }
 }

CommandLineParser/Core/Parsing/Resolvers/StringResolver.cs

@@ -5,7 +5,7 @@ namespace MatthiWare.CommandLine.Core.Parsing.Resolvers
 {
     internal class StringResolver : ArgumentResolver<string>
     {
-        public override bool CanResolve(ArgumentModel model) => true;
+        public override bool CanResolve(ArgumentModel model) => model.HasValue;
 
         public override string Resolve(ArgumentModel model) => model.HasValue ? model.Value : null;
     }