Skip to content

Commit

Permalink
Simplify GenericDictionaryEquivalencyStep (#2191)
Browse files Browse the repository at this point in the history 
* Remove always true check

`comparands.Expectation` is null-checked at the first thing in `Handle`

* Make a single generic call

We would currently either call into AssertSameLengthMethod or AssertDictionaryEquivalenceMethod
This saves us a static field

* Use patterns over TryGet

* Inline Handle
  • Loading branch information
@jnyrup
jnyrup committed Apr 30, 2023
1 parent 6a575c9 commit 22a6039
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 91 deletions.
31 changes: 11 additions & 20 deletions Src/FluentAssertions/Equivalency/Steps/DictionaryInterfaceInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,8 @@ private DictionaryInterfaceInfo(Type key, Type value)
/// <remarks>>
/// The <paramref name="role"/> is used to describe the <paramref name="target"/> in failure messages.
/// </remarks>
public static bool TryGetFrom(Type target, string role, out DictionaryInterfaceInfo result)
public static DictionaryInterfaceInfo FindFrom(Type target, string role)
{
result = null;

var interfaces = GetDictionaryInterfacesFrom(target);

if (interfaces.Length > 1)
Expand All @@ -51,14 +49,12 @@ public static bool TryGetFrom(Type target, string role, out DictionaryInterfaceI
$"{string.Join(", ", (IEnumerable<DictionaryInterfaceInfo>)interfaces)}", nameof(role));
}

if (interfaces.Length == 1)
if (interfaces.Length == 0)
{
result = interfaces.Single();

return true;
return null;
}

return false;
return interfaces[0];
}

