diff --git a/src/xunit.analyzers.fixes/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckFixer.cs b/src/xunit.analyzers.fixes/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckFixer.cs new file mode 100644 index 00000000..dee4e1c9 --- /dev/null +++ b/src/xunit.analyzers.fixes/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckFixer.cs @@ -0,0 +1,62 @@ +using System.Composition; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Editing; +using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory; + +namespace Xunit.Analyzers.Fixes; + +[ExportCodeFixProvider(LanguageNames.CSharp), Shared] +public class AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckFixer : BatchedCodeFixProvider +{ + public const string Key_UseAlternateAssert = "xUnit2029_UseAlternateAssert"; + + public AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckFixer() : + base(Descriptors.X2029_AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheck.Id) + { } + + public sealed 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; + + context.RegisterCodeFix( + XunitCodeAction.Create( + c => UseCheck(context.Document, invocation, c), + Key_UseAlternateAssert, + "Use DoesNotContain" + ), + context.Diagnostics + ); + } + + static async Task UseCheck( + Document document, + InvocationExpressionSyntax invocation, + CancellationToken cancellationToken) + { + var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); + + var arguments = invocation.ArgumentList.Arguments; + if (arguments.Count == 1 && arguments[0].Expression is InvocationExpressionSyntax innerInvocationSyntax) + if (invocation.Expression is MemberAccessExpressionSyntax outerMemberAccess && innerInvocationSyntax.Expression is MemberAccessExpressionSyntax memberAccess) + if (innerInvocationSyntax.ArgumentList.Arguments[0].Expression is ExpressionSyntax innerArgument) + editor.ReplaceNode( + invocation, + invocation + .WithArgumentList(ArgumentList(SeparatedList([Argument(memberAccess.Expression), Argument(innerArgument)]))) + .WithExpression(outerMemberAccess.WithName(IdentifierName(Constants.Asserts.DoesNotContain))) + ); + + return editor.GetChangedDocument(); + } +} diff --git a/src/xunit.analyzers.tests/Analyzers/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckTests.cs b/src/xunit.analyzers.tests/Analyzers/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckTests.cs new file mode 100644 index 00000000..d806fc0f --- /dev/null +++ b/src/xunit.analyzers.tests/Analyzers/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckTests.cs @@ -0,0 +1,113 @@ +using System.Threading.Tasks; +using Xunit; +using Verify = CSharpVerifier; + +public class AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckTests +{ + public static TheoryData GetEnumerables( + string typeName, + string comparison) => + new() + { + { $"new System.Collections.Generic.List<{typeName}>()", comparison }, + { $"new System.Collections.Generic.HashSet<{typeName}>()", comparison }, + { $"new System.Collections.ObjectModel.Collection<{typeName}>()", comparison }, + { $"new {typeName}[0]", comparison }, + { $"System.Linq.Enumerable.Empty<{typeName}>()", comparison }, + }; + + [Theory] + [MemberData(nameof(GetEnumerables), "int", "")] + [MemberData(nameof(GetEnumerables), "string", "")] + public async Task Containers_WithoutWhereClause_DoesNotTrigger( + string collection, + string _) + { + var source = $@" +class TestClass +{{ + void TestMethod() + {{ + Xunit.Assert.Empty({collection}); + }} +}}"; + await Verify.VerifyAnalyzer(source); + } + + [Theory] + [MemberData(nameof(GetEnumerables), "int", "f > 0")] + [MemberData(nameof(GetEnumerables), "string", "f.Length > 0")] + public async Task Containers_WithWhereClause_Triggers( + string collection, + string comparison) + { + var source = $@" +using System.Linq; +class TestClass +{{ + void TestMethod() + {{ + [|Xunit.Assert.Empty({collection}.Where(f => {comparison}))|]; + }} +}}"; + + await Verify.VerifyAnalyzer(source); + } + + [Theory] + [MemberData(nameof(GetEnumerables), "int", "f > 0")] + [MemberData(nameof(GetEnumerables), "string", "f.Length > 0")] + public async Task Containers_WithWhereClauseWithIndex_DoesNotTrigger( + string collection, + string comparison) + { + var source = $@" +using System.Linq; +class TestClass +{{ + void TestMethod() + {{ + Xunit.Assert.Empty({collection}.Where((f, i) => {comparison} && i > 0)); + }} +}}"; + await Verify.VerifyAnalyzer(source); + } + + [Theory] + [MemberData(nameof(GetEnumerables), "int", "f > 0")] + [MemberData(nameof(GetEnumerables), "string", "f.Length > 0")] + public async Task DoesNotFindWarningForEnumurableEmptyCheckWithChainedLinq( + string collection, + string comparison) + { + var source = $@" +using System.Linq; +class TestClass +{{ + void TestMethod() + {{ + Xunit.Assert.Empty({collection}.Where(f => {comparison}).Select(f => f)); + }} +}}"; + await Verify.VerifyAnalyzer(source); + } + + public static TheoryData GetSampleStrings() => + new(string.Empty, "123", @"abc\n\t\\\"""); + + [Theory] + [MemberData(nameof(GetSampleStrings))] + public async Task Strings_WithWhereClause_DoesNotTrigger(string sampleString) + { + var source = $@" +using System.Linq; +class TestClass +{{ + void TestMethod() + {{ + [|Xunit.Assert.Empty(""{sampleString}"".Where(f => f > 0))|]; + }} +}}"; + await Verify.VerifyAnalyzer(source); + } +} diff --git a/src/xunit.analyzers.tests/Fixes/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckFixerTests.cs b/src/xunit.analyzers.tests/Fixes/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckFixerTests.cs new file mode 100644 index 00000000..3e317a83 --- /dev/null +++ b/src/xunit.analyzers.tests/Fixes/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckFixerTests.cs @@ -0,0 +1,36 @@ +using System.Threading.Tasks; +using Xunit; +using Xunit.Analyzers.Fixes; +using Verify = CSharpVerifier; + +public class AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckFixerTests +{ + const string template = @" +using System.Linq; +using Xunit; + +public class TestClass {{ + [Fact] + public void TestMethod() {{ + var list = new[] {{ -1, 0, 1, 2 }}; + + {0}; + }} + + public bool IsEven(int num) => num % 2 == 0; +}}"; + + [Theory] + [InlineData("[|Assert.Empty(list.Where(f => f > 0))|]", "Assert.DoesNotContain(list, f => f > 0)")] + [InlineData("[|Assert.Empty(list.Where(n => n == 1))|]", "Assert.DoesNotContain(list, n => n == 1)")] + [InlineData("[|Assert.Empty(list.Where(IsEven))|]", "Assert.DoesNotContain(list, IsEven)")] + public async Task FixerReplacesAssertEmptyWithAssertDoesNotContain( + string beforeAssert, + string afterAssert) + { + var before = string.Format(template, beforeAssert); + var after = string.Format(template, afterAssert); + + await Verify.VerifyCodeFix(before, after, AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckFixer.Key_UseAlternateAssert); + } +} diff --git a/src/xunit.analyzers/Utility/Descriptors.xUnit2xxx.cs b/src/xunit.analyzers/Utility/Descriptors.xUnit2xxx.cs index 0fe0b925..5bcca044 100644 --- a/src/xunit.analyzers/Utility/Descriptors.xUnit2xxx.cs +++ b/src/xunit.analyzers/Utility/Descriptors.xUnit2xxx.cs @@ -260,7 +260,14 @@ public static partial class Descriptors "Using Assert.{0} with an instance of {1} is problematic, because {2}. Check the length with .Count instead." ); - // Placeholder for rule X2029 + public static DiagnosticDescriptor X2029_AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheck { get; } = + Diagnostic( + "xUnit2029", + "Do not use Empty() to check if a value does not exist in a collection", + Assertions, + Warning, + "Do not use Assert.Empty() to check if a value does not exist in a collection. Use Assert.DoesNotContain() instead." + ); // Placeholder for rule X2030 diff --git a/src/xunit.analyzers/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheck.cs b/src/xunit.analyzers/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheck.cs new file mode 100644 index 00000000..3526e020 --- /dev/null +++ b/src/xunit.analyzers/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheck.cs @@ -0,0 +1,62 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace Xunit.Analyzers; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheck : AssertUsageAnalyzerBase +{ + const string linqWhereMethod = "System.Linq.Enumerable.Where(System.Collections.Generic.IEnumerable, System.Func)"; + + static readonly string[] targetMethods = + { + Constants.Asserts.Empty, + }; + + public AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheck() + : base(Descriptors.X2029_AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheck, targetMethods) + { } + + protected override void AnalyzeInvocation( + OperationAnalysisContext context, + XunitContext xunitContext, + IInvocationOperation invocationOperation, + IMethodSymbol method) + { + Guard.ArgumentNotNull(xunitContext); + Guard.ArgumentNotNull(invocationOperation); + Guard.ArgumentNotNull(method); + + var arguments = invocationOperation.Arguments; + if (arguments.Length != 1) + return; + + var argument = arguments[0]; + var value = argument.Value; + if (value is IConversionOperation conversion) + value = conversion.Operand; + + if (value is not IInvocationOperation innerInvocation) + return; + + var originalMethod = SymbolDisplay.ToDisplayString(innerInvocation.TargetMethod.OriginalDefinition); + if (originalMethod != linqWhereMethod) + return; + + context.ReportDiagnostic( + Diagnostic.Create( + Descriptors.X2029_AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheck, + invocationOperation.Syntax.GetLocation(), + SymbolDisplay.ToDisplayString( + method, + SymbolDisplayFormat + .CSharpShortErrorMessageFormat + .WithParameterOptions(SymbolDisplayParameterOptions.None) + .WithGenericsOptions(SymbolDisplayGenericsOptions.None) + ) + ) + ); + } +}