From 6a55818bb1398352ffde319e4f9e052d5681bb27 Mon Sep 17 00:00:00 2001 From: Salvatore Isaja Date: Mon, 10 Oct 2022 19:23:53 +0200 Subject: [PATCH 01/12] Treat record structs as records (issue #1808) --- Src/FluentAssertions/Common/TypeExtensions.cs | 18 +++-- .../RecordSpecs.cs | 15 +++++ .../Types/TypeExtensionsSpecs.cs | 66 +++++++++++++++++++ 3 files changed, 93 insertions(+), 6 deletions(-) diff --git a/Src/FluentAssertions/Common/TypeExtensions.cs b/Src/FluentAssertions/Common/TypeExtensions.cs index 996602dbbd..ba992b3b97 100644 --- a/Src/FluentAssertions/Common/TypeExtensions.cs +++ b/Src/FluentAssertions/Common/TypeExtensions.cs @@ -584,12 +584,18 @@ private static bool IsAnonymousType(this Type type) public static bool IsRecord(this Type type) { return TypeIsRecordCache.GetOrAdd(type, static t => - t.GetMethod("$") is not null && - t.GetTypeInfo() - .DeclaredProperties - .FirstOrDefault(p => p.Name == "EqualityContract")? - .GetMethod? - .GetCustomAttribute(typeof(CompilerGeneratedAttribute)) is not null); + { + bool isRecord = t.GetMethod("$") is not null && + t.GetTypeInfo() + .DeclaredProperties + .FirstOrDefault(p => p.Name == "EqualityContract")? + .GetMethod? + .GetCustomAttribute(typeof(CompilerGeneratedAttribute)) is not null; + bool isRecordStruct = t.BaseType == typeof(ValueType) && + t.GetMethods().Where(m => m.Name == "op_Inequality").SelectMany(m => m.GetCustomAttributes(typeof(CompilerGeneratedAttribute))).Any() && + t.GetMethods().Where(m => m.Name == "op_Equality").SelectMany(m => m.GetCustomAttributes(typeof(CompilerGeneratedAttribute))).Any(); + return isRecord || isRecordStruct; + }); } private static bool IsKeyValuePair(Type type) diff --git a/Tests/FluentAssertions.Equivalency.Specs/RecordSpecs.cs b/Tests/FluentAssertions.Equivalency.Specs/RecordSpecs.cs index d7931e25df..d70c9de403 100644 --- a/Tests/FluentAssertions.Equivalency.Specs/RecordSpecs.cs +++ b/Tests/FluentAssertions.Equivalency.Specs/RecordSpecs.cs @@ -16,6 +16,16 @@ public void When_the_subject_is_a_record_it_should_compare_it_by_its_members() actual.Should().BeEquivalentTo(expected); } + [Fact] + public void When_the_subject_is_a_readonly_record_struct_it_should_compare_it_by_its_members() + { + var actual = new MyReadonlyRecordStruct("foo", new[] { "bar", "zip", "foo" }); + + var expected = new MyReadonlyRecordStruct("foo", new[] { "bar", "zip", "foo" }); + + actual.Should().BeEquivalentTo(expected); + } + [Fact] public void When_the_subject_is_a_record_it_should_mention_that_in_the_configuration_output() { @@ -90,4 +100,9 @@ private record MyRecord public string[] CollectionProperty { get; init; } } + + private readonly record struct MyReadonlyRecordStruct(string StringField, string[] CollectionProperty) + { + public readonly string StringField = StringField; + } } diff --git a/Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs b/Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs index 382d15fa5d..c7f08cfa46 100644 --- a/Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs +++ b/Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Immutable; using System.Linq; using System.Reflection; using FluentAssertions.Common; @@ -124,6 +125,33 @@ public void When_getting_fake_implicit_conversion_operator_from_a_type_with_fake result.Should().NotBeNull(); } + [Theory] + [InlineData(typeof(MyRecord), true)] + [InlineData(typeof(MyRecordStruct), true)] + [InlineData(typeof(MyRecordStructWithOverriddenEquality), true)] + [InlineData(typeof(MyReadonlyRecordStruct), true)] + [InlineData(typeof(MyStruct), false)] + [InlineData(typeof(MyStructWithOverriddenEquality), false)] + [InlineData(typeof(MyClass), false)] + [InlineData(typeof(int), false)] + [InlineData(typeof(string), false)] + public void IsRecord_should_detect_records_correctly(Type type, bool expected) + { + type.IsRecord().Should().Be(expected); + } + + [Fact] + public void When_checking_if_anonymous_type_is_record_it_should_return_false() + { + new { Value = 42 }.GetType().IsRecord().Should().Be(false); + } + + [Fact] + public void When_checking_if_class_with_multiple_equality_methods_is_record_it_should_return_false() + { + typeof(ImmutableArray).IsRecord().Should().Be(false); + } + private static MethodInfo GetFakeConversionOperator(Type type, string name, BindingFlags bindingAttr, Type returnType) { MethodInfo[] methods = type.GetMethods(bindingAttr); @@ -153,4 +181,42 @@ private TypeWithFakeConversionOperators(int value) public static byte op_Explicit(TypeWithFakeConversionOperators typeWithFakeConversionOperators) => (byte)typeWithFakeConversionOperators.value; #pragma warning restore SA1300, IDE1006 } + + private record MyRecord(int Value); + + private record struct MyRecordStruct(int Value); + + private record struct MyRecordStructWithOverriddenEquality(int Value) + { + public bool Equals(MyRecordStructWithOverriddenEquality other) => Value == other.Value; + + public override int GetHashCode() => Value; + } + + private readonly record struct MyReadonlyRecordStruct(int Value); + + private struct MyStruct + { + public int Value { get; set; } + } + + private struct MyStructWithOverriddenEquality : IEquatable + { + public int Value { get; set; } + + public bool Equals(MyStructWithOverriddenEquality other) => Value == other.Value; + + public override bool Equals(object obj) => obj is MyStructWithOverriddenEquality other && Equals(other); + + public override int GetHashCode() => Value; + + public static bool operator ==(MyStructWithOverriddenEquality left, MyStructWithOverriddenEquality right) => left.Equals(right); + + public static bool operator !=(MyStructWithOverriddenEquality left, MyStructWithOverriddenEquality right) => !left.Equals(right); + } + + private class MyClass + { + public int Value { get; set; } + } } From 2045c6a700e7e304610810b9db1320744db5f954 Mon Sep 17 00:00:00 2001 From: Salvatore Isaja Date: Wed, 12 Oct 2022 21:03:48 +0200 Subject: [PATCH 02/12] Use BeFalse in IsRecord assertions --- Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs b/Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs index c7f08cfa46..3a1df75233 100644 --- a/Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs +++ b/Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs @@ -143,13 +143,13 @@ public void IsRecord_should_detect_records_correctly(Type type, bool expected) [Fact] public void When_checking_if_anonymous_type_is_record_it_should_return_false() { - new { Value = 42 }.GetType().IsRecord().Should().Be(false); + new { Value = 42 }.GetType().IsRecord().Should().BeFalse(); } [Fact] public void When_checking_if_class_with_multiple_equality_methods_is_record_it_should_return_false() { - typeof(ImmutableArray).IsRecord().Should().Be(false); + typeof(ImmutableArray).IsRecord().Should().BeFalse(); } private static MethodInfo GetFakeConversionOperator(Type type, string name, BindingFlags bindingAttr, Type returnType) From df8558b7515949f386ac435122788359a0b8cabc Mon Sep 17 00:00:00 2001 From: Salvatore Isaja Date: Sun, 16 Oct 2022 12:06:46 +0200 Subject: [PATCH 03/12] Update releases.md and reformat TypeExtensions.IsRecord. --- Src/FluentAssertions/Common/TypeExtensions.cs | 12 ++++++++++-- docs/_pages/releases.md | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/Src/FluentAssertions/Common/TypeExtensions.cs b/Src/FluentAssertions/Common/TypeExtensions.cs index ba992b3b97..e5326ce50c 100644 --- a/Src/FluentAssertions/Common/TypeExtensions.cs +++ b/Src/FluentAssertions/Common/TypeExtensions.cs @@ -591,9 +591,17 @@ public static bool IsRecord(this Type type) .FirstOrDefault(p => p.Name == "EqualityContract")? .GetMethod? .GetCustomAttribute(typeof(CompilerGeneratedAttribute)) is not null; + bool isRecordStruct = t.BaseType == typeof(ValueType) && - t.GetMethods().Where(m => m.Name == "op_Inequality").SelectMany(m => m.GetCustomAttributes(typeof(CompilerGeneratedAttribute))).Any() && - t.GetMethods().Where(m => m.Name == "op_Equality").SelectMany(m => m.GetCustomAttributes(typeof(CompilerGeneratedAttribute))).Any(); + t.GetMethods() + .Where(m => m.Name == "op_Inequality") + .SelectMany(m => m.GetCustomAttributes(typeof(CompilerGeneratedAttribute))) + .Any() && + t.GetMethods() + .Where(m => m.Name == "op_Equality") + .SelectMany(m => m.GetCustomAttributes(typeof(CompilerGeneratedAttribute))) + .Any(); + return isRecord || isRecordStruct; }); } diff --git a/docs/_pages/releases.md b/docs/_pages/releases.md index 4e47815073..fb519883d2 100644 --- a/docs/_pages/releases.md +++ b/docs/_pages/releases.md @@ -25,7 +25,7 @@ sidebar: ### Enhancements * Included the time difference in the error message of `BeCloseTo` - [#2013](https://github.com/fluentassertions/fluentassertions/pull/2013) - +* Treated record structs and readonly record structs as records, thus comparing them by member by default - [#2009](https://github.com/fluentassertions/fluentassertions/pull/2009) ## 6.7.0 ### What's new From 3ca8329d184e287b81bea05fbfb2f136d98eb2a3 Mon Sep 17 00:00:00 2001 From: Salvatore Isaja Date: Mon, 17 Oct 2022 07:42:15 +0200 Subject: [PATCH 04/12] Improve documentation about record structs --- Src/FluentAssertions/Common/TypeExtensions.cs | 35 ++++++++++--------- docs/_pages/objectgraphs.md | 4 +-- docs/_pages/releases.md | 1 + 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/Src/FluentAssertions/Common/TypeExtensions.cs b/Src/FluentAssertions/Common/TypeExtensions.cs index e5326ce50c..fe2937bbeb 100644 --- a/Src/FluentAssertions/Common/TypeExtensions.cs +++ b/Src/FluentAssertions/Common/TypeExtensions.cs @@ -585,22 +585,25 @@ public static bool IsRecord(this Type type) { return TypeIsRecordCache.GetOrAdd(type, static t => { - bool isRecord = t.GetMethod("$") is not null && - t.GetTypeInfo() - .DeclaredProperties - .FirstOrDefault(p => p.Name == "EqualityContract")? - .GetMethod? - .GetCustomAttribute(typeof(CompilerGeneratedAttribute)) is not null; - - bool isRecordStruct = t.BaseType == typeof(ValueType) && - t.GetMethods() - .Where(m => m.Name == "op_Inequality") - .SelectMany(m => m.GetCustomAttributes(typeof(CompilerGeneratedAttribute))) - .Any() && - t.GetMethods() - .Where(m => m.Name == "op_Equality") - .SelectMany(m => m.GetCustomAttributes(typeof(CompilerGeneratedAttribute))) - .Any(); + var isRecord = t.GetMethod("$") is not null && + t.GetTypeInfo() + .DeclaredProperties + .FirstOrDefault(p => p.Name == "EqualityContract")? + .GetMethod? + .GetCustomAttribute(typeof(CompilerGeneratedAttribute)) is not null; + + // As noted here: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-10.0/record-structs#open-questions + // recognizing record structs from metadata is an open point. The following check is based on common sense + // and heuristic testing, apparently giving good results but not supported by official documentation. + var isRecordStruct = t.BaseType == typeof(ValueType) && + t.GetMethods() + .Where(m => m.Name == "op_Inequality") + .SelectMany(m => m.GetCustomAttributes(typeof(CompilerGeneratedAttribute))) + .Any() && + t.GetMethods() + .Where(m => m.Name == "op_Equality") + .SelectMany(m => m.GetCustomAttributes(typeof(CompilerGeneratedAttribute))) + .Any(); return isRecord || isRecordStruct; }); diff --git a/docs/_pages/objectgraphs.md b/docs/_pages/objectgraphs.md index fc37bf3496..a38e1f5c5d 100644 --- a/docs/_pages/objectgraphs.md +++ b/docs/_pages/objectgraphs.md @@ -39,7 +39,7 @@ orderDto.Should().BeEquivalentTo(order, options => ### Value Types -To determine whether Fluent Assertions should recurs into an object's properties or fields, it needs to understand what types have value semantics and what types should be treated as reference types. The default behavior is to treat every type that overrides `Object.Equals` as an object that was designed to have value semantics. Anonymous types, records and tuples also override this method, but because the community proved us that they use them quite often in equivalency comparisons, we decided to always compare them by their members. +To determine whether Fluent Assertions should recurs into an object's properties or fields, it needs to understand what types have value semantics and what types should be treated as reference types. The default behavior is to treat every type that overrides `Object.Equals` as an object that was designed to have value semantics. Anonymous types, records, record structs, readonly record structs and tuples also override this method, but because the community proved us that they use them quite often in equivalency comparisons, we decided to always compare them by their members. You can easily override this by using the `ComparingByValue`, `ComparingByMembers`, `ComparingRecordsByValue` and `ComparingRecordsByMembers` options for individual assertions: @@ -48,7 +48,7 @@ subject.Should().BeEquivalentTo(expected, options => options.ComparingByValue()); ``` -For records, this works like this: +For records, record structs and readonly record structs this works like this: ```csharp actual.Should().BeEquivalentTo(expected, options => options diff --git a/docs/_pages/releases.md b/docs/_pages/releases.md index fb519883d2..3301440e33 100644 --- a/docs/_pages/releases.md +++ b/docs/_pages/releases.md @@ -26,6 +26,7 @@ sidebar: ### Enhancements * Included the time difference in the error message of `BeCloseTo` - [#2013](https://github.com/fluentassertions/fluentassertions/pull/2013) * Treated record structs and readonly record structs as records, thus comparing them by member by default - [#2009](https://github.com/fluentassertions/fluentassertions/pull/2009) + ## 6.7.0 ### What's new From e94230b5600645d217c2cd65434ac9a4e1038f97 Mon Sep 17 00:00:00 2001 From: Salvatore Isaja Date: Sun, 23 Oct 2022 10:27:47 +0200 Subject: [PATCH 05/12] No need to be explicit about readonly record structs --- .../FluentAssertions.Equivalency.Specs/RecordSpecs.cs | 10 +++++----- docs/_pages/objectgraphs.md | 2 +- docs/_pages/releases.md | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Tests/FluentAssertions.Equivalency.Specs/RecordSpecs.cs b/Tests/FluentAssertions.Equivalency.Specs/RecordSpecs.cs index d70c9de403..9ce6473ae1 100644 --- a/Tests/FluentAssertions.Equivalency.Specs/RecordSpecs.cs +++ b/Tests/FluentAssertions.Equivalency.Specs/RecordSpecs.cs @@ -17,11 +17,11 @@ public void When_the_subject_is_a_record_it_should_compare_it_by_its_members() } [Fact] - public void When_the_subject_is_a_readonly_record_struct_it_should_compare_it_by_its_members() + public void When_the_subject_is_a_record_struct_it_should_compare_it_by_its_members() { - var actual = new MyReadonlyRecordStruct("foo", new[] { "bar", "zip", "foo" }); + var actual = new MyRecordStruct("foo", new[] { "bar", "zip", "foo" }); - var expected = new MyReadonlyRecordStruct("foo", new[] { "bar", "zip", "foo" }); + var expected = new MyRecordStruct("foo", new[] { "bar", "zip", "foo" }); actual.Should().BeEquivalentTo(expected); } @@ -101,8 +101,8 @@ private record MyRecord public string[] CollectionProperty { get; init; } } - private readonly record struct MyReadonlyRecordStruct(string StringField, string[] CollectionProperty) + private record struct MyRecordStruct(string StringField, string[] CollectionProperty) { - public readonly string StringField = StringField; + public string StringField = StringField; } } diff --git a/docs/_pages/objectgraphs.md b/docs/_pages/objectgraphs.md index a38e1f5c5d..35b9fe7a7a 100644 --- a/docs/_pages/objectgraphs.md +++ b/docs/_pages/objectgraphs.md @@ -39,7 +39,7 @@ orderDto.Should().BeEquivalentTo(order, options => ### Value Types -To determine whether Fluent Assertions should recurs into an object's properties or fields, it needs to understand what types have value semantics and what types should be treated as reference types. The default behavior is to treat every type that overrides `Object.Equals` as an object that was designed to have value semantics. Anonymous types, records, record structs, readonly record structs and tuples also override this method, but because the community proved us that they use them quite often in equivalency comparisons, we decided to always compare them by their members. +To determine whether Fluent Assertions should recurs into an object's properties or fields, it needs to understand what types have value semantics and what types should be treated as reference types. The default behavior is to treat every type that overrides `Object.Equals` as an object that was designed to have value semantics. Anonymous types, records, record structs and tuples also override this method, but because the community proved us that they use them quite often in equivalency comparisons, we decided to always compare them by their members. You can easily override this by using the `ComparingByValue`, `ComparingByMembers`, `ComparingRecordsByValue` and `ComparingRecordsByMembers` options for individual assertions: diff --git a/docs/_pages/releases.md b/docs/_pages/releases.md index 3301440e33..ed5492d92b 100644 --- a/docs/_pages/releases.md +++ b/docs/_pages/releases.md @@ -25,7 +25,7 @@ sidebar: ### Enhancements * Included the time difference in the error message of `BeCloseTo` - [#2013](https://github.com/fluentassertions/fluentassertions/pull/2013) -* Treated record structs and readonly record structs as records, thus comparing them by member by default - [#2009](https://github.com/fluentassertions/fluentassertions/pull/2009) +* Changed `BeEquivalentTo` to treat record structs like records, thus comparing them by member by default - [#2009](https://github.com/fluentassertions/fluentassertions/pull/2009) ## 6.7.0 From 105b5cab7d6ae1e2df1de464508201f57b51c844 Mon Sep 17 00:00:00 2001 From: Salvatore Isaja Date: Fri, 6 Jan 2023 18:18:15 +0100 Subject: [PATCH 06/12] Simplify expression to guess whether a type is a record struct. --- Src/FluentAssertions/Common/TypeExtensions.cs | 10 ++-------- docs/_pages/objectgraphs.md | 2 +- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/Src/FluentAssertions/Common/TypeExtensions.cs b/Src/FluentAssertions/Common/TypeExtensions.cs index 9016e9fbde..3dab6ab180 100644 --- a/Src/FluentAssertions/Common/TypeExtensions.cs +++ b/Src/FluentAssertions/Common/TypeExtensions.cs @@ -598,14 +598,8 @@ public static bool IsRecord(this Type type) // recognizing record structs from metadata is an open point. The following check is based on common sense // and heuristic testing, apparently giving good results but not supported by official documentation. var isRecordStruct = t.BaseType == typeof(ValueType) && - t.GetMethods() - .Where(m => m.Name == "op_Inequality") - .SelectMany(m => m.GetCustomAttributes(typeof(CompilerGeneratedAttribute))) - .Any() && - t.GetMethods() - .Where(m => m.Name == "op_Equality") - .SelectMany(m => m.GetCustomAttributes(typeof(CompilerGeneratedAttribute))) - .Any(); + t.GetMethod("op_Equality", BindingFlags.Static | BindingFlags.Public | BindingFlags.DeclaredOnly, null, new[] { t, t }, null)? + .GetCustomAttribute(typeof(CompilerGeneratedAttribute)) is not null; return isRecord || isRecordStruct; }); diff --git a/docs/_pages/objectgraphs.md b/docs/_pages/objectgraphs.md index 35b9fe7a7a..958da1b5c8 100644 --- a/docs/_pages/objectgraphs.md +++ b/docs/_pages/objectgraphs.md @@ -48,7 +48,7 @@ subject.Should().BeEquivalentTo(expected, options => options.ComparingByValue()); ``` -For records, record structs and readonly record structs this works like this: +For records and record structs this works like this: ```csharp actual.Should().BeEquivalentTo(expected, options => options From 49293506457bc1ed9f9fedcf88139679c2c6bcf0 Mon Sep 17 00:00:00 2001 From: Salvatore Isaja Date: Fri, 6 Jan 2023 18:31:31 +0100 Subject: [PATCH 07/12] Add test case for record struct with custom PrintMembers. --- .../Types/TypeExtensionsSpecs.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs b/Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs index 3a1df75233..9a09835afc 100644 --- a/Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs +++ b/Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs @@ -128,6 +128,7 @@ public void When_getting_fake_implicit_conversion_operator_from_a_type_with_fake [Theory] [InlineData(typeof(MyRecord), true)] [InlineData(typeof(MyRecordStruct), true)] + [InlineData(typeof(MyRecordStructWithCustomPrintMembers), true)] [InlineData(typeof(MyRecordStructWithOverriddenEquality), true)] [InlineData(typeof(MyReadonlyRecordStruct), true)] [InlineData(typeof(MyStruct), false)] @@ -186,6 +187,15 @@ private record MyRecord(int Value); private record struct MyRecordStruct(int Value); + private record struct MyRecordStructWithCustomPrintMembers(int Value) + { + private bool PrintMembers(System.Text.StringBuilder builder) + { + builder.Append(Value); + return true; + } + } + private record struct MyRecordStructWithOverriddenEquality(int Value) { public bool Equals(MyRecordStructWithOverriddenEquality other) => Value == other.Value; From 68869a74dc136fd9e3b4569c9776a8257a8f975f Mon Sep 17 00:00:00 2001 From: Salvatore Isaja Date: Fri, 6 Jan 2023 18:44:44 +0100 Subject: [PATCH 08/12] Add test case for value tuples not being mistaken for a record struct. --- Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs b/Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs index 9a09835afc..fe249ffebb 100644 --- a/Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs +++ b/Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs @@ -147,6 +147,12 @@ public void When_checking_if_anonymous_type_is_record_it_should_return_false() new { Value = 42 }.GetType().IsRecord().Should().BeFalse(); } + [Fact] + public void When_checking_if_value_tuple_is_record_it_should_return_false() + { + (42, "the answer").GetType().IsRecord().Should().BeFalse(); + } + [Fact] public void When_checking_if_class_with_multiple_equality_methods_is_record_it_should_return_false() { From 916df636485654335110b7f641162180f8dae11a Mon Sep 17 00:00:00 2001 From: Salvatore Isaja Date: Fri, 6 Jan 2023 21:55:54 +0100 Subject: [PATCH 09/12] Add a test case for a false positive in record struct detection. --- .../Types/TypeExtensionsSpecs.cs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs b/Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs index fe249ffebb..7ff238aa2b 100644 --- a/Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs +++ b/Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs @@ -132,6 +132,7 @@ public void When_getting_fake_implicit_conversion_operator_from_a_type_with_fake [InlineData(typeof(MyRecordStructWithOverriddenEquality), true)] [InlineData(typeof(MyReadonlyRecordStruct), true)] [InlineData(typeof(MyStruct), false)] + [InlineData(typeof(MyStructWithFakeCompilerGeneratedEquality), true)] // false positive! [InlineData(typeof(MyStructWithOverriddenEquality), false)] [InlineData(typeof(MyClass), false)] [InlineData(typeof(int), false)] @@ -216,6 +217,25 @@ private struct MyStruct public int Value { get; set; } } + // Note that this struct is mistakenly detected as a record struct by the current version of TypeExtensions.IsRecord. + // This cannot be avoided at present, unless something is changed at language level, + // or a smarter way to check for record structs is found. + private struct MyStructWithFakeCompilerGeneratedEquality : IEquatable + { + public int Value { get; set; } + + public bool Equals(MyStructWithFakeCompilerGeneratedEquality other) => Value == other.Value; + + public override bool Equals(object obj) => obj is MyStructWithFakeCompilerGeneratedEquality other && Equals(other); + + public override int GetHashCode() => Value; + + [System.Runtime.CompilerServices.CompilerGenerated] + public static bool operator ==(MyStructWithFakeCompilerGeneratedEquality left, MyStructWithFakeCompilerGeneratedEquality right) => left.Equals(right); + + public static bool operator !=(MyStructWithFakeCompilerGeneratedEquality left, MyStructWithFakeCompilerGeneratedEquality right) => !left.Equals(right); + } + private struct MyStructWithOverriddenEquality : IEquatable { public int Value { get; set; } From cbb3cd597fd2d6f8b0b96f95d36352b686f062b2 Mon Sep 17 00:00:00 2001 From: Salvatore Isaja Date: Sun, 8 Jan 2023 18:09:50 +0100 Subject: [PATCH 10/12] Check for record struct also looks for PrintMembers per @jnyrup suggestion. --- Src/FluentAssertions/Common/TypeExtensions.cs | 36 ++++++++++--------- .../Types/TypeExtensionsSpecs.cs | 31 +++++++++++++--- 2 files changed, 47 insertions(+), 20 deletions(-) diff --git a/Src/FluentAssertions/Common/TypeExtensions.cs b/Src/FluentAssertions/Common/TypeExtensions.cs index 23a1f316d3..0559e7dc7d 100644 --- a/Src/FluentAssertions/Common/TypeExtensions.cs +++ b/Src/FluentAssertions/Common/TypeExtensions.cs @@ -5,6 +5,7 @@ using System.Linq.Expressions; using System.Reflection; using System.Runtime.CompilerServices; +using System.Text; using FluentAssertions.Equivalency; namespace FluentAssertions.Common; @@ -587,22 +588,25 @@ private static bool IsAnonymousType(this Type type) public static bool IsRecord(this Type type) { - return TypeIsRecordCache.GetOrAdd(type, static t => - { - var isRecord = t.GetMethod("$", BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly) is { } && - t.GetProperty("EqualityContract", BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.DeclaredOnly)? - .GetMethod? - .GetCustomAttribute(typeof(CompilerGeneratedAttribute)) is { }; - - // As noted here: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-10.0/record-structs#open-questions - // recognizing record structs from metadata is an open point. The following check is based on common sense - // and heuristic testing, apparently giving good results but not supported by official documentation. - var isRecordStruct = t.BaseType == typeof(ValueType) && - t.GetMethod("op_Equality", BindingFlags.Static | BindingFlags.Public | BindingFlags.DeclaredOnly, null, new[] { t, t }, null)? - .GetCustomAttribute(typeof(CompilerGeneratedAttribute)) is not null; - - return isRecord || isRecordStruct; - }); + return TypeIsRecordCache.GetOrAdd(type, static t => t.IsRecordClass() || t.IsRecordStruct()); + } + + private static bool IsRecordClass(this Type type) + { + return type.GetMethod("$", BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly) is { } && + type.GetProperty("EqualityContract", BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.DeclaredOnly)? + .GetMethod?.IsDecoratedWith() == true; + } + + private static bool IsRecordStruct(this Type type) + { + // As noted here: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-10.0/record-structs#open-questions + // recognizing record structs from metadata is an open point. The following check is based on common sense + // and heuristic testing, apparently giving good results but not supported by official documentation. + return type.BaseType == typeof(ValueType) && + type.GetMethod("PrintMembers", BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.DeclaredOnly, null, new[] { typeof(StringBuilder) }, null) is { } && + type.GetMethod("op_Equality", BindingFlags.Static | BindingFlags.Public | BindingFlags.DeclaredOnly, null, new[] { type, type }, null)? + .IsDecoratedWith() == true; } private static bool IsKeyValuePair(Type type) diff --git a/Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs b/Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs index 7ff238aa2b..bb95403d3d 100644 --- a/Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs +++ b/Tests/FluentAssertions.Specs/Types/TypeExtensionsSpecs.cs @@ -132,7 +132,8 @@ public void When_getting_fake_implicit_conversion_operator_from_a_type_with_fake [InlineData(typeof(MyRecordStructWithOverriddenEquality), true)] [InlineData(typeof(MyReadonlyRecordStruct), true)] [InlineData(typeof(MyStruct), false)] - [InlineData(typeof(MyStructWithFakeCompilerGeneratedEquality), true)] // false positive! + [InlineData(typeof(MyStructWithFakeCompilerGeneratedEquality), false)] + [InlineData(typeof(MyStructWithFakeCompilerGeneratedEqualityAndPrintMembers), true)] // false positive! [InlineData(typeof(MyStructWithOverriddenEquality), false)] [InlineData(typeof(MyClass), false)] [InlineData(typeof(int), false)] @@ -217,9 +218,6 @@ private struct MyStruct public int Value { get; set; } } - // Note that this struct is mistakenly detected as a record struct by the current version of TypeExtensions.IsRecord. - // This cannot be avoided at present, unless something is changed at language level, - // or a smarter way to check for record structs is found. private struct MyStructWithFakeCompilerGeneratedEquality : IEquatable { public int Value { get; set; } @@ -236,6 +234,31 @@ private struct MyStructWithFakeCompilerGeneratedEquality : IEquatable !left.Equals(right); } + // Note that this struct is mistakenly detected as a record struct by the current version of TypeExtensions.IsRecord. + // This cannot be avoided at present, unless something is changed at language level, + // or a smarter way to check for record structs is found. + private struct MyStructWithFakeCompilerGeneratedEqualityAndPrintMembers : IEquatable + { + public int Value { get; set; } + + public bool Equals(MyStructWithFakeCompilerGeneratedEqualityAndPrintMembers other) => Value == other.Value; + + public override bool Equals(object obj) => obj is MyStructWithFakeCompilerGeneratedEqualityAndPrintMembers other && Equals(other); + + public override int GetHashCode() => Value; + + [System.Runtime.CompilerServices.CompilerGenerated] + public static bool operator ==(MyStructWithFakeCompilerGeneratedEqualityAndPrintMembers left, MyStructWithFakeCompilerGeneratedEqualityAndPrintMembers right) => left.Equals(right); + + public static bool operator !=(MyStructWithFakeCompilerGeneratedEqualityAndPrintMembers left, MyStructWithFakeCompilerGeneratedEqualityAndPrintMembers right) => !left.Equals(right); + + private bool PrintMembers(System.Text.StringBuilder builder) + { + builder.Append(Value); + return true; + } + } + private struct MyStructWithOverriddenEquality : IEquatable { public int Value { get; set; } From f8d0b399274d9eff38df963f1d513a95670ff356 Mon Sep 17 00:00:00 2001 From: Jonas Nyrup Date: Wed, 11 Jan 2023 09:27:43 +0100 Subject: [PATCH 11/12] Update docs/_pages/objectgraphs.md Co-authored-by: IT-VBFK <49762557+IT-VBFK@users.noreply.github.com> --- docs/_pages/objectgraphs.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/_pages/objectgraphs.md b/docs/_pages/objectgraphs.md index 958da1b5c8..b2080e85ce 100644 --- a/docs/_pages/objectgraphs.md +++ b/docs/_pages/objectgraphs.md @@ -48,7 +48,7 @@ subject.Should().BeEquivalentTo(expected, options => options.ComparingByValue()); ``` -For records and record structs this works like this: +For `record`s and `record struct`s this works like this: ```csharp actual.Should().BeEquivalentTo(expected, options => options From 3be69bb59719ffd7443ce25ae364f2e0caefafd8 Mon Sep 17 00:00:00 2001 From: Jonas Nyrup Date: Wed, 11 Jan 2023 11:43:47 +0100 Subject: [PATCH 12/12] Update docs/_pages/objectgraphs.md Co-authored-by: IT-VBFK <49762557+IT-VBFK@users.noreply.github.com> --- docs/_pages/objectgraphs.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/_pages/objectgraphs.md b/docs/_pages/objectgraphs.md index b2080e85ce..37e5b5bb7a 100644 --- a/docs/_pages/objectgraphs.md +++ b/docs/_pages/objectgraphs.md @@ -39,7 +39,7 @@ orderDto.Should().BeEquivalentTo(order, options => ### Value Types -To determine whether Fluent Assertions should recurs into an object's properties or fields, it needs to understand what types have value semantics and what types should be treated as reference types. The default behavior is to treat every type that overrides `Object.Equals` as an object that was designed to have value semantics. Anonymous types, records, record structs and tuples also override this method, but because the community proved us that they use them quite often in equivalency comparisons, we decided to always compare them by their members. +To determine whether Fluent Assertions should recurs into an object's properties or fields, it needs to understand what types have value semantics and what types should be treated as reference types. The default behavior is to treat every type that overrides `Object.Equals` as an object that was designed to have value semantics. Anonymous types, `record`s, `record struct`s and tuples also override this method, but because the community proved us that they use them quite often in equivalency comparisons, we decided to always compare them by their members. You can easily override this by using the `ComparingByValue`, `ComparingByMembers`, `ComparingRecordsByValue` and `ComparingRecordsByMembers` options for individual assertions: