From 0c18601e0ad6e31017935fea8df122b9d49f3c97 Mon Sep 17 00:00:00 2001 From: Evangelink Date: Tue, 19 May 2026 19:53:17 +0200 Subject: [PATCH] Assert.That: restore friendly method-call display from #6691 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #8307 re-architected `Assert.That` and silently regressed two display fixes from the original #6691 work: 1. Any instance method named `Get(...)` was rendered as `obj[...]` in failure details, regardless of whether the receiver was an array. So `box.Get(1) == 0` produced `box[1] = 101` instead of `box.Get(1) = 101`. The original fix gated this on `callExpr.Object.Type.IsArray`. 2. Non-boolean method calls fell back to a raw `callExpr.ToString()` cleanup, losing the friendly `this.Method(...)` / `Type.Method(...)` / `receiver.Method(...)` rendering. Inherited methods on `this`, captured-this extension methods, and static methods on the same type all suffered. Re-introduces: * `IsMultiDimensionalArrayGetCall` — scopes the `Get -> obj[...]` special case to actual array receivers (matches multi-dim arrays like `int[,]` whose `Get` is runtime-synthesized on the array type itself, not on `System.Array`). * `GetMethodCallDisplayName` — builds the friendly receiver string for non-boolean method calls. * `IsCapturedThis` — recognizes the compiler-synthesized `<>4__this` field used in closures AND the no-closure `ConstantExpression` form, guarded by `ce.Type == ce.Value.GetType()` so a base-typed local isn't mislabeled as `this` (while inherited methods on `this` are still rendered correctly via `IsAssignableFrom`). Four regression tests added in `AssertTests.That.cs`: * `That_ArbitraryGetMethodOnNonArray_RendersAsMethodCall_NotIndexer` * `That_StaticMethodOnSameType_RendersWithTypeNamePrefix` * `That_InstanceMethodOnThis_RendersAsThisMethod` * `That_ExtensionMethodOnThis_RendersAsThisMethod` All 918 `AssertTests` pass on `net9.0`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../TestFramework/Assertions/Assert.That.cs | 92 ++++++++++++++++++- .../Assertions/AssertTests.That.cs | 64 +++++++++++++ 2 files changed, 151 insertions(+), 5 deletions(-) diff --git a/src/TestFramework/TestFramework/Assertions/Assert.That.cs b/src/TestFramework/TestFramework/Assertions/Assert.That.cs index dadb59c4db..ba53bfa2dc 100644 --- a/src/TestFramework/TestFramework/Assertions/Assert.That.cs +++ b/src/TestFramework/TestFramework/Assertions/Assert.That.cs @@ -828,9 +828,12 @@ 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 (IsMultiDimensionalArrayGetCall(callExpr)) { - // Handle multi-dimensional array indexers (e.g., array.Get(i, j) displayed as array[i, j]) + // Handle multi-dimensional array indexers (e.g., array.Get(i, j) displayed as array[i, j]). + // Only matches when the receiver is actually an array (runtime-synthesized Get on int[,]), + // so arbitrary user-defined `Get(...)` methods on non-array types are NOT mis-rendered as + // `obj[...]` — they go through the regular method-call path below (issue #6691). string objectName = GetCleanMemberName(callExpr.Object); string indexDisplay = string.Join(", ", callExpr.Arguments.Select(arg => GetIndexArgumentDisplay(arg, evaluationCache))); string indexerDisplay = $"{objectName}[{indexDisplay}]"; @@ -856,9 +859,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 @@ -872,6 +876,84 @@ private static void HandleMethodCallExpression(MethodCallExpression callExpr, Di } } + /// + /// Multidimensional arrays (e.g. int[,]) expose runtime-synthesized + /// Get/Set/Address methods on the array type itself — not on + /// — so we check the receiver type rather than the declaring type to + /// catch them. Arbitrary user-defined instance methods named Get on non-array types + /// fall through to the regular method-call display path (issue #6691). + /// + private static bool IsMultiDimensionalArrayGetCall(MethodCallExpression callExpr) + => callExpr.Method.Name == GetMethodName + && callExpr.Object is not null + && callExpr.Object.Type.IsArray + && callExpr.Arguments.Count > 0; + + /// + /// 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 this render as this.Method(...); extension methods + /// use the first argument as the receiver. Fixes issue #6691. + /// + 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: use the declaring type's full name (cleaned) so nested + // types keep their nesting context (Outer.Inner.Method rather than Inner.Method). + string typeName = callExpr.Method.DeclaringType is { } dt + ? CleanTypeName(dt.FullName ?? dt.Name) + : 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})"; + } + + /// + /// Returns if is a reference to the enclosing + /// instance (this) — either accessed via the compiler-synthesized display-class field + /// (named like <>4__this) or as a 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 , + /// so inherited methods on this still render as this.Method(...) without + /// mis-labeling base-typed locals as this. + /// + 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)); + private static bool IsFuncOrActionType(Type? type) { if (type is null) diff --git a/test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.That.cs b/test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.That.cs index ebff5d5e87..7b6852835f 100644 --- a/test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.That.cs +++ b/test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.That.cs @@ -1343,6 +1343,70 @@ public void That_NullDelegateTypedMember_StillAppearsInDetails() ex.Message.Should().Contain("Callback"); 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]` (mis-leading), 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() + { + var box = new GetterBox(); + int expected = 0; + + Action act = () => Assert.That(() => box.Get(1) == expected); + + AssertFailedException ex = act.Should().Throw().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() + { + int expected = 0; + + Action act = () => Assert.That(() => GetAnimal() == expected); + + AssertFailedException ex = act.Should().Throw().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() + { + int expected = 0; + + Action act = () => Assert.That(() => GetAnimalInstance() == expected); + + AssertFailedException ex = act.Should().Throw().Which; + // Captured-this instance methods should render as `this.MethodName(...)`. + ex.Message.Should().Contain($"this.{nameof(GetAnimalInstance)}"); + } + + public void That_ExtensionMethodOnThis_RendersAsThisMethod() + { + string expected = "wrong"; + + Action act = () => Assert.That(() => this.GetGreeting() == expected); + + AssertFailedException ex = act.Should().Throw().Which; + ex.Message.Should().Contain($"this.{nameof(AssertTestsExtensions.GetGreeting)}"); + } + + private sealed class GetterBox + { + public int Get(int key) => key + 100; + } +} + +internal static class AssertTestsExtensions +{ + public static string GetGreeting(this AssertTests _) => "hello"; } internal static class MutableBoxHelper