Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
7 changes: 7 additions & 0 deletions TUnit.Analyzers.CodeFixers/TwoPhase/XUnitTwoPhaseAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,13 @@ private bool IsXUnitAssertion(InvocationExpressionSyntax invocation)
if (args.Count < 1) return (AssertionConversionKind.Single, null, false, null);

var collection = args[0].Expression.ToString();

if (args.Count >= 2)
{
var predicate = args[1].Expression.ToString();
return (AssertionConversionKind.Single, $"await Assert.That({collection}).HasSingleItem({predicate})", true, null);
}

return (AssertionConversionKind.Single, $"await Assert.That({collection}).HasSingleItem()", true, null);
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue: Code fixer doesn't distinguish between xUnit's predicate and expected-value overloads

Both code fixers assume any second argument to Assert.Single is a predicate, but xUnit has a non-generic overload Assert.Single(IEnumerable collection, object? expected) where the second argument is an expected value, not a predicate.

When users migrate code like:

Assert.Single(myCollection, expectedObject);

The fixer converts it to:

Assert.That(myCollection).HasSingleItem(expectedObject);

This fails to compile because HasSingleItem expects Func<TItem, bool>, not an object value.

Why this matters:
xUnit provides four Assert.Single overloads (see xUnit source):

  1. Assert.Single(IEnumerable)
  2. Assert.Single(IEnumerable, object?) ← problematic overload
  3. Assert.Single<T>(IEnumerable<T>)
  4. Assert.Single<T>(IEnumerable<T>, Predicate<T>)

Suggested approach:
Use semantic analysis to distinguish between these overloads, similar to how ConvertContains handles parameter type checking (XUnitTwoPhaseAnalyzer.cs:369-377):

var symbol = SemanticModel.GetSymbolInfo(memberAccess).Symbol;
if (symbol is IMethodSymbol { Parameters.Length: 2 } methodSymbol)
{
    // Check if second parameter is Predicate<T> vs object
    if (methodSymbol.Parameters[1].Type is INamedTypeSymbol { DelegateInvokeMethod: not null })
    {
        // This is the predicate overload - safe to convert
    }
    else
    {
        // This is the object expected overload - skip or handle differently
    }
}

Why semantic analysis is better:

  • Accurately distinguishes overloads based on actual types, not syntax
  • Prevents compilation errors in migrated code
  • Consistent with the defensive pattern already used elsewhere in this codebase

See

if (args.Count >= 2)
{
var predicate = args[1].Expression.ToString();
return (AssertionConversionKind.Single, $"await Assert.That({collection}).HasSingleItem({predicate})", true, null);
}

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Both code fixers now use semantic analysis to check if the second parameter is a delegate type (DelegateInvokeMethod: not null) before converting to HasSingleItem(predicate). For the object? expected-value overload, the two-phase analyzer falls back to HasSingleItem() with a TODO comment, and the code fix provider falls back to plain HasSingleItem().

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -419,4 +419,94 @@ public void MyTest()
"""
);
}

[Test]
public async Task Xunit_Single_With_Predicate_Preserves_Lambda()
{
await Verifier
.VerifyCodeFixAsync(
"""
using System.Threading.Tasks;
using System.Collections.Generic;

