Resolves #345 (#346)

* Resolves #345 * PluralLocalizationFormatter does treat numeric string as valid argument * Restore behavior of v3.1.0 and before (don't convert numeric string to decimal for arg values) * Bump version to v3.2.2 * Update appveyor and github CI scripts
axuno · Aug 3, 2023 · e98917a · e98917a
1 parent c8a99c0
commit e98917a
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 62 deletions.
diff --git a/.github/workflows/SonarCloud.yml b/.github/workflows/SonarCloud.yml
@@ -13,25 +13,26 @@ 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
       - name: Cache SonarCloud packages
-        uses: actions/cache@v1
+        uses: actions/cache@v3
         with:
           path: ~\sonar\cache
           key: ${{ runner.os }}-sonar
           restore-keys: ${{ runner.os }}-sonar
       - name: Cache SonarCloud scanner
         id: cache-sonar-scanner
-        uses: actions/cache@v1
+        uses: actions/cache@v3
         with:
           path: .\.sonar\scanner
           key: ${{ runner.os }}-sonar-scanner
@@ -48,13 +49,13 @@ jobs:
           SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
         shell: powershell
         run: |
-          .\.sonar\scanner\dotnet-sonarscanner begin /k:"${{ github.event.repository.owner.login }}_SmartFormat" /o:"${{ github.event.repository.owner.login }}" /d:sonar.login="${{ secrets.SONAR_TOKEN }}" /d:sonar.host.url="https://sonarcloud.io" /d:sonar.exclusions="**/SmartFormat.ZString/**/*" /d:sonar.cs.opencover.reportsPaths="./src/SmartFormat.Tests/**/coverage*.xml"
+          .\.sonar\scanner\dotnet-sonarscanner begin /k:"${{ github.event.repository.owner.login }}_SmartFormat" /o:"${{ github.event.repository.owner.login }}" /d:sonar.token="${{ secrets.SONAR_TOKEN }}" /d:sonar.host.url="https://sonarcloud.io" /d:sonar.exclusions="**/SmartFormat.ZString/**/*" /d:sonar.cs.opencover.reportsPaths="./src/SmartFormat.Tests/**/coverage*.xml"
           dotnet sln ./src/SmartFormat.sln remove ./src/Demo/Demo.csproj ./src/Demo.NetFramework/Demo.NetFramework.csproj ./src/Performance/Performance.csproj ./src/Performance_v27/Performance_v27.csproj 
           dotnet add ./src/SmartFormat.Tests/SmartFormat.Tests.csproj package AltCover
           dotnet restore ./src/SmartFormat.sln
           dotnet build ./src/SmartFormat.sln --no-restore /verbosity:minimal /t:rebuild /p:configuration=release /nowarn:CS1591,CS0618 /p:IncludeSymbols=true /p:SymbolPackageFormat=snupkg /p:ContinuousIntegrationBuild=true /p:Version=${{ env.version }} /p:FileVersion=${{ env.versionFile }}
           dotnet test ./src/SmartFormat.sln --no-build --verbosity normal /p:configuration=release /p:AltCover=true /p:AltCoverXmlReport="coverage.xml" /p:AltCoverStrongNameKey="../SmartFormat/SmartFormat.snk" /p:AltCoverAssemblyExcludeFilter="SmartFormat.Tests|SmartFormat.ZString|NUnit3.TestAdapter"
-          .\.sonar\scanner\dotnet-sonarscanner end /d:sonar.login="${{ secrets.SONAR_TOKEN }}"
+          .\.sonar\scanner\dotnet-sonarscanner end /d:sonar.token="${{ secrets.SONAR_TOKEN }}"
       - name: Pack
         run: |
           echo "Packing Version: ${{ env.version }}, File Version: ${{ env.versionFile }}"

diff --git a/appveyor.yml b/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) {

diff --git a/src/Directory.Build.props b/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>

diff --git a/src/SmartFormat.Tests/Extensions/PluralLocalizationFormatterTests.cs b/src/SmartFormat.Tests/Extensions/PluralLocalizationFormatterTests.cs
@@ -49,6 +49,32 @@ 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("1234")] // don't convert numeric string to decimal, see https://github.com/axuno/SmartFormat/issues/345
+    [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 +363,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));
-    }
 }
diff --git a/src/SmartFormat/Extensions/PluralLocalizationFormatter.cs b/src/SmartFormat/Extensions/PluralLocalizationFormatter.cs
@@ -14,6 +14,7 @@ namespace SmartFormat.Extensions;
 
 /// <summary>
 /// A class to format following culture specific pluralization rules.
+/// The range of values the formatter can process is from <see cref="decimal.MinValue"/> to <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);
             }
         }