Implement IEquatable<T> on value types overriding Equals (and enable CA1066/1077) (dotnet#63690)

Author: stephentoub

eng/CodeAnalysis.src.globalconfig

@@ -152,10 +152,10 @@ dotnet_diagnostic.CA1064.severity = none
 dotnet_diagnostic.CA1065.severity = none
 
 # CA1066: Implement IEquatable when overriding Object.Equals
-dotnet_diagnostic.CA1066.severity = none
+dotnet_diagnostic.CA1066.severity = warning
 
 # CA1067: Override Object.Equals(object) when implementing IEquatable<T>
-dotnet_diagnostic.CA1067.severity = none
+dotnet_diagnostic.CA1067.severity = warning
 
 # CA1068: CancellationToken parameters must come last
 dotnet_diagnostic.CA1068.severity = none

src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj

@@ -140,8 +140,6 @@
     <Compile Include="$(BclSourcesRoot)\System\Diagnostics\StackFrame.CoreCLR.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Diagnostics\StackFrameHelper.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Diagnostics\StackTrace.CoreCLR.cs" />
-    <Compile Include="$(BclSourcesRoot)\System\Diagnostics\SymbolStore\SymAddressKind.cs" />
-    <Compile Include="$(BclSourcesRoot)\System\Diagnostics\SymbolStore\Token.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Enum.CoreCLR.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Environment.CoreCLR.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Exception.CoreCLR.cs" />

src/coreclr/System.Private.CoreLib/src/System/Diagnostics/SymbolStore/SymAddressKind.cs

@@ -194,7 +194,9 @@ public int this[int index]
         }
     }
 
+#pragma warning disable CA1066 // IEquatable<MetadataImport> interface implementation isn't used
     internal readonly struct MetadataImport
+#pragma warning restore CA1067
     {
         private readonly IntPtr m_metadataImport2;
         private readonly object? m_keepalive;

src/coreclr/System.Private.CoreLib/src/System/Diagnostics/SymbolStore/Token.cs

@@ -1246,7 +1246,7 @@ public void GetObjectData(SerializationInfo info, StreamingContext context)
         }
     }
 
