Fix option groups (#575)

* Add MyGet package provider to consume the daily builds. (#566) * Options groups take in account default value * Do not allow options groups and exclusive set names to be used together * Multiple group errors are shown together * MissingGroupOptionError compare option names Co-authored-by: Mohamed Hassan <[email protected]>
commandlineparser · Feb 2, 2020 · 32bbe00 · 32bbe00
1 parent 5196aa9
commit 32bbe00
Show file tree

Hide file tree

Showing 7 changed files with 180 additions and 9 deletions.
diff --git a/appveyor.yml b/appveyor.yml
@@ -1,12 +1,12 @@
 #version should be only changed with RELEASE eminent, see RELEASE.md
 
-version: 2.7.83-beta-{build}
+version: 2.7.84-beta-{build}
 
 image: Visual Studio 2019
 
 clone_depth: 1
 pull_requests:
-  do_not_increment_build_number: true
+  do_not_increment_build_number: false
 
 init:
   - ps: |
@@ -15,8 +15,9 @@ init:
       if ($env:APPVEYOR_REPO_TAG -eq "true") {
         $ver = $env:APPVEYOR_REPO_TAG_NAME
         if($ver.StartsWith("v") -eq $true) { $ver = $ver.Substring(1) }
-        Update-AppveyorBuild -Version $ver
-      }
+        Update-AppveyorBuild -Version $ver       
+      }   
+  - ps:  Write-Host "APPVEYOR_BUILD_VERSION='$env:APPVEYOR_BUILD_VERSION'" -ForegroundColor Yellow
 
 environment:
   matrix:
@@ -57,3 +58,12 @@ deploy:
   artifact: 'NuGetPackages'
   on:
     APPVEYOR_REPO_TAG: true
+
+#myget 
+- provider: NuGet
+  server: https://www.myget.org/F/commandlineparser/api/v2/package
+  api_key:
+    secure: ltHh/DsAk+Y7qbJwzUO4+i1U+7uGTLVYXTdW0+Rk2z7jqj5DDNNlih9J8K7bU4bH
+  artifact: 'NuGetPackages'	
+  symbol_server: https://www.myget.org/F/commandlineparser/symbols/api/v2/package
+
diff --git a/src/CommandLine/Core/SpecificationPropertyRules.cs b/src/CommandLine/Core/SpecificationPropertyRules.cs
@@ -18,12 +18,35 @@ public static IEnumerable<Func<IEnumerable<SpecificationProperty>, IEnumerable<E
                 {
                     EnforceMutuallyExclusiveSet(),
                     EnforceGroup(),
+                    EnforceMutuallyExclusiveSetAndGroupAreNotUsedTogether(),
                     EnforceRequired(),
                     EnforceRange(),
                     EnforceSingle(tokens)
                 };
         }
 
+        private static Func<IEnumerable<SpecificationProperty>, IEnumerable<Error>> EnforceMutuallyExclusiveSetAndGroupAreNotUsedTogether()
+        {
+            return specProps =>
+            {
+                var options =
+                    from sp in specProps
+                    where sp.Specification.IsOption()
+                    let o = (OptionSpecification)sp.Specification
+                    where o.SetName.Length > 0
+                    where o.Group.Length > 0
+                    select o;
+
+                if (options.Any())
+                {
+                    return from o in options
+                           select new GroupOptionAmbiguityError(new NameInfo(o.ShortName, o.LongName));
+                }
+
+                return Enumerable.Empty<Error>();
+            };
+        }
+
         private static Func<IEnumerable<SpecificationProperty>, IEnumerable<Error>> EnforceGroup()
         {
             return specProps =>
@@ -36,14 +59,15 @@ where o.Group.Length > 0
                     select new
                     {
                         Option = o,
-                        Value = sp.Value
+                        Value = sp.Value,
+                        DefaultValue = sp.Specification.DefaultValue
                     };
 
                 var groups = from o in optionsValues
                              group o by o.Option.Group into g
                              select g;
 
-                var errorGroups = groups.Where(gr => gr.All(g => g.Value.IsNothing()));
+                var errorGroups = groups.Where(gr => gr.All(g => g.Value.IsNothing() && g.DefaultValue.IsNothing()));
 
                 if (errorGroups.Any())
                 {

diff --git a/src/CommandLine/Error.cs b/src/CommandLine/Error.cs
@@ -2,6 +2,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Linq;
 
 namespace CommandLine
 {
@@ -74,8 +75,11 @@ public enum ErrorType
         /// <summary>
         /// Value of <see cref="CommandLine.MissingGroupOptionError"/> type.
         /// </summary>
-        MissingGroupOptionError
-
+        MissingGroupOptionError,
+        /// <summary>
+        /// Value of <see cref="CommandLine.GroupOptionAmbiguityError"/> type.
+        /// </summary>
+        GroupOptionAmbiguityError
     }
 
     /// <summary>
@@ -532,7 +536,7 @@ internal InvalidAttributeConfigurationError()
         }
     }
 
