Resolves axuno#345

* PluralLocalizationFormatter does treat numeric string as valid argument
* Restore behavior of v3.1.0 and before
* Bump version to v3.2.2
* Update appveyor and github workflow CI

.github/workflows/SonarCloud.yml

@@ -13,13 +13,14 @@ jobs:
     # (PRs from forks can't access secrets other than secrets.GITHUB_TOKEN for security reasons)
     if: ${{ !github.event.pull_request.head.repo.fork }}
     env:
-      version: '3.2.1'
-      versionFile: '3.2.1'
+      version: '3.2.2'
+      versionFile: '3.2.2'
     steps:
-      - name: Set up JDK 11
-        uses: actions/setup-java@v1
+      - name: Set up JDK 17
+        uses: actions/setup-java@v3
         with:
-          java-version: 1.11
+          distribution: 'microsoft'
+          java-version: '17'
       - uses: actions/checkout@v3
         with:
           fetch-depth: 0   # Shallow clones should be disabled for a better relevancy of analysis

appveyor.yml

@@ -23,7 +23,7 @@ for:
       - ps: dotnet restore SmartFormat.sln --verbosity quiet
       - ps: dotnet add .\SmartFormat.Tests\SmartFormat.Tests.csproj package AltCover
       - ps: |
-          $version = "3.2.1"
+          $version = "3.2.2"
           $versionFile = $version + "." + ${env:APPVEYOR_BUILD_NUMBER}
 
           if ($env:APPVEYOR_PULL_REQUEST_NUMBER) {

src/Directory.Build.props

@@ -8,8 +8,8 @@
         <Copyright>Copyright 2011-$(CurrentYear) SmartFormat Project</Copyright>
         <RepositoryUrl>https://github.com/axuno/SmartFormat.git</RepositoryUrl>
         <PublishRepositoryUrl>true</PublishRepositoryUrl>
-        <Version>3.2.1</Version>
-        <FileVersion>3.2.1</FileVersion>
+        <Version>3.2.2</Version>
+        <FileVersion>3.2.2</FileVersion>
         <AssemblyVersion>3.0.0</AssemblyVersion> <!--only update AssemblyVersion with major releases -->
         <LangVersion>latest</LangVersion>
         <EnableNETAnalyzers>true</EnableNETAnalyzers>

src/SmartFormat.Tests/Extensions/PluralLocalizationFormatterTests.cs

@@ -49,6 +49,31 @@ public void Explicit_Formatter_Without_IEnumerable_Arg_Should_Throw()
         Assert.That(() => smart.Format("{0:plural:One|Two}", new object()), Throws.Exception.TypeOf<FormattingException>());
     }
 
+    [TestCase("")] // no string
+    [TestCase(false)] // no boolean
+    [TestCase(3.40282347E+38f)] // float.MaxValue exceeds decimal.MaxValue
+    public void Explicit_Formatter_Without_Valid_Argument_Should_Throw(object arg)
+    {
+        var smart = Smart.CreateDefaultSmartFormat();
+        Assert.That(() => smart.Format("{0:plural:One|Two}", arg), Throws.Exception.TypeOf<FormattingException>(), "Invalid argument type or value");
+    }
+
+    [TestCase("String", "String")]
+    [TestCase(false, "other")]
+    [TestCase(default(string?), "other")]
+    public void AutoDetect_Formatter_Should_Not_Handle_bool_string_null(object arg, string expected)
+    {
+        var smart = new SmartFormatter()
+            .AddExtensions(new DefaultSource())
+            .AddExtensions(new PluralLocalizationFormatter { CanAutoDetect = true },
+                new ConditionalFormatter { CanAutoDetect = true },
+                new DefaultFormatter());
+
+        // Result comes from ConditionalFormatter!
+        var result = smart.Format("{0:{}|other}", arg);
+        Assert.That(result, Is.EqualTo(expected));
+    }
+
     [TestCase(0)]
     [TestCase(1)]
     [TestCase(100)]
@@ -337,34 +362,4 @@ public void Pluralization_With_Changed_SplitChar(int numOfPeople, string format,
         var result = smart.Format(format, data);
         Assert.That(result, Is.EqualTo(expected));
     }
