Globalization update from CoreRT - part 2 (#7020)

- Persian calendar date formatting
- Fix IsPrefix handling of empty strings
- Formatting changes

6 files changed, 72 insertions, 40 deletions

diff --git a/src/mscorlib/corefx/System/Globalization/CalendarData.cs b/src/mscorlib/corefx/System/Globalization/CalendarData.cs
index 3b89b1eb55..2dbd1b8069 100644
--- a/src/mscorlib/corefx/System/Globalization/CalendarData.cs
+++ b/src/mscorlib/corefx/System/Globalization/CalendarData.cs
@@ -242,6 +242,13 @@ namespace System.Globalization
                     this.saEraNames = JapaneseCalendar.EraNames();
                     break;
 
+                case CalendarId.PERSIAN:
+                    if (this.saEraNames == null || this.saEraNames.Length == 0 || String.IsNullOrEmpty(this.saEraNames[0]))
+                    {
+                        this.saEraNames = new String[] { "\x0647\x002e\x0634" };
+                    }
+                    break;
+
                 default:
                     // Most calendars are just "A.D."
                     this.saEraNames = Invariant.saEraNames;
@@ -296,6 +303,14 @@ namespace System.Globalization
                         this.saAbbrevEraNames[0] = this.saEraNames[0];
                     }
                     break;
+
+                case CalendarId.PERSIAN:
+                    if (this.saAbbrevEraNames == null || this.saAbbrevEraNames.Length == 0 || String.IsNullOrEmpty(this.saAbbrevEraNames[0]))
+                    {
+                        this.saAbbrevEraNames = this.saEraNames;
+                    }
+                    break;
+
                 default:
                     // Most calendars just use the full name
                     this.saAbbrevEraNames = this.saEraNames;
@@ -313,7 +328,7 @@ namespace System.Globalization
             //
 
             // Get a culture name
-            // TODO: NLS Arrowhead Arrowhead - note that this doesn't handle the new calendars (lunisolar, etc)
+            // TODO: Note that this doesn't handle the new calendars (lunisolar, etc)
             String culture = CalendarIdToCultureName(calendarId);
 
             // Return our calendar
diff --git a/src/mscorlib/corefx/System/Globalization/CompareInfo.Windows.cs b/src/mscorlib/corefx/System/Globalization/CompareInfo.Windows.cs
index d7d0e9c8e6..744a48b107 100644
--- a/src/mscorlib/corefx/System/Globalization/CompareInfo.Windows.cs
+++ b/src/mscorlib/corefx/System/Globalization/CompareInfo.Windows.cs
@@ -79,13 +79,16 @@ namespace System.Globalization
 
             int tmpHash = 0;
 
-            if (Interop.mincore.LCMapStringEx(_sortHandle != IntPtr.Zero ? null : _sortName,
-                                              LCMAP_HASH | (uint)GetNativeCompareFlags(options),
-                                              source, source.Length,
-                                              &tmpHash, sizeof(int),
-                                              null, null, _sortHandle) == 0)
+            fixed (char* pSource = source)
             {
-                Environment.FailFast("LCMapStringEx failed!");
+                if (Interop.mincore.LCMapStringEx(_sortHandle != IntPtr.Zero ? null : _sortName,
+                                                  LCMAP_HASH | (uint)GetNativeCompareFlags(options),
+                                                  pSource, source.Length,
+                                                  &tmpHash, sizeof(int),
+                                                  null, null, _sortHandle) == 0)
+                {
+                    Environment.FailFast("LCMapStringEx failed!");
+                }
             }
 
             return tmpHash;
@@ -168,6 +171,8 @@ namespace System.Globalization
             Contract.Assert(target != null);
             Contract.Assert((options & CompareOptions.OrdinalIgnoreCase) == 0);
 
+            // TODO: Consider moving this up to the relevent APIs we need to ensure this behavior for
+            // and add a precondition that target is not empty. 
             if (target.Length == 0)
                 return startIndex;       // keep Whidbey compatibility
 
@@ -199,6 +204,8 @@ namespace System.Globalization
             Contract.Assert(target != null);
             Contract.Assert((options & CompareOptions.OrdinalIgnoreCase) == 0);
 
+            // TODO: Consider moving this up to the relevent APIs we need to ensure this behavior for
+            // and add a precondition that target is not empty. 
             if (target.Length == 0)
                 return startIndex;       // keep Whidbey compatibility
 
@@ -266,6 +273,7 @@ namespace System.Globalization
         private const int FIND_FROMSTART = 0x00400000;
         private const int FIND_FROMEND = 0x00800000;
 
