Char to split options and formats is limited to pipe, comma, tilde (a…

…xuno#254) * SplitChar for formatters is unified and checked for validity * Affects ChooseFormatter, ConditionalFormatter, IsMatchFormatter, ListFormatter, PluralLocalizationFormatter, SubStringFormatter
axunonb · Mar 4, 2022 · fdbebdc · fdbebdc
1 parent c614eea
commit fdbebdc
Show file tree

Hide file tree

Showing 16 changed files with 101 additions and 35 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -33,7 +33,7 @@ e) `DictionarySource`
 
 The test setup for `ObjectPoolPerformanceTests` is included in the repo. It's obvious, that test results depend a lot on the input format string and the type of data arguments. Still, the results give a good impression of the improvements in `v3.0` compared to `v2.7`.
 
-Results under NetStandard2.1:
+Comparison under NetStandard2.1:
 ```
 |         Method |     N |     Mean |   Error |  StdDev |      Gen 0 |     Gen 1 | Gen 2 | Allocated |
 |--------------- |------ |---------:|--------:|--------:|-----------:|----------:|------:|----------:|

diff --git a/src/SmartFormat.Tests/Extensions/ChooseFormatterTests.cs b/src/SmartFormat.Tests/Extensions/ChooseFormatterTests.cs
@@ -38,9 +38,9 @@ public void Choose_should_work_with_numbers_strings_and_booleans(string format,
         public void Choose_With_Changed_SplitChar()
         {
             var smart = Smart.CreateDefaultSmartFormat();
-            // Set SplitChar from | to TAB, so we can use | for the output string
-            smart.GetFormatterExtension<ChooseFormatter>()!.SplitChar = '\t';
-            var result = smart.Format("{0:choose(1\t2\t3):|one|\t|two|\t|three|}", 2);
+            // Set SplitChar from | to ~, so we can use | for the output string
+            smart.GetFormatterExtension<ChooseFormatter>()!.SplitChar = '~';
+            var result = smart.Format("{0:choose(1~2~3):|one|~|two|~|three|}", 2);
             Assert.That(result, Is.EqualTo("|two|"));
         }
 

diff --git a/src/SmartFormat.Tests/Extensions/ConditionalFormatterTests.cs b/src/SmartFormat.Tests/Extensions/ConditionalFormatterTests.cs
@@ -39,13 +39,13 @@ public void Test_Bool(string format, string expected)
             smart.Test(format, args, expected);
         }
 
-        [TestCase("{0:cond:|Yes|\t|No|}", 0, "|Yes|")]
-        [TestCase("{0:cond:|Yes|\t|No|}", 1, "|No|")]
+        [TestCase("{0:cond:|Yes|~|No|}", 0, "|Yes|")]
+        [TestCase("{0:cond:|Yes|~|No|}", 1, "|No|")]
         public void Test_With_Changed_SplitChar(string format, int arg, string expected)
         {
             var smart = Smart.CreateDefaultSmartFormat();
-            // Set SplitChar from | to TAB, so we can use | for the output string
-            smart.GetFormatterExtension<ConditionalFormatter>()!.SplitChar = '\t';
+            // Set SplitChar from | to ~, so we can use | for the output string
+            smart.GetFormatterExtension<ConditionalFormatter>()!.SplitChar = '~';
             var result = smart.Format(format, arg);
             Assert.That(result, Is.EqualTo(expected));
         }

diff --git a/src/SmartFormat.Tests/Extensions/IsMatchFormatterTests.cs b/src/SmartFormat.Tests/Extensions/IsMatchFormatterTests.cs
@@ -53,14 +53,14 @@ public void Less_Than_2_Format_Options_Should_Throw()
         }
 
         // The "{}" in the format will output the input variable
-        [TestCase("{theValue:ismatch(^.+123.+$):|Has match for '{}'|\t|No match|}", "|Has match for 'Some123Content'|")]
-        [TestCase("{theValue:ismatch(^.+999.+$):|Has match for '{}'|\t|No match|}", "|No match|")]
+        [TestCase("{theValue:ismatch(^.+123.+$):|Has match for '{}'|~|No match|}", "|Has match for 'Some123Content'|")]
+        [TestCase("{theValue:ismatch(^.+999.+$):|Has match for '{}'|~|No match|}", "|No match|")]
         public void Test_With_Changed_SplitChar(string format, string expected)
         {
             var variable = new Dictionary<string, object> { {"theValue", "Some123Content"}};
             var smart = GetFormatter();;
-            // Set SplitChar from | to TAB, so we can use | for the output string
-            smart.GetFormatterExtension<IsMatchFormatter>()!.SplitChar = '\t';
+            // Set SplitChar from | to ~, so we can use | for the output string
+            smart.GetFormatterExtension<IsMatchFormatter>()!.SplitChar = '~';
             var result = smart.Format(format, variable);
             Assert.That(result, Is.EqualTo(expected));
         }

diff --git a/src/SmartFormat.Tests/Extensions/ListFormatterTests.cs b/src/SmartFormat.Tests/Extensions/ListFormatterTests.cs
@@ -33,18 +33,20 @@ public void Simple_List()
         {
             var smart = Smart.CreateDefaultSmartFormat();
             var items = new[] { "one", "two", "three" };
-            var result = smart.Format("{0:list:{}|, |, and }", new object[] { items }); // important: not only "items" as the parameter
+            // Important: You cannot use "items" as an indexed parameter directly,
+            // as it would be used as arg0="one", arg1="two", arg2="three"
+            var result = smart.Format("{0:list:{}|, |, and }", (System.Collections.IList) items);
             Assert.AreEqual("one, two, and three", result);
         }
 
         [Test]
         public void Simple_List_Changed_SplitChar()
         {
             var smart = Smart.CreateDefaultSmartFormat();
-            // Set SplitChar from | to TAB, so we can use | for the output string
-            smart.GetFormatterExtension<ListFormatter>()!.SplitChar = '\t';
+            // Set SplitChar from | to ~, so we can use | for the output string
+            smart.GetFormatterExtension<ListFormatter>()!.SplitChar = '~';
             var items = new[] { "one", "two", "three" };
-            var result = smart.Format("{0:list:{}\t|\t|}", new object[] { items }); // important: not only "items" as the parameter
+            var result = smart.Format("{0:list:{}~|~|}", (System.Collections.IList) items);
             Assert.AreEqual("one|two|three", result);
         }
 

diff --git a/src/SmartFormat.Tests/Extensions/PluralLocalizationFormatterTests.cs b/src/SmartFormat.Tests/Extensions/PluralLocalizationFormatterTests.cs
@@ -279,8 +279,8 @@ public void Nested_PlaceHolders_Pluralization(int numOfPeople, string format, bo
             Assert.That(result, numOfPeople == 1 ? Is.EqualTo("There is a person.") : Is.EqualTo("There are 2 people."));
         }
 
-        [TestCase(1, "There {People.Count:plural:|is a person|.\t|are {} people|.}", "There |is a person|.")]
-        [TestCase(2, "There {People.Count:plural:|is a person|.\t|are {} people|.}", "There |are 2 people|.")]
+        [TestCase(1, "There {People.Count:plural:|is a person|.~|are {} people|.}", "There |is a person|.")]
+        [TestCase(2, "There {People.Count:plural:|is a person|.~|are {} people|.}", "There |are 2 people|.")]
         public void Pluralization_With_Changed_SplitChar(int numOfPeople, string format, string expected)
         {
             var data = numOfPeople == 1
@@ -290,7 +290,7 @@ public void Pluralization_With_Changed_SplitChar(int numOfPeople, string format,
             var smart = new SmartFormatter()
                 .AddExtensions(new ReflectionSource())
                 // Set SplitChar from | to TAB, so we can use | for the output string
-                .AddExtensions(new PluralLocalizationFormatter { SplitChar = '\t'},
+                .AddExtensions(new PluralLocalizationFormatter { SplitChar = '~'},
                     new DefaultFormatter());
 
             var result = smart.Format(format, data);

diff --git a/src/SmartFormat/Core/Parsing/LiteralText.cs b/src/SmartFormat/Core/Parsing/LiteralText.cs
@@ -3,7 +3,6 @@
 // Licensed under the MIT license.
 
 using System;
-using System.Buffers;
 using SmartFormat.Core.Settings;
 using SmartFormat.Pooling.ObjectPools;
 using SmartFormat.Pooling.SmartPools;
@@ -92,4 +91,4 @@ public override void Clear()
             _toStringCache = null;
         }
     }
-}
+}
diff --git a/src/SmartFormat/Core/Settings/PoolSettings.cs b/src/SmartFormat/Core/Settings/PoolSettings.cs
@@ -11,7 +11,7 @@ public struct PoolSettings
     {
         /// <summary>
         /// Gets or sets whether object pools will tackle created and returned objects.
-        /// The pools are still used for creating new instances, but without tracking.
+        /// If <see langword="false"/>, the pools are still used for creating new instances, but without tracking.
         /// <para>The setting is respected by all subclasses of <see cref="Pooling.SpecializedPools.SpecializedPoolAbstract{T}"/>.</para>
         /// Default is <see langword="true"/>.<br/>
         /// This setting will have immediate effect at any time.

diff --git a/src/SmartFormat/Extensions/ChooseFormatter.cs b/src/SmartFormat/Extensions/ChooseFormatter.cs
@@ -17,12 +17,18 @@ namespace SmartFormat.Extensions
     public class ChooseFormatter : IFormatter
     {
         private CultureInfo? _cultureInfo;
+        private char _splitChar = '|';
 
         /// <summary>
         /// Gets or sets the character used to split the option text literals.
+        /// Valid characters are: | (pipe) , (comma)  ~ (tilde)
         /// </summary>
-        public char SplitChar { get; set; } = '|';
-
+        public char SplitChar
+        {
+            get => _splitChar;
+            set => _splitChar = Utilities.Validation.GetValidSplitCharOrThrow(value);
+        }
+
         /// <summary>
         /// Obsolete. <see cref="IFormatter"/>s only have one unique name.
         /// </summary>

diff --git a/src/SmartFormat/Extensions/ConditionalFormatter.cs b/src/SmartFormat/Extensions/ConditionalFormatter.cs
@@ -4,7 +4,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Reflection;
 using System.Text.RegularExpressions;
 using SmartFormat.Core.Extensions;
 using SmartFormat.Core.Parsing;
@@ -22,6 +21,8 @@ private static readonly Regex _complexConditionPattern
                 //   Description:      and/or    comparator     value
                 RegexOptions.IgnorePatternWhitespace | RegexOptions.Compiled);
 
+        private char _splitChar = '|';
+
         /// <summary>
         /// Obsolete. <see cref="IFormatter"/>s only have one unique name.
         /// </summary>
@@ -36,16 +37,21 @@ private static readonly Regex _complexConditionPattern
 
         /// <summary>
         /// Gets or sets the character used to split the option text literals.
+        /// Valid characters are: | (pipe) , (comma)  ~ (tilde)
         /// </summary>
-        public char SplitChar { get; set; } = '|';
-
+        public char SplitChar
+        {
+            get => _splitChar;
+            set => _splitChar = Validation.GetValidSplitCharOrThrow(value);
+        }
+
         ///<inheritdoc />
         public bool TryEvaluateFormat(IFormattingInfo formattingInfo)
         {
             var format = formattingInfo.Format;
             var current = formattingInfo.CurrentValue;
 
-            // See if the format string contains un-nested "|":
+            // See if the format string contains un-nested splits
             var parameters = format is not null ? format.Split(SplitChar) : new List<Format>(0);
 
             // Check whether arguments can be handled by this formatter
@@ -151,8 +157,7 @@ public bool TryEvaluateFormat(IFormattingInfo formattingInfo)
                 default:
                 {
                     // Object: Something|Nothing
-                    var arg = current;
-                    paramIndex = arg != null ? 0 : 1;
+                    paramIndex = current != null ? 0 : 1;
                     break;
                 }
             }

diff --git a/src/SmartFormat/Extensions/IsMatchFormatter.cs b/src/SmartFormat/Extensions/IsMatchFormatter.cs
@@ -28,6 +28,8 @@ namespace SmartFormat.Extensions
     /// </example>
     public class IsMatchFormatter : IFormatter, IInitializer
     {
+        private char _splitChar = '|';
+
         /// <summary>
         /// Obsolete. <see cref="IFormatter"/>s only have one unique name.
         /// </summary>
@@ -42,8 +44,13 @@ public class IsMatchFormatter : IFormatter, IInitializer
 
         /// <summary>
         /// Gets or sets the character used to split the option text literals.
+        /// Valid characters are: | (pipe) , (comma)  ~ (tilde)
         /// </summary>
-        public char SplitChar { get; set; } = '|';
+        public char SplitChar
+        {
+            get => _splitChar;
+            set => _splitChar = Utilities.Validation.GetValidSplitCharOrThrow(value);
+        }
 
         ///<inheritdoc />
         public bool TryEvaluateFormat(IFormattingInfo formattingInfo)

diff --git a/src/SmartFormat/Extensions/ListFormatter.cs b/src/SmartFormat/Extensions/ListFormatter.cs
@@ -43,6 +43,7 @@ public class ListFormatter : IFormatter, ISource, IInitializer
         // Will be overridden during Initialize()
         private SmartSettings _smartSettings = new();
         private bool _isInitialized = false;
+        private char _splitChar = '|';
 
         /// <summary>
         /// Obsolete. <see cref="IFormatter"/>s only have one unique name.
@@ -58,8 +59,13 @@ public class ListFormatter : IFormatter, ISource, IInitializer
 
         /// <summary>
         /// Gets or sets the character used to split the option text literals.
+        /// Valid characters are: | (pipe) , (comma)  ~ (tilde)
         /// </summary>
-        public char SplitChar { get; set; } = '|';
+        public char SplitChar
+        {
+            get => _splitChar;
+            set => _splitChar = Utilities.Validation.GetValidSplitCharOrThrow(value);
+        }
 
         /// <summary>
         /// This allows an integer to be used as a selector to index an array (or list).

diff --git a/src/SmartFormat/Extensions/NullFormatter.cs b/src/SmartFormat/Extensions/NullFormatter.cs
@@ -20,6 +20,8 @@ namespace SmartFormat.Extensions
     /// </example>
     public class NullFormatter : IFormatter
     {
+        private char _splitChar = '|';
+
         /// <summary>
         /// Obsolete. <see cref="IFormatter"/>s only have one unique name.
         /// </summary>
@@ -34,8 +36,13 @@ public class NullFormatter : IFormatter
 
         /// <summary>
         /// Gets or sets the character used to split the option text literals.
+        /// Valid characters are: | (pipe) , (comma)  ~ (tilde)
         /// </summary>
-        public char SplitChar { get; set; } = '|';
+        public char SplitChar
+        {
+            get => _splitChar;
+            set => _splitChar = Utilities.Validation.GetValidSplitCharOrThrow(value);
+        }
 
         ///<inheritdoc />
         public bool TryEvaluateFormat(IFormattingInfo formattingInfo)

diff --git a/src/SmartFormat/Extensions/PluralLocalizationFormatter.cs b/src/SmartFormat/Extensions/PluralLocalizationFormatter.cs
@@ -17,6 +17,8 @@ namespace SmartFormat.Extensions
     /// </summary>
     public class PluralLocalizationFormatter : IFormatter
     {
+        private char _splitChar = '|';
+
         /// <summary>
         /// CTOR for the plugin with rules for many common languages.
         /// </summary>
@@ -73,8 +75,13 @@ public PluralLocalizationFormatter(string defaultTwoLetterIsoLanguageName)
 
         /// <summary>
         /// Gets or sets the character used to split the option text literals.
+        /// Valid characters are: | (pipe) , (comma)  ~ (tilde)
         /// </summary>
-        public char SplitChar { get; set; } = '|';
+        public char SplitChar
+        {
+            get => _splitChar;
+            set => _splitChar = Validation.GetValidSplitCharOrThrow(value);
+        }
 
         ///<inheritdoc />
         public bool TryEvaluateFormat(IFormattingInfo formattingInfo)

diff --git a/src/SmartFormat/Extensions/SubStringFormatter.cs b/src/SmartFormat/Extensions/SubStringFormatter.cs
@@ -12,6 +12,8 @@ namespace SmartFormat.Extensions
     /// </summary>
     public class SubStringFormatter : IFormatter
     {
+        private char _splitChar = ',';
+
         /// <summary>
         /// Obsolete. <see cref="IFormatter"/>s only have one unique name.
         /// </summary>
@@ -26,8 +28,13 @@ public class SubStringFormatter : IFormatter
 
         /// <summary>
         /// Gets or sets the character used to split the option text literals.
+        /// Valid characters are: | (pipe) , (comma)  ~ (tilde)
         /// </summary>
-        public char SplitChar { get; set; } = ',';
+        public char SplitChar
+        {
+            get => _splitChar;
+            set => _splitChar = Utilities.Validation.GetValidSplitCharOrThrow(value);
+        }
 
         /// <summary>
         /// Get or set the string to display for NULL values, defaults to <see cref="string.Empty"/>.

diff --git a/src/SmartFormat/Utilities/Validation.cs b/src/SmartFormat/Utilities/Validation.cs
@@ -0,0 +1,20 @@
+//
+// Copyright SmartFormat Project maintainers and contributors.
+// Licensed under the MIT license.
+//
+
+using System;
+
+namespace SmartFormat.Utilities
+{
+    internal class Validation
+    {
+        public static char GetValidSplitCharOrThrow(char toCheck)
+        {
+            var valid = new[] { '|', ',', '~' };
+            return toCheck == valid[0] || toCheck == valid[1] || toCheck == valid[2]
+                ? toCheck
+                : throw new ArgumentException($"Only '{valid[0]}', '{valid[1]}' and '{valid[2]}' are valid split chars.");
+        }
+    }
+}