Skip to content

Commit 1d18680

Browse files
Youssef1313Youssef1313
authored
Improve collection.Count/Length detection in MSTEST0037 (#6428)
1 parent f05f838 commit 1d18680

File tree

3 files changed

+158
-114
lines changed

3 files changed

+158
-114
lines changed

‎src/Analyzers/MSTest.Analyzers.CodeFixes/UseProperAssertMethodsFixer.cs‎

Lines changed: 34 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -70,16 +70,16 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
7070
switch (mode)
7171
{
7272
case UseProperAssertMethodsAnalyzer.CodeFixModeSimple:
73-
createChangedDocument = ct => FixAssertMethodForSimpleModeAsync(context.Document, diagnostic.AdditionalLocations[0], diagnostic.AdditionalLocations[1], root, simpleNameSyntax, properAssertMethodName, ct);
73+
createChangedDocument = ct => FixAssertMethodForSimpleModeAsync(context.Document, diagnostic.AdditionalLocations, root, simpleNameSyntax, properAssertMethodName, ct);
7474
break;
7575
case UseProperAssertMethodsAnalyzer.CodeFixModeAddArgument:
7676
createChangedDocument = ct => FixAssertMethodForAddArgumentModeAsync(context.Document, diagnostic.AdditionalLocations[0], diagnostic.AdditionalLocations[1], diagnostic.AdditionalLocations[2], root, simpleNameSyntax, properAssertMethodName, ct);
7777
break;
7878
case UseProperAssertMethodsAnalyzer.CodeFixModeRemoveArgument:
7979
createChangedDocument = ct => FixAssertMethodForRemoveArgumentModeAsync(context.Document, diagnostic.AdditionalLocations, root, simpleNameSyntax, properAssertMethodName, diagnostic.Properties.ContainsKey(UseProperAssertMethodsAnalyzer.NeedsNullableBooleanCastKey), ct);
8080
break;
81-
case UseProperAssertMethodsAnalyzer.CodeFixModeCollectionCount:
82-
createChangedDocument = ct => FixAssertMethodForCollectionCountModeAsync(context.Document, diagnostic.AdditionalLocations, root, simpleNameSyntax, properAssertMethodName, ct);
81+
case UseProperAssertMethodsAnalyzer.CodeFixModeRemoveArgumentAndReplaceArgument:
82+
createChangedDocument = ct => FixAssertMethodForRemoveArgumentAndReplaceArgumentModeAsync(context.Document, diagnostic.AdditionalLocations, root, simpleNameSyntax, properAssertMethodName, ct);
8383
break;
8484
default:
8585
break;
@@ -96,25 +96,29 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
9696
}
9797
}
9898

99-
private static async Task<Document> FixAssertMethodForSimpleModeAsync(Document document, Location conditionLocationToBeReplaced, Location replacementExpressionLocation, SyntaxNode root, SimpleNameSyntax simpleNameSyntax, string properAssertMethodName, CancellationToken cancellationToken)
99+
private static async Task<Document> FixAssertMethodForSimpleModeAsync(Document document, IReadOnlyList<Location> additionalLocations, SyntaxNode root, SimpleNameSyntax simpleNameSyntax, string properAssertMethodName, CancellationToken cancellationToken)
100100
{
101-
// This doesn't properly handle cases like Assert.IsTrue(message: "My message", condition: x == null)
102-
// The proper handling of this may be Assert.IsNull(message: "My message", value: x)
103-
// Or: Assert.IsNull(x, "My message")
104-
// For now this is not handled.
105-
if (root.FindNode(conditionLocationToBeReplaced.SourceSpan) is not ArgumentSyntax conditionNodeToBeReplaced)
106-
{
107-
return document;
108-
}
101+
DocumentEditor editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
102+
FixInvocationMethodName(editor, simpleNameSyntax, properAssertMethodName);
109103

110-
if (root.FindNode(replacementExpressionLocation.SourceSpan) is not ExpressionSyntax replacementExpressionNode)
104+
for (int i = 0; i < additionalLocations.Count; i += 2)
111105
{
112-
return document;
113-
}
106+
// This doesn't properly handle cases like Assert.IsTrue(message: "My message", condition: x == null)
107+
// The proper handling of this may be Assert.IsNull(message: "My message", value: x)
108+
// Or: Assert.IsNull(x, "My message")
109+
// For now this is not handled.
110+
if (root.FindNode(additionalLocations[i].SourceSpan) is not ArgumentSyntax conditionNodeToBeReplaced)
111+
{
112+
return document;
113+
}
114114

115-
DocumentEditor editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
116-
FixInvocationMethodName(editor, simpleNameSyntax, properAssertMethodName);
117-
editor.ReplaceNode(conditionNodeToBeReplaced, SyntaxFactory.Argument(replacementExpressionNode).WithAdditionalAnnotations(Formatter.Annotation));
115+
if (root.FindNode(additionalLocations[i + 1].SourceSpan, getInnermostNodeForTie: true) is not ExpressionSyntax replacementExpressionNode)
116+
{
117+
return document;
118+
}
119+
120+
editor.ReplaceNode(conditionNodeToBeReplaced, SyntaxFactory.Argument(replacementExpressionNode).WithAdditionalAnnotations(Formatter.Annotation));
121+
}
118122

