-
-
Notifications
You must be signed in to change notification settings - Fork 108
Fix IsEquivalentTo to respect IEquatable<T> for value types like Vector2 #4263
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ internal static class ReflectionHelper | |
| { | ||
| /// <summary> | ||
| /// Gets all public instance properties and fields to compare for structural equivalency. | ||
| /// Filters out indexed properties (like indexers) that require parameters. | ||
| /// </summary> | ||
| /// <param name="type">The type to get members from.</param> | ||
| /// <returns>A list of PropertyInfo and FieldInfo members.</returns> | ||
|
|
@@ -19,7 +20,17 @@ public static List<MemberInfo> GetMembersToCompare( | |
| Type type) | ||
| { | ||
| var members = new List<MemberInfo>(); | ||
| members.AddRange(type.GetProperties(BindingFlags.Public | BindingFlags.Instance)); | ||
|
|
||
| // Filter out indexed properties (properties with parameters like this[int index]) | ||
| var properties = type.GetProperties(BindingFlags.Public | BindingFlags.Instance); | ||
| foreach (var prop in properties) | ||
| { | ||
| if (prop.GetIndexParameters().Length == 0) | ||
| { | ||
| members.Add(prop); | ||
| } | ||
| } | ||
|
Comment on lines
+26
to
+32
|
||
|
|
||
| members.AddRange(type.GetFields(BindingFlags.Public | BindingFlags.Instance)); | ||
| return members; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| using System.Collections.Concurrent; | ||
| using System.Diagnostics.CodeAnalysis; | ||
|
|
||
| namespace TUnit.Assertions.Conditions.Helpers; | ||
|
|
||
|
|
@@ -59,6 +60,8 @@ public static void ClearCustomPrimitives() | |
| /// </summary> | ||
| /// <param name="type">The type to check.</param> | ||
| /// <returns>True if the type should use value equality; false for structural comparison.</returns> | ||
| [UnconditionalSuppressMessage("Trimming", "IL2067", | ||
| Justification = "This method is only called from code paths that already require reflection (StructuralEqualityComparer)")] | ||
| public static bool IsPrimitiveOrWellKnownType(Type type) | ||
| { | ||
| // Check user-defined primitives first (fast path for common case) | ||
|
|
@@ -67,18 +70,55 @@ public static bool IsPrimitiveOrWellKnownType(Type type) | |
| return true; | ||
| } | ||
|
|
||
| return type.IsPrimitive | ||
| || type.IsEnum | ||
| || type == typeof(string) | ||
| || type == typeof(decimal) | ||
| || type == typeof(DateTime) | ||
| || type == typeof(DateTimeOffset) | ||
| || type == typeof(TimeSpan) | ||
| || type == typeof(Guid) | ||
| if (type.IsPrimitive | ||
| || type.IsEnum | ||
| || type == typeof(string) | ||
| || type == typeof(decimal) | ||
| || type == typeof(DateTime) | ||
| || type == typeof(DateTimeOffset) | ||
| || type == typeof(TimeSpan) | ||
| || type == typeof(Guid) | ||
| #if NET6_0_OR_GREATER | ||
| || type == typeof(DateOnly) | ||
| || type == typeof(TimeOnly) | ||
| || type == typeof(DateOnly) | ||
| || type == typeof(TimeOnly) | ||
| #endif | ||
| ; | ||
| ) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| // Check if the type is a value type (struct) that implements IEquatable<T> for itself | ||
| // Value types like Vector2, Matrix3x2, etc. that implement IEquatable<T> | ||
| // should use value equality rather than structural comparison. | ||
| // We only check value types to avoid affecting records/classes that may have | ||
| // collection properties requiring structural comparison. | ||
| if (type.IsValueType && ImplementsSelfEquatable(type)) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Checks if a type implements IEquatable{T} where T is the type itself. | ||
| /// </summary> | ||
| private static bool ImplementsSelfEquatable( | ||
| [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.Interfaces)] | ||
| Type type) | ||
| { | ||
| // Iterate through interfaces to find IEquatable<T> where T is the type itself | ||
| // This approach is AOT-compatible as it doesn't use MakeGenericType | ||
| foreach (var iface in type.GetInterfaces()) | ||
| { | ||
| if (iface.IsGenericType | ||
| && iface.GetGenericTypeDefinition() == typeof(IEquatable<>) | ||
| && iface.GenericTypeArguments[0] == type) | ||
| { | ||
| return true; | ||
| } | ||
| } | ||
|
Comment on lines
+112
to
+120
|
||
|
|
||
| return false; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue number in the comment is incorrect. The PR description indicates this fixes issue #4081, but the comment references #3722.