public class MyClass
{
public void MyTest()
{
var properties = new List<Property>
{
new Property { Prop = "myKey" }
};

{|#0:Xunit.Assert.Single(properties, p => p.Prop == "myKey")|};
}
}

public class Property
{
public string Prop { get; set; }
}
""",
Verifier.Diagnostic(Rules.XUnitAssertion)
.WithLocation(0),
"""
using System.Threading.Tasks;
using System.Collections.Generic;

public class MyClass
{
public void MyTest()
{
var properties = new List<Property>
{
new Property { Prop = "myKey" }
};

Assert.That(properties).HasSingleItem(p => p.Prop == "myKey");
}
}

public class Property
{
public string Prop { get; set; }
}
"""
);
}

[Test]
public async Task Xunit_Single_Without_Predicate()
{
await Verifier
.VerifyCodeFixAsync(
"""
using System.Threading.Tasks;
using System.Collections.Generic;

public class MyClass
{
public void MyTest()
{
var items = new List<int> { 42 };
{|#0:Xunit.Assert.Single(items)|};
}
}
""",
Verifier.Diagnostic(Rules.XUnitAssertion)
.WithLocation(0),
"""
using System.Threading.Tasks;
using System.Collections.Generic;

public class MyClass
{
public void MyTest()
{
var items = new List<int> { 42 };
Assert.That(items).HasSingleItem();
}
}
"""
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,9 @@ private static async Task<Document> ConvertAssertionAsync(CodeFixContext context

// "All" is handled separately in ConvertAssertionAsync

"Single" => SyntaxFactory.ParseExpression($"Assert.That({actual}).HasSingleItem()"),
"Single" => argumentListArguments.Count >= 2
? SyntaxFactory.ParseExpression($"Assert.That({argumentListArguments[0]}).HasSingleItem({argumentListArguments[1]})")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue: Same overload ambiguity exists here

This code fixer has the same issue as XUnitTwoPhaseAnalyzer.cs - it assumes any second argument to Assert.Single is a predicate.

xUnit's Assert.Single(IEnumerable, object?) overload would be incorrectly converted, causing compilation errors because HasSingleItem expects Func<TItem, bool>, not an object value.

Suggested fix:
Add semantic analysis before line 191 to check the parameter type:

"Single" => argumentListArguments.Count >= 2 && IsPredicateOverload(invocation, 1)
    ? SyntaxFactory.ParseExpression($"Assert.That({argumentListArguments[0]}).HasSingleItem({argumentListArguments[1]})")
    : SyntaxFactory.ParseExpression($"Assert.That({argumentListArguments[0]}).HasSingleItem()"),

Where IsPredicateOverload uses SemanticModel to verify the second parameter is actually a delegate type, not object?.

See the detailed explanation and suggested implementation approach in the comment on XUnitTwoPhaseAnalyzer.cs.

See

"Single" => argumentListArguments.Count >= 2
? SyntaxFactory.ParseExpression($"Assert.That({argumentListArguments[0]}).HasSingleItem({argumentListArguments[1]})")
: SyntaxFactory.ParseExpression($"Assert.That({argumentListArguments[0]}).HasSingleItem()"),

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in the same commit - extracted to an async Single method matching the pattern used by Contains.

: SyntaxFactory.ParseExpression($"Assert.That({argumentListArguments[0]}).HasSingleItem()"),

"IsType" => isGeneric
? SyntaxFactory.ParseExpression($"Assert.That({actual}).IsTypeOf<{genericArgs}>()")
Expand Down
24 changes: 24 additions & 0 deletions TUnit.Assertions.Tests/ListAssertionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,30 @@ public async Task Test_List_HasSingleItem()
await Assert.That(list).HasSingleItem();
}

[Test]
public async Task Test_List_HasSingleItem_WithPredicate()
{
IList<int> list = new List<int> { 1, 2, 3, 4, 5 };
var item = await Assert.That(list).HasSingleItem(x => x == 3);
await Assert.That(item).IsEqualTo(3);
}

[Test]
public async Task Test_List_HasSingleItem_WithPredicate_Fails_WhenNoneMatch()
{
IList<int> list = new List<int> { 1, 2, 3 };
var action = async () => await Assert.That(list).HasSingleItem(x => x > 10);
await Assert.That(action).ThrowsException();
}

[Test]
public async Task Test_List_HasSingleItem_WithPredicate_Fails_WhenMultipleMatch()
{
IList<int> list = new List<int> { 1, 2, 3, 4, 5 };
var action = async () => await Assert.That(list).HasSingleItem(x => x > 3);
await Assert.That(action).ThrowsException();
}

[Test]
public async Task Test_List_IsInOrder()
{
Expand Down
33 changes: 33 additions & 0 deletions TUnit.Assertions/Collections/CollectionChecks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,39 @@ public static AssertionResult CheckHasSingleItem<TItem>(ICollectionAdapter<TItem
return AssertionResult.Failed($"it had {count} item(s)");
}

/// <summary>
/// Checks if exactly one item in the collection matches the predicate.
/// Returns the matching item via out parameter.
/// </summary>
public static AssertionResult CheckHasSingleItemPredicate<TItem>(
ICollectionAdapter<TItem> adapter,
Func<TItem, bool> predicate,
out TItem? matchingItem)
{
matchingItem = default;
var matchCount = 0;

foreach (var item in adapter.AsEnumerable())
{
if (predicate(item))
{
matchCount++;
if (matchCount == 1)
{
matchingItem = item;
}
}
}

if (matchCount == 1)
{
return AssertionResult.Passed;
}

matchingItem = default;
return AssertionResult.Failed($"{matchCount} item(s) matched the predicate");
}

/// <summary>
/// Checks if all items satisfy the predicate.
/// </summary>
Expand Down
59 changes: 59 additions & 0 deletions TUnit.Assertions/Conditions/CollectionAssertions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,65 @@ private async Task<TItem> ExecuteAndReturnItemAsync()
}
}

/// <summary>
/// Asserts that a collection contains exactly one item matching the predicate.
/// When awaited, returns the matching item for further assertions.
/// Delegates to CollectionChecks for the actual logic.
/// </summary>
[AssertionExtension("HasSingleItem")]
public class HasSingleItemPredicateAssertion<TCollection, TItem> : Sources.CollectionAssertionBase<TCollection, TItem>
where TCollection : IEnumerable<TItem>
{
private readonly Func<TItem, bool> _predicate;
private readonly string _predicateDescription;
private TItem? _singleItem;

public HasSingleItemPredicateAssertion(
AssertionContext<TCollection> context,
Func<TItem, bool> predicate,
string predicateDescription)
: base(context)
{
_predicate = predicate ?? throw new ArgumentNullException(nameof(predicate));
_predicateDescription = predicateDescription;
}

protected override Task<AssertionResult> CheckAsync(EvaluationMetadata<TCollection> metadata)
{
if (metadata.Exception != null)
{
return Task.FromResult(AssertionResult.Failed($"threw {metadata.Exception.GetType().Name}"));
}

if (metadata.Value == null)
{
return Task.FromResult(AssertionResult.Failed("value was null"));
}

var adapter = new EnumerableAdapter<TItem>(metadata.Value);
var result = CollectionChecks.CheckHasSingleItemPredicate(adapter, _predicate, out _singleItem);
return Task.FromResult(result);
}

protected override string GetExpectation() => $"to have exactly one item matching {_predicateDescription}";

/// <summary>
/// Enables await syntax that returns the matching item.
/// This allows both chaining (.And) and item capture (await).
/// </summary>
public new System.Runtime.CompilerServices.TaskAwaiter<TItem> GetAwaiter()
{
return ExecuteAndReturnItemAsync().GetAwaiter();
}

private async Task<TItem> ExecuteAndReturnItemAsync()
{
await AssertAsync();

return _singleItem!;
}
}

/// <summary>
/// Asserts that a collection contains an item matching the predicate.
/// When awaited, returns the found item for further assertions.
Expand Down
13 changes: 13 additions & 0 deletions TUnit.Assertions/Sources/CollectionAssertionBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,19 @@ public HasSingleItemAssertion<TCollection, TItem> HasSingleItem()
return new HasSingleItemAssertion<TCollection, TItem>(Context);
}

/// <summary>
/// Asserts that the collection contains exactly one item matching the predicate.
/// This instance method enables calling HasSingleItem with proper type inference.
/// Example: await Assert.That(list).HasSingleItem(x => x > 5);
/// </summary>
public HasSingleItemPredicateAssertion<TCollection, TItem> HasSingleItem(
Func<TItem, bool> predicate,
[CallerArgumentExpression(nameof(predicate))] string? expression = null)
{
Context.ExpressionBuilder.Append($".HasSingleItem({expression})");
return new HasSingleItemPredicateAssertion<TCollection, TItem>(Context, predicate, expression ?? "predicate");
}

/// <summary>
/// Asserts that the collection contains only distinct (unique) items.
/// This instance method enables calling HasDistinctItems with proper type inference.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,7 @@ namespace .Collections
public static . CheckDoesNotOverlap<TItem>(.<TItem> adapter, .<TItem> other) { }
public static . CheckHasDistinctItems<TItem>(.<TItem> adapter, .<TItem>? comparer = null) { }
public static . CheckHasSingleItem<TItem>(.<TItem> adapter) { }
public static . CheckHasSingleItemPredicate<TItem>(.<TItem> adapter, <TItem, bool> predicate, out TItem? matchingItem) { }
public static . CheckIsEmpty<TItem>(.<TItem> adapter) { }
public static . CheckIsInDescendingOrder<TItem>(.<TItem> adapter, .<TItem>? comparer = null) { }
public static . CheckIsInOrder<TItem>(.<TItem> adapter, .<TItem>? comparer = null) { }
Expand Down Expand Up @@ -1216,6 +1217,15 @@ namespace .Conditions
public .<TItem> GetAwaiter() { }
protected override string GetExpectation() { }
}
[.("HasSingleItem")]
public class HasSingleItemPredicateAssertion<TCollection, TItem> : .<TCollection, TItem>
where TCollection : .<TItem>
{
public HasSingleItemPredicateAssertion(.<TCollection> context, <TItem, bool> predicate, string predicateDescription) { }
protected override .<.> CheckAsync(.<TCollection> metadata) { }
public .<TItem> GetAwaiter() { }
protected override string GetExpectation() { }
}
[.<.>("IsSuccessStatusCode", CustomName="IsNotSuccessStatusCode", ExpectationMessage="have a success status code", NegateLogic=true)]
[.<.>("IsSuccessStatusCode", ExpectationMessage="have a success status code")]
public static class HttpResponseMessageAssertionExtensions
Expand Down Expand Up @@ -3809,6 +3819,11 @@ namespace .Extensions
public static .<TCollection, TItem> HasSingleItem<TCollection, TItem>(this .<TCollection> source)
where TCollection : .<TItem> { }
}
public static class HasSingleItemPredicateAssertionExtensions
{
public static .<TCollection, TItem> HasSingleItem<TCollection, TItem>(this .<TCollection> source, <TItem, bool> predicate, string predicateDescription, [.("predicate")] string? predicateExpression = null, [.("predicateDescription")] string? predicateDescriptionExpression = null)
where TCollection : .<TItem> { }
}
public static class HttpResponseMessageAssertionExtensions
{
public static ._HasContentType_String_Assertion HasContentType(this .<.> source, string contentType, [.("contentType")] string? contentTypeExpression = null) { }
Expand Down Expand Up @@ -5651,6 +5666,7 @@ namespace .Sources
public .<TCollection, TItem> HasCount(int expectedCount, [.("expectedCount")] string? expression = null) { }
public .<TCollection, TItem> HasDistinctItems() { }
public .<TCollection, TItem> HasSingleItem() { }
public .<TCollection, TItem> HasSingleItem(<TItem, bool> predicate, [.("predicate")] string? expression = null) { }
public .<TTarget, TCollection> IsAssignableTo<TTarget>() { }
public .<TCollection, TItem> IsEmpty() { }
public .<TCollection, TItem> IsInDescendingOrder() { }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,7 @@ namespace .Collections
public static . CheckDoesNotOverlap<TItem>(.<TItem> adapter, .<TItem> other) { }
public static . CheckHasDistinctItems<TItem>(.<TItem> adapter, .<TItem>? comparer = null) { }
public static . CheckHasSingleItem<TItem>(.<TItem> adapter) { }
public static . CheckHasSingleItemPredicate<TItem>(.<TItem> adapter, <TItem, bool> predicate, out TItem? matchingItem) { }
public static . CheckIsEmpty<TItem>(.<TItem> adapter) { }
public static . CheckIsInDescendingOrder<TItem>(.<TItem> adapter, .<TItem>? comparer = null) { }
public static . CheckIsInOrder<TItem>(.<TItem> adapter, .<TItem>? comparer = null) { }
Expand Down Expand Up @@ -1199,6 +1200,15 @@ namespace .Conditions
public .<TItem> GetAwaiter() { }
protected override string GetExpectation() { }
}
[.("HasSingleItem")]
public class HasSingleItemPredicateAssertion<TCollection, TItem> : .<TCollection, TItem>
where TCollection : .<TItem>
{
public HasSingleItemPredicateAssertion(.<TCollection> context, <TItem, bool> predicate, string predicateDescription) { }
protected override .<.> CheckAsync(.<TCollection> metadata) { }
public .<TItem> GetAwaiter() { }
protected override string GetExpectation() { }
}
[.<.>("IsSuccessStatusCode", CustomName="IsNotSuccessStatusCode", ExpectationMessage="have a success status code", NegateLogic=true)]
[.<.>("IsSuccessStatusCode", ExpectationMessage="have a success status code")]
public static class HttpResponseMessageAssertionExtensions
Expand Down Expand Up @@ -3775,6 +3785,11 @@ namespace .Extensions
public static .<TCollection, TItem> HasSingleItem<TCollection, TItem>(this .<TCollection> source)
where TCollection : .<TItem> { }
}
public static class HasSingleItemPredicateAssertionExtensions
{
public static .<TCollection, TItem> HasSingleItem<TCollection, TItem>(this .<TCollection> source, <TItem, bool> predicate, string predicateDescription, [.("predicate")] string? predicateExpression = null, [.("predicateDescription")] string? predicateDescriptionExpression = null)
where TCollection : .<TItem> { }
}
public static class HttpResponseMessageAssertionExtensions
{
public static ._HasContentType_String_Assertion HasContentType(this .<.> source, string contentType, [.("contentType")] string? contentTypeExpression = null) { }
Expand Down Expand Up @@ -5598,6 +5613,7 @@ namespace .Sources
public .<TCollection, TItem> HasCount(int expectedCount, [.("expectedCount")] string? expression = null) { }
public .<TCollection, TItem> HasDistinctItems() { }
public .<TCollection, TItem> HasSingleItem() { }
public .<TCollection, TItem> HasSingleItem(<TItem, bool> predicate, [.("predicate")] string? expression = null) { }
public .<TTarget, TCollection> IsAssignableTo<TTarget>() { }
public .<TCollection, TItem> IsEmpty() { }
public .<TCollection, TItem> IsInDescendingOrder() { }
Expand Down
Loading
Loading