-
Notifications
You must be signed in to change notification settings - Fork 70
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
xunit/xunit#2123: Analyzer to convert Assert.Collection to Assert.Sin…
…gle (#168)
- Loading branch information
1 parent
0c15399
commit 0a86d7e
Showing
6 changed files
with
448 additions
and
2 deletions.
There are no files selected for viewing
205 changes: 205 additions & 0 deletions
205
src/xunit.analyzers.fixes/X2000/AssertSingleShouldBeUsedForSingleParameterFixer.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,205 @@ | ||
using System.Collections.Immutable; | ||
using System.Composition; | ||
using System.Globalization; | ||
using System.Linq; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Microsoft.CodeAnalysis; | ||
using Microsoft.CodeAnalysis.CodeActions; | ||
using Microsoft.CodeAnalysis.CodeFixes; | ||
using Microsoft.CodeAnalysis.CSharp; | ||
using Microsoft.CodeAnalysis.CSharp.Syntax; | ||
using Microsoft.CodeAnalysis.Editing; | ||
using Microsoft.CodeAnalysis.Formatting; | ||
using Microsoft.CodeAnalysis.Simplification; | ||
using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory; | ||
|
||
namespace Xunit.Analyzers.Fixes; | ||
|
||
[ExportCodeFixProvider(LanguageNames.CSharp), Shared] | ||
public class AssertSingleShouldBeUsedForSingleParameterFixer : BatchedCodeFixProvider | ||
{ | ||
private const string DefaultParameterName = "item"; | ||
public const string Key_UseSingleMethod = "xUnit2023_UseSingleMethod"; | ||
|
||
public AssertSingleShouldBeUsedForSingleParameterFixer() : | ||
base(Descriptors.X2023_AssertSingleShouldBeUsedForSingleParameter.Id) | ||
{ } | ||
|
||
static string GetSafeVariableName( | ||
string targetParameterName, | ||
ImmutableHashSet<string> localSymbols) | ||
{ | ||
var idx = 2; | ||
var result = targetParameterName; | ||
|
||
while (localSymbols.Contains(result)) | ||
result = string.Format(CultureInfo.InvariantCulture, "{0}_{1}", targetParameterName, idx++); | ||
|
||
return result; | ||
} | ||
|
||
public override async Task RegisterCodeFixesAsync(CodeFixContext context) | ||
{ | ||
var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); | ||
if (root is null) | ||
return; | ||
|
||
var invocation = root.FindNode(context.Span).FirstAncestorOrSelf<InvocationExpressionSyntax>(); | ||
if (invocation is null) | ||
return; | ||
|
||
var diagnostic = context.Diagnostics.FirstOrDefault(); | ||
if (diagnostic is null) | ||
return; | ||
if (!diagnostic.Properties.TryGetValue(Constants.Properties.Replacement, out var replacement)) | ||
return; | ||
if (replacement is null) | ||
return; | ||
|
||
context.RegisterCodeFix( | ||
CodeAction.Create( | ||
string.Format("Use Assert.{0}", replacement), | ||
ct => UseSingleMethod(context.Document, invocation, replacement, ct), | ||
Key_UseSingleMethod | ||
), | ||
context.Diagnostics | ||
); | ||
} | ||
|
||
static async Task<Document> UseSingleMethod( | ||
Document document, | ||
InvocationExpressionSyntax invocation, | ||
string replacementMethod, | ||
CancellationToken cancellationToken) | ||
{ | ||
var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); | ||
|
||
if (invocation.Expression is MemberAccessExpressionSyntax memberAccess && | ||
invocation.ArgumentList.Arguments[0].Expression is IdentifierNameSyntax collectionVariable) | ||
{ | ||
var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); | ||
if (semanticModel != null && invocation.Parent != null) | ||
{ | ||
var startLocation = invocation.GetLocation().SourceSpan.Start; | ||
var localSymbols = semanticModel.LookupSymbols(startLocation).OfType<ILocalSymbol>().Select(s => s.Name).ToImmutableHashSet(); | ||
var replacementNode = | ||
invocation | ||
.WithArgumentList(ArgumentList(SeparatedList(new[] { Argument(collectionVariable) }))) | ||
.WithExpression(memberAccess.WithName(IdentifierName(replacementMethod))); | ||
|
||
if (invocation.ArgumentList.Arguments[1].Expression is SimpleLambdaExpressionSyntax lambdaExpression) | ||
{ | ||
var originalParameterName = lambdaExpression.Parameter.Identifier.Text; | ||
var parameterName = GetSafeVariableName(originalParameterName, localSymbols); | ||
|
||
if (parameterName != originalParameterName) | ||
{ | ||
var body = lambdaExpression.Body; | ||
var tokens = | ||
body | ||
.DescendantTokens() | ||
.Where(t => t.Kind() == SyntaxKind.IdentifierToken && t.Text == originalParameterName) | ||
.ToArray(); | ||
body = body.ReplaceTokens(tokens, (t1, t2) => Identifier(t2.LeadingTrivia, parameterName, t2.TrailingTrivia)); | ||
lambdaExpression = lambdaExpression.WithBody(body); | ||
} | ||
|
||
var oneItemVariableStatement = | ||
OneItemVariableStatement(parameterName, replacementNode) | ||
.WithLeadingTrivia(invocation.GetLeadingTrivia()); | ||
|
||
ReplaceCollectionWithSingle(editor, oneItemVariableStatement, invocation.Parent); | ||
AppendLambdaStatements(editor, oneItemVariableStatement, lambdaExpression); | ||
} | ||
else if (invocation.ArgumentList.Arguments[1].Expression is IdentifierNameSyntax identifierExpression) | ||
{ | ||
var isMethod = semanticModel.GetSymbolInfo(identifierExpression).Symbol?.Kind == SymbolKind.Method; | ||
if (isMethod) | ||
{ | ||
var parameterName = GetSafeVariableName(DefaultParameterName, localSymbols); | ||
|
||
var oneItemVariableStatement = | ||
OneItemVariableStatement(parameterName, replacementNode) | ||
.WithLeadingTrivia(invocation.GetLeadingTrivia()); | ||
|
||
ReplaceCollectionWithSingle(editor, oneItemVariableStatement, invocation.Parent); | ||
AppendMethodInvocation(editor, oneItemVariableStatement, identifierExpression, parameterName); | ||
} | ||
} | ||
} | ||
} | ||
|
||
return editor.GetChangedDocument(); | ||
} | ||
|
||
static LocalDeclarationStatementSyntax OneItemVariableStatement( | ||
string parameterName, | ||
InvocationExpressionSyntax replacementNode) | ||
{ | ||
var equalsToReplacementNode = EqualsValueClause(replacementNode); | ||
|
||
var oneItemVariableDeclaration = VariableDeclaration( | ||
ParseTypeName("var"), | ||
SingletonSeparatedList( | ||
VariableDeclarator(Identifier(parameterName)) | ||
.WithInitializer(equalsToReplacementNode) | ||
) | ||
).NormalizeWhitespace(); | ||
|
||
return LocalDeclarationStatement(oneItemVariableDeclaration); | ||
} | ||
|
||
static void ReplaceCollectionWithSingle( | ||
DocumentEditor editor, | ||
LocalDeclarationStatementSyntax oneItemVariableStatement, | ||
SyntaxNode invocationParent) | ||
{ | ||
editor.ReplaceNode( | ||
invocationParent, | ||
oneItemVariableStatement | ||
); | ||
} | ||
|
||
static void AppendLambdaStatements( | ||
DocumentEditor editor, | ||
LocalDeclarationStatementSyntax oneItemVariableStatement, | ||
SimpleLambdaExpressionSyntax lambdaExpression) | ||
{ | ||
if (lambdaExpression.ExpressionBody is InvocationExpressionSyntax lambdaBody) | ||
{ | ||
editor.InsertAfter( | ||
oneItemVariableStatement, | ||
ExpressionStatement(lambdaBody) | ||
.WithAdditionalAnnotations(Formatter.Annotation, Simplifier.Annotation) | ||
); | ||
} | ||
else if (lambdaExpression.Block != null && lambdaExpression.Block.Statements.Count != 0) | ||
{ | ||
editor.InsertAfter( | ||
oneItemVariableStatement, | ||
lambdaExpression.Block.Statements.Select( | ||
(s, i) => s.WithAdditionalAnnotations(Formatter.Annotation, Simplifier.Annotation) | ||
) | ||
); | ||
} | ||
} | ||
|
||
static void AppendMethodInvocation( | ||
DocumentEditor editor, | ||
LocalDeclarationStatementSyntax oneItemVariableStatement, | ||
IdentifierNameSyntax methodExpression, | ||
string parameterName) | ||
{ | ||
editor.InsertAfter( | ||
oneItemVariableStatement, | ||
ExpressionStatement( | ||
InvocationExpression( | ||
methodExpression, | ||
ArgumentList(SingletonSeparatedList(Argument(IdentifierName(parameterName)))) | ||
) | ||
) | ||
.WithAdditionalAnnotations(Formatter.Annotation, Simplifier.Annotation) | ||
); | ||
} | ||
} |
49 changes: 49 additions & 0 deletions
49
src/xunit.analyzers.tests/Analyzers/X2000/AssertSingleShouldBeUsedForSingleParameterTests.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
using Xunit; | ||
using Verify = CSharpVerifier<Xunit.Analyzers.AssertSingleShouldBeUsedForSingleParameter>; | ||
|
||
public class AssertSingleShouldBeUsedForSingleParameterTests | ||
{ | ||
[Fact] | ||
public async void FindsInfo_ForSingleItemCollectionCheck() | ||
{ | ||
var code = @" | ||
using Xunit; | ||
using System.Collections.Generic; | ||
public class TestClass { | ||
[Fact] | ||
public void TestMethod() { | ||
IEnumerable<object> collection = new List<object>() { new object() }; | ||
Assert.Collection(collection, item => Assert.NotNull(item)); | ||
} | ||
}"; | ||
|
||
var expected = | ||
Verify | ||
.Diagnostic() | ||
.WithSpan(10, 9, 10, 68) | ||
.WithArguments("Collection"); | ||
|
||
await Verify.VerifyAnalyzer(code, expected); | ||
} | ||
|
||
[Fact] | ||
public async void DoesNotFindInfo_ForMultipleItemCollectionCheck() | ||
{ | ||
var code = @" | ||
using Xunit; | ||
using System.Collections.Generic; | ||
public class TestClass { | ||
[Fact] | ||
public void TestMethod() { | ||
IEnumerable<object> collection = new List<object>() { new object(), new object() }; | ||
Assert.Collection(collection, item1 => Assert.NotNull(item1), item2 => Assert.NotNull(item2)); | ||
} | ||
}"; | ||
|
||
await Verify.VerifyAnalyzer(code); | ||
} | ||
} |
135 changes: 135 additions & 0 deletions
135
...xunit.analyzers.tests/Fixes/X2000/AssertSingleShouldBeUsedForSingleParameterFixerTests.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,135 @@ | ||
using Microsoft.CodeAnalysis.CSharp; | ||
using Xunit; | ||
using Xunit.Analyzers.Fixes; | ||
using Verify = CSharpVerifier<Xunit.Analyzers.AssertSingleShouldBeUsedForSingleParameter>; | ||
|
||
public class AssertSingleShouldBeUsedForSingleParameterFixerTests | ||
{ | ||
public static TheoryData<string, string> Statements = new() | ||
{ | ||
{ | ||
"[|Assert.Collection(collection, item => Assert.NotNull(item))|]", | ||
@"var item = Assert.Single(collection); | ||
Assert.NotNull(item);" | ||
}, | ||
{ | ||
@"var item = 42; | ||
[|Assert.Collection(collection, item => Assert.NotNull(item))|]", | ||
@"var item = 42; | ||
var item_2 = Assert.Single(collection); | ||
Assert.NotNull(item_2);" | ||
}, | ||
{ | ||
"[|Assert.Collection(collection, item => { Assert.NotNull(item); })|]", | ||
@"var item = Assert.Single(collection); | ||
Assert.NotNull(item);" | ||
}, | ||
{ | ||
@"var item = 42; | ||
[|Assert.Collection(collection, item => { Assert.NotNull(item); })|]", | ||
@"var item = 42; | ||
var item_2 = Assert.Single(collection); | ||
Assert.NotNull(item_2);" | ||
}, | ||
{ | ||
"[|Assert.Collection(collection, item => { Assert.NotNull(item); Assert.NotNull(item); })|]", | ||
@"var item = Assert.Single(collection); | ||
Assert.NotNull(item); Assert.NotNull(item);" | ||
}, | ||
{ | ||
@"var item = 42; | ||
[|Assert.Collection(collection, item => { Assert.NotNull(item); Assert.NotNull(item); })|]", | ||
@"var item = 42; | ||
var item_2 = Assert.Single(collection); | ||
Assert.NotNull(item_2); Assert.NotNull(item_2);" | ||
}, | ||
{ | ||
@"[|Assert.Collection(collection, item => { | ||
if (item != null) { | ||
Assert.NotNull(item); | ||
Assert.NotNull(item); | ||
} | ||
})|]", | ||
@"var item = Assert.Single(collection); | ||
if (item != null) | ||
{ | ||
Assert.NotNull(item); | ||
Assert.NotNull(item); | ||
}" | ||
}, | ||
{ | ||
@"var item = 42; | ||
[|Assert.Collection(collection, item => { | ||
if (item != null) { | ||
Assert.NotNull(item); | ||
Assert.NotNull(item); | ||
} | ||
})|]", | ||
@"var item = 42; | ||
var item_2 = Assert.Single(collection); | ||
if (item_2 != null) | ||
{ | ||
Assert.NotNull(item_2); | ||
Assert.NotNull(item_2); | ||
}" | ||
}, | ||
{ | ||
"[|Assert.Collection(collection, ElementInspector)|]", | ||
@"var item = Assert.Single(collection); | ||
ElementInspector(item);" | ||
}, | ||
{ | ||
@"var item = 42; | ||
var item_2 = 21.12; | ||
[|Assert.Collection(collection, ElementInspector)|]", | ||
@"var item = 42; | ||
var item_2 = 21.12; | ||
var item_3 = Assert.Single(collection); | ||
ElementInspector(item_3);" | ||
}, | ||
}; | ||
|
||
const string beforeTemplate = @" | ||
using Xunit; | ||
using System.Collections.Generic; | ||
public class TestClass {{ | ||
[Fact] | ||
public void TestMethod() {{ | ||
IEnumerable<object> collection = new List<object>() {{ new object() }}; | ||
{0}; | ||
}} | ||
private void ElementInspector(object obj) | ||
{{ }} | ||
}}"; | ||
|
||
const string afterTemplate = @" | ||
using Xunit; | ||
using System.Collections.Generic; | ||
public class TestClass {{ | ||
[Fact] | ||
public void TestMethod() {{ | ||
IEnumerable<object> collection = new List<object>() {{ new object() }}; | ||
{0} | ||
}} | ||
private void ElementInspector(object obj) | ||
{{ }} | ||
}}"; | ||
|
||
[Theory] | ||
[MemberData(nameof(Statements))] | ||
public async void ReplacesCollectionMethod( | ||
string statementBefore, | ||
string statementAfter) | ||
{ | ||
var before = string.Format(beforeTemplate, statementBefore); | ||
var after = string.Format(afterTemplate, statementAfter); | ||
|
||
await Verify.VerifyCodeFix(LanguageVersion.CSharp8, before, after, AssertSingleShouldBeUsedForSingleParameterFixer.Key_UseSingleMethod); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,5 @@ | ||
{ | ||
"$schema": "https://xUnit.net/schema/current/xunit.runner.schema.json", | ||
"diagnosticMessages": true, | ||
"maxParallelThreads": -1, | ||
"shadowCopy": false | ||
} |
Oops, something went wrong.