Create analyzer/fixer for Assert.NotEmpty (#185)

xunit · Jul 24, 2024 · 0122310 · 0122310
1 parent 4f2297b
commit 0122310
Show file tree

Hide file tree

Showing 6 changed files with 144 additions and 69 deletions.
diff --git a/...dForCollectionDoesNotContainCheckFixer.cs → ...yShouldNotBeUsedForContainsChecksFixer.cs b/...dForCollectionDoesNotContainCheckFixer.cs → ...yShouldNotBeUsedForContainsChecksFixer.cs
@@ -1,4 +1,5 @@
 using System.Composition;
+using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis;
@@ -11,12 +12,19 @@
 namespace Xunit.Analyzers.Fixes;
 
 [ExportCodeFixProvider(LanguageNames.CSharp), Shared]
-public class AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckFixer : BatchedCodeFixProvider
+public class AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecksFixer : BatchedCodeFixProvider
 {
-	public const string Key_UseAlternateAssert = "xUnit2029_UseAlternateAssert";
+	public const string Key_UseDoesNotContain = "xUnit2029_UseDoesNotContain";
+	public const string Key_UseContains = "xUnit2030_UseContains";
 
-	public AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckFixer() :
-		base(Descriptors.X2029_AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheck.Id)
+	static readonly string[] targetDiagnostics =
+	[
+		Descriptors.X2029_AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheck.Id,
+		Descriptors.X2030_AssertNotEmptyShouldNotBeUsedForCollectionContainsCheck.Id,
+	];
+
+	public AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecksFixer() :
+		base(targetDiagnostics)
 	{ }
 
 	public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
@@ -29,11 +37,32 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
 		if (invocation is null)
 			return;
 
+		if (context.Diagnostics.Length != 1)
+			return;
+
+		var diagnostic = context.Diagnostics.Single();
+		string replaceAssert;
+		string equivalenceKey;
+		string title;
+
+		if (diagnostic.Id == targetDiagnostics[0])
+		{
+			replaceAssert = Constants.Asserts.DoesNotContain;
+			equivalenceKey = Key_UseDoesNotContain;
+			title = "Use DoesNotContain";
+		}
+		else
+		{
+			replaceAssert = Constants.Asserts.Contains;
+			equivalenceKey = Key_UseContains;
+			title = "Use Contains";
+		}
+
 		context.RegisterCodeFix(
 			XunitCodeAction.Create(
-				c => UseCheck(context.Document, invocation, c),
-				Key_UseAlternateAssert,
-				"Use DoesNotContain"
+				c => UseCheck(context.Document, invocation, replaceAssert, c),
+				equivalenceKey,
+				title
 			),
 			context.Diagnostics
 		);
@@ -42,6 +71,7 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
 	static async Task<Document> UseCheck(
 		Document document,
 		InvocationExpressionSyntax invocation,
+		string replaceAssert,
 		CancellationToken cancellationToken)
 	{
 		var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
@@ -54,7 +84,7 @@ static async Task<Document> UseCheck(
 						invocation,
 						invocation
 							.WithArgumentList(ArgumentList(SeparatedList([Argument(memberAccess.Expression), Argument(innerArgument)])))
-							.WithExpression(outerMemberAccess.WithName(IdentifierName(Constants.Asserts.DoesNotContain)))
+							.WithExpression(outerMemberAccess.WithName(IdentifierName(replaceAssert)))
 					);
 
 		return editor.GetChangedDocument();

diff --git a/...dForCollectionDoesNotContainCheckTests.cs → ...yShouldNotBeUsedForContainsChecksTests.cs b/...dForCollectionDoesNotContainCheckTests.cs → ...yShouldNotBeUsedForContainsChecksTests.cs
@@ -1,8 +1,8 @@
 using System.Threading.Tasks;
 using Xunit;
-using Verify = CSharpVerifier<Xunit.Analyzers.AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheck>;
+using Verify = CSharpVerifier<Xunit.Analyzers.AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecks>;
 
-public class AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckTests
+public class AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecksTests
 {
 	public static TheoryData<string, string> GetEnumerables(
 		string typeName,
@@ -27,6 +27,7 @@ public async Task Containers_WithoutWhereClause_DoesNotTrigger(
 			class TestClass {{
 			    void TestMethod() {{
 			        Xunit.Assert.Empty({0});
+					Xunit.Assert.NotEmpty({0});
 			    }}
 			}}
 			""", collection);
@@ -37,7 +38,7 @@ void TestMethod() {{
 	[Theory]
 	[MemberData(nameof(GetEnumerables), "int", "f > 0")]
 	[MemberData(nameof(GetEnumerables), "string", "f.Length > 0")]
-	public async Task Containers_WithWhereClause_Triggers(
+	public async Task Containers_WithWhereClauseWithIndex_DoesNotTrigger(
 		string collection,
 		string comparison)
 	{
@@ -46,7 +47,8 @@ public async Task Containers_WithWhereClause_Triggers(
 
 			class TestClass {{
 			    void TestMethod() {{
-			        [|Xunit.Assert.Empty({0}.Where(f => {1}))|];
+			        Xunit.Assert.Empty({0}.Where((f, i) => {1} && i > 0));
+					Xunit.Assert.NotEmpty({0}.Where((f, i) => {1} && i > 0));
 			    }}
 			}}
 			""", collection, comparison);
@@ -57,7 +59,7 @@ void TestMethod() {{
 	[Theory]
 	[MemberData(nameof(GetEnumerables), "int", "f > 0")]
 	[MemberData(nameof(GetEnumerables), "string", "f.Length > 0")]
-	public async Task Containers_WithWhereClauseWithIndex_DoesNotTrigger(
+	public async Task EnumurableEmptyCheck_WithChainedLinq_DoesNotTrigger(
 		string collection,
 		string comparison)
 	{
@@ -66,7 +68,8 @@ public async Task Containers_WithWhereClauseWithIndex_DoesNotTrigger(
 
 			class TestClass {{
 			    void TestMethod() {{
-			        Xunit.Assert.Empty({0}.Where((f, i) => {1} && i > 0));
+			        Xunit.Assert.Empty({0}.Where(f => {1}).Select(f => f));
+					Xunit.Assert.NotEmpty({0}.Where(f => {1}).Select(f => f));
 			    }}
 			}}
 			""", collection, comparison);
@@ -77,7 +80,7 @@ void TestMethod() {{
 	[Theory]
 	[MemberData(nameof(GetEnumerables), "int", "f > 0")]
 	[MemberData(nameof(GetEnumerables), "string", "f.Length > 0")]
-	public async Task EnumurableEmptyCheck_WithChainedLinq_DoesNotTrigger(
+	public async Task Containers_WithWhereClause_Triggers(
 		string collection,
 		string comparison)
 	{
@@ -86,10 +89,12 @@ public async Task EnumurableEmptyCheck_WithChainedLinq_DoesNotTrigger(
 
 			class TestClass {{
 			    void TestMethod() {{
-			        Xunit.Assert.Empty({0}.Where(f => {1}).Select(f => f));
+			        {{|xUnit2029:Xunit.Assert.Empty({0}.Where(f => {1}))|}};
+					{{|xUnit2030:Xunit.Assert.NotEmpty({0}.Where(f => {1}))|}};
 			    }}
 			}}
 			""", collection, comparison);
+
 		await Verify.VerifyAnalyzer(source);
 	}
 
@@ -104,7 +109,8 @@ public async Task Strings_WithWhereClause_Triggers(string sampleString)
 
 			class TestClass {{
 			    void TestMethod() {{
-			        [|Xunit.Assert.Empty("{0}".Where(f => f > 0))|];
+			        {{|xUnit2029:Xunit.Assert.Empty("{0}".Where(f => f > 0))|}};
+					{{|xUnit2030:Xunit.Assert.NotEmpty("{0}".Where(f => f > 0))|}};
 			    }}
 			}}
 			""", sampleString);

diff --git a/...zers.tests/Fixes/X2000/AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecksFixerTests.cs b/...zers.tests/Fixes/X2000/AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecksFixerTests.cs
@@ -0,0 +1,64 @@
+using System.Threading.Tasks;
+using Xunit;
+using Xunit.Analyzers.Fixes;
+using Verify = CSharpVerifier<Xunit.Analyzers.AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecks>;
+
+public class AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecksFixerTests
+{
+	const string template = /* lang=c#-test */ """
+		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(
+		/* lang=c#-test */ "{|xUnit2029:Assert.Empty(list.Where(f => f > 0))|}",
+		/* lang=c#-test */ "Assert.DoesNotContain(list, f => f > 0)")]
+	[InlineData(
+		/* lang=c#-test */ "{|xUnit2029:Assert.Empty(list.Where(n => n == 1))|}",
+		/* lang=c#-test */ "Assert.DoesNotContain(list, n => n == 1)")]
+	[InlineData(
+		/* lang=c#-test */ "{|xUnit2029:Assert.Empty(list.Where(IsEven))|}",
+		/* lang=c#-test */ "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, AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecksFixer.Key_UseDoesNotContain);
+	}
+
+	[Theory]
+	[InlineData(
+		/* lang=c#-test */ "{|xUnit2030:Assert.NotEmpty(list.Where(f => f > 0))|}",
+		/* lang=c#-test */ "Assert.Contains(list, f => f > 0)")]
+	[InlineData(
+		/* lang=c#-test */ "{|xUnit2030:Assert.NotEmpty(list.Where(n => n == 1))|}",
+		/* lang=c#-test */ "Assert.Contains(list, n => n == 1)")]
+	[InlineData(
+		/* lang=c#-test */ "{|xUnit2030:Assert.NotEmpty(list.Where(IsEven))|}",
+		/* lang=c#-test */ "Assert.Contains(list, IsEven)")]
+
+	public async Task FixerReplacesAssertNotEmptyWithAssertContains(
+		string beforeAssert,
+		string afterAssert)
+	{
+		var before = string.Format(template, beforeAssert);
+		var after = string.Format(template, afterAssert);
+
+		await Verify.VerifyCodeFix(before, after, AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecksFixer.Key_UseContains);
+	}
+}
diff --git a/...tests/Fixes/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckFixerTests.cs b/...tests/Fixes/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckFixerTests.cs
diff --git a/src/xunit.analyzers/Utility/Descriptors.xUnit2xxx.cs b/src/xunit.analyzers/Utility/Descriptors.xUnit2xxx.cs
@@ -263,13 +263,20 @@ public static partial class Descriptors
 	public static DiagnosticDescriptor X2029_AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheck { get; } =
 		Diagnostic(
 			"xUnit2029",
-			"Do not use Empty() to check if a value does not exist in a collection",
+			"Do not use Assert.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."
+			"Do not use Assert.Empty to check if a value does not exist in a collection. Use Assert.DoesNotContain instead."
 		);
 
-	// Placeholder for rule X2030
+	public static DiagnosticDescriptor X2030_AssertNotEmptyShouldNotBeUsedForCollectionContainsCheck { get; } =
+		Diagnostic(
+			"xUnit2030",
+			"Do not use Assert.NotEmpty to check if a value exists in a collection",
+			Assertions,
+			Warning,
+			"Do not use Assert.NotEmpty to check if a value exists in a collection. Use Assert.Contains instead."
+		);
 
 	// Placeholder for rule X2031
 

diff --git a/...BeUsedForCollectionDoesNotContainCheck.cs → ...tEmptyShouldNotBeUsedForContainsChecks.cs b/...BeUsedForCollectionDoesNotContainCheck.cs → ...tEmptyShouldNotBeUsedForContainsChecks.cs
@@ -6,17 +6,24 @@
 namespace Xunit.Analyzers;
 
 [DiagnosticAnalyzer(LanguageNames.CSharp)]
-public class AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheck : AssertUsageAnalyzerBase
+public class AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecks : AssertUsageAnalyzerBase
 {
 	const string linqWhereMethod = "System.Linq.Enumerable.Where<TSource>(System.Collections.Generic.IEnumerable<TSource>, System.Func<TSource, bool>)";
 
+	static readonly DiagnosticDescriptor[] targetDescriptors =
+	[
+		Descriptors.X2029_AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheck,
+		Descriptors.X2030_AssertNotEmptyShouldNotBeUsedForCollectionContainsCheck,
+	];
+
 	static readonly string[] targetMethods =
-	{
+	[
 		Constants.Asserts.Empty,
-	};
+		Constants.Asserts.NotEmpty,
+	];
 
-	public AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheck()
-		: base(Descriptors.X2029_AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheck, targetMethods)
+	public AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecks() :
+		base(targetDescriptors, targetMethods)
 	{ }
 
 	protected override void AnalyzeInvocation(
@@ -45,9 +52,13 @@ protected override void AnalyzeInvocation(
 		if (originalMethod != linqWhereMethod)
 			return;
 
+		var descriptor = method.Name == Constants.Asserts.Empty
+			? targetDescriptors[0]
+			: targetDescriptors[1];
+
 		context.ReportDiagnostic(
 			Diagnostic.Create(
-				Descriptors.X2029_AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheck,
+				descriptor,
 				invocationOperation.Syntax.GetLocation(),
 				SymbolDisplay.ToDisplayString(
 					method,