-
-
Notifications
You must be signed in to change notification settings - Fork 111
feat: add HasSingleItem(predicate) overload #4832
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
bdad834
8ad3550
0172c33
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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]})") | ||||||||||
|
||||||||||
| "Single" => argumentListArguments.Count >= 2 | |
| ? SyntaxFactory.ParseExpression($"Assert.That({argumentListArguments[0]}).HasSingleItem({argumentListArguments[1]})") | |
| : SyntaxFactory.ParseExpression($"Assert.That({argumentListArguments[0]}).HasSingleItem()"), |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.Singleis a predicate, but xUnit has a non-generic overloadAssert.Single(IEnumerable collection, object? expected)where the second argument is an expected value, not a predicate.When users migrate code like:
The fixer converts it to:
This fails to compile because
HasSingleItemexpectsFunc<TItem, bool>, not an object value.Why this matters:
xUnit provides four
Assert.Singleoverloads (see xUnit source):Assert.Single(IEnumerable)Assert.Single(IEnumerable, object?)← problematic overloadAssert.Single<T>(IEnumerable<T>)Assert.Single<T>(IEnumerable<T>, Predicate<T>)Suggested approach:
Use semantic analysis to distinguish between these overloads, similar to how
ConvertContainshandles parameter type checking (XUnitTwoPhaseAnalyzer.cs:369-377):Why semantic analysis is better:
See
TUnit/TUnit.Analyzers.CodeFixers/TwoPhase/XUnitTwoPhaseAnalyzer.cs
Lines 352 to 357 in bdad834
There was a problem hiding this comment.
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 toHasSingleItem(predicate). For theobject?expected-value overload, the two-phase analyzer falls back toHasSingleItem()with a TODO comment, and the code fix provider falls back to plainHasSingleItem().