run on STJ

Author: EgorBo

src/libraries/Common/src/Polyfills/EncodingPolyfills.cs

@@ -10,19 +10,29 @@ namespace System.Text;
 /// <summary>Provides downlevel polyfills for span-based Encoding APIs.</summary>
 internal static class EncodingPolyfills
 {
-    public static unsafe int GetByteCount(this Encoding encoding, ReadOnlySpan<char> chars)
+    public static int GetByteCount(this Encoding encoding, ReadOnlySpan<char> chars)
     {
-        fixed (char* charsPtr = &GetNonNullPinnableReference(chars))
+        // SAFETY-TODO: review this unsafe usage.
+        // Either document why it's safe, refactor to remove it, or mark the enclosing member 'unsafe'.
+        unsafe
         {
-            return encoding.GetByteCount(charsPtr, chars.Length);
+            fixed (char* charsPtr = &GetNonNullPinnableReference(chars))
+            {
+                return encoding.GetByteCount(charsPtr, chars.Length);
+            }
         }
     }
 
-    public static unsafe int GetCharCount(this Encoding encoding, ReadOnlySpan<byte> bytes)
+    public static int GetCharCount(this Encoding encoding, ReadOnlySpan<byte> bytes)
     {
-        fixed (byte* bytesPtr = &GetNonNullPinnableReference(bytes))
+        // SAFETY-TODO: review this unsafe usage.
+        // Either document why it's safe, refactor to remove it, or mark the enclosing member 'unsafe'.
+        unsafe
         {
-            return encoding.GetCharCount(bytesPtr, bytes.Length);
+            fixed (byte* bytesPtr = &GetNonNullPinnableReference(bytes))
+            {
+                return encoding.GetCharCount(bytesPtr, bytes.Length);
+            }
         }
     }
 
@@ -31,29 +41,44 @@ public static int GetBytes(this Encoding encoding, string str, Span<byte> bytes)
         return GetBytes(encoding, str.AsSpan(), bytes);
     }
 
-    public static unsafe int GetBytes(this Encoding encoding, ReadOnlySpan<char> chars, Span<byte> bytes)
+    public static int GetBytes(this Encoding encoding, ReadOnlySpan<char> chars, Span<byte> bytes)
     {
-        fixed (char* charsPtr = &GetNonNullPinnableReference(chars))
-        fixed (byte* bytesPtr = &GetNonNullPinnableReference(bytes))
+        // SAFETY-TODO: review this unsafe usage.
+        // Either document why it's safe, refactor to remove it, or mark the enclosing member 'unsafe'.
+        unsafe
         {
-            return encoding.GetBytes(charsPtr, chars.Length, bytesPtr, bytes.Length);
+            fixed (char* charsPtr = &GetNonNullPinnableReference(chars))
+            fixed (byte* bytesPtr = &GetNonNullPinnableReference(bytes))
+            {
+                return encoding.GetBytes(charsPtr, chars.Length, bytesPtr, bytes.Length);
+            }
         }
     }
 
-    public static unsafe int GetChars(this Encoding encoding, ReadOnlySpan<byte> bytes, Span<char> chars)
+    public static int GetChars(this Encoding encoding, ReadOnlySpan<byte> bytes, Span<char> chars)
     {
-        fixed (byte* bytesPtr = &GetNonNullPinnableReference(bytes))
-        fixed (char* charsPtr = &GetNonNullPinnableReference(chars))
+        // SAFETY-TODO: review this unsafe usage.
+        // Either document why it's safe, refactor to remove it, or mark the enclosing member 'unsafe'.
+        unsafe
         {
-            return encoding.GetChars(bytesPtr, bytes.Length, charsPtr, chars.Length);
+            fixed (byte* bytesPtr = &GetNonNullPinnableReference(bytes))
+            fixed (char* charsPtr = &GetNonNullPinnableReference(chars))
+            {
+                return encoding.GetChars(bytesPtr, bytes.Length, charsPtr, chars.Length);
+            }
         }
     }
 
-    public static unsafe string GetString(this Encoding encoding, ReadOnlySpan<byte> bytes)
+    public static string GetString(this Encoding encoding, ReadOnlySpan<byte> bytes)
     {
-        fixed (byte* bytesPtr = &GetNonNullPinnableReference(bytes))
+        // SAFETY-TODO: review this unsafe usage.
+        // Either document why it's safe, refactor to remove it, or mark the enclosing member 'unsafe'.
+        unsafe
         {
-            return encoding.GetString(bytesPtr, bytes.Length);
+            fixed (byte* bytesPtr = &GetNonNullPinnableReference(bytes))
+            {
+                return encoding.GetString(bytesPtr, bytes.Length);
+            }
         }
     }
 
