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

Fix ordering collections by the identity function #1802

Merged
merged 2 commits into from Feb 11, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 6 additions & 8 deletions Src/FluentAssertions/Collections/GenericCollectionAssertions.cs
Expand Up @@ -2993,13 +2993,11 @@ public AndConstraint<TAssertions> StartWith(T element, string because = "", para
direction,
unordered);

string orderString = GetExpressionOrderString(propertyExpression);

Execute.Assertion
.ForCondition(unordered.SequenceEqual(expectation))
.BecauseOf(because, becauseArgs)
.FailWith("Expected {context:collection} {0} to be ordered {1}{reason} and result in {2}.",
Subject, orderString, expectation);
() => Subject, () => GetExpressionOrderString(propertyExpression), () => expectation);

return new AndConstraint<SubsequentOrderingAssertions<T>>(
new SubsequentOrderingAssertions<T>(Subject, expectation));
Expand Down Expand Up @@ -3188,11 +3186,13 @@ private bool IsValidProperty<TSelector>(Expression<Func<T, TSelector>> propertyE
Guard.ThrowIfArgumentIsNull(propertyExpression, nameof(propertyExpression),
"Cannot assert collection ordering without specifying a property.");

propertyExpression.ValidateMemberPath();

return Execute.Assertion
.ForCondition(Subject is not null)
.BecauseOf(because, becauseArgs)
.ForCondition(Subject is not null)
.FailWith("Expected {context:collection} to be ordered by {0}{reason} but found <null>.",
propertyExpression.GetMemberPath());
() => propertyExpression.GetMemberPath());
}

private AndConstraint<TAssertions> NotBeOrderedBy<TSelector>(
Expand All @@ -3214,13 +3214,11 @@ private bool IsValidProperty<TSelector>(Expression<Func<T, TSelector>> propertyE
direction,
unordered);

string orderString = GetExpressionOrderString(propertyExpression);

Execute.Assertion
.ForCondition(!unordered.SequenceEqual(expectation))
.BecauseOf(because, becauseArgs)
.FailWith("Expected {context:collection} {0} to not be ordered {1}{reason} and not result in {2}.",
Subject, orderString, expectation);
() => Subject, () => GetExpressionOrderString(propertyExpression), () => expectation);
}

return new AndConstraint<TAssertions>((TAssertions)this);
Expand Down
58 changes: 58 additions & 0 deletions Src/FluentAssertions/Common/ExpressionExtensions.cs
Expand Up @@ -103,6 +103,64 @@ internal static class ExpressionExtensions
return new MemberPath(typeof(TDeclaringType), declaringType, segmentPath.Replace(".[", "[", StringComparison.Ordinal));
}

/// <summary>
/// Validates that the expression can be used to construct a <see cref="MemberPath"/>.
/// </summary>
public static void ValidateMemberPath<TDeclaringType, TPropertyType>(
this Expression<Func<TDeclaringType, TPropertyType>> expression)
{
Guard.ThrowIfArgumentIsNull(expression, nameof(expression), "Expected an expression, but found <null>.");

Expression node = expression;

while (node is not null)
{
#pragma warning disable IDE0010 // System.Linq.Expressions.ExpressionType has many members we do not care about
switch (node.NodeType)
#pragma warning restore IDE0010
{
case ExpressionType.Lambda:
node = ((LambdaExpression)node).Body;
break;

case ExpressionType.Convert:
case ExpressionType.ConvertChecked:
var unaryExpression = (UnaryExpression)node;
node = unaryExpression.Operand;
break;

case ExpressionType.MemberAccess:
var memberExpression = (MemberExpression)node;
node = memberExpression.Expression;

break;

case ExpressionType.ArrayIndex:
var binaryExpression = (BinaryExpression)node;
node = binaryExpression.Left;

break;

case ExpressionType.Parameter:
node = null;
break;

case ExpressionType.Call:
var methodCallExpression = (MethodCallExpression)node;
if (methodCallExpression.Method.Name != "get_Item" || methodCallExpression.Arguments.Count != 1 || methodCallExpression.Arguments[0] is not ConstantExpression)
{
throw new ArgumentException(GetUnsupportedExpressionMessage(expression.Body), nameof(expression));
}

node = methodCallExpression.Object;
break;

default:
throw new ArgumentException(GetUnsupportedExpressionMessage(expression.Body), nameof(expression));
}
}
}

private static string GetUnsupportedExpressionMessage(Expression expression) =>
$"Expression <{expression}> cannot be used to select a member.";
}
Expand Down
4 changes: 2 additions & 2 deletions Src/FluentAssertions/Common/MemberPath.cs
Expand Up @@ -31,9 +31,9 @@ public MemberPath(Type reflectedType, Type declaringType, string dottedPath)

public MemberPath(string dottedPath)
{
Guard.ThrowIfArgumentIsNullOrEmpty(
Guard.ThrowIfArgumentIsNull(
dottedPath, nameof(dottedPath),
"A member path cannot be null or empty");
"A member path cannot be null");

this.dottedPath = dottedPath;
}
Expand Down
Expand Up @@ -14,6 +14,11 @@ internal class MappedPathMatchingRule : IMemberMatchingRule

public MappedPathMatchingRule(string expectationMemberPath, string subjectMemberPath)
{
Guard.ThrowIfArgumentIsNullOrEmpty(expectationMemberPath,
nameof(expectationMemberPath), "A member path cannot be null");
Guard.ThrowIfArgumentIsNullOrEmpty(subjectMemberPath,
nameof(subjectMemberPath), "A member path cannot be null");

expectationPath = new MemberPath(expectationMemberPath);
subjectPath = new MemberPath(subjectMemberPath);

Expand Down
Expand Up @@ -81,6 +81,19 @@ public void When_asserting_the_items_in_an_unordered_collection_are_ordered_asce
" but found {1, 6, 12, 15, 12, 17, 26} where item at index 3 is in wrong order.");
}

[Fact]
public void Items_can_be_ordered_by_the_identity_function()
{
// Arrange
var collection = new[] { 1, 2 };

// Act
Action action = () => collection.Should().BeInAscendingOrder(x => x);

// Assert
action.Should().NotThrow();
}

#endregion

#region Not Be In Ascending Order
Expand Down
1 change: 1 addition & 0 deletions docs/_pages/releases.md
Expand Up @@ -12,6 +12,7 @@ sidebar:
### What's New

### Fixes
* Fix regression introduced in 6.5.0 where `collection.Should().BeInAscendingOrder(x => x)` would fail - [#1802](https://github.com/fluentassertions/fluentassertions/pull/1802)

### Fixes (Extensibility)

Expand Down