Skip to content

Commit 5b89e90

Browse files
thomhurstthomhurst
committed
fix: address PR #5785 review feedback
- Remove ShouldEntryPointAttribute (was never wired up; the generated extensions live on IShouldSource<T> not T, so they never collided with FluentAssertions in the first place — only the hand-written Should() overloads do, and the attribute couldn't suppress those) - Link EquatableArray<T> from TUnit.Core.SourceGenerator/Models/ instead of carrying a 4th copy in this repo - Strip dead method-level covariance branch and the unused NeedsMethodLevelCovariance / TypeParamIsTypeParameter / TypeParamConstraintName fields on AssertionData; emit always uses IShouldSource<typeParam> direct now - Skip generating extensions for collection assertions whose Should counterpart is a hand-crafted instance method on ShouldCollectionSource<T> (CollectionIsInOrderAssertion, CollectionAllAssertion, etc.) so the two paths can't drift or shadow each other - Add NullSourceTests covering null string/list sources to verify the null-forgiving propagation in ShouldCollectionSource produces meaningful AssertionException rather than NRE
1 parent 543e79d commit 5b89e90

5 files changed

Lines changed: 65 additions & 102 deletions

File tree

‎TUnit.Assertions.Should.SourceGenerator/EquatableArray.cs‎

Lines changed: 0 additions & 52 deletions
This file was deleted.

‎TUnit.Assertions.Should.SourceGenerator/ShouldExtensionGenerator.cs‎

Lines changed: 29 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Text;
66
using Microsoft.CodeAnalysis;
77
using TUnit.Assertions.SourceGenerator.Generators;
8+
using TUnit.Core.SourceGenerator.Models;
89

910
namespace TUnit.Assertions.Should.SourceGenerator;
1011

@@ -23,6 +24,24 @@ public sealed class ShouldExtensionGenerator : IIncrementalGenerator
2324
private const string AssertionContextFullName = "TUnit.Assertions.Core.AssertionContext`1";
2425
private const string ShouldExtensionsNamespace = "TUnit.Assertions.Should.Extensions";
2526

27+
/// <summary>
28+
/// Assertion classes whose Should counterparts live as hand-crafted instance methods on
29+
/// <c>ShouldCollectionSource&lt;T&gt;</c>. Skipped by the generator so the two paths can't drift
30+
/// or shadow each other. Instance methods are required because the generated extension form
31+
/// (<c>Method&lt;TCollection, TItem&gt;</c> with constraint) can't infer <c>TItem</c> from a
32+
/// constraint alone — see <c>ShouldCollectionSource</c> for the rationale.
33+
/// </summary>
34+
private static readonly HashSet<string> InstanceMethodAssertions = new(StringComparer.Ordinal)
35+
{
36+
"CollectionIsInOrderAssertion",
37+
"CollectionIsInDescendingOrderAssertion",
38+
"CollectionAllAssertion",
39+
"CollectionAnyAssertion",
40+
"HasSingleItemAssertion",
41+
"HasSingleItemPredicateAssertion",
42+
"HasDistinctItemsAssertion",
43+
};
44+
2645
private static readonly SymbolDisplayFormat NoGlobalFormat =
2746
SymbolDisplayFormat.FullyQualifiedFormat
2847
.WithGlobalNamespaceStyle(SymbolDisplayGlobalNamespaceStyle.Omitted)
@@ -154,6 +173,11 @@ private static void CollectFromType(INamedTypeSymbol type, CollectionContext ctx
154173
return;
155174
}
156175

176+
if (InstanceMethodAssertions.Contains(type.Name))
177+
{
178+
return;
179+
}
180+
157181
// Skip referenced types whose Should{ClassName}Extensions has already been baked in
158182
// another reference (typically TUnit.Assertions.Should.dll); re-emitting would cause
159183
// CS0121 ambiguities. Types declared in the current compilation are always emitted
@@ -247,7 +271,7 @@ private static void CollectFromType(INamedTypeSymbol type, CollectionContext ctx
247271
p.HasExplicitDefaultValue ? FormatDefaultValue(p.ExplicitDefaultValue, p.Type) : null));
248272
}
249273

250-
ctorData.Add(new ConstructorData(paramData.ToImmutable(), TryGetRucMessage(ctor.GetAttributes())));
274+
ctorData.Add(new ConstructorData(new EquatableArray<ParameterData>(paramData), TryGetRucMessage(ctor.GetAttributes())));
251275
}
252276

