Skip to content

Commit

Permalink
Create analyzer/fixer for Assert.NotEmpty (#185)
Browse files Browse the repository at this point in the history
  • Loading branch information
RieBi authored Jul 24, 2024
1 parent 4f2297b commit 0122310
Show file tree
Hide file tree
Showing 6 changed files with 144 additions and 69 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Composition;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
Expand All @@ -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)
Expand All @@ -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
);
Expand All @@ -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);
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -27,6 +27,7 @@ public async Task Containers_WithoutWhereClause_DoesNotTrigger(
class TestClass {{
void TestMethod() {{
Xunit.Assert.Empty({0});
Xunit.Assert.NotEmpty({0});
}}
}}
""", collection);
Expand All @@ -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)
{
Expand All @@ -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);
Expand All @@ -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)
{
Expand All @@ -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);
Expand All @@ -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)
{
Expand All @@ -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);
}

Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}

This file was deleted.

13 changes: 10 additions & 3 deletions src/xunit.analyzers/Utility/Descriptors.xUnit2xxx.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 0122310

Please sign in to comment.