-    public sealed class MissingGroupOptionError : Error
+    public sealed class MissingGroupOptionError : Error, IEquatable<Error>, IEquatable<MissingGroupOptionError>
     {
         public const string ErrorMessage = "At least one option in a group must have value.";
 
@@ -555,5 +559,33 @@ public IEnumerable<NameInfo> Names
         {
             get { return names; }
         }
+
+        public new bool Equals(Error obj)
+        {
+            var other = obj as MissingGroupOptionError;
+            if (other != null)
+            {
+                return Equals(other);
+            }
+
+            return base.Equals(obj);
+        }
+
+        public bool Equals(MissingGroupOptionError other)
+        {
+            if (other == null)
+            {
+                return false;
+            }
+
+            return Group.Equals(other.Group) && Names.SequenceEqual(other.Names);
+        }
+    }
+
+    public sealed class GroupOptionAmbiguityError : NamedError
+    {
+        internal GroupOptionAmbiguityError(NameInfo option)
+            : base(ErrorType.GroupOptionAmbiguityError, option)
+        { }
     }
 }
diff --git a/tests/CommandLine.Tests/Fakes/Options_With_Multiple_Groups.cs b/tests/CommandLine.Tests/Fakes/Options_With_Multiple_Groups.cs
@@ -0,0 +1,20 @@
+namespace CommandLine.Tests.Fakes
+{
+    public class Options_With_Multiple_Groups
+    {
+        [Option('v', "version")]
+        public string Version { get; set; }
+
+        [Option("option11", Group = "err-group")]
+        public string Option11 { get; set; }
+
+        [Option("option12", Group = "err-group")]
+        public string Option12 { get; set; }
+
+        [Option("option21", Group = "err-group2")]
+        public string Option21 { get; set; }
+
+        [Option("option22", Group = "err-group2")]
+        public string Option22 { get; set; }
+    }
+}
diff --git a/tests/CommandLine.Tests/Fakes/Simple_Options_With_OptionGroup_MutuallyExclusiveSet.cs b/tests/CommandLine.Tests/Fakes/Simple_Options_With_OptionGroup_MutuallyExclusiveSet.cs
@@ -0,0 +1,14 @@
+namespace CommandLine.Tests.Fakes
+{
+    public class Simple_Options_With_OptionGroup_MutuallyExclusiveSet
+    {
+        [Option(HelpText = "Define a string value here.", Group = "test", SetName = "setname", Default = "qwerty123")]
+        public string StringValue { get; set; }
+
+        [Option('s', "shortandlong", HelpText = "Example with both short and long name.", Group = "test", SetName = "setname")]
+        public string ShortAndLong { get; set; }
+
+        [Option('x', HelpText = "Define a boolean or switch value here.")]
+        public bool BoolValue { get; set; }
+    }
+}
diff --git a/tests/CommandLine.Tests/Fakes/Simple_Options_With_OptionGroup_WithOptionDefaultValue.cs b/tests/CommandLine.Tests/Fakes/Simple_Options_With_OptionGroup_WithOptionDefaultValue.cs
@@ -0,0 +1,14 @@
+namespace CommandLine.Tests.Fakes
+{
+    public class Simple_Options_With_OptionGroup_WithOptionDefaultValue
+    {
+        [Option(HelpText = "Define a string value here.", Required = true, Group = "test", Default = "qwerty123")]
+        public string StringValue { get; set; }
+
+        [Option('s', "shortandlong", HelpText = "Example with both short and long name.", Required = true, Group = "test")]
+        public string ShortAndLong { get; set; }
+
+        [Option('x', HelpText = "Define a boolean or switch value here.")]
+        public bool BoolValue { get; set; }
+    }
+}
diff --git a/tests/CommandLine.Tests/Unit/Core/InstanceBuilderTests.cs b/tests/CommandLine.Tests/Unit/Core/InstanceBuilderTests.cs
@@ -1110,6 +1110,34 @@ public void Options_In_Group_With_No_Values_Generates_MissingGroupOptionError()
             ((NotParsed<Options_With_Group>)result).Errors.Should().BeEquivalentTo(expectedResult);
         }
 
