More testing, minor usability improvements (#44)

Author: reubeno

src/NClap/Help/TwoColumnArgumentHelpLayout.cs

@@ -31,7 +31,7 @@ private TwoColumnArgumentHelpLayout(TwoColumnArgumentHelpLayout other) : base(ot
         /// <summary>
         /// Optional maximum widths of columns; null indicates no preference.
         /// </summary>
-        public int?[] ColumnWidths = new int?[2] { null, null };
+        public int?[] ColumnWidths { get; } = new int?[2] { null, null };
 
         /// <summary>
         /// Optionally specifies separator string to be used between columns,

src/NClap/Metadata/ArgumentBaseAttribute.cs

@@ -117,6 +117,14 @@ public string[] ConflictsWith
         /// </summary>
         public bool AllowEmpty { get; set; }
 
+        /// <summary>
+        /// Optionally specifies the strings that may be used as element
+        /// separators for multiple elements that have been expressed in the
+        /// same token.  Only relevant for parsing collection types, and
+        /// ignored otherwise.
+        /// </summary>
+        public string[] ElementSeparators { get; set; } = new[] { "," };
+
         /// <summary>
         /// Optionally provides a type that implements IStringParser, and which
         /// should be used for parsing strings for this argument instead of the

src/NClap/NClap.csproj

@@ -21,7 +21,7 @@
     <Description>NClap is a .NET library for parsing command-line arguments and building interactive command shells. It's driven by a declarative attribute syntax, and easy to extend.</Description>
     <Company>Microsoft Corporation</Company>
     <Copyright>Copyright © Microsoft Corporation. All right reserved.</Copyright>
-    <Version>2.0.1</Version>
+    <Version>2.1.0</Version>
     <AssemblyVersion>$(Version)</AssemblyVersion>
     <FileVersion>$(Version)</FileVersion>
     <PackageVersion>$(Version)</PackageVersion>

src/NClap/Parser/ArgumentParser.cs

@@ -365,6 +365,7 @@ private static ArgumentParseContext CreateParseContext(ArgumentDefinition argume
             new ArgumentParseContext
             {
                 NumberOptions = argument.Attribute.NumberOptions,
+                ElementSeparators = argument.Attribute.ElementSeparators,
                 AllowEmpty = argument.Attribute.AllowEmpty,
                 FileSystemReader = options.FileSystemReader,
                 ParserContext = options.Context,

src/NClap/Repl/Loop.cs

@@ -42,10 +42,7 @@ public IEnumerable<string> GetCompletions(IEnumerable<string> tokens, int tokenI
         /// for the argument set that will be dynamically created for this loop.</param>
         public Loop(Type commandType, ILoopClient loopClient, ArgumentSetAttribute argSetAttribute = null)
         {
-            if (commandType == null)
-            {
-                throw new ArgumentNullException(nameof(commandType));
-            }
+            if (commandType == null) throw new ArgumentNullException(nameof(commandType));
 
             _client = loopClient ?? throw new ArgumentNullException(nameof(loopClient));
             _client.TokenCompleter = new TokenCompleter(this);
@@ -219,7 +216,7 @@ private static Type ConstructCommandType(Type inputType, out Func<object> factor
             }
 
             // See if it is an enum; if so, use it as the inner type for a command group.
-            if (inputType.GetTypeInfo().IsEnum)
+            else if (inputType.GetTypeInfo().IsEnum)
             {
                 loopType = typeof(CommandGroup<>).MakeGenericType(new[] { inputType });
             }

src/NClap/Types/ArgumentParseContext.cs

@@ -1,4 +1,7 @@
 using NClap.Metadata;
+using System;
+using System.Collections.Generic;
+using System.Linq;
 
 namespace NClap.Types
 {
@@ -8,6 +11,7 @@ namespace NClap.Types
     public class ArgumentParseContext
     {
         private IFileSystemReader _fileSystemReader = Parser.FileSystemReader.Create();
+        private IReadOnlyList<string> _elementSeparators = new List<string> { "," };
 
         /// <summary>
         /// The default context for parsing.
@@ -35,18 +39,31 @@ public IFileSystemReader FileSystemReader
         /// <summary>
         /// Options for parsing numeric arguments.
         /// </summary>
-        public NumberOptions NumberOptions { get; set; }
+        public NumberOptions NumberOptions { get; set; } = NumberOptions.None;
 
         /// <summary>
         /// True to allow "empty" arguments (e.g. empty strings); false to
         /// consider them invalid.
         /// </summary>
-        public bool AllowEmpty { get; set; }
+        public bool AllowEmpty { get; set; } = false;
 
         /// <summary>
         /// True for parsing to be case sensitive; false to be case insensitive.
         /// </summary>
-        public bool CaseSensitive { get; set; }
+        public bool CaseSensitive { get; set; } = false;
+
+        /// <summary>
+        /// Strings that may separate multiple elements stored in the same token.
+        /// </summary>
+        public IReadOnlyList<string> ElementSeparators
+        {
+            get => _elementSeparators;
+            set
+            {
+                if (value == null) _elementSeparators = new string[] { };
+                _elementSeparators = value.ToList();
+            }
+        }
 
         /// <summary>
         /// Optionally provides a reference to the object containing the one to

src/NClap/Types/CollectionArgumentTypeBase.cs

@@ -13,8 +13,6 @@ namespace NClap.Types
     /// </summary>
     internal abstract class CollectionArgumentTypeBase : ArgumentTypeBase, ICollectionArgumentType
     {
-        private const char ElementSeparatorChar = ',';
-
         private readonly IArgumentType _elementArgumentType;
 
         /// <summary>
@@ -73,16 +71,29 @@ public override IEnumerable<string> GetCompletions(ArgumentCompletionContext con
                 throw new ArgumentNullException(nameof(valueToComplete));
             }
 
-            var tokens = valueToComplete.Split(ElementSeparatorChar);
+            string[] tokens;
+            if (context.ParseContext.ElementSeparators.Any())
+            {
+                tokens = valueToComplete.Split(
+                    context.ParseContext.ElementSeparators.ToArray(),
+                    StringSplitOptions.None);
+            }
+            else
+            {
+                tokens = new[] { valueToComplete };
+            }
+
             Debug.Assert(tokens.Length >= 1);
 
             var currentTokenIndex = tokens.Length - 1;
             var currentToken = tokens[currentTokenIndex];
 
             tokens[currentTokenIndex] = string.Empty;
 
+            var preferredSeparator = GetPreferredElementSeparatorOrDefault(context.ParseContext) ?? string.Empty;
+
             return _elementArgumentType.GetCompletions(context, currentToken)
-                       .Select(completion => string.Join(ElementSeparatorChar.ToString(), tokens) + completion);
+                       .Select(completion => string.Join(preferredSeparator, tokens) + completion);
         }
 
         /// <summary>
@@ -109,7 +120,17 @@ public override IEnumerable<string> GetCompletions(ArgumentCompletionContext con
         /// <returns>The parsed object.</returns>
         protected override object Parse(ArgumentParseContext context, string stringToParse)
         {
-            var elementStrings = stringToParse.Split(ElementSeparatorChar);
+            string[] elementStrings;
+            if (context.ElementSeparators.Any())
+            {
+                elementStrings = stringToParse.Split(
+                    context.ElementSeparators.ToArray(),
+                    StringSplitOptions.None);
+            }
+            else
+            {
+                elementStrings = new[] { stringToParse };
+            }
 
             var parsedElements = elementStrings.Select(elementString =>
             {
@@ -136,5 +157,14 @@ protected override object Parse(ArgumentParseContext context, string stringToPar
         /// <param name="collection">Collection to enumerate.</param>
         /// <returns>The enumerated objects.</returns>
         protected abstract IEnumerable<object> GetElements(object collection);
+
+        /// <summary>
+        /// Given a parser context, returns the preferred element separator. Returns null
+        /// if no such separator exists.
+        /// </summary>
+        /// <param name="context">Parser context.</param>
+        /// <returns>Preferred element separator, if one exists; null otherwise.</returns>
+        private static string GetPreferredElementSeparatorOrDefault(ArgumentParseContext context) =>
+            context.ElementSeparators.FirstOrDefault();
     }
 }

src/Tests/UnitTests/AssemblyTests.cs

@@ -1,4 +1,6 @@
-using System.Linq;
+using System;
+using System.Collections.Generic;
+using System.Linq;
 using System.Reflection;
 using FluentAssertions;
 using FluentAssertions.Types;
@@ -9,15 +11,17 @@ namespace NClap.Tests
     [TestClass]
     public class AssemblyTests
     {
-        public static readonly Assembly AssemblyUnderTest = typeof(CommandLineParser).GetTypeInfo().Assembly;
+        public static readonly Type RepresentativeType = typeof(CommandLineParser);
+
+        public static readonly Assembly AssemblyUnderTest = RepresentativeType.GetTypeInfo().Assembly;
 
         [TestMethod]
-        public void VerifyNamespace()
+        public void TestThatAllPublicTypesInAssemblyStartWithCorrectNamespacePrefix()
         {
             const string expectedNs = nameof(NClap);
             const string expectedNsWithDot = expectedNs + ".";
 
-            foreach (var type in AllTypes.From(AssemblyUnderTest).Where(t => t.GetTypeInfo().IsPublic))
+            foreach (var type in AllPublicTypes())
             {
                 var ns = type.Namespace;
                 if (ns != expectedNs)
@@ -26,5 +30,29 @@ public void VerifyNamespace()
                 }
             }
         }
+
+        [TestMethod]
+        public void TestThatNoPublicTypeInAssemblyContainsNonPrivateField()
+        {
+            var noNonPrivateFields = true;
+            foreach (var type in AllPublicTypes().Where(t => !t.IsEnum))
+            {
+                foreach (var member in type.GetTypeInfo().GetFields().Where(f => !IsConstant(f)))
+                {
+                    if (!member.IsPrivate)
+                    {
+                        Console.Error.WriteLine($"Found non-private field in public type: {type.FullName} :: {member.Name}");
+                        noNonPrivateFields = false;
+                    }
+                }
+            }
+
+            noNonPrivateFields.Should().BeTrue();
+        }
+
+        private static bool IsConstant(FieldInfo field) =>
+            field.IsLiteral && !field.IsInitOnly;
+
+        private static IEnumerable<Type> AllPublicTypes() => AllTypes.From(AssemblyUnderTest).Where(t => t.GetTypeInfo().IsPublic);
     }
 }

src/Tests/UnitTests/Exceptions/InvalidArgumentSetExceptionTests.cs

@@ -3,6 +3,7 @@
 using NClap.Exceptions;
 using NClap.Metadata;
 using NClap.Tests.Metadata;
+using System;
 
 namespace NClap.Tests.Exceptions
 {
@@ -32,5 +33,15 @@ public void ArgumentConstructor()
             exn.InnerMessage.Should().Be(innerMessage);
             exn.Message.Should().Contain(innerMessage);
         }
+
+        [TestMethod]
+        public void TestThatProvidedInnerExceptionIsAccessibleInConstructedObject()
+        {
+            const string anyMessage = "Some message";
+            var innerExn = new NotImplementedException();
+            var exn = new InvalidArgumentSetException(anyMessage, innerExn);
+            exn.InnerException.Should().BeSameAs(innerExn);
+            exn.InnerMessage.Should().Be(anyMessage);
+        }
     }
 }

src/Tests/UnitTests/Exceptions/InvalidCommandExceptionTests.cs

@@ -0,0 +1,18 @@
+using FluentAssertions;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using NClap.Exceptions;
+
+namespace NClap.Tests.Exceptions
+{
+    [TestClass]
+    public class InvalidCommandExceptionTests
+    {
+        [TestMethod]
+        public void TestThatInnerMessageIsEmbeddedInConstructedObject()
+        {
+            const string anyMessage = "Some message";
+            var exn = new InvalidCommandException(anyMessage);
+            exn.Message.Should().Be(anyMessage);
+        }
+    }
+}