@@ -73,9 +98,14 @@ public static bool TryGetChars(this Encoding encoding, ReadOnlySpan<byte> bytes,
     }
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
-    private static unsafe ref readonly T GetNonNullPinnableReference<T>(ReadOnlySpan<T> buffer)
+    private static ref readonly T GetNonNullPinnableReference<T>(ReadOnlySpan<T> buffer)
     {
-        // Based on the internal implementation from MemoryMarshal.
-        return ref buffer.Length != 0 ? ref MemoryMarshal.GetReference(buffer) : ref Unsafe.AsRef<T>((void*)1);
+        // SAFETY-TODO: review this unsafe usage.
+        // Either document why it's safe, refactor to remove it, or mark the enclosing member 'unsafe'.
+        unsafe
+        {
+            // Based on the internal implementation from MemoryMarshal.
+            return ref buffer.Length != 0 ? ref MemoryMarshal.GetReference(buffer) : ref Unsafe.AsRef<T>((void*)1);
+        }
     }
 }

src/libraries/Common/src/Polyfills/SinglePolyfills.cs

@@ -11,10 +11,15 @@ internal static class SinglePolyfills
     extension(float)
     {
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        public static unsafe bool IsFinite(float f)
+        public static bool IsFinite(float f)
         {
-            uint bits = *(uint*)&f;
-            return (~bits & 0x7F80_0000U) != 0;
+            // SAFETY-TODO: review this unsafe usage.
+            // Either document why it's safe, refactor to remove it, or mark the enclosing member 'unsafe'.
+            unsafe
+            {
+                uint bits = *(uint*)&f;
+                return (~bits & 0x7F80_0000U) != 0;
+            }
         }
     }
 }

src/libraries/Common/src/Polyfills/StringBuilderPolyfills.cs

@@ -11,11 +11,16 @@ internal static class StringBuilderPolyfills
     extension(StringBuilder stringBuilder)
     {
         /// <summary>Polyfill for StringBuilder.Append(ReadOnlySpan&lt;char&gt;) which is not available on .NET Standard 2.0.</summary>
-        public unsafe StringBuilder Append(ReadOnlySpan<char> span)
+        public StringBuilder Append(ReadOnlySpan<char> span)
         {
-            fixed (char* ptr = &MemoryMarshal.GetReference(span))
+            // SAFETY-TODO: review this unsafe usage.
+            // Either document why it's safe, refactor to remove it, or mark the enclosing member 'unsafe'.
+            unsafe
             {
-                return stringBuilder.Append(ptr, span.Length);
+                fixed (char* ptr = &MemoryMarshal.GetReference(span))
+                {
+                    return stringBuilder.Append(ptr, span.Length);
+                }
             }
         }
     }

src/libraries/Common/src/System/Text/AsciiPolyfills.cs

@@ -13,39 +13,44 @@ public static bool IsValid(string value)
             return IsValid(value.AsSpan());
         }
 
-        public static unsafe bool IsValid(ReadOnlySpan<char> value)
+        public static bool IsValid(ReadOnlySpan<char> value)
         {
-            fixed (char* src = value)
+            // SAFETY-TODO: review this unsafe usage.
+            // Either document why it's safe, refactor to remove it, or mark the enclosing member 'unsafe'.
+            unsafe
             {
-                uint* ptrUInt32 = (uint*)src;
-                int length = value.Length;
-
-                while (length >= 4)
+                fixed (char* src = value)
                 {
-                    if (!AllCharsInUInt32AreAscii(ptrUInt32[0] | ptrUInt32[1]))
+                    uint* ptrUInt32 = (uint*)src;
+                    int length = value.Length;
+
+                    while (length >= 4)
                     {
-                        return false;
-                    }
+                        if (!AllCharsInUInt32AreAscii(ptrUInt32[0] | ptrUInt32[1]))
+                        {
+                            return false;
+                        }
 
-                    ptrUInt32 += 2;
-                    length -= 4;
-                }
+                        ptrUInt32 += 2;
+                        length -= 4;
+                    }
 
-                char* ptrChar = (char*)ptrUInt32;
-                while (length-- > 0)
-                {
-                    char ch = *ptrChar++;
-                    if (ch >= 0x80)
+                    char* ptrChar = (char*)ptrUInt32;
+                    while (length-- > 0)
                     {
-                        return false;
+                        char ch = *ptrChar++;
+                        if (ch >= 0x80)
+                        {
+                            return false;
+                        }
                     }
                 }
-            }
 
-            return true;
+                return true;
 
-            [MethodImpl(MethodImplOptions.AggressiveInlining)]
-            static bool AllCharsInUInt32AreAscii(uint value) => (value & ~0x007F_007Fu) == 0;
+                [MethodImpl(MethodImplOptions.AggressiveInlining)]
+                static bool AllCharsInUInt32AreAscii(uint value) => (value & ~0x007F_007Fu) == 0;
+            }
         }
     }
 }

src/libraries/System.Private.CoreLib/src/System/Text/Rune.cs

@@ -257,37 +257,42 @@ private static unsafe Rune ChangeCaseCultureAware(Rune rune, TextInfo textInfo,
             }
         }
 #else
