Skip to content
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
117 changes: 112 additions & 5 deletions src/TestFramework/TestFramework/Assertions/Assert.That.cs
Original file line number Diff line number Diff line change
Expand Up @@ -891,9 +891,14 @@ private static void HandleMethodCallExpression(MethodCallExpression callExpr, Di
// (to avoid showing both "list" and "list[0]")
ExtractVariablesFromExpression(callExpr.Arguments[0], details, evaluationCache, suppressIntermediateValues);
}
else if (callExpr.Method.Name == GetMethodName && callExpr.Object is not null && callExpr.Arguments.Count > 0)
else if (IsArrayGetCall(callExpr))
{
// Handle multi-dimensional array indexers (e.g., array.Get(i, j) displayed as array[i, j])
// Handle array indexers (e.g., array.Get(i, j) displayed as array[i, j]).
// In practice this only fires for multidimensional arrays — single-dimensional
// arrays surface as ArrayIndex in LINQ expressions, not as a Get method call.
// We gate on the receiver actually being an array so arbitrary user-defined
// `Get(...)` methods on non-array types are NOT mis-rendered as `obj[...]`
// (issue #6691); they go through the regular method-call path below.
string objectName = GetCleanMemberName(callExpr.Object);
string indexDisplay = string.Join(", ", callExpr.Arguments.Select(GetIndexArgumentDisplay));
string indexerDisplay = $"{objectName}[{indexDisplay}]";
Expand All @@ -919,9 +924,10 @@ private static void HandleMethodCallExpression(MethodCallExpression callExpr, Di
}
else
{
// For non-boolean methods, capture the method call itself
// (e.g., "list.Count" when used in a comparison)
string methodCallDisplay = GetCleanMemberName(callExpr);
// For non-boolean methods, capture the method call itself using a friendly receiver
// (issue #6691): static methods get the declaring type, captured-this instance methods
// render as `this.Method(...)`, extension methods on `this` also render as `this.Method(...)`.
string methodCallDisplay = GetMethodCallDisplayName(callExpr);
TryAddExpressionValue(callExpr, methodCallDisplay, details, evaluationCache);

// Don't extract from the object to avoid duplication
Expand All @@ -935,6 +941,107 @@ private static void HandleMethodCallExpression(MethodCallExpression callExpr, Di
}
}

/// <summary>
/// Matches <c>array.Get(i[, j[, k...]])</c> calls on an array receiver. In practice this only
/// fires for multidimensional arrays (e.g. <c>int[,]</c>) which expose runtime-synthesized
/// <c>Get</c>/<c>Set</c>/<c>Address</c> methods on the array type itself — not on
/// <see cref="Array"/>; single-dimensional arrays surface as <see cref="ExpressionType.ArrayIndex"/>
/// rather than a method call. Gating on the receiver actually being an array prevents arbitrary
/// user-defined instance methods named <c>Get</c> from being mis-rendered as <c>obj[...]</c>
/// (issue #6691).
/// </summary>
private static bool IsArrayGetCall(MethodCallExpression callExpr)
=> callExpr.Method.Name == GetMethodName
&& callExpr.Object is not null
&& callExpr.Object.Type.IsArray
Comment thread
Evangelink marked this conversation as resolved.
&& callExpr.Arguments.Count > 0;

/// <summary>
/// Builds a friendly display name for a method-call expression so the failure message uses the
/// same syntax the user wrote. Static methods get prefixed with their declaring type's name;
/// instance methods on captured <c>this</c> render as <c>this.Method(...)</c>; extension methods
/// use the first argument as the receiver. Fixes issue #6691.
/// </summary>
private static string GetMethodCallDisplayName(MethodCallExpression callExpr)
{
string methodName = callExpr.Method.Name;

// Extension methods are static methods on a static class marked [Extension]; the receiver is the
// first argument. Render like the user wrote: receiver.Method(rest).
if (callExpr.Object is null
&& callExpr.Method.IsDefined(typeof(ExtensionAttribute), inherit: false)
&& callExpr.Arguments.Count > 0)
{
Expression firstArg = callExpr.Arguments[0];
Type receiverParamType = callExpr.Method.GetParameters()[0].ParameterType;
string receiver = IsCapturedThis(firstArg, receiverParamType)
? "this"
: GetCleanMemberName(firstArg);
string extArgs = string.Join(", ", callExpr.Arguments.Skip(1).Select(static a => CleanExpressionText(a.ToString())));
return $"{receiver}.{methodName}({extArgs})";
}

string argsStr = string.Join(", ", callExpr.Arguments.Select(static a => CleanExpressionText(a.ToString())));

if (callExpr.Object is null)
{
// Regular static method: prefix with a friendly type display (no namespace, nested
// types separated with `.` instead of the reflection `+`) so nested types keep their
// nesting context (Outer.Inner.Method rather than Inner.Method).
string typeName = callExpr.Method.DeclaringType is { } dt
? GetFriendlyTypeName(dt)
: NullAngleBrackets;
return $"{typeName}.{methodName}({argsStr})";
}

if (IsCapturedThis(callExpr.Object, callExpr.Method.DeclaringType))
{
return $"this.{methodName}({argsStr})";
}

string objectDisplay = GetCleanMemberName(callExpr.Object);
return $"{objectDisplay}.{methodName}({argsStr})";
}

/// <summary>
/// Returns a user-friendly display name for <paramref name="type"/>: BCL aliases via
/// <see cref="CleanTypeName(string)"/>, namespace stripped, and nested-type separators
/// (reflection's <c>+</c>) converted to <c>.</c>.
/// </summary>
private static string GetFriendlyTypeName(Type type)
{
string raw = type.Name;
string cleaned = CleanTypeName(raw);
if (!ReferenceEquals(cleaned, raw))
{
return cleaned;
}

// Walk up the nesting chain to produce Outer.Inner instead of Outer+Inner.
return type.IsNested && type.DeclaringType is { } declaring
? $"{GetFriendlyTypeName(declaring)}.{type.Name}"
: type.Name;
}

/// <summary>
/// Returns <see langword="true"/> if <paramref name="objectExpr"/> is a reference to the enclosing
/// instance (<c>this</c>) — either accessed via the compiler-synthesized display-class field
/// (named like <c>&lt;&gt;4__this</c>) or as a <see cref="ConstantExpression"/> representing
/// the enclosing instance (no-closure case). For the constant form we require the expression's
/// static type to exactly match its runtime type and be assignable to <paramref name="declaringType"/>,
/// so inherited methods on <c>this</c> still render as <c>this.Method(...)</c> without
/// mis-labeling base-typed locals as <c>this</c>.
/// </summary>
private static bool IsCapturedThis(Expression objectExpr, Type? declaringType)
=> (objectExpr is MemberExpression me
&& me.Member.Name.StartsWith("<>", StringComparison.Ordinal)
&& me.Member.Name.EndsWith("__this", StringComparison.Ordinal))
|| (declaringType is not null
&& objectExpr is ConstantExpression ce
&& ce.Value is not null
&& ce.Type == ce.Value.GetType()
&& declaringType.IsAssignableFrom(ce.Type));

/// <summary>
/// Evaluates only the *sub-children* of a writable assignment target (the Left of an Assign or
/// compound assignment, or the Operand of a Pre/Post Increment/Decrement). Walking the writable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1514,6 +1514,65 @@ public void That_NullDelegateTypedMember_StillAppearsInDetails()
ex.Message.Should().Contain("null");
}

