From ad50269e7dd4db96e53f615974f858827b70d6e7 Mon Sep 17 00:00:00 2001 From: "John J. Aylward" Date: Fri, 22 May 2020 10:50:30 -0400 Subject: [PATCH 1/3] move Errors and Value up to abstract class definition --- src/CommandLine/ParserResult.cs | 55 ++++++++++++++++----------------- 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/src/CommandLine/ParserResult.cs b/src/CommandLine/ParserResult.cs index 20761ada..d5c2dfc3 100644 --- a/src/CommandLine/ParserResult.cs +++ b/src/CommandLine/ParserResult.cs @@ -9,7 +9,7 @@ namespace CommandLine public sealed class TypeInfo { private readonly Type current; - private readonly IEnumerable choices; + private readonly IEnumerable choices; private TypeInfo(Type current, IEnumerable choices) { @@ -64,10 +64,20 @@ public abstract class ParserResult private readonly ParserResultType tag; private readonly TypeInfo typeInfo; - internal ParserResult(ParserResultType tag, TypeInfo typeInfo) + internal ParserResult(IEnumerable errors, TypeInfo typeInfo) { - this.tag = tag; + this.tag = ParserResultType.NotParsed; this.typeInfo = typeInfo; + Errors = errors; + Value = default; + } + + internal ParserResult(T value, TypeInfo typeInfo) + { + this.tag = ParserResultType.Parsed; + this.typeInfo = typeInfo; + Errors = new Error[0]; + Value = value; } /// @@ -82,6 +92,16 @@ public TypeInfo TypeInfo { get { return typeInfo; } } + + /// + /// Gets the instance with parsed values. If one or more errors occures, is returned. + /// + public T Value { get; } + + /// + /// Gets the sequence of parsing errors. If there are no errors, then an empty IEnumerable is returned. + /// + public IEnumerable Errors { get; } } /// @@ -90,12 +110,9 @@ public TypeInfo TypeInfo /// The type with attributes that define the syntax of parsing rules. public sealed class Parsed : ParserResult, IEquatable> { - private readonly T value; - internal Parsed(T value, TypeInfo typeInfo) - : base(ParserResultType.Parsed, typeInfo) + : base(value, typeInfo) { - this.value = value; } internal Parsed(T value) @@ -103,13 +120,6 @@ internal Parsed(T value) { } - /// - /// Gets the instance with parsed values. - /// - public T Value - { - get { return value; } - } /// /// Determines whether the specified is equal to the current . @@ -118,8 +128,7 @@ public T Value /// true if the specified is equal to the current ; otherwise, false. public override bool Equals(object obj) { - var other = obj as Parsed; - if (other != null) + if (obj is Parsed other) { return Equals(other); } @@ -159,21 +168,12 @@ public bool Equals(Parsed other) /// The type with attributes that define the syntax of parsing rules. public sealed class NotParsed : ParserResult, IEquatable> { - private readonly IEnumerable errors; internal NotParsed(TypeInfo typeInfo, IEnumerable errors) - : base(ParserResultType.NotParsed, typeInfo) + : base(errors, typeInfo) { - this.errors = errors; } - /// - /// Gets the sequence of parsing errors. - /// - public IEnumerable Errors - { - get { return errors; } - } /// /// Determines whether the specified is equal to the current . @@ -182,8 +182,7 @@ public IEnumerable Errors /// true if the specified is equal to the current ; otherwise, false. public override bool Equals(object obj) { - var other = obj as NotParsed; - if (other != null) + if (obj is NotParsed other) { return Equals(other); } From 3094bfb1f044b9409a858956ddec72bc514fbf21 Mon Sep 17 00:00:00 2001 From: "John J. Aylward" Date: Fri, 22 May 2020 11:05:51 -0400 Subject: [PATCH 2/3] add null checks to constructors to ensure valid data --- src/CommandLine/ParserResult.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/CommandLine/ParserResult.cs b/src/CommandLine/ParserResult.cs index d5c2dfc3..c7c9c833 100644 --- a/src/CommandLine/ParserResult.cs +++ b/src/CommandLine/ParserResult.cs @@ -67,17 +67,17 @@ public abstract class ParserResult internal ParserResult(IEnumerable errors, TypeInfo typeInfo) { this.tag = ParserResultType.NotParsed; - this.typeInfo = typeInfo; - Errors = errors; + this.typeInfo = typeInfo ?? TypeInfo.Create(typeof(T)); + Errors = errors ?? new Error[0]; Value = default; } internal ParserResult(T value, TypeInfo typeInfo) { + Value = value ?? throw new ArgumentNullException(nameof(value)); this.tag = ParserResultType.Parsed; - this.typeInfo = typeInfo; + this.typeInfo = typeInfo ?? TypeInfo.Create(value.GetType()); Errors = new Error[0]; - Value = value; } /// From d3b6315e7ec25c6a568bbbf6ec93278eb8ae59d7 Mon Sep 17 00:00:00 2001 From: "John J. Aylward" Date: Fri, 22 May 2020 18:01:39 -0400 Subject: [PATCH 3/3] Adds test cases --- tests/CommandLine.Tests/Unit/Issue543Tests.cs | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 tests/CommandLine.Tests/Unit/Issue543Tests.cs diff --git a/tests/CommandLine.Tests/Unit/Issue543Tests.cs b/tests/CommandLine.Tests/Unit/Issue543Tests.cs new file mode 100644 index 00000000..61a83512 --- /dev/null +++ b/tests/CommandLine.Tests/Unit/Issue543Tests.cs @@ -0,0 +1,108 @@ +using System.Collections.Generic; +using System.Linq; +using Xunit; +using CommandLine.Text; + +namespace CommandLine.Tests.Unit +{ + //Reference: PR# 634 + public class Issue543Tests + { + + private const int ERROR_SUCCESS = 0; + + [Fact] + public void Parser_GiveHelpArgument_ExpectSuccess() + { + var result = Parser.Default.ParseArguments(new[] { "--help" }); + + Assert.Equal(ParserResultType.NotParsed, result.Tag); + Assert.Null(result.Value); + Assert.NotEmpty(result.Errors); + } + + [Fact] + public void Parser_GiveConnectionStringAndJobId_ExpectSuccess() + { + var result = Parser.Default.ParseArguments(new[] { + "-c", "someConnectionString", + "-j", "1234", + }); + + Assert.Equal(ParserResultType.Parsed, result.Tag); + Assert.NotNull(result.Value); + Assert.Empty(result.Errors); + Assert.Equal("someConnectionString", result.Value.ConnectionString); + Assert.Equal(1234, result.Value.JobId); + } + + [Fact] + public void Parser_GiveVerb1_ExpectSuccess() + { + var result = Parser.Default.ParseArguments(new[] { + "verb1", + "-j", "1234", + }); + + Assert.Equal(ParserResultType.Parsed, result.Tag); + Assert.Empty(result.Errors); + Assert.NotNull(result.Value); + Assert.NotNull(result.Value as Verb1Options); + Assert.Equal(1234, (result.Value as Verb1Options).JobId); + } + + [Fact] + public void Parser_GiveVerb2_ExpectSuccess() + { + var result = Parser.Default.ParseArguments(new[] { + "verb2", + "-c", "someConnectionString", + }); + + Assert.Equal(ParserResultType.Parsed, result.Tag); + Assert.Empty(result.Errors); + Assert.NotNull(result.Value); + Assert.NotNull(result.Value as Verb2Options); + Assert.Equal("someConnectionString", (result.Value as Verb2Options).ConnectionString); + } + + // Options + internal class Options + { + [Option('c', "connectionString", Required = true, HelpText = "Texts.ExplainConnection")] + public string ConnectionString { get; set; } + + [Option('j', "jobId", Required = true, HelpText = "Texts.ExplainJob")] + public int JobId { get; set; } + + [Usage(ApplicationAlias = "Importer.exe")] + public static IEnumerable Examples + { + get => new[] { + new Example("Texts.ExplainExampleExecution", new Options() { + ConnectionString="Server=MyServer;Database=MyDatabase", + JobId = 5 + }), + }; + } + } + + // Options + [Verb("verb1")] + internal class Verb1Options + { + [Option('j', "jobId", Required = false, HelpText = "Texts.ExplainJob")] + public int JobId { get; set; } + } + + // Options + [Verb("verb2")] + internal class Verb2Options + { + [Option('c', "connectionString", Required = false, HelpText = "Texts.ExplainConnection")] + public string ConnectionString { get; set; } + } + + } +} +