Enable new CA1311 (ToLower/Upper culture) and CA1852 (seal internal/p…

…rivate types) rules (#68268)

* Enable new CA1311 (ToLower/Upper culture) and CA1852 (seal internal/private types)

CA1311 flagged a few issues, all addressed by using ToLowerInvariant/ToUpperInvariant.

CA1852 flagged a bunch that previous cleanups around sealing types missed or that are new since.  Sealing types then highlighted places where protected or virtual members were being exposed unnecessarily, so those were fixed, too.  Adding sealed to things also highlighted some discrepancies in the order of "unsafe sealed" keywords, where the vast majority in the repo were "sealed unsafe", so I fixed the few that weren't.

* Address PR feedback and fix more CI failures

* Fix unit test build error

eng/CodeAnalysis.src.globalconfig

@@ -190,6 +190,9 @@ dotnet_diagnostic.CA1309.severity = none
 # CA1310: Specify StringComparison for correctness
 dotnet_diagnostic.CA1310.severity = suggestion
 
+# CA1311: Specify a culture or use an invariant version
+dotnet_diagnostic.CA1311.severity = warning
+
 # CA1401: P/Invokes should not be visible
 dotnet_diagnostic.CA1401.severity = warning
 
@@ -402,6 +405,9 @@ dotnet_diagnostic.CA1850.severity = warning
 # CA1851: Possible multiple enumerations of 'IEnumerable' collection
 dotnet_diagnostic.CA1851.severity = suggestion
 
+# CA1852: Seal internal types
+dotnet_diagnostic.CA1852.severity = warning
+
 # CA2000: Dispose objects before losing scope
 dotnet_diagnostic.CA2000.severity = none
 

eng/CodeAnalysis.test.globalconfig

@@ -189,6 +189,9 @@ dotnet_diagnostic.CA1309.severity = none
 # CA1310: Specify StringComparison for correctness
 dotnet_diagnostic.CA1310.severity = none
 
+# CA1311: Specify a culture or use an invariant version
+dotnet_diagnostic.CA1311.severity = none
+
 # CA1401: P/Invokes should not be visible
 dotnet_diagnostic.CA1401.severity = none
 
@@ -399,6 +402,9 @@ dotnet_diagnostic.CA1850.severity = none
 # CA1851: Possible multiple enumerations of 'IEnumerable' collection
 dotnet_diagnostic.CA1851.severity = none
 
+# CA1852: Seal internal types
+dotnet_diagnostic.CA1852.severity = none
+
 # CA2000: Dispose objects before losing scope
 dotnet_diagnostic.CA2000.severity = none
 

eng/Versions.props

@@ -44,7 +44,7 @@
     <MicrosoftCodeAnalysisCSharpCodeStyleVersion>4.3.0-1.22206.2</MicrosoftCodeAnalysisCSharpCodeStyleVersion>
     <MicrosoftCodeAnalysisCSharpWorkspacesVersion>4.3.0-1.22206.2</MicrosoftCodeAnalysisCSharpWorkspacesVersion>
     <MicrosoftCodeAnalysisCSharpVersion>4.3.0-1.22206.2</MicrosoftCodeAnalysisCSharpVersion>
-    <MicrosoftCodeAnalysisNetAnalyzersVersion>7.0.0-preview1.22212.1</MicrosoftCodeAnalysisNetAnalyzersVersion>
+    <MicrosoftCodeAnalysisNetAnalyzersVersion>7.0.0-preview1.22218.1</MicrosoftCodeAnalysisNetAnalyzersVersion>
     <MicrosoftCodeAnalysisVersion>4.3.0-1.22206.2</MicrosoftCodeAnalysisVersion>
     <!--
       TODO: Remove pinned version once arcade supplies a 4.3 compiler.

src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs

@@ -1512,7 +1512,7 @@ internal static MetadataImport GetMetadataImport(RuntimeModule module)
         #endregion
     }
 
