Skip to content

Commit b729b0b

Browse files
MatthieeMatthiee
authored
Required option with default value would cause error (#34)
* Add failing test case * Switch to extension method * Fix OptioNotFoundException message * Fix OptionNotFoundException pt2 * Fixes #30 default value was not taken into consideration when option was required. Error was thrown when it shouldn't.
1 parent daf751f commit b729b0b

File tree

6 files changed

+94
-42
lines changed

6 files changed

+94
-42
lines changed

‎CommandLineParser.Tests/CustomerReportedTests.cs‎

Lines changed: 57 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,22 @@
1-
using MatthiWare.CommandLine;
2-
1+
using System;
2+
using System.Collections.Generic;
3+
using MatthiWare.CommandLine;
4+
using MatthiWare.CommandLine.Core.Attributes;
35
using Xunit;
46

57
namespace MatthiWare.CommandLine.Tests
68
{
79
public class CustomerReportedTests
810
{
11+
#region Issue_12
912
/// <summary>
1013
/// Running with *no* parameters at all crashes the command line parser #12
1114
/// https://github.com/MatthiWare/CommandLineParser.Core/issues/12
1215
/// </summary>
1316
[Theory]
14-
[InlineData(true, true, false)]
17+
[InlineData(true, false, false)]
1518
[InlineData(false, false, false)]
16-
[InlineData(true, true, true)]
19+
[InlineData(true, false, true)]
1720
[InlineData(false, false, true)]
1821
public void NoCommandLineArgumentsCrashesParser_Issue_12(bool required, bool outcome, bool empty)
1922
{
@@ -35,5 +38,55 @@ private class OptionsModelIssue_12
3538
{
3639
public int Test { get; set; }
3740
}
41+
#endregion
42+
43+
#region Issue_30_AutoPrintUsageAndErrors
44+
/// <summary>
45+
/// AutoPrintUsageAndErrors always prints even when everything is *fine* #30
46+
/// https://github.com/MatthiWare/CommandLineParser.Core/issues/30
47+
/// </summary>
48+
[Theory]
49+
[InlineData(null, null)]
50+
[InlineData("true", null)]
51+
[InlineData("false", null)]
52+
[InlineData(null, "bla")]
53+
public void AutoPrintUsageAndErrorsShouldNotPrintWhenEverythingIsFIne(string verbose, string path)
54+
{
55+
var parser = new CommandLineParser<OptionsModelIssue_30>();
56+
57+
var items = new List<string>();
58+
AddItemToArray(verbose);
59+
AddItemToArray(path);
60+
61+
var parsed = parser.Parse(items.ToArray());
62+
63+
if (parsed.HasErrors)
64+
{
65+
foreach (var err in parsed.Errors)
66+
throw err;
67+
}
68+
69+
void AddItemToArray(string item)
70+
{
71+
if (item != null)
72+
items.Add(item);
73+
}
74+
}
75+
76+
private class OptionsModelIssue_30
77+
{
78+
[Required]
79+
[Name("v", "verb")]
80+
[DefaultValue(true)]
81+
[Description("Verbose description")]
82+
public bool Verbose { get; set; }
83+
84+
[Required]
85+
[Name("p", "path")]
86+
[DefaultValue(@"C:\Some\Path")]
87+
[Description("Path description")]
88+
public string Path { get; set; }
89+
}
90+
#endregion
3891
}
3992
}

‎CommandLineParser/CommandLineParser.xml‎

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎CommandLineParser/CommandLineParser`TOption.cs‎

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -351,9 +351,9 @@ private void ParseOptions(IList<Exception> errors, ParseResult<TOption> result,
351351
{
352352
break;
353353
}
354-
else if (!found && option.IsRequired)
354+
else if (!found && option.IsRequired && !option.HasDefault)
355355
{
356-
errors.Add(new OptionNotFoundException(ParserOptions, option));
356+
errors.Add(new OptionNotFoundException(option));
357357

358358
continue;
359359
}
@@ -452,7 +452,7 @@ private void InitialzeModel()
452452
{
453453
var attributes = propInfo.GetCustomAttributes(true);
454454

455-
var lambda = GetLambdaExpression(propInfo, out string key);
455+
var lambda = propInfo.GetLambdaExpression(out string key);
456456

457457
var actions = new List<Action>(4);
458458
bool ignoreSet = false;
@@ -499,19 +499,6 @@ private void InitialzeModel()
499499
foreach (var action in actions)
500500
action();
501501
}
502-
503-
LambdaExpression GetLambdaExpression(PropertyInfo propInfo, out string key)
504-
{
505-
var entityType = propInfo.DeclaringType;
506-
var propType = propInfo.PropertyType;
507-
var parameter = Expression.Parameter(entityType, entityType.FullName);
508-
var property = Expression.Property(parameter, propInfo);
509-
var funcType = typeof(Func<,>).MakeGenericType(entityType, propType);
510-
511-
key = $"{entityType.ToString()}.{propInfo.Name}";
512-
513-
return Expression.Lambda(funcType, property, parameter);
514-
}
515502
}
516503
}
517504
}

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

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,9 @@ public override ICommandParserResult Parse(IArgumentManager argumentManager)
130130
{
131131
break;
132132
}
133-
else if (!found && option.IsRequired)
133+
else if (!found && option.IsRequired && !option.HasDefault)
134134
{
135-
errors.Add(new OptionNotFoundException(m_parserOptions, option));
135+
errors.Add(new OptionNotFoundException(option));
136136

137137
continue;
138138
}
@@ -317,7 +317,7 @@ private void InitialzeModel()
317317
{
318318
var attributes = propInfo.GetCustomAttributes(true);
319319

320-
var lambda = GetLambdaExpression(propInfo, out string key);
320+
var lambda = propInfo.GetLambdaExpression(out string key);
321321

322322
var actions = new List<Action>(4);
323323
bool ignoreSet = false;
@@ -364,19 +364,6 @@ private void InitialzeModel()
364364
foreach (var action in actions)
365365
action();
366366
}
367-
368-
LambdaExpression GetLambdaExpression(PropertyInfo propInfo, out string key)
369-
{
370-
var entityType = propInfo.DeclaringType;
371-
var propType = propInfo.PropertyType;
372-
var parameter = Expression.Parameter(entityType, entityType.FullName);
373-
var property = Expression.Property(parameter, propInfo);
374-
var funcType = typeof(Func<,>).MakeGenericType(entityType, propType);
375-
376-
key = $"{entityType.ToString()}.{propInfo.Name}";
377-
378-
return Expression.Lambda(funcType, property, parameter);
379-
}
380367
}
381368

382369
/// <summary>

‎CommandLineParser/Core/Exceptions/OptionNotFoundException.cs‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,21 +14,25 @@ public class OptionNotFoundException : KeyNotFoundException
1414
/// </summary>
1515
public ICommandLineOption Option { get; private set; }
1616

17-
public OptionNotFoundException(CommandLineParserOptions parserOptions, ICommandLineOption option)
18-
: base(CreateMessage(parserOptions, option))
17+
/// <summary>
18+
/// Creates a new <see cref="OptionNotFoundException"/> for a given <see cref="ICommandLineOption"/>
19+
/// </summary>
20+
/// <param name="option">The option that was not found</param>
21+
public OptionNotFoundException(ICommandLineOption option)
22+
: base(CreateMessage(option))
1923
{ }
2024

21-
private static string CreateMessage(CommandLineParserOptions parserOptions, ICommandLineOption option)
25+
private static string CreateMessage(ICommandLineOption option)
2226
{
2327
bool hasShort = option.HasShortName;
2428
bool hasLong = option.HasLongName;
2529
bool hasBoth = hasShort && hasLong;
2630

2731
string hasBothSeperator = hasBoth ? "|" : string.Empty;
28-
string shortName = hasShort ? $"{parserOptions.PrefixShortOption}{option.ShortName}" : string.Empty;
29-
string longName = hasLong ? $"{parserOptions.PrefixLongOption}{option.LongName}" : string.Empty;
32+
string shortName = hasShort ? $"{option.ShortName}" : string.Empty;
33+
string longName = hasLong ? $"{option.LongName}" : string.Empty;
3034

31-
return $"Required argument {shortName}{hasBothSeperator}{longName} not found!";
35+
return $"Required option '{shortName}{hasBothSeperator}{longName}' not found!";
3236
}
3337
}
3438
}

‎CommandLineParser/Core/Utils/ExtensionMethods.cs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
using System;
2+
using System.Linq.Expressions;
3+
using System.Reflection;
24
using System.Text;
35

46
namespace MatthiWare.CommandLine.Core.Utils
@@ -30,6 +32,19 @@ public static bool IsAssignableToGenericType(this Type self, Type genericType)
3032
return IsAssignableToGenericType(baseType, genericType);
3133
}
3234

35+
public static LambdaExpression GetLambdaExpression(this PropertyInfo propInfo, out string key)
36+
{
37+
var entityType = propInfo.DeclaringType;
38+
var propType = propInfo.PropertyType;
39+
var parameter = Expression.Parameter(entityType, entityType.FullName);
40+
var property = Expression.Property(parameter, propInfo);
41+
var funcType = typeof(Func<,>).MakeGenericType(entityType, propType);
42+
43+
key = $"{entityType.ToString()}.{propInfo.Name}";
44+
45+
return Expression.Lambda(funcType, property, parameter);
46+
}
47+
3348
public static StringBuilder AppendIf(this StringBuilder self, bool contition, string text)
3449
{
3550
if (contition)

0 commit comments

Comments
 (0)