-        private static unsafe Rune ChangeCaseCultureAware(Rune rune, CultureInfo culture, bool toUpper)
+        private static Rune ChangeCaseCultureAware(Rune rune, CultureInfo culture, bool toUpper)
         {
-            Debug.Assert(culture != null, "This should've been checked by the caller.");
+            // SAFETY-TODO: review this unsafe usage.
+            // Either document why it's safe, refactor to remove it, or mark the enclosing member 'unsafe'.
+            unsafe
+            {
+                Debug.Assert(culture != null, "This should've been checked by the caller.");
 
-            Span<char> original = stackalloc char[MaxUtf16CharsPerRune]; // worst case scenario = 2 code units (for a surrogate pair)
-            Span<char> modified = stackalloc char[MaxUtf16CharsPerRune]; // case change should preserve UTF-16 code unit count
+                Span<char> original = stackalloc char[MaxUtf16CharsPerRune]; // worst case scenario = 2 code units (for a surrogate pair)
+                Span<char> modified = stackalloc char[MaxUtf16CharsPerRune]; // case change should preserve UTF-16 code unit count
 
-            int charCount = rune.EncodeToUtf16(original);
-            original = original.Slice(0, charCount);
-            modified = modified.Slice(0, charCount);
+                int charCount = rune.EncodeToUtf16(original);
+                original = original.Slice(0, charCount);
+                modified = modified.Slice(0, charCount);
 
-            if (toUpper)
-            {
-                MemoryExtensions.ToUpper(original, modified, culture);
-            }
-            else
-            {
-                MemoryExtensions.ToLower(original, modified, culture);
-            }
+                if (toUpper)
+                {
+                    MemoryExtensions.ToUpper(original, modified, culture);
+                }
+                else
+                {
+                    MemoryExtensions.ToLower(original, modified, culture);
+                }
 
-            // We use simple case folding rules, which disallows moving between the BMP and supplementary
-            // planes when performing a case conversion. The helper methods which reconstruct a Rune
-            // contain debug asserts for this condition.
+                // We use simple case folding rules, which disallows moving between the BMP and supplementary
+                // planes when performing a case conversion. The helper methods which reconstruct a Rune
+                // contain debug asserts for this condition.
 
-            if (rune.IsBmp)
-            {
-                return UnsafeCreate(modified[0]);
-            }
-            else
-            {
-                return UnsafeCreate(UnicodeUtility.GetScalarFromUtf16SurrogatePair(modified[0], modified[1]));
+                if (rune.IsBmp)
+                {
+                    return UnsafeCreate(modified[0]);
+                }
+                else
+                {
+                    return UnsafeCreate(UnicodeUtility.GetScalarFromUtf16SurrogatePair(modified[0], modified[1]));
+                }
             }
         }
 #endif
@@ -793,21 +798,26 @@ public int EncodeToUtf8(Span<byte> destination)
         /// <param name="other">The rune to compare with the current instance.</param>
         /// <param name="comparisonType">One of the enumeration values that specifies the rules to use in the comparison.</param>
         /// <returns><see langword="true"/> if the current instance and <paramref name="other"/> are equal; otherwise, <see langword="false"/>.</returns>
-        public unsafe bool Equals(Rune other, StringComparison comparisonType)
+        public bool Equals(Rune other, StringComparison comparisonType)
         {
-            if (comparisonType is StringComparison.Ordinal)
+            // SAFETY-TODO: review this unsafe usage.
+            // Either document why it's safe, refactor to remove it, or mark the enclosing member 'unsafe'.
+            unsafe
             {
-                return this == other;
-            }
+                if (comparisonType is StringComparison.Ordinal)
+                {
+                    return this == other;
+                }
 
-            // Convert this to span
-            ReadOnlySpan<char> thisChars = AsSpan(stackalloc char[MaxUtf16CharsPerRune]);
+                // Convert this to span
+                ReadOnlySpan<char> thisChars = AsSpan(stackalloc char[MaxUtf16CharsPerRune]);
 
-            // Convert other to span
-            ReadOnlySpan<char> otherChars = other.AsSpan(stackalloc char[MaxUtf16CharsPerRune]);
+                // Convert other to span
+                ReadOnlySpan<char> otherChars = other.AsSpan(stackalloc char[MaxUtf16CharsPerRune]);
 
-            // Compare span equality
-            return thisChars.Equals(otherChars, comparisonType);
+                // Compare span equality
+                return thisChars.Equals(otherChars, comparisonType);
+            }
         }
 
         public override int GetHashCode() => Value;

src/libraries/System.Text.Json/src/System.Text.Json.csproj

@@ -3,6 +3,7 @@
   <PropertyGroup>
     <TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppPrevious);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <EnableUnsafeMethodModifierAnalyzer>true</EnableUnsafeMethodModifierAnalyzer>
     <!-- For this project, we want warnings if there are public APIs/types without properly formatted XML comments. -->
     <SkipIntellisenseNoWarnCS1591>true</SkipIntellisenseNoWarnCS1591>
     <NoWarn>$(NoWarn);CS8969</NoWarn>