-    internal unsafe class Signature
+    internal sealed unsafe class Signature
     {
         #region Definitions
         internal enum MdSigCallingConvention : byte

src/coreclr/System.Private.CoreLib/src/System/Threading/Overlapped.cs

@@ -27,7 +27,7 @@ namespace System.Threading
 {
     #region class _IOCompletionCallback
 
-    internal unsafe partial class _IOCompletionCallback
+    internal sealed unsafe partial class _IOCompletionCallback
     {
         // call back helper
         internal static void PerformIOCompletionCallback(uint errorCode, uint numBytes, NativeOverlapped* pNativeOverlapped)

src/coreclr/System.Private.CoreLib/src/System/__Canon.cs

@@ -1,6 +1,8 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+#pragma warning disable CA1852 // __Canon should not be sealed
+
 using System.Runtime.InteropServices;
 
 namespace System

src/coreclr/System.Private.CoreLib/src/System/__ComObject.cs

@@ -1,6 +1,8 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+#pragma warning disable CA1852 // __ComObject should not be sealed
+
 using System.Collections;
 using System.Diagnostics.CodeAnalysis;
 using System.Reflection;

src/coreclr/nativeaot/Directory.Build.props

@@ -34,7 +34,7 @@
     <NoWarn>$(NoWarn);CS8602;CS8603;CS8604;CS8618;CS8625;CS8632;CS8765</NoWarn>
 
     <!-- we should just fix -->
-    <NoWarn>$(NoWarn);CA1810;CA1823;CA1825;CA2208;SA1129;SA1205;SA1400;SA1517</NoWarn>
+    <NoWarn>$(NoWarn);CA1810;CA1823;CA1825;CA1852;CA2208;SA1129;SA1205;SA1400;SA1517</NoWarn>
 
     <!-- Arrays as attribute arguments is not CLS-compliant -->
     <NoWarn>$(NoWarn);CS3016</NoWarn>

src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/CompilerServices/FixedBufferAttribute.cs

@@ -3,7 +3,7 @@
 
 namespace System.Runtime.CompilerServices
 {
-    internal unsafe sealed class FixedBufferAttribute : Attribute
+    internal sealed unsafe class FixedBufferAttribute : Attribute
     {
         public FixedBufferAttribute(Type elementType, int length)
         {

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Overlapped.cs

@@ -27,7 +27,7 @@ namespace System.Threading
 {
     #region class OverlappedData
 
-    internal unsafe sealed class OverlappedData
+    internal sealed unsafe class OverlappedData
     {
         internal IAsyncResult _asyncResult;
         internal object _callback; // IOCompletionCallback or _IOCompletionCallback

src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/ExecutionEnvironmentImplementation.MappingTables.cs

@@ -103,7 +103,7 @@ private static unsafe bool TryGetNativeReaderForBlob(NativeFormatModuleInfo modu
         /// </summary>
         /// <param name="runtimeTypeHandle">Runtime handle of the type in question</param>
         /// <param name="qTypeDefinition">TypeDef handle for the type</param>
-        public unsafe sealed override bool TryGetMetadataForNamedType(RuntimeTypeHandle runtimeTypeHandle, out QTypeDefinition qTypeDefinition)
+        public sealed unsafe override bool TryGetMetadataForNamedType(RuntimeTypeHandle runtimeTypeHandle, out QTypeDefinition qTypeDefinition)
         {
             Debug.Assert(!RuntimeAugments.IsGenericType(runtimeTypeHandle));
             return TypeLoaderEnvironment.Instance.TryGetMetadataForNamedType(runtimeTypeHandle, out qTypeDefinition);
@@ -115,7 +115,7 @@ public unsafe sealed override bool TryGetMetadataForNamedType(RuntimeTypeHandle
         // Preconditions:
         //    runtimeTypeHandle is a typedef or a generic type instance (not a constructed type such as an array)
         //
-        public unsafe sealed override bool IsReflectionBlocked(RuntimeTypeHandle runtimeTypeHandle)
+        public sealed unsafe override bool IsReflectionBlocked(RuntimeTypeHandle runtimeTypeHandle)
         {
             // For generic types, use the generic type definition
             runtimeTypeHandle = GetTypeDefinition(runtimeTypeHandle);
@@ -163,7 +163,7 @@ public unsafe sealed override bool IsReflectionBlocked(RuntimeTypeHandle runtime
         /// </summary>
         /// <param name="qTypeDefinition">TypeDef handle for the type to look up</param>
         /// <param name="runtimeTypeHandle">Runtime type handle (MethodTable) for the given type</param>
-        public unsafe sealed override bool TryGetNamedTypeForMetadata(QTypeDefinition qTypeDefinition, out RuntimeTypeHandle runtimeTypeHandle)
+        public sealed unsafe override bool TryGetNamedTypeForMetadata(QTypeDefinition qTypeDefinition, out RuntimeTypeHandle runtimeTypeHandle)
         {
             return TypeLoaderEnvironment.Instance.TryGetOrCreateNamedTypeForMetadata(qTypeDefinition, out runtimeTypeHandle);
         }
@@ -180,7 +180,7 @@ public unsafe sealed override bool TryGetNamedTypeForMetadata(QTypeDefinition qT
         /// <param name="runtimeTypeHandle">MethodTable of the type in question</param>
         /// <param name="metadataReader">Metadata reader for the type</param>
         /// <param name="typeRefHandle">Located TypeRef handle</param>
-        public unsafe sealed override bool TryGetTypeReferenceForNamedType(RuntimeTypeHandle runtimeTypeHandle, out MetadataReader metadataReader, out TypeReferenceHandle typeRefHandle)
+        public sealed unsafe override bool TryGetTypeReferenceForNamedType(RuntimeTypeHandle runtimeTypeHandle, out MetadataReader metadataReader, out TypeReferenceHandle typeRefHandle)
         {
             return TypeLoaderEnvironment.TryGetTypeReferenceForNamedType(runtimeTypeHandle, out metadataReader, out typeRefHandle);
         }
@@ -203,7 +203,7 @@ public unsafe sealed override bool TryGetTypeReferenceForNamedType(RuntimeTypeHa
         /// <param name="metadataReader">Metadata reader for module containing the type reference</param>
         /// <param name="typeRefHandle">TypeRef handle to look up</param>
         /// <param name="runtimeTypeHandle">Resolved MethodTable for the type reference</param>
-        public unsafe sealed override bool TryGetNamedTypeForTypeReference(MetadataReader metadataReader, TypeReferenceHandle typeRefHandle, out RuntimeTypeHandle runtimeTypeHandle)
+        public sealed unsafe override bool TryGetNamedTypeForTypeReference(MetadataReader metadataReader, TypeReferenceHandle typeRefHandle, out RuntimeTypeHandle runtimeTypeHandle)
         {
             return TypeLoaderEnvironment.TryGetNamedTypeForTypeReference(metadataReader, typeRefHandle, out runtimeTypeHandle);
         }
@@ -217,7 +217,7 @@ public unsafe sealed override bool TryGetNamedTypeForTypeReference(MetadataReade
         //
         // This is not equivalent to calling TryGetMultiDimTypeForElementType() with a rank of 1!
         //
-        public unsafe sealed override bool TryGetArrayTypeForElementType(RuntimeTypeHandle elementTypeHandle, out RuntimeTypeHandle arrayTypeHandle)
+        public sealed unsafe override bool TryGetArrayTypeForElementType(RuntimeTypeHandle elementTypeHandle, out RuntimeTypeHandle arrayTypeHandle)
         {
             if (RuntimeAugments.IsGenericTypeDefinition(elementTypeHandle))
             {
@@ -238,7 +238,7 @@ public unsafe sealed override bool TryGetArrayTypeForElementType(RuntimeTypeHand
         //
         // This is not equivalent to calling TryGetMultiDimTypeElementType() with a rank of 1!
         //
-        public unsafe sealed override bool TryGetArrayTypeElementType(RuntimeTypeHandle arrayTypeHandle, out RuntimeTypeHandle elementTypeHandle)
+        public sealed unsafe override bool TryGetArrayTypeElementType(RuntimeTypeHandle arrayTypeHandle, out RuntimeTypeHandle elementTypeHandle)
         {
             elementTypeHandle = RuntimeAugments.GetRelatedParameterTypeHandle(arrayTypeHandle);
             return true;
@@ -254,7 +254,7 @@ public unsafe sealed override bool TryGetArrayTypeElementType(RuntimeTypeHandle
         //
         // Calling this with rank 1 is not equivalent to calling TryGetArrayTypeForElementType()!
         //
-        public unsafe sealed override bool TryGetMultiDimArrayTypeForElementType(RuntimeTypeHandle elementTypeHandle, int rank, out RuntimeTypeHandle arrayTypeHandle)
+        public sealed unsafe override bool TryGetMultiDimArrayTypeForElementType(RuntimeTypeHandle elementTypeHandle, int rank, out RuntimeTypeHandle arrayTypeHandle)
         {
             if (RuntimeAugments.IsGenericTypeDefinition(elementTypeHandle))
             {
@@ -277,7 +277,7 @@ public unsafe sealed override bool TryGetMultiDimArrayTypeForElementType(Runtime
         // Preconditions:
         //     targetTypeHandle is a valid RuntimeTypeHandle.
         //
-        public unsafe sealed override bool TryGetPointerTypeForTargetType(RuntimeTypeHandle targetTypeHandle, out RuntimeTypeHandle pointerTypeHandle)
+        public sealed unsafe override bool TryGetPointerTypeForTargetType(RuntimeTypeHandle targetTypeHandle, out RuntimeTypeHandle pointerTypeHandle)
         {
             return TypeLoaderEnvironment.Instance.TryGetPointerTypeForTargetType(targetTypeHandle, out pointerTypeHandle);
         }
@@ -289,7 +289,7 @@ public unsafe sealed override bool TryGetPointerTypeForTargetType(RuntimeTypeHan
         // Preconditions:
         //      pointerTypeHandle is a valid RuntimeTypeHandle of type pointer.
         //
-        public unsafe sealed override bool TryGetPointerTypeTargetType(RuntimeTypeHandle pointerTypeHandle, out RuntimeTypeHandle targetTypeHandle)
+        public sealed unsafe override bool TryGetPointerTypeTargetType(RuntimeTypeHandle pointerTypeHandle, out RuntimeTypeHandle targetTypeHandle)
         {
             targetTypeHandle = RuntimeAugments.GetRelatedParameterTypeHandle(pointerTypeHandle);
             return true;
@@ -302,7 +302,7 @@ public unsafe sealed override bool TryGetPointerTypeTargetType(RuntimeTypeHandle
         // Preconditions:
         //     targetTypeHandle is a valid RuntimeTypeHandle.
         //
-        public unsafe sealed override bool TryGetByRefTypeForTargetType(RuntimeTypeHandle targetTypeHandle, out RuntimeTypeHandle byRefTypeHandle)
+        public sealed unsafe override bool TryGetByRefTypeForTargetType(RuntimeTypeHandle targetTypeHandle, out RuntimeTypeHandle byRefTypeHandle)
         {
             return TypeLoaderEnvironment.Instance.TryGetByRefTypeForTargetType(targetTypeHandle, out byRefTypeHandle);
         }
@@ -314,7 +314,7 @@ public unsafe sealed override bool TryGetByRefTypeForTargetType(RuntimeTypeHandl
         // Preconditions:
         //      byRefTypeHandle is a valid RuntimeTypeHandle of a byref.
         //
-        public unsafe sealed override bool TryGetByRefTypeTargetType(RuntimeTypeHandle byRefTypeHandle, out RuntimeTypeHandle targetTypeHandle)
+        public sealed unsafe override bool TryGetByRefTypeTargetType(RuntimeTypeHandle byRefTypeHandle, out RuntimeTypeHandle targetTypeHandle)
         {
             targetTypeHandle = RuntimeAugments.GetRelatedParameterTypeHandle(byRefTypeHandle);
             return true;
@@ -328,7 +328,7 @@ public unsafe sealed override bool TryGetByRefTypeTargetType(RuntimeTypeHandle b
         //      runtimeTypeDefinitionHandle is a valid RuntimeTypeHandle for a generic type.
         //      genericTypeArgumentHandles is an array of valid RuntimeTypeHandles.
         //
-        public unsafe sealed override bool TryGetConstructedGenericTypeForComponents(RuntimeTypeHandle genericTypeDefinitionHandle, RuntimeTypeHandle[] genericTypeArgumentHandles, out RuntimeTypeHandle runtimeTypeHandle)
+        public sealed unsafe override bool TryGetConstructedGenericTypeForComponents(RuntimeTypeHandle genericTypeDefinitionHandle, RuntimeTypeHandle[] genericTypeArgumentHandles, out RuntimeTypeHandle runtimeTypeHandle)
         {
             if (TypeLoaderEnvironment.Instance.TryLookupConstructedGenericTypeForComponents(genericTypeDefinitionHandle, genericTypeArgumentHandles, out runtimeTypeHandle))
             {
@@ -1386,7 +1386,7 @@ public sealed override FieldAccessor TryGetFieldAccessor(
         //
         // This resolves RuntimeMethodHandles for methods declared on non-generic types (declaringTypeHandle is an output of this method.)
         //
-        public unsafe sealed override bool TryGetMethodFromHandle(RuntimeMethodHandle runtimeMethodHandle, out RuntimeTypeHandle declaringTypeHandle, out QMethodDefinition methodHandle, out RuntimeTypeHandle[] genericMethodTypeArgumentHandles)
+        public sealed unsafe override bool TryGetMethodFromHandle(RuntimeMethodHandle runtimeMethodHandle, out RuntimeTypeHandle declaringTypeHandle, out QMethodDefinition methodHandle, out RuntimeTypeHandle[] genericMethodTypeArgumentHandles)
         {
             MethodNameAndSignature nameAndSignature;
             methodHandle = default(QMethodDefinition);
@@ -1407,7 +1407,7 @@ public sealed override bool TryGetMethodFromHandleAndType(RuntimeMethodHandle ru
         //
         // This resolves RuntimeFieldHandles for fields declared on non-generic types (declaringTypeHandle is an output of this method.)
         //
-        public unsafe sealed override bool TryGetFieldFromHandle(RuntimeFieldHandle runtimeFieldHandle, out RuntimeTypeHandle declaringTypeHandle, out FieldHandle fieldHandle)
+        public sealed unsafe override bool TryGetFieldFromHandle(RuntimeFieldHandle runtimeFieldHandle, out RuntimeTypeHandle declaringTypeHandle, out FieldHandle fieldHandle)
         {
             fieldHandle = default(FieldHandle);
 

src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/FieldAccessors/PointerTypeFieldAccessorForStaticFields.cs

@@ -16,7 +16,7 @@ public PointerTypeFieldAccessorForStaticFields(IntPtr cctorContext, IntPtr stati
         {
         }
 
-        protected unsafe sealed override object GetFieldBypassCctor()
+        protected sealed unsafe override object GetFieldBypassCctor()
         {
             if (FieldBase == FieldTableFlags.GCStatic)
             {
@@ -33,7 +33,7 @@ protected unsafe sealed override object GetFieldBypassCctor()
             return RuntimeAugments.LoadPointerTypeField(threadStaticRegion, FieldOffset, FieldTypeHandle);
         }
 
-        protected unsafe sealed override void UncheckedSetFieldBypassCctor(object value)
+        protected sealed unsafe override void UncheckedSetFieldBypassCctor(object value)
         {
             if (FieldBase == FieldTableFlags.GCStatic)
             {

src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/FieldAccessors/ReferenceTypeFieldAccessorForStaticFields.cs

@@ -16,7 +16,7 @@ public ReferenceTypeFieldAccessorForStaticFields(IntPtr cctorContext, IntPtr sta
         {
         }
 
-        protected unsafe sealed override object GetFieldBypassCctor()
+        protected sealed unsafe override object GetFieldBypassCctor()
         {
             if (FieldBase == FieldTableFlags.GCStatic)
             {
@@ -33,7 +33,7 @@ protected unsafe sealed override object GetFieldBypassCctor()
             return RuntimeAugments.LoadReferenceTypeField(threadStaticRegion, FieldOffset);
         }
 
-        protected unsafe sealed override void UncheckedSetFieldBypassCctor(object value)
+        protected sealed unsafe override void UncheckedSetFieldBypassCctor(object value)
         {
             if (FieldBase == FieldTableFlags.GCStatic)
             {

src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/FieldAccessors/ValueTypeFieldAccessorForStaticFields.cs

@@ -16,7 +16,7 @@ public ValueTypeFieldAccessorForStaticFields(IntPtr cctorContext, IntPtr statics
         {
         }
 
-        protected unsafe sealed override object GetFieldBypassCctor()
+        protected sealed unsafe override object GetFieldBypassCctor()
         {
             if (FieldBase == FieldTableFlags.GCStatic)
             {
@@ -33,7 +33,7 @@ protected unsafe sealed override object GetFieldBypassCctor()
             return RuntimeAugments.LoadValueTypeField(threadStaticRegion, FieldOffset, FieldTypeHandle);
         }
 
-        protected unsafe sealed override void UncheckedSetFieldBypassCctor(object value)
+        protected sealed unsafe override void UncheckedSetFieldBypassCctor(object value)
         {
             if (FieldBase == FieldTableFlags.GCStatic)
             {

src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs

@@ -34,7 +34,7 @@
 
 namespace Internal.JitInterface
 {
-    internal unsafe sealed partial class CorInfoImpl
+    internal sealed unsafe partial class CorInfoImpl
     {
         //
         // Global initialization and state

src/coreclr/tools/Common/TypeSystem/Common/MetadataTypeSystemContext.cs

@@ -94,7 +94,7 @@ public override DefType GetWellKnownType(WellKnownType wellKnownType, bool throw
             return type;
         }
 
-        protected sealed internal override bool ComputeHasStaticConstructor(TypeDesc type)
+        protected internal sealed override bool ComputeHasStaticConstructor(TypeDesc type)
         {
             if (type is MetadataType)
             {
@@ -103,7 +103,7 @@ protected sealed internal override bool ComputeHasStaticConstructor(TypeDesc typ
             return false;
         }
 
-        protected sealed internal override bool IsIDynamicInterfaceCastableInterface(DefType type)
+        protected internal sealed override bool IsIDynamicInterfaceCastableInterface(DefType type)
         {
             MetadataType t = (MetadataType)type;
             return t.Module == SystemModule