Fix: Auto-detection of PluralLocalizationFormatter does not throw for…

… values not convertible to decimal (#330)

* Resolves #329

Current behavior, introduced in v3.2.0:
When PluralLocalizationFormatter.CanAutoDetect == true, values not convertible to decimal will throw then trying to IConvertible.ToDecimal(...)

New behavior:
When PluralLocalizationFormatter.CanAutoDetect == true, for values not convertible to decimal, IFormatter.TryEvaluateFormat(...) will return false

* Make PluralLocalizationFormatter.TryGetDecimalValue static

.github/workflows/SonarCloud.yml

@@ -13,14 +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.1.0'
-      versionFile: '3.1.0'
+      version: '3.2.1'
+      versionFile: '3.2.1'
     steps:
       - name: Set up JDK 11
         uses: actions/setup-java@v1
         with:
           java-version: 1.11
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
         with:
           fetch-depth: 0   # Shallow clones should be disabled for a better relevancy of analysis
       - name: Cache SonarCloud packages
@@ -60,7 +60,7 @@ jobs:
           echo "Packing Version: ${{ env.version }}, File Version: ${{ env.versionFile }}"
           dotnet pack ./src/SmartFormat.Deploy.sln /verbosity:minimal --configuration release /p:IncludeSymbols=true /p:SymbolPackageFormat=snupkg /p:ContinuousIntegrationBuild=true /p:PackageOutputPath=${{ github.workspace }}/artifacts /p:Version=${{ env.version }} /p:FileVersion=${{ env.versionFile }}
       - name: Upload Artifacts
-        uses: actions/upload-artifact@v2
+        uses: actions/upload-artifact@v3
         with:
           name: Packages_${{ env.version }}
           path: ${{ github.workspace }}/artifacts/

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.0"
+          $version = "3.2.1"
           $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.0</Version>
-        <FileVersion>3.2.0</FileVersion>
+        <Version>3.2.1</Version>
+        <FileVersion>3.2.1</FileVersion>
         <AssemblyVersion>3.0.0</AssemblyVersion> <!--only update AssemblyVersion with major releases -->
         <LangVersion>latest</LangVersion>
         <EnableNETAnalyzers>true</EnableNETAnalyzers>

src/SmartFormat.Tests/Extensions/PluralLocalizationFormatterTests.cs

@@ -350,4 +350,21 @@ public void DoesNotHandle_Bool_WhenCanAutoDetect_IsTrue()
         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

@@ -123,19 +123,24 @@ public bool TryEvaluateFormat(IFormattingInfo formattingInfo)
         // 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" });'
-        if (current is IConvertible convertible && current is not bool)
-            value = convertible.ToDecimal(null);
-        else if (current is IEnumerable<object> objects)
-            value = objects.Count();
-        else
+
+        switch (current)
         {
-            // Auto detection calls just return a failure to evaluate
-            if (string.IsNullOrEmpty(formattingInfo.Placeholder?.FormatterName))
-                return false;
+            case IConvertible convertible when current is not bool && 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;
 
-            // 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, 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"}'");
+            }
         }
 
         // Get the specific plural rule, or the default rule:
@@ -154,6 +159,20 @@ public bool TryEvaluateFormat(IFormattingInfo formattingInfo)
         return true;
     }
 
+    private static bool TryGetDecimalValue(IConvertible convertible, IFormatProvider? provider,  out decimal value)
+    {
+        try
+        {
+            value = convertible.ToDecimal(provider);
+            return true;
+        }
+        catch
+        {
+            value = default;
+            return false;
+        }
+    }
+
     private static PluralRules.PluralRuleDelegate GetPluralRule(IFormattingInfo formattingInfo)
     {
         // Determine the culture