// ---- #6691 method-call display: locks down the friendly receiver rendering ---------------
// Without the fix, arbitrary `Get(...)` instance methods on non-array types render as
// `obj[arg]` (misleading), instance methods on `this` render with the full type name, and
// static methods on the same type show as `Method(args)` with no receiver context.
private static int GetAnimal() => 42;

public int GetAnimalInstance() => 7;

public void That_ArbitraryGetMethodOnNonArray_RendersAsMethodCall_NotIndexer()
Comment thread
Evangelink marked this conversation as resolved.
{
var box = new GetterBox();
int expected = 0;

Action act = () => Assert.That(() => box.Get(1) == expected);

AssertFailedException ex = act.Should().Throw<AssertFailedException>().Which;
// Must render as `box.Get(1)`, not as `box[1]` (former #6691 regression).
ex.Message.Should().Contain("box.Get(1)");
ex.Message.Should().NotContain("box[1]");
}

public void That_StaticMethodOnSameType_RendersWithTypeNamePrefix()
Comment thread
Evangelink marked this conversation as resolved.
{
int expected = 0;

Action act = () => Assert.That(() => GetAnimal() == expected);

AssertFailedException ex = act.Should().Throw<AssertFailedException>().Which;
// The declaring type should prefix the static method call so the user can locate it.
ex.Message.Should().Contain(nameof(GetAnimal));
ex.Message.Should().Contain(nameof(AssertTests));
}

public void That_InstanceMethodOnThis_RendersAsThisMethod()
Comment thread
Evangelink marked this conversation as resolved.
{
int expected = 0;

Action act = () => Assert.That(() => GetAnimalInstance() == expected);

AssertFailedException ex = act.Should().Throw<AssertFailedException>().Which;
// Captured-this instance methods should render as `this.MethodName(...)`.
ex.Message.Should().Contain($"this.{nameof(GetAnimalInstance)}");
}

public void That_ExtensionMethodOnThis_RendersAsThisMethod()
Comment thread
Evangelink marked this conversation as resolved.
{
string expected = "wrong";

Action act = () => Assert.That(() => this.GetGreeting() == expected);

AssertFailedException ex = act.Should().Throw<AssertFailedException>().Which;
ex.Message.Should().Contain($"this.{nameof(AssertTestsExtensions.GetGreeting)}");
}

private sealed class GetterBox
{
public int Get(int key) => key + 100;
}

// ---- Object-typed sub-expressions: locks down current behavior when two side-effecting
// method calls return the same mutable reference. The cache stores reference values, so by
// the time details are extracted both slots point to the post-mutation object; with
Expand Down Expand Up @@ -1561,6 +1620,11 @@ public Shape GetValueWithSideEffect()
}
}

internal static class AssertTestsExtensions
{
public static string GetGreeting(this AssertTests _) => "hello";
}

internal static class MutableBoxHelper
{
public static int ComputeValue() => 42;
Expand Down
Loading