Skip to content

Commit 4b9a1b2

Browse files
PopSlimePopSlime
authored
Fix the ICU time format conversion logic (#103681)
* Fix the ICU time format conversion logic Revise the ICU time format conversion logic to support all unquoted literal texts and the `B` and `b` pattern symbols. Fix #103592 * Clarify literal texts in the conversion logic * Add tests for verifying time patterns Add tests verifying that all the short and long time patterns either use a 24-hour clock or have an AM/PM designator. * Fix literal single quote and literal backslash conversion * Refactor the literal quote conversion logic * Revise the test logic to ignore literal texts and check pattern redundancy Modify the test logic so that it recognizes literal texts correctly, and fails if 12-hour and 24-hour clocks are used at the same time. * Revise the test logic to ensure all cultures are tested * Add comments to clarify the backslash conversion * Refactor the conversion logic Simplify some logic and improve readability. * Exclude bad ICU patterns from the tests * Exclude the VerifyTimePatterns tests from hybrid globalization on browser * Add missing usings * Improve readability of the for-loops
1 parent 078d720 commit 4b9a1b2

File tree

4 files changed

+128
-9
lines changed

4 files changed

+128
-9
lines changed

‎src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Icu.cs

+43-9
Original file line numberDiff line numberDiff line change
@@ -364,32 +364,59 @@ private static string ConvertIcuTimeFormatString(ReadOnlySpan<char> icuFormatStr
364364
char current = icuFormatString[i];
365365
switch (current)
366366
{
367+
case '\\':
368+
// The ICU format does not use backslashes to escape while the .NET format does
369+
result[resultPos++] = '\\';
370+
result[resultPos++] = '\\';
371+
break;
367372
case '\'':
368-
result[resultPos++] = icuFormatString[i++];
369-
while (i < icuFormatString.Length)
373+
static bool HandleQuoteLiteral(ReadOnlySpan<char> icuFormatString, ref int i, Span<char> result, ref int resultPos)
374+
{
375+
if (i + 1 < icuFormatString.Length && icuFormatString[i + 1] == '\'')
376+
{
377+
result[resultPos++] = '\\';
378+
result[resultPos++] = '\'';
379+
i++;
380+
return true;
381+
}
382+
result[resultPos++] = '\'';
383+
return false;
384+
}
385+
386+
if (HandleQuoteLiteral(icuFormatString, ref i, result, ref resultPos))
387+
{
388+
break;
389+
}
390+
i++;
391+
for (; i < icuFormatString.Length; i++)
370392
{
371393
current = icuFormatString[i];
372-
result[resultPos++] = current;
373394
if (current == '\'')
374395
{
396+
if (HandleQuoteLiteral(icuFormatString, ref i, result, ref resultPos))
397+
{
398+
continue;
399+
}
375400
break;
376401
}
377-
i++;
402+
if (current == '\\')
403+
{
404+
// The same backslash escaping mentioned above
405+
result[resultPos++] = '\\';
406+
}
407+
result[resultPos++] = current;
378408
}
379409
break;
380410

381-
case ':':
382-
case '.':
383411
case 'H':
384412
case 'h':
385413
case 'm':
386414
case 's':
387-
case ' ':
388-
case '\u00A0': // no-break space
389-
case '\u202F': // narrow no-break space
390415
result[resultPos++] = current;
391416
break;
392417
case 'a': // AM/PM
418+
case 'b': // am, pm, noon, midnight
419+
case 'B': // flexible day periods
393420
if (!amPmAdded)
394421
{
395422
amPmAdded = true;
@@ -398,6 +425,13 @@ private static string ConvertIcuTimeFormatString(ReadOnlySpan<char> icuFormatStr
398425
}
399426
break;
400427

428+
default:
429+
// Characters that are not ASCII letters are literal texts
430+
if (!char.IsAsciiLetter(current))
431+
{
432+
result[resultPos++] = current;
433+
}
434+
break;
401435
}
402436
}
403437

‎src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoData.cs

+15
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.Collections.Generic;
45
using System.Runtime.InteropServices;
56
using Xunit;
67

@@ -58,5 +59,19 @@ public static Exception GetCultureNotSupportedException(CultureInfo cultureInfo)
5859
cultureInfo.Name,
5960
cultureInfo.Calendar.GetType().Name));
6061
}
62+
63+
// These cultures have bad ICU time patterns below the corresponding versions
64+
// They are excluded from the VerifyTimePatterns tests
65+
public static readonly Dictionary<string, Version> _badIcuTimePatterns = new Dictionary<string, Version>()
66+
{
67+
{ "mi", new Version(65, 0) },
68+
{ "mi-NZ", new Version(65, 0) },
69+
};
70+
public static bool HasBadIcuTimePatterns(CultureInfo culture)
71+
{
72+
return PlatformDetection.IsIcuGlobalizationAndNotHybridOnBrowser
73+
&& _badIcuTimePatterns.TryGetValue(culture.Name, out var version)
74+
&& PlatformDetection.ICUVersion < version;
75+
}
6176
}
6277
}

