From e98917a299292e0ff238c890d03e5b8ee0bd46e1 Mon Sep 17 00:00:00 2001 From: axunonb Date: Thu, 3 Aug 2023 17:12:09 +0200 Subject: [PATCH] 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 --- .github/workflows/SonarCloud.yml | 19 ++++--- appveyor.yml | 2 +- src/Directory.Build.props | 4 +- .../PluralLocalizationFormatterTests.cs | 56 +++++++++---------- .../Extensions/PluralLocalizationFormatter.cs | 39 +++++++------ 5 files changed, 58 insertions(+), 62 deletions(-) diff --git a/.github/workflows/SonarCloud.yml b/.github/workflows/SonarCloud.yml index 547f0cbb..0be93b08 100644 --- 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 index 1ab2889b..b352f4a0 100644 --- 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 index 3736ab5e..754858ee 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -8,8 +8,8 @@ Copyright 2011-$(CurrentYear) SmartFormat Project https://github.com/axuno/SmartFormat.git true - 3.2.1 - 3.2.1 + 3.2.2 + 3.2.2 3.0.0 latest true diff --git a/src/SmartFormat.Tests/Extensions/PluralLocalizationFormatterTests.cs b/src/SmartFormat.Tests/Extensions/PluralLocalizationFormatterTests.cs index 6c1e182b..352ca6fa 100644 --- 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()); } + [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(), "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 index ae637b36..33605324 100644 --- a/src/SmartFormat/Extensions/PluralLocalizationFormatter.cs +++ b/src/SmartFormat/Extensions/PluralLocalizationFormatter.cs @@ -14,6 +14,7 @@ namespace SmartFormat.Extensions; /// /// A class to format following culture specific pluralization rules. +/// The range of values the formatter can process is from to . /// public class PluralLocalizationFormatter : IFormatter { @@ -79,7 +80,7 @@ public PluralLocalizationFormatter(string defaultTwoLetterIsoLanguageName) /// called by its name in the input format string. /// /// Auto detection only works with more than 1 format argument. - /// Is recommended to set to . This will be the default in a future version. + /// It is recommended to set to . This will be the default in a future version. /// /// /// @@ -102,31 +103,30 @@ 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 objects: value = objects.Count(); @@ -134,12 +134,11 @@ public bool TryEvaluateFormat(IFormattingInfo formattingInfo) 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); } }