-
-    [Test]
-    public void DoesNotHandle_Bool_WhenCanAutoDetect_IsTrue()
-    {
-        var smart = new SmartFormatter()
-            .AddExtensions(new DefaultSource())
-            .AddExtensions(new PluralLocalizationFormatter { CanAutoDetect = true }, // Should not handle the bool
-            new ConditionalFormatter { CanAutoDetect = true }, // Should handle the bool
-            new DefaultFormatter());
-
-        var result = smart.Format(new CultureInfo("ar"), "{0:yes|no}", true);
-        Assert.That(result, Is.EqualTo("yes"));
-    }
-
-    [TestCase("A", "[A]")]
-    [TestCase(default(string?), "null")]
-    public void DoesNotHandle_NonDecimalValues_WhenCanAutoDetect_IsTrue(string? category, string expected)
-    {
-        var smart = new SmartFormatter()
-            .AddExtensions(new DefaultSource())
-            .AddExtensions(new ReflectionSource())
-            // Should not handle because "Category" value cannot convert to decimal
-            .AddExtensions(new PluralLocalizationFormatter { CanAutoDetect = true },
-                // Should detect and handle the format
-                new ConditionalFormatter { CanAutoDetect = true },
-                new DefaultFormatter());
-
-        var result = smart.Format(new CultureInfo("en"), "{Category:[{}]|null}", new { Category = category });
-        Assert.That(result, Is.EqualTo(expected));
-    }
 }

src/SmartFormat/Extensions/PluralLocalizationFormatter.cs

@@ -14,6 +14,7 @@ namespace SmartFormat.Extensions;
 
 /// <summary>
 /// A class to format following culture specific pluralization rules.
+/// The maximum value the formatter can process is <see cref="decimal.MaxValue"/>.
 /// </summary>
 public class PluralLocalizationFormatter : IFormatter
 {
@@ -79,7 +80,7 @@ public PluralLocalizationFormatter(string defaultTwoLetterIsoLanguageName)
     /// called by its name in the input format string.
     /// <para/>
     /// <b>Auto detection only works with more than 1 format argument.
-    /// Is recommended to set <see cref="CanAutoDetect"/> to <see langword="false"/>. This will be the default in a future version.
+    /// It is recommended to set <see cref="CanAutoDetect"/> to <see langword="false"/>. This will be the default in a future version.
     /// </b>
     /// </summary>
     /// <remarks>
@@ -102,44 +103,42 @@ public bool TryEvaluateFormat(IFormattingInfo formattingInfo)
     {
         var format = formattingInfo.Format;
         var current = formattingInfo.CurrentValue;
-
-        if (format == null) return false;
 
-        // Extract the plural words from the format string:
+        if (format == null) return false;
+
+        // Extract the plural words from the format string
         var pluralWords = format.Split(SplitChar);
 
+        var useAutoDetection = string.IsNullOrEmpty(formattingInfo.Placeholder?.FormatterName);
+
         // This extension requires at least two plural words for auto detection
-        // For locales
-        if (pluralWords.Count <= 1 && string.IsNullOrEmpty(formattingInfo.Placeholder?.FormatterName))
-        {
-            // Auto detection calls just return a failure to evaluate
-            return false;
-        }
+        // Valid types for auto detection are checked later
+        if (useAutoDetection && pluralWords.Count <= 1) return false;
 
         decimal value;
 
-        // Check whether arguments can be handled by this formatter
-
-        // We can format numbers, and IEnumerables. For IEnumerables we look at the number of items
-        // in the collection: this means the user can e.g. use the same parameter for both plural and list, for example
-        // 'Smart.Format("The following {0:plural:person is|people are} impressed: {0:list:{}|, |, and}", new[] { "bob", "alice" });'
+        /*
+         Check whether arguments can be handled by this formatter:
 
+         We can format numbers, and IEnumerables. For IEnumerables we look at the number of items
+         in the collection: this means the user can e.g. use the same parameter for both plural and list, for example
+         'Smart.Format("The following {0:plural:person is|people are} impressed: {0:list:{}|, |, and}", new[] { "bob", "alice" });'
+        */
         switch (current)
         {
-            case IConvertible convertible when current is not bool && TryGetDecimalValue(convertible, null, out value):
+            case IConvertible convertible when convertible is not bool or string && TryGetDecimalValue(convertible, null, out value):
                 break;
             case IEnumerable<object> objects:
                 value = objects.Count();
                 break;
             default:
             {
                 // Auto detection calls just return a failure to evaluate
-                if (string.IsNullOrEmpty(formattingInfo.Placeholder?.FormatterName))
-                    return false;
+                if (useAutoDetection) return false;
 
                 // throw, if the formatter has been called explicitly
-                throw new FormatException(
-                    $"Formatter named '{formattingInfo.Placeholder?.FormatterName}' can format numbers and IEnumerables, but the argument was of type '{current?.GetType().ToString() ?? "null"}'");
+                throw new FormattingException(format,
+                    $"Formatter named '{formattingInfo.Placeholder?.FormatterName}' can format numbers and IEnumerables, but the argument was of type '{current?.GetType().ToString() ?? "null"}'", 0);
             }
         }