/// <summary>
Expand All @@ -69,10 +65,8 @@ public static bool TryGetFrom(Type target, string role, out DictionaryInterfaceI
/// <remarks>>
/// The <paramref name="role"/> is used to describe the <paramref name="target"/> in failure messages.
/// </remarks>
public static bool TryGetFromWithKey(Type target, string role, Type key, out DictionaryInterfaceInfo result)
public static DictionaryInterfaceInfo FindFromWithKey(Type target, string role, Type key)
{
result = null;

var suitableDictionaryInterfaces = GetDictionaryInterfacesFrom(target)
.Where(info => info.Key.IsAssignableFrom(key))
.ToArray();
Expand All @@ -83,16 +77,15 @@ public static bool TryGetFromWithKey(Type target, string role, Type key, out Dic
AssertionScope.Current.FailWith(
$"The {role} implements multiple IDictionary interfaces taking a key of {key}. ");

return false;
return null;
}

if (suitableDictionaryInterfaces.Length == 0)
{
return false;
return null;
}

result = suitableDictionaryInterfaces.Single();
return true;
return suitableDictionaryInterfaces[0];
}

private static DictionaryInterfaceInfo[] GetDictionaryInterfacesFrom(Type target)
Expand All @@ -118,7 +111,7 @@ private static DictionaryInterfaceInfo[] GetDictionaryInterfacesFrom(Type target
/// <returns>
/// <see langword="true"/> if the conversion succeeded or <see langword="false"/> otherwise.
/// </returns>
public bool TryConvertFrom(object convertable, out object dictionary)
public object ConvertFrom(object convertable)
{
Type[] enumerables = convertable.GetType().GetClosedGenericInterfaces(typeof(IEnumerable<>));

Expand All @@ -132,12 +125,10 @@ public bool TryConvertFrom(object convertable, out object dictionary)
Type pairValueType = suitableKeyValuePairCollection.GenericTypeArguments.Last();

var methodInfo = ConvertToDictionaryMethod.MakeGenericMethod(Key, pairValueType);
dictionary = methodInfo.Invoke(null, new[] { convertable });
return true;
return methodInfo.Invoke(null, new[] { convertable });
}

dictionary = null;
return false;
return null;
}

private static Dictionary<TKey, TValue> ConvertToDictionaryInternal<TKey, TValue>(
Expand Down
109 changes: 38 additions & 71 deletions Src/FluentAssertions/Equivalency/Steps/GenericDictionaryEquivalencyStep.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@ namespace FluentAssertions.Equivalency.Steps;
public class GenericDictionaryEquivalencyStep : IEquivalencyStep
{
#pragma warning disable SA1110 // Allow opening parenthesis on new line to reduce line length
private static readonly MethodInfo AssertSameLengthMethod =
new Func<IDictionary<object, object>, IDictionary<object, object>, bool>(AssertSameLength).GetMethodInfo()
.GetGenericMethodDefinition();

private static readonly MethodInfo AssertDictionaryEquivalenceMethod =
new Action<EquivalencyValidationContext, IEquivalencyValidator, IEquivalencyAssertionOptions,
IDictionary<object, object>, IDictionary<object, object>>
Expand All @@ -26,11 +22,13 @@ public class GenericDictionaryEquivalencyStep : IEquivalencyStep
if (comparands.Expectation != null)
{
Type expectationType = comparands.GetExpectedType(context.Options);
bool isDictionary = DictionaryInterfaceInfo.TryGetFrom(expectationType, "expectation", out var expectedDictionary);

if (isDictionary)
if (DictionaryInterfaceInfo.FindFrom(expectationType, "expectation") is { } expectedDictionary)
{
Handle(comparands, expectedDictionary, context, nestedValidator);
if (AssertSubjectIsNotNull(comparands.Subject)
&& EnsureSubjectIsDictionary(comparands, expectedDictionary) is { } actualDictionary)
{
AssertDictionaryEquivalence(comparands, context, nestedValidator, actualDictionary, expectedDictionary);
}

return EquivalencyResult.AssertionCompleted;
}
Expand All @@ -39,74 +37,36 @@ public class GenericDictionaryEquivalencyStep : IEquivalencyStep
return EquivalencyResult.ContinueWithNext;
}

private static void Handle(Comparands comparands, DictionaryInterfaceInfo expectedDictionary,
IEquivalencyValidationContext context,
IEquivalencyValidator nestedValidator)
{
if (AssertSubjectIsNotNull(comparands.Subject)
&& AssertExpectationIsNotNull(comparands.Subject, comparands.Expectation))
{
var (isDictionary, actualDictionary) = EnsureSubjectIsDictionary(comparands, expectedDictionary);

if (isDictionary && AssertSameLength(comparands, actualDictionary, expectedDictionary))
{
AssertDictionaryEquivalence(comparands, context, nestedValidator, actualDictionary, expectedDictionary);
}
}
}

private static bool AssertSubjectIsNotNull(object subject)
{
return AssertionScope.Current
.ForCondition(subject is not null)
.FailWith("Expected {context:Subject} not to be {0}{reason}.", new object[] { null });
}

private static bool AssertExpectationIsNotNull(object subject, object expectation)
{
return AssertionScope.Current
.ForCondition(expectation is not null)
.FailWith("Expected {context:Subject} to be {0}{reason}, but found {1}.", null, subject);
}

private static (bool isDictionary, DictionaryInterfaceInfo info) EnsureSubjectIsDictionary(Comparands comparands,
private static DictionaryInterfaceInfo EnsureSubjectIsDictionary(Comparands comparands,
DictionaryInterfaceInfo expectedDictionary)
{
bool isDictionary = DictionaryInterfaceInfo.TryGetFromWithKey(comparands.Subject.GetType(), "subject",
expectedDictionary.Key, out var actualDictionary);
var actualDictionary = DictionaryInterfaceInfo.FindFromWithKey(comparands.Subject.GetType(), "subject",
expectedDictionary.Key);

if (!isDictionary && expectedDictionary.TryConvertFrom(comparands.Subject, out var convertedSubject))
if (actualDictionary is null && expectedDictionary.ConvertFrom(comparands.Subject) is { } convertedSubject)
{
comparands.Subject = convertedSubject;
isDictionary = DictionaryInterfaceInfo.TryGetFrom(comparands.Subject.GetType(), "subject", out actualDictionary);
actualDictionary = DictionaryInterfaceInfo.FindFrom(comparands.Subject.GetType(), "subject");
}

if (!isDictionary)
if (actualDictionary is null)
{
AssertionScope.Current.FailWith(
$"Expected {{context:subject}} to be a dictionary or collection of key-value pairs that is keyed to type {expectedDictionary.Key}. " +
$"It implements {actualDictionary}.");
}

return (isDictionary, actualDictionary);
return actualDictionary;
}

private static bool AssertSameLength(Comparands comparands, DictionaryInterfaceInfo actualDictionary,
DictionaryInterfaceInfo expectedDictionary)
{
if (comparands.Subject is ICollection subjectCollection
&& comparands.Expectation is ICollection expectationCollection
&& subjectCollection.Count == expectationCollection.Count)
{
return true;
}

return (bool)AssertSameLengthMethod
.MakeGenericMethod(actualDictionary.Key, actualDictionary.Value, expectedDictionary.Key, expectedDictionary.Value)
.Invoke(null, new[] { comparands.Subject, comparands.Expectation });
}

private static bool AssertSameLength<TSubjectKey, TSubjectValue, TExpectedKey, TExpectedValue>(
private static void FailWithLengthDifference<TSubjectKey, TSubjectValue, TExpectedKey, TExpectedValue>(
IDictionary<TSubjectKey, TSubjectValue> subject, IDictionary<TExpectedKey, TExpectedValue> expectation)

// Type constraint of TExpectedKey is asymmetric in regards to TSubjectKey
Expand All @@ -119,7 +79,7 @@ private static bool AssertExpectationIsNotNull(object subject, object expectatio
bool hasMissingKeys = keyDifference.MissingKeys.Count > 0;
bool hasAdditionalKeys = keyDifference.AdditionalKeys.Any();

return Execute.Assertion
Execute.Assertion
.WithExpectation("Expected {context:subject} to be a dictionary with {0} item(s){reason}, ", expectation.Count)
.ForCondition(!hasMissingKeys || hasAdditionalKeys)
.FailWith("but it misses key(s) {0}", keyDifference.MissingKeys)
Expand Down Expand Up @@ -183,32 +143,39 @@ private static bool AssertExpectationIsNotNull(object subject, object expectatio
IDictionary<TExpectedKey, TExpectedValue> expectation)
where TExpectedKey : TSubjectKey
{
foreach (TExpectedKey key in expectation.Keys)
if (subject.Count != expectation.Count)
{
if (subject.TryGetValue(key, out TSubjectValue subjectValue))
FailWithLengthDifference(subject, expectation);
}
else
{
foreach (TExpectedKey key in expectation.Keys)
{
if (options.IsRecursive)
if (subject.TryGetValue(key, out TSubjectValue subjectValue))
{
// Run the child assertion without affecting the current context
using (new AssertionScope())
if (options.IsRecursive)
{
var nestedComparands = new Comparands(subject[key], expectation[key], typeof(TExpectedValue));

parent.RecursivelyAssertEquality(nestedComparands,
context.AsDictionaryItem<TExpectedKey, TExpectedValue>(key));
// Run the child assertion without affecting the current context
using (new AssertionScope())
{
var nestedComparands = new Comparands(subject[key], expectation[key], typeof(TExpectedValue));

parent.RecursivelyAssertEquality(nestedComparands,
context.AsDictionaryItem<TExpectedKey, TExpectedValue>(key));
}
}
else
{
subjectValue.Should().Be(expectation[key], context.Reason.FormattedMessage, context.Reason.Arguments);
}
}
else
{
subjectValue.Should().Be(expectation[key], context.Reason.FormattedMessage, context.Reason.Arguments);
AssertionScope.Current
.BecauseOf(context.Reason)
.FailWith("Expected {context:subject} to contain key {0}{reason}.", key);
}
}
else
{
AssertionScope.Current
.BecauseOf(context.Reason)
.FailWith("Expected {context:subject} to contain key {0}{reason}.", key);
}
}
}

Expand Down

0 comments on commit 22a6039

Please sign in to comment.