-    public unsafe partial struct ModuleHandle
+    public unsafe partial struct ModuleHandle : IEquatable<ModuleHandle>
     {
         #region Public Static Members
         public static readonly ModuleHandle EmptyHandle;

src/coreclr/System.Private.CoreLib/src/System/Reflection/MdImport.cs

@@ -6,7 +6,7 @@
 
 namespace System
 {
-    public struct ModuleHandle
+    public struct ModuleHandle : IEquatable<ModuleHandle>
     {
         public static readonly ModuleHandle EmptyHandle;
 

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

@@ -34,6 +34,8 @@
 //   - The TypeUnifier extension class provides a more friendly interface to the rest of the codebase.
 //
 
+#pragma warning disable CA1067 // override Equals because it implements IEquatable<T>
+
 namespace System.Reflection.Runtime.General
 {
     internal static partial class TypeUnifier

src/coreclr/nativeaot/System.Private.CoreLib/src/System/ModuleHandle.cs

@@ -10,6 +10,8 @@
 
 using Internal.Reflection.Tracing;
 
+#pragma warning disable CA1067 // override Equals because it implements IEquatable<T>
+
 namespace System.Reflection.Runtime.TypeInfos
 {
     //

src/coreclr/nativeaot/System.Private.Reflection.Core/src/System/Reflection/Runtime/General/TypeUnifier.NativeFormat.cs

@@ -20,6 +20,7 @@ public void EmitSource()
         WriteLine("#pragma warning disable 649");
         WriteLine("#pragma warning disable 169");
         WriteLine("#pragma warning disable 282 // There is no defined ordering between fields in multiple declarations of partial class or struct");
+        WriteLine("#pragma warning disable CA1066 // IEquatable<T> implementations aren't used");
         WriteLine("#pragma warning disable IDE0059");
         WriteLine();
 

src/coreclr/nativeaot/System.Private.Reflection.Core/src/System/Reflection/Runtime/TypeInfos/RuntimeNamedTypeInfo.cs

@@ -6,6 +6,7 @@
 #pragma warning disable 649
 #pragma warning disable 169
 #pragma warning disable 282 // There is no defined ordering between fields in multiple declarations of partial class or struct
+#pragma warning disable CA1066 // IEquatable<T> implementations aren't used
 #pragma warning disable IDE0059
 
 using System;

src/coreclr/tools/Common/Internal/Metadata/NativeFormat/Generator/ReaderGen.cs

@@ -1,12 +1,9 @@
 // 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 169
-
-// There is no defined ordering between fields in multiple declarations of partial class or struct
-#pragma warning disable 282
-
+#pragma warning disable 282 // There is no defined ordering between fields in multiple declarations of partial class or struct
+#pragma warning disable CA1066 // IEquatable<T> implementations aren't used
 
 using System;
 using System.IO;

src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeFormatReaderGen.cs

@@ -12,7 +12,9 @@ namespace Internal.TypeSystem
     /// type system do not have a known size. This type is used to make such sizes viral through the type layout
     /// computations)
     /// </summary>
+#pragma warning disable CA1066 // IEquatable<T> implementation wouldn't be used
     public struct LayoutInt
+#pragma warning restore CA1066
     {
         private int _value;
 

src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeMetadataReader.cs

@@ -2,9 +2,8 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
-using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
 using System.Runtime.InteropServices;
-using System.Text;
 
 internal static partial class Interop
 {
@@ -33,6 +32,19 @@ public bool IsIPv6
             private  uint _isIPv6;                             // Non-zero if this is an IPv6 address; zero for IPv4.
             internal uint ScopeId;                             // Scope ID (IPv6 only)
 
+            public override unsafe int GetHashCode()
+            {
+                HashCode h = default;
+                fixed (byte* ptr = Address)
+                {
+                    h.AddBytes(new ReadOnlySpan<byte>(ptr, IsIPv6 ? IPv6AddressBytes : IPv4AddressBytes));
+                }
+                return h.ToHashCode();
+            }
+
+            public override bool Equals([NotNullWhen(true)] object? obj) =>
+                obj is IPAddress other && Equals(other);
+
             public bool Equals(IPAddress other)
             {
                 int addressByteCount;

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

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using System.Diagnostics.CodeAnalysis;
 
 namespace Microsoft.Extensions.DependencyInjection.ServiceLookup
 {
@@ -33,10 +34,14 @@ public ServiceCacheKey(Type? type, int slot)
             Slot = slot;
         }
 
-        public bool Equals(ServiceCacheKey other)
-        {
-            return Type == other.Type && Slot == other.Slot;
-        }
+        /// <summary>Indicates whether the current instance is equal to another instance of the same type.</summary>
+        /// <param name="other">An instance to compare with this instance.</param>
+        /// <returns>true if the current instance is equal to the other instance; otherwise, false.</returns>
+        public bool Equals(ServiceCacheKey other) =>
+            Type == other.Type && Slot == other.Slot;
+
+        public override bool Equals([NotNullWhen(true)] object? obj) =>
+            obj is ServiceCacheKey other && Equals(other);
 
         public override int GetHashCode()
         {

src/libraries/Common/src/Interop/Unix/System.Native/Interop.IPAddress.cs

@@ -6,7 +6,7 @@
 
 namespace Microsoft.Extensions.Logging
 {
-    public readonly partial struct EventId
+    public readonly partial struct EventId : System.IEquatable<Microsoft.Extensions.Logging.EventId>
     {
         private readonly object _dummy;
         private readonly int _dummyPrimitive;

src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/ServiceCacheKey.cs

@@ -1,14 +1,15 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System;
 using System.Diagnostics.CodeAnalysis;
 
 namespace Microsoft.Extensions.Logging
 {
     /// <summary>
     /// Identifies a logging event. The primary identifier is the "Id" property, with the "Name" property providing a short description of this type of event.
     /// </summary>
-    public readonly struct EventId
+    public readonly struct EventId : IEquatable<EventId>
     {
         /// <summary>
         /// Implicitly creates an EventId from the given <see cref="int"/>.

src/libraries/Microsoft.Extensions.Logging.Abstractions/ref/Microsoft.Extensions.Logging.Abstractions.cs

@@ -15,7 +15,9 @@ public partial class ImmutableDictionary<TKey, TValue>
         /// <summary>
         /// Contains all the key/values in the collection that hash to the same value.
         /// </summary>
+#pragma warning disable CA1066 // Implement IEquatable when overriding Object.Equals
         internal readonly struct HashBucket : IEnumerable<KeyValuePair<TKey, TValue>>
+#pragma warning restore CA1066
         {
             /// <summary>
             /// One of the values in this bucket.

src/libraries/Microsoft.Extensions.Logging.Abstractions/src/EventId.cs

@@ -30,7 +30,9 @@ internal enum OperationResult
         /// <summary>
         /// Contains all the keys in the collection that hash to the same value.
         /// </summary>
+#pragma warning disable CA1066 // Implement IEquatable when overriding Object.Equals
         internal readonly struct HashBucket
+#pragma warning restore CA1066
         {
             /// <summary>
             /// One of the values in this bucket.

src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableDictionary_2.HashBucket.cs

@@ -6,7 +6,7 @@
 
 namespace System.Collections.Specialized
 {
-    public partial struct BitVector32
+    public partial struct BitVector32 : System.IEquatable<System.Collections.Specialized.BitVector32>
     {
         private int _dummyPrimitive;
         public BitVector32(System.Collections.Specialized.BitVector32 value) { throw null; }
@@ -18,11 +18,12 @@ public partial struct BitVector32
         public static int CreateMask(int previous) { throw null; }
         public static System.Collections.Specialized.BitVector32.Section CreateSection(short maxValue) { throw null; }
         public static System.Collections.Specialized.BitVector32.Section CreateSection(short maxValue, System.Collections.Specialized.BitVector32.Section previous) { throw null; }
+        public bool Equals(System.Collections.Specialized.BitVector32 other) { throw null; }
         public override bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] object? o) { throw null; }
         public override int GetHashCode() { throw null; }
         public override string ToString() { throw null; }
         public static string ToString(System.Collections.Specialized.BitVector32 value) { throw null; }
-        public readonly partial struct Section
+        public readonly partial struct Section : System.IEquatable<System.Collections.Specialized.BitVector32.Section>
         {
             private readonly int _dummyPrimitive;
             public short Mask { get { throw null; } }

src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableHashSet_1.HashBucket.cs

@@ -10,7 +10,7 @@ namespace System.Collections.Specialized
     ///    <para>Provides a simple light bit vector with easy integer or Boolean access to
     ///       a 32 bit storage.</para>
     /// </devdoc>
-    public struct BitVector32
+    public struct BitVector32 : IEquatable<BitVector32>
     {
         private uint _data;
 
@@ -151,7 +151,12 @@ private static Section CreateSectionHelper(short maxValue, short priorMask, shor
             return new Section(mask, offset);
         }
 
-        public override bool Equals([NotNullWhen(true)] object? o) => o is BitVector32 other && _data == other._data;
+        public override bool Equals([NotNullWhen(true)] object? o) => o is BitVector32 other && Equals(other);
+
+        /// <summary>Indicates whether the current instance is equal to another instance of the same type.</summary>
+        /// <param name="other">An instance to compare with this instance.</param>
+        /// <returns>true if the current instance is equal to the other instance; otherwise, false.</returns>
+        public bool Equals(BitVector32 other) => _data == other._data;
 
         public override int GetHashCode() => _data.GetHashCode();
 
@@ -182,7 +187,7 @@ public override string ToString()
         ///    <para>
         ///       Represents an section of the vector that can contain a integer number.</para>
         /// </devdoc>
-        public readonly struct Section
+        public readonly struct Section : IEquatable<Section>
         {
             private readonly short _mask;
             private readonly short _offset;

src/libraries/System.Collections.Specialized/ref/System.Collections.Specialized.cs

@@ -518,17 +518,32 @@ public static void EqualsTest()
             Assert.True(new BitVector32(0).Equals(original));
             Assert.True(original.Equals(new BitVector32(0)));
 
+            Assert.True(original.Equals((object)original));
+            Assert.True(new BitVector32().Equals((object)original));
+            Assert.True(original.Equals((object)new BitVector32()));
+            Assert.True(new BitVector32(0).Equals((object)original));
+            Assert.True(original.Equals((object)new BitVector32(0)));
+
             BitVector32 other = new BitVector32(int.MaxValue / 2 - 1);
             Assert.True(other.Equals(other));
             Assert.True(new BitVector32(int.MaxValue / 2 - 1).Equals(other));
             Assert.True(other.Equals(new BitVector32(int.MaxValue / 2 - 1)));
 
+            Assert.True(other.Equals((object)other));
+            Assert.True(new BitVector32(int.MaxValue / 2 - 1).Equals((object)other));
+            Assert.True(other.Equals((object)new BitVector32(int.MaxValue / 2 - 1)));
+
             Assert.False(other.Equals(original));
             Assert.False(original.Equals(other));
             Assert.False(other.Equals(null));
             Assert.False(original.Equals(null));
             Assert.False(other.Equals(int.MaxValue / 2 - 1));
             Assert.False(original.Equals(0));
+
+            Assert.False(other.Equals((object)original));
+            Assert.False(original.Equals((object)other));
+            Assert.False(other.Equals(int.MaxValue / 2 - 1));
+            Assert.False(original.Equals(0));
         }
 
         [Fact]