119123
return editor.GetChangedDocument();
120124
}
@@ -213,7 +217,7 @@ private static async Task<Document> FixAssertMethodForRemoveArgumentModeAsync(
213217
return editor.GetChangedDocument();
214218
}
215219

216-
private static async Task<Document> FixAssertMethodForCollectionCountModeAsync(
220+
private static async Task<Document> FixAssertMethodForRemoveArgumentAndReplaceArgumentModeAsync(
217221
Document document,
218222
IReadOnlyList<Location> additionalLocations,
219223
SyntaxNode root,
@@ -223,61 +227,29 @@ private static async Task<Document> FixAssertMethodForCollectionCountModeAsync(
223227
{
224228
// Handle collection count transformations:
225229
// Assert.AreEqual(0, list.Count) -> Assert.IsEmpty(list)
226-
// Assert.AreEqual(3, list.Count) -> Assert.HasCount(3, list)
227230
// Assert.AreEqual(list.Count, 0) -> Assert.IsEmpty(list)
228-
// Assert.AreEqual(list.Count, 3) -> Assert.HasCount(3, list)
229-
if (root.FindNode(additionalLocations[0].SourceSpan) is not ArgumentSyntax firstArgument ||
230-
root.FindNode(additionalLocations[1].SourceSpan) is not ArgumentSyntax ||
231-
firstArgument.Parent is not ArgumentListSyntax argumentList)
231+
if (root.FindNode(additionalLocations[0].SourceSpan) is not ArgumentSyntax expectedArgumentToRemove)
232232
{
233233
return document;
234234
}
235235

236-
if (root.FindNode(additionalLocations[2].SourceSpan) is not ExpressionSyntax collectionExpression)
236+
if (root.FindNode(additionalLocations[1].SourceSpan) is not ArgumentSyntax argumentToBeReplaced ||
237+
root.FindNode(additionalLocations[2].SourceSpan) is not ExpressionSyntax replacement)
237238
{
238239
return document;
239240
}
240241

241-
DocumentEditor editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
242-
FixInvocationMethodName(editor, simpleNameSyntax, properAssertMethodName);
243-
244-
// Preserve any additional arguments beyond the first two (expected/actual)
245-
var additionalArguments = argumentList.Arguments.Skip(2).ToList();
246-
247-
ArgumentListSyntax newArgumentList;
248-
249-
if (properAssertMethodName == "IsEmpty")
250-
{
251-
// For IsEmpty, we just need the collection argument plus any additional arguments
252-
var newArguments = new List<ArgumentSyntax>
253-
{
254-
SyntaxFactory.Argument(collectionExpression).WithAdditionalAnnotations(Formatter.Annotation),
255-
};
256-
newArguments.AddRange(additionalArguments);
257-
newArgumentList = argumentList.WithArguments(SyntaxFactory.SeparatedList(newArguments));
258-
}
259-
else // HasCount
242+
if (expectedArgumentToRemove.Parent is not ArgumentListSyntax argumentList)
260243
{
261-
// For HasCount, we need count and collection arguments plus any additional arguments
262-
// additionalLocations[3] should contain the count expression
263-
if (additionalLocations.Count > 3 &&
264-
root.FindNode(additionalLocations[3].SourceSpan) is ArgumentSyntax countArgument)
265-
{
266-
var newArguments = new List<ArgumentSyntax>
267-
{
268-
SyntaxFactory.Argument(countArgument.Expression).WithAdditionalAnnotations(Formatter.Annotation),
269-
SyntaxFactory.Argument(collectionExpression).WithAdditionalAnnotations(Formatter.Annotation),
270-
};
271-
newArguments.AddRange(additionalArguments);
272-
newArgumentList = argumentList.WithArguments(SyntaxFactory.SeparatedList(newArguments));
273-
}
274-
else
275-
{
276-
// Fallback: something went wrong, don't apply the fix
277-
return document;
278-
}
244+
return document;
279245
}
280246

247+
DocumentEditor editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
248+
FixInvocationMethodName(editor, simpleNameSyntax, properAssertMethodName);
249+
250+
int argumentIndexToRemove = argumentList.Arguments.IndexOf(expectedArgumentToRemove);
251+
ArgumentListSyntax newArgumentList = argumentList.ReplaceNode(argumentToBeReplaced, argumentToBeReplaced.WithExpression(replacement));
252+
newArgumentList = newArgumentList.WithArguments(newArgumentList.Arguments.RemoveAt(argumentIndexToRemove));
281253
editor.ReplaceNode(argumentList, newArgumentList);
282254

283255
return editor.GetChangedDocument();

0 commit comments

Comments
 (0)