+        // TODO: Instead of this method could we just have upstack code call IndexOfOrdinal with ignoreCase = false?
         private static unsafe int FastIndexOfString(string source, string target, int startIndex, int sourceCount, int targetCount, bool findLastIndex)
         {
             int retValue = -1;
@@ -367,6 +375,9 @@ namespace System.Globalization
             if ((options & CompareOptions.IgnoreWidth) != 0) { nativeCompareFlags |= NORM_IGNOREWIDTH; }
             if ((options & CompareOptions.StringSort) != 0) { nativeCompareFlags |= SORT_STRINGSORT; }
 
+            // TODO: Can we try for GetNativeCompareFlags to never
+            // take Ordinal or OrdinalIgnoreCase.  This value is not part of Win32, we just handle it special
+            // in some places.
             // Suffix & Prefix shouldn't use this, make sure to turn off the NORM_LINGUISTIC_CASING flag
             if (options == CompareOptions.Ordinal) { nativeCompareFlags = COMPARE_OPTIONS_ORDINAL; }
 
diff --git a/src/mscorlib/corefx/System/Globalization/CompareInfo.cs b/src/mscorlib/corefx/System/Globalization/CompareInfo.cs
index 3f819ddf93..77778af23c 100644
--- a/src/mscorlib/corefx/System/Globalization/CompareInfo.cs
+++ b/src/mscorlib/corefx/System/Globalization/CompareInfo.cs
@@ -379,13 +379,17 @@ namespace System.Globalization
                     SR.ArgumentNull_String);
             }
             Contract.EndContractBlock();
-            int prefixLen = prefix.Length;
 
-            if (prefixLen == 0)
+            if (prefix.Length == 0)
             {
                 return (true);
             }
 
+            if (source.Length == 0)
+            {
+                return false;
+            }
+
             if (options == CompareOptions.OrdinalIgnoreCase)
             {
                 return source.StartsWith(prefix, StringComparison.OrdinalIgnoreCase);
@@ -425,15 +429,13 @@ namespace System.Globalization
                     SR.ArgumentNull_String);
             }
             Contract.EndContractBlock();
-            int suffixLen = suffix.Length;
-            int sourceLength = source.Length;
 
-            if (suffixLen == 0)
+            if (suffix.Length == 0)
             {
                 return (true);
             }
 
-            if (sourceLength == 0)
+            if (source.Length == 0)
             {
                 return false;
             }
diff --git a/src/mscorlib/corefx/System/Globalization/CultureInfo.cs b/src/mscorlib/corefx/System/Globalization/CultureInfo.cs
index 5e8e6254fd..f2b3742ab4 100644
--- a/src/mscorlib/corefx/System/Globalization/CultureInfo.cs
+++ b/src/mscorlib/corefx/System/Globalization/CultureInfo.cs
@@ -120,6 +120,11 @@ namespace System.Globalization
         // All of the following will be created on demand.
         //
 
+        // WARNING: We allow diagnostic tools to directly inspect these three members (s_InvariantCultureInfo, s_DefaultThreadCurrentUICulture and s_DefaultThreadCurrentCulture)
+        // See https://github.com/dotnet/corert/blob/master/Documentation/design-docs/diagnostics/diagnostics-tools-contract.md for more details. 
+        // Please do not change the type, the name, or the semantic usage of this member without understanding the implication for tools. 
+        // Get in touch with the diagnostics team if you have questions.
+
         //The Invariant culture;
         private static volatile CultureInfo s_InvariantCultureInfo;
 
@@ -212,6 +217,8 @@ namespace System.Globalization
         // We do this to try to return the system UI language and the default user languages
         // This method will fallback if this fails (like Invariant)
         //
