diff --git a/src/xunit.analyzers.fixes/X2000/AssertSingleShouldBeUsedForSingleParameterFixer.cs b/src/xunit.analyzers.fixes/X2000/AssertSingleShouldBeUsedForSingleParameterFixer.cs new file mode 100644 index 00000000..095d74f7 --- /dev/null +++ b/src/xunit.analyzers.fixes/X2000/AssertSingleShouldBeUsedForSingleParameterFixer.cs @@ -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 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(); + 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 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().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) + ); + } +} diff --git a/src/xunit.analyzers.tests/Analyzers/X2000/AssertSingleShouldBeUsedForSingleParameterTests.cs b/src/xunit.analyzers.tests/Analyzers/X2000/AssertSingleShouldBeUsedForSingleParameterTests.cs new file mode 100644 index 00000000..f31be186 --- /dev/null +++ b/src/xunit.analyzers.tests/Analyzers/X2000/AssertSingleShouldBeUsedForSingleParameterTests.cs @@ -0,0 +1,49 @@ +using Xunit; +using Verify = CSharpVerifier; + +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 collection = new List() { 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 collection = new List() { new object(), new object() }; + + Assert.Collection(collection, item1 => Assert.NotNull(item1), item2 => Assert.NotNull(item2)); + } +}"; + + await Verify.VerifyAnalyzer(code); + } +} diff --git a/src/xunit.analyzers.tests/Fixes/X2000/AssertSingleShouldBeUsedForSingleParameterFixerTests.cs b/src/xunit.analyzers.tests/Fixes/X2000/AssertSingleShouldBeUsedForSingleParameterFixerTests.cs new file mode 100644 index 00000000..51d00361 --- /dev/null +++ b/src/xunit.analyzers.tests/Fixes/X2000/AssertSingleShouldBeUsedForSingleParameterFixerTests.cs @@ -0,0 +1,135 @@ +using Microsoft.CodeAnalysis.CSharp; +using Xunit; +using Xunit.Analyzers.Fixes; +using Verify = CSharpVerifier; + +public class AssertSingleShouldBeUsedForSingleParameterFixerTests +{ + public static TheoryData 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 collection = new List() {{ 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 collection = new List() {{ 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); + } +} diff --git a/src/xunit.analyzers.tests/xunit.runner.json b/src/xunit.analyzers.tests/xunit.runner.json index 7b91a367..33d08cc8 100644 --- a/src/xunit.analyzers.tests/xunit.runner.json +++ b/src/xunit.analyzers.tests/xunit.runner.json @@ -1,6 +1,5 @@ { "$schema": "https://xUnit.net/schema/current/xunit.runner.schema.json", "diagnosticMessages": true, - "maxParallelThreads": -1, "shadowCopy": false } diff --git a/src/xunit.analyzers/Utility/Descriptors.cs b/src/xunit.analyzers/Utility/Descriptors.cs index 50189118..da7aee2e 100644 --- a/src/xunit.analyzers/Utility/Descriptors.cs +++ b/src/xunit.analyzers/Utility/Descriptors.cs @@ -624,7 +624,14 @@ public static DiagnosticDescriptor X2019_AssertThrowsShouldNotBeUsedForAsyncThro "Do not negate your value when calling Assert.{0}. Call Assert.{1} without the negation instead." ); - // Placeholder for rule X2023 + public static DiagnosticDescriptor X2023_AssertSingleShouldBeUsedForSingleParameter { get; } = + Rule( + "xUnit2023", + "Do not use collection methods for single-item collections", + Assertions, + Info, + "Do not use Assert.{0} if there is one element in the collection. Use Assert.Single instead." + ); // Placeholder for rule X2024 diff --git a/src/xunit.analyzers/X2000/AssertSingleShouldBeUsedForSingleParameter.cs b/src/xunit.analyzers/X2000/AssertSingleShouldBeUsedForSingleParameter.cs new file mode 100644 index 00000000..2580de62 --- /dev/null +++ b/src/xunit.analyzers/X2000/AssertSingleShouldBeUsedForSingleParameter.cs @@ -0,0 +1,51 @@ +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace Xunit.Analyzers; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class AssertSingleShouldBeUsedForSingleParameter : AssertUsageAnalyzerBase +{ + static readonly string[] targetMethods = + { + Constants.Asserts.Collection + }; + + public AssertSingleShouldBeUsedForSingleParameter() : + base(Descriptors.X2023_AssertSingleShouldBeUsedForSingleParameter, targetMethods) + { } + + protected override void AnalyzeInvocation( + OperationAnalysisContext context, + XunitContext xunitContext, + IInvocationOperation invocationOperation, + IMethodSymbol method) + { + if (invocationOperation.Arguments.Length != 2) + return; + + var secondParameter = invocationOperation.Arguments[1]; + if (secondParameter.ArgumentKind != ArgumentKind.ParamArray) + return; + + if (secondParameter.Value is not IArrayCreationOperation operation) + return; + + if (operation.DimensionSizes.Length != 1 || (int)(operation.DimensionSizes[0].ConstantValue.Value ?? 0) != 1) + return; + + var builder = ImmutableDictionary.CreateBuilder(); + builder[Constants.Properties.Replacement] = Constants.Asserts.Single; + + context.ReportDiagnostic( + Diagnostic.Create( + Descriptors.X2023_AssertSingleShouldBeUsedForSingleParameter, + invocationOperation.Syntax.GetLocation(), + builder.ToImmutable(), + method.Name + ) + ); + } +}