253277
if (ctorData.Count == 0)
@@ -259,12 +283,6 @@ private static void CollectFromType(INamedTypeSymbol type, CollectionContext ctx
259283

260284
var typeParam = assertionBaseType.TypeArguments[0];
261285
var typeParamDisplay = typeParam.ToDisplayString(NoGlobalFormat);
262-
var typeParamIsTypeParameter = typeParam is ITypeParameterSymbol;
263-
// No method-level covariance: every extension targets IShouldSource<typeParam> directly.
264-
// Source-type narrowing happens at the Should() entry overloads (mirroring Assert.That),
265-
// which avoids inference failures on element-typed assertions like BeInOrder where there's
266-
// no parameter to bind TItem from.
267-
var needsMethodLevelCovariance = false;
268286

269287
var classGenericParams = ImmutableArray.CreateBuilder<GenericParamData>();
270288
if (type.IsGenericType)
@@ -279,17 +297,14 @@ private static void CollectFromType(INamedTypeSymbol type, CollectionContext ctx
279297
ClassName: type.Name,
280298
ClassFullName: type.ToDisplayString(NameWithoutTypeArgsFormat),
281299
IsClassGeneric: type.IsGenericType,
282-
ClassGenericParams: classGenericParams.ToImmutable(),
300+
ClassGenericParams: new EquatableArray<GenericParamData>(classGenericParams),
283301
MethodName: methodName!,
284302
NegatedMethodName: negatedMethodName,
285303
OverloadResolutionPriority: overloadPriority,
286304
ShouldNameOverride: shouldNameOverride,
287305
ShouldNegatedOverride: shouldNegatedOverride,
288306
TypeParamDisplay: typeParamDisplay,
289-
TypeParamIsTypeParameter: typeParamIsTypeParameter,
290-
NeedsMethodLevelCovariance: needsMethodLevelCovariance,
291-
TypeParamConstraintName: CovarianceHelper.GetConstraintTypeName(typeParamDisplay, typeParam),
292-
Constructors: ctorData.ToImmutable(),
307+
Constructors: new EquatableArray<ConstructorData>(ctorData),
293308
RequiresUnreferencedCodeMessage: rucMessage));
294309
}
295310

@@ -401,24 +416,9 @@ private static void EmitMethod(StringBuilder sb, AssertionData data, Constructor
401416
}
402417
}
403418

404-
string sourceType;
405-
string contextExpr;
406419
var assertionTypeParamForCtor = data.TypeParamDisplay;
407-
408-
if (data.NeedsMethodLevelCovariance)
409-
{
410-
var covariantParam = CovarianceHelper.GetCovariantTypeParamName(classGenericList);
411-
allMethodGenerics.Add(covariantParam);
412-
sourceType = $"global::TUnit.Assertions.Should.Core.IShouldSource<{covariantParam}>";
413-
constraints.Add($"where {covariantParam} : {data.TypeParamConstraintName}");
414-
// Lambda body upcasts TActual to typeParam via the constraint; the signature shift handles variance.
415-
contextExpr = $"source.Context.Map<{data.TypeParamDisplay}>(static x => x)";
416-
}
417-
else
418-
{
419-
sourceType = $"global::TUnit.Assertions.Should.Core.IShouldSource<{data.TypeParamDisplay}>";
420-
contextExpr = "source.Context";
421-
}
420+
var sourceType = $"global::TUnit.Assertions.Should.Core.IShouldSource<{data.TypeParamDisplay}>";
421+
var contextExpr = "source.Context";
422422

423423
var classGenericSuffix = data.IsClassGeneric
424424
? "<" + string.Join(", ", classGenericList) + ">"
@@ -535,9 +535,6 @@ private sealed record AssertionData(
535535
string? ShouldNameOverride,
536536
string? ShouldNegatedOverride,
537537
string TypeParamDisplay,
538-
bool TypeParamIsTypeParameter,
539-
bool NeedsMethodLevelCovariance,
540-
string TypeParamConstraintName,
541538
EquatableArray<ConstructorData> Constructors,
542539
string? RequiresUnreferencedCodeMessage);
543540

‎TUnit.Assertions.Should.SourceGenerator/TUnit.Assertions.Should.SourceGenerator.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
</PropertyGroup>
1616
<ItemGroup>
1717
<Compile Include="..\TUnit.Assertions.SourceGenerator\Generators\CovarianceHelper.cs" Link="Shared\CovarianceHelper.cs" />
18+
<Compile Include="..\TUnit.Core.SourceGenerator\Models\EquatableArray.cs" Link="Shared\EquatableArray.cs" />
1819
</ItemGroup>
1920

2021
<ItemGroup>

‎TUnit.Assertions.Should.Tests/NullSourceTests.cs‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
using TUnit.Assertions.Exceptions;
2+
using TUnit.Assertions.Should;
3+
using TUnit.Assertions.Should.Extensions;
4+
5+
namespace TUnit.Assertions.Should.Tests;
6+
7+
/// <summary>
8+
/// Verifies null sources produce a meaningful assertion failure rather than an NRE.
9+
/// </summary>
10+
public class NullSourceTests
11+
{
12+
[Test]
13+
public async Task Null_string_BeEqualTo_fails_with_assertion_exception()
14+
{
15+
string? value = null;
16+
await Assert.That(async () => await value.Should().BeEqualTo("expected"))
17+
.Throws<AssertionException>();
18+
}
19+
20+
[Test]
21+
public async Task Null_collection_Contain_fails_with_assertion_exception()
22+
{
23+
List<int>? list = null;
24+
await Assert.That(async () => await list.Should().Contain(1))
25+
.Throws<AssertionException>();
26+
}
27+
28+
[Test]
29+
public async Task Null_collection_BeInOrder_fails_with_assertion_exception()
30+
{
31+
List<int>? list = null;
32+
await Assert.That(async () => await list.Should().BeInOrder())
33+
.Throws<AssertionException>();
34+
}
35+
}

‎TUnit.Assertions.Should/Attributes/ShouldEntryPointAttribute.cs‎

Lines changed: 0 additions & 18 deletions
This file was deleted.

0 commit comments

Comments
 (0)