+        // TODO: It would appear that this is only ever called with userOveride = true
+        // and this method only has one caller.  Can we fold it into the caller?
         private static CultureInfo GetCultureByName(String name, bool userOverride)
         {
             CultureInfo ci = null;
@@ -243,15 +250,14 @@ namespace System.Globalization
 
         internal static bool VerifyCultureName(String cultureName, bool throwException)
         {
-            // This function is used by ResourceManager.GetResourceFileName(). 
+            // This function is used by ResourceManager.GetResourceFileName().
             // ResourceManager searches for resource using CultureInfo.Name,
             // so we should check against CultureInfo.Name.
 
             for (int i = 0; i < cultureName.Length; i++)
             {
                 char c = cultureName[i];
-                // TODO: NLS Arrowhead - This is broken, names can only be RFC4646 names (ie: a-zA-Z0-9).
-                // TODO: NLS Arrowhead - This allows any unicode letter/digit
+                // TODO: Names can only be RFC4646 names (ie: a-zA-Z0-9) while this allows any unicode letter/digit
                 if (Char.IsLetterOrDigit(c) || c == '-' || c == '_')
                 {
                     continue;
@@ -416,10 +422,9 @@ namespace System.Globalization
         public static CultureInfo DefaultThreadCurrentCulture
         {
             get { return s_DefaultThreadCurrentCulture; }
-            [System.Security.SecuritySafeCritical]  // auto-generated
             set
             {
-                // If you add pre-conditions to this method, check to see if you also need to 
+                // If you add pre-conditions to this method, check to see if you also need to
                 // add them to Thread.CurrentCulture.set.
 
                 s_DefaultThreadCurrentCulture = value;
@@ -429,13 +434,12 @@ namespace System.Globalization
         public static CultureInfo DefaultThreadCurrentUICulture
         {
             get { return s_DefaultThreadCurrentUICulture; }
-            [System.Security.SecuritySafeCritical]  // auto-generated
             set
             {
                 //If they're trying to use a Culture with a name that we can't use in resource lookup,
                 //don't even let them set it on the thread.
 
-                // If you add more pre-conditions to this method, check to see if you also need to 
+                // If you add more pre-conditions to this method, check to see if you also need to
                 // add them to Thread.CurrentUICulture.set.
 
                 if (value != null)
@@ -560,7 +564,6 @@ namespace System.Globalization
         ////////////////////////////////////////////////////////////////////////
         public virtual String DisplayName
         {
-            [System.Security.SecuritySafeCritical]  // auto-generated
             get
             {
                 Contract.Ensures(Contract.Result<String>() != null);
@@ -581,7 +584,6 @@ namespace System.Globalization
         ////////////////////////////////////////////////////////////////////////
         public virtual String NativeName
         {
-            [System.Security.SecuritySafeCritical]  // auto-generated
             get
             {
                 Contract.Ensures(Contract.Result<String>() != null);
@@ -600,7 +602,6 @@ namespace System.Globalization
         ////////////////////////////////////////////////////////////////////////
         public virtual String EnglishName
         {
-            [System.Security.SecuritySafeCritical]  // auto-generated
             get
             {
                 Contract.Ensures(Contract.Result<String>() != null);
@@ -611,7 +612,6 @@ namespace System.Globalization
         // ie: en
         public virtual String TwoLetterISOLanguageName
         {
-            [System.Security.SecuritySafeCritical]  // auto-generated
             get
             {
                 Contract.Ensures(Contract.Result<String>() != null);
diff --git a/src/mscorlib/corefx/System/Globalization/JapaneseCalendar.Windows.cs b/src/mscorlib/corefx/System/Globalization/JapaneseCalendar.Windows.cs
index 3636e90d74..6a9df97200 100644
--- a/src/mscorlib/corefx/System/Globalization/JapaneseCalendar.Windows.cs
+++ b/src/mscorlib/corefx/System/Globalization/JapaneseCalendar.Windows.cs
@@ -2,6 +2,8 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Diagnostics;
+
 using Internal.Runtime.Augments;
 
 namespace System.Globalization
diff --git a/src/mscorlib/corefx/System/Globalization/TextInfo.Windows.cs b/src/mscorlib/corefx/System/Globalization/TextInfo.Windows.cs
index 0dce392d35..238c51217b 100644
--- a/src/mscorlib/corefx/System/Globalization/TextInfo.Windows.cs
+++ b/src/mscorlib/corefx/System/Globalization/TextInfo.Windows.cs
@@ -51,7 +51,7 @@ namespace System.Globalization
             }
             else
             {
-                int result;
+                int ret;
 
                 // Check for Invariant to avoid A/V in LCMapStringEx
                 uint linguisticCasing = IsInvariantLocale(_textInfoName) ? 0 : LCMAP_LINGUISTIC_CASING;
@@ -59,27 +59,29 @@ namespace System.Globalization
                 //
                 //  Create the result string.
                 //
-                char[] buffer = new char[nLengthInput];
-                fixed (char* pBuffer = buffer)
+                string result = string.FastAllocateString(nLengthInput);
+
+                fixed (char* pSource = s)
+                fixed (char* pResult = result)
                 {
-                    result = Interop.mincore.LCMapStringEx(_sortHandle != IntPtr.Zero ? null : _textInfoName,
-                                                           toUpper ? LCMAP_UPPERCASE | linguisticCasing : LCMAP_LOWERCASE | linguisticCasing,
-                                                           s,
-                                                           nLengthInput,
-                                                           pBuffer,
-                                                           nLengthInput,
-                                                           null,
-                                                           null,
-                                                           _sortHandle);
+                    ret = Interop.mincore.LCMapStringEx(_sortHandle != IntPtr.Zero ? null : _textInfoName,
+                                                        toUpper ? LCMAP_UPPERCASE | linguisticCasing : LCMAP_LOWERCASE | linguisticCasing,
+                                                        pSource,
+                                                        nLengthInput,
+                                                        pResult,
+                                                        nLengthInput,
+                                                        null,
+                                                        null,
+                                                        _sortHandle);
                 }
 
-                if (0 == result)
+                if (0 == ret)
                 {
                     throw new InvalidOperationException(SR.InvalidOperation_ReadOnly);
                 }
 
-                Contract.Assert(result == nLengthInput, "Expected getting the same length of the original string");
-                return new string(buffer, 0, result);
+                Contract.Assert(ret == nLengthInput, "Expected getting the same length of the original string");
+                return result;
             }
         }
 
@@ -92,7 +94,7 @@ namespace System.Globalization
 
             Interop.mincore.LCMapStringEx(_sortHandle != IntPtr.Zero ? null : _textInfoName,
                                           toUpper ? LCMAP_UPPERCASE | linguisticCasing : LCMAP_LOWERCASE | linguisticCasing,
-                                          new string(c, 1),
+                                          &c,
                                           1,
                                           &retVal,
                                           1,