+        [Fact]
+        public void Options_In_Group_With_No_Values_Generates_MissingGroupOptionErrors()
+        {
+            // Fixture setup
+            var optionNames1 = new List<NameInfo>
+            {
+                new NameInfo("", "option11"),
+                new NameInfo("", "option12")
+            };
+            var optionNames2 = new List<NameInfo>
+            {
+                new NameInfo("", "option21"),
+                new NameInfo("", "option22")
+            };
+            var expectedResult = new[]
+            {
+                new MissingGroupOptionError("err-group", optionNames1),
+                new MissingGroupOptionError("err-group2", optionNames2)
+            };
+
+            // Exercize system 
+            var result = InvokeBuild<Options_With_Multiple_Groups>(
+                new[] { "-v 10.42" });
+
+            // Verify outcome
+            ((NotParsed<Options_With_Multiple_Groups>)result).Errors.Should().BeEquivalentTo(expectedResult);
+        }
+
         [Theory]
         [InlineData("-v", "10.5", "--option1", "test1", "--option2", "test2")]
         [InlineData("-v", "10.5", "--option1", "test1")]
@@ -1164,6 +1192,35 @@ public void Options_In_Group_Ignore_Option_Group_If_Option_Group_Name_Empty()
             errors.Should().BeEquivalentTo(expectedResult);
         }
 
+        [Fact]
+        public void Options_In_Group_Use_Option_Default_Value_When_Available()
+        {
+            // Exercize system 
+            var result = InvokeBuild<Simple_Options_With_OptionGroup_WithOptionDefaultValue>(new string[] { "-x" });
+
+            // Verify outcome
+            result.Should().BeOfType<Parsed<Simple_Options_With_OptionGroup_WithOptionDefaultValue>>();
+        }
+
+        [Fact]
+        public void Options_In_Group_Do_Not_Allow_Mutually_Exclusive_Set()
+        {
+            var expectedResult = new[]
+            {
+                new GroupOptionAmbiguityError(new NameInfo("", "stringvalue")),
+                new GroupOptionAmbiguityError(new NameInfo("s", "shortandlong"))
+            };
+
+            // Exercize system 
+            var result = InvokeBuild<Simple_Options_With_OptionGroup_MutuallyExclusiveSet>(new string[] { "-x" });
+
+            // Verify outcome
+            result.Should().BeOfType<NotParsed<Simple_Options_With_OptionGroup_MutuallyExclusiveSet>>();
+            var errors = ((NotParsed<Simple_Options_With_OptionGroup_MutuallyExclusiveSet>)result).Errors;
+
+            errors.Should().BeEquivalentTo(expectedResult);
+        }
+
         private class ValueWithNoSetterOptions
         {
             [Value(0, MetaName = "Test", Default = 0)]