Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the ICU time format conversion logic #103681

Merged
merged 13 commits into from
Jun 21, 2024
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -364,32 +364,59 @@ private static string ConvertIcuTimeFormatString(ReadOnlySpan<char> icuFormatStr
char current = icuFormatString[i];
switch (current)
{
case '\\':
PopSlime marked this conversation as resolved.
Show resolved Hide resolved
// The ICU format does not use backslashes to escape while the .NET format does
result[resultPos++] = '\\';
PopSlime marked this conversation as resolved.
Show resolved Hide resolved
result[resultPos++] = '\\';
break;
case '\'':
result[resultPos++] = icuFormatString[i++];
while (i < icuFormatString.Length)
static bool HandleQuoteLiteral(ReadOnlySpan<char> icuFormatString, ref int i, Span<char> result, ref int resultPos)
{
if (i + 1 < icuFormatString.Length && icuFormatString[i + 1] == '\'')
{
result[resultPos++] = '\\';
result[resultPos++] = '\'';
i++;
return true;
}
result[resultPos++] = '\'';
return false;
PopSlime marked this conversation as resolved.
Show resolved Hide resolved
}

if (HandleQuoteLiteral(icuFormatString, ref i, result, ref resultPos))
{
break;
}
i++;
for (; i < icuFormatString.Length; i++)
{
current = icuFormatString[i];
result[resultPos++] = current;
if (current == '\'')
{
if (HandleQuoteLiteral(icuFormatString, ref i, result, ref resultPos))
{
continue;
}
break;
}
i++;
if (current == '\\')
{
// The same backslash escaping mentioned above
result[resultPos++] = '\\';
}
result[resultPos++] = current;
}
break;

case ':':
case '.':
case 'H':
case 'h':
case 'm':
case 's':
case ' ':
case '\u00A0': // no-break space
case '\u202F': // narrow no-break space
result[resultPos++] = current;
break;
case 'a': // AM/PM
case 'b': // am, pm, noon, midnight
case 'B': // flexible day periods
tarekgh marked this conversation as resolved.
Show resolved Hide resolved
if (!amPmAdded)
{
amPmAdded = true;
Expand All @@ -398,6 +425,13 @@ private static string ConvertIcuTimeFormatString(ReadOnlySpan<char> icuFormatStr
}
break;

default:
// Characters that are not ASCII letters are literal texts
if (!char.IsAsciiLetter(current))
{
result[resultPos++] = current;
}
break;
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Runtime.InteropServices;
using Xunit;

Expand Down Expand Up @@ -58,5 +59,19 @@ public static Exception GetCultureNotSupportedException(CultureInfo cultureInfo)
cultureInfo.Name,
cultureInfo.Calendar.GetType().Name));
}

// These cultures have bad ICU time patterns below the corresponding versions
// They are excluded from the VerifyTimePatterns tests
public static readonly Dictionary<string, Version> _badIcuTimePatterns = new Dictionary<string, Version>()
{
{ "mi", new Version(65, 0) },
{ "mi-NZ", new Version(65, 0) },
};
public static bool HasBadIcuTimePatterns(CultureInfo culture)
{
return PlatformDetection.IsIcuGlobalizationAndNotHybridOnBrowser
&& _badIcuTimePatterns.TryGetValue(culture.Name, out var version)
&& PlatformDetection.ICUVersion < version;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,41 @@ public void LongTimePattern_CheckReadingTimeFormatWithSingleQuotes_ICU()
}
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
public void LongTimePattern_VerifyTimePatterns()
{
Assert.All(CultureInfo.GetCultures(CultureTypes.AllCultures), culture => {
if (DateTimeFormatInfoData.HasBadIcuTimePatterns(culture))
{
return;
}
var pattern = culture.DateTimeFormat.LongTimePattern;
bool use24Hour = false;
bool use12Hour = false;
bool useAMPM = false;
for (var i = 0; i < pattern.Length; i++)
{
switch (pattern[i])
{
case 'H': use24Hour = true; break;
case 'h': use12Hour = true; break;
case 't': useAMPM = true; break;
case '\\': i++; break;
case '\'':
i++;
for (; i < pattern.Length; i++)
{
var c = pattern[i];
if (c == '\'') break;
if (c == '\\') i++;
}
break;
}
}
Assert.True((use24Hour || useAMPM) && (use12Hour ^ use24Hour), $"Bad long time pattern for culture {culture.Name}: '{pattern}'");
});
}

[Fact]
public void LongTimePattern_CheckTimeFormatWithSpaces()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,41 @@ public void ShortTimePattern_SetReadOnly_ThrowsInvalidOperationException()
Assert.Throws<InvalidOperationException>(() => DateTimeFormatInfo.InvariantInfo.ShortTimePattern = "HH:mm");
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
public void ShortTimePattern_VerifyTimePatterns()
{
Assert.All(CultureInfo.GetCultures(CultureTypes.AllCultures), culture => {
if (DateTimeFormatInfoData.HasBadIcuTimePatterns(culture))
{
return;
}
var pattern = culture.DateTimeFormat.ShortTimePattern;
bool use24Hour = false;
bool use12Hour = false;
bool useAMPM = false;
for (var i = 0; i < pattern.Length; i++)
{
switch (pattern[i])
{
case 'H': use24Hour = true; break;
case 'h': use12Hour = true; break;
case 't': useAMPM = true; break;
case '\\': i++; break;
case '\'':
i++;
for (; i < pattern.Length; i++)
{
var c = pattern[i];
if (c == '\'') break;
if (c == '\\') i++;
}
break;
}
}
Assert.True((use24Hour || useAMPM) && (use12Hour ^ use24Hour), $"Bad short time pattern for culture {culture.Name}: '{pattern}'");
});
}

[Fact]
public void ShortTimePattern_CheckTimeFormatWithSpaces()
{
Expand Down
Loading