‎src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongTimePattern.cs

+35
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,41 @@ public void LongTimePattern_CheckReadingTimeFormatWithSingleQuotes_ICU()
284284
}
285285
}
286286

287+
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
288+
public void LongTimePattern_VerifyTimePatterns()
289+
{
290+
Assert.All(CultureInfo.GetCultures(CultureTypes.AllCultures), culture => {
291+
if (DateTimeFormatInfoData.HasBadIcuTimePatterns(culture))
292+
{
293+
return;
294+
}
295+
var pattern = culture.DateTimeFormat.LongTimePattern;
296+
bool use24Hour = false;
297+
bool use12Hour = false;
298+
bool useAMPM = false;
299+
for (var i = 0; i < pattern.Length; i++)
300+
{
301+
switch (pattern[i])
302+
{
303+
case 'H': use24Hour = true; break;
304+
case 'h': use12Hour = true; break;
305+
case 't': useAMPM = true; break;
306+
case '\\': i++; break;
307+
case '\'':
308+
i++;
309+
for (; i < pattern.Length; i++)
310+
{
311+
var c = pattern[i];
312+
if (c == '\'') break;
313+
if (c == '\\') i++;
314+
}
315+
break;
316+
}
317+
}
318+
Assert.True((use24Hour || useAMPM) && (use12Hour ^ use24Hour), $"Bad long time pattern for culture {culture.Name}: '{pattern}'");
319+
});
320+
}
321+
287322
[Fact]
288323
public void LongTimePattern_CheckTimeFormatWithSpaces()
289324
{

‎src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoShortTimePattern.cs

+35
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,41 @@ public void ShortTimePattern_SetReadOnly_ThrowsInvalidOperationException()
255255
Assert.Throws<InvalidOperationException>(() => DateTimeFormatInfo.InvariantInfo.ShortTimePattern = "HH:mm");
256256
}
257257

258+
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
259+
public void ShortTimePattern_VerifyTimePatterns()
260+
{
261+
Assert.All(CultureInfo.GetCultures(CultureTypes.AllCultures), culture => {
262+
if (DateTimeFormatInfoData.HasBadIcuTimePatterns(culture))
263+
{
264+
return;
265+
}
266+
var pattern = culture.DateTimeFormat.ShortTimePattern;
267+
bool use24Hour = false;
268+
bool use12Hour = false;
269+
bool useAMPM = false;
270+
for (var i = 0; i < pattern.Length; i++)
271+
{
272+
switch (pattern[i])
273+
{
274+
case 'H': use24Hour = true; break;
275+
case 'h': use12Hour = true; break;
276+
case 't': useAMPM = true; break;
277+
case '\\': i++; break;
278+
case '\'':
279+
i++;
280+
for (; i < pattern.Length; i++)
281+
{
282+
var c = pattern[i];
283+
if (c == '\'') break;
284+
if (c == '\\') i++;
285+
}
286+
break;
287+
}
288+
}
289+
Assert.True((use24Hour || useAMPM) && (use12Hour ^ use24Hour), $"Bad short time pattern for culture {culture.Name}: '{pattern}'");
290+
});
291+
}
292+
258293
[Fact]
259294
public void ShortTimePattern_CheckTimeFormatWithSpaces()
260295
{

0 commit comments

Comments
 (0)