Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jan 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions eng/CodeAnalysis.src.globalconfig
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,9 @@ internal unsafe struct MetadataEnumResult
}
}

#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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

namespace System
{
public struct ModuleHandle
public struct ModuleHandle : IEquatable<ModuleHandle>
{
public static readonly ModuleHandle EmptyHandle;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

using Internal.Reflection.Tracing;

#pragma warning disable CA1067 // override Equals because it implements IEquatable<T>

namespace System.Reflection.Runtime.TypeInfos
{
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/tools/Common/TypeSystem/Common/LayoutInt.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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"/>.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand All @@ -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; } }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Loading