From 01223107dbef6fd9e54728773dbf4e0421a8e637 Mon Sep 17 00:00:00 2001 From: Sviatoslav Zubar Date: Wed, 24 Jul 2024 18:55:11 +0200 Subject: [PATCH] Create analyzer/fixer for Assert.NotEmpty (#185) --- ...yShouldNotBeUsedForContainsChecksFixer.cs} | 46 ++++++++++--- ...yShouldNotBeUsedForContainsChecksTests.cs} | 24 ++++--- ...uldNotBeUsedForContainsChecksFixerTests.cs | 64 +++++++++++++++++++ ...CollectionDoesNotContainCheckFixerTests.cs | 43 ------------- .../Utility/Descriptors.xUnit2xxx.cs | 13 +++- ...tEmptyShouldNotBeUsedForContainsChecks.cs} | 23 +++++-- 6 files changed, 144 insertions(+), 69 deletions(-) rename src/xunit.analyzers.fixes/X2000/{AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckFixer.cs => AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecksFixer.cs} (60%) rename src/xunit.analyzers.tests/Analyzers/X2000/{AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckTests.cs => AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecksTests.cs} (82%) create mode 100644 src/xunit.analyzers.tests/Fixes/X2000/AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecksFixerTests.cs delete mode 100644 src/xunit.analyzers.tests/Fixes/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckFixerTests.cs rename src/xunit.analyzers/X2000/{AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheck.cs => AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecks.cs} (72%) diff --git a/src/xunit.analyzers.fixes/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckFixer.cs b/src/xunit.analyzers.fixes/X2000/AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecksFixer.cs similarity index 60% rename from src/xunit.analyzers.fixes/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckFixer.cs rename to src/xunit.analyzers.fixes/X2000/AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecksFixer.cs index dee4e1c9..35b01e83 100644 --- a/src/xunit.analyzers.fixes/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckFixer.cs +++ b/src/xunit.analyzers.fixes/X2000/AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecksFixer.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 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 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/src/xunit.analyzers.tests/Analyzers/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckTests.cs b/src/xunit.analyzers.tests/Analyzers/X2000/AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecksTests.cs similarity index 82% rename from src/xunit.analyzers.tests/Analyzers/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckTests.cs rename to src/xunit.analyzers.tests/Analyzers/X2000/AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecksTests.cs index 6d017834..9f90e667 100644 --- a/src/xunit.analyzers.tests/Analyzers/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckTests.cs +++ b/src/xunit.analyzers.tests/Analyzers/X2000/AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecksTests.cs @@ -1,8 +1,8 @@ using System.Threading.Tasks; using Xunit; -using Verify = CSharpVerifier; +using Verify = CSharpVerifier; -public class AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckTests +public class AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecksTests { public static TheoryData 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/src/xunit.analyzers.tests/Fixes/X2000/AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecksFixerTests.cs b/src/xunit.analyzers.tests/Fixes/X2000/AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecksFixerTests.cs new file mode 100644 index 00000000..5a283645 --- /dev/null +++ b/src/xunit.analyzers.tests/Fixes/X2000/AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecksFixerTests.cs @@ -0,0 +1,64 @@ +using System.Threading.Tasks; +using Xunit; +using Xunit.Analyzers.Fixes; +using Verify = CSharpVerifier; + +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/src/xunit.analyzers.tests/Fixes/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckFixerTests.cs b/src/xunit.analyzers.tests/Fixes/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckFixerTests.cs deleted file mode 100644 index efe54c55..00000000 --- a/src/xunit.analyzers.tests/Fixes/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckFixerTests.cs +++ /dev/null @@ -1,43 +0,0 @@ -using System.Threading.Tasks; -using Xunit; -using Xunit.Analyzers.Fixes; -using Verify = CSharpVerifier; - -public class AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckFixerTests -{ - 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 */ "[|Assert.Empty(list.Where(f => f > 0))|]", - /* lang=c#-test */ "Assert.DoesNotContain(list, f => f > 0)")] - [InlineData( - /* lang=c#-test */ "[|Assert.Empty(list.Where(n => n == 1))|]", - /* lang=c#-test */ "Assert.DoesNotContain(list, n => n == 1)")] - [InlineData( - /* lang=c#-test */ "[|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, AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckFixer.Key_UseAlternateAssert); - } -} diff --git a/src/xunit.analyzers/Utility/Descriptors.xUnit2xxx.cs b/src/xunit.analyzers/Utility/Descriptors.xUnit2xxx.cs index 5bcca044..a16e0882 100644 --- 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/src/xunit.analyzers/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheck.cs b/src/xunit.analyzers/X2000/AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecks.cs similarity index 72% rename from src/xunit.analyzers/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheck.cs rename to src/xunit.analyzers/X2000/AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecks.cs index 3526e020..a5ed3fe1 100644 --- a/src/xunit.analyzers/X2000/AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheck.cs +++ b/src/xunit.analyzers/X2000/AssertEmptyOrNotEmptyShouldNotBeUsedForContainsChecks.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(System.Collections.Generic.IEnumerable, System.Func)"; + 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,