Skip to content

Commit

Permalink
Create analyzer/fixer for Assert.Empty (#184)
Browse files Browse the repository at this point in the history
  • Loading branch information
RieBi committed Jul 7, 2024
1 parent db1f0c2 commit a67a4c7
Show file tree
Hide file tree
Showing 5 changed files with 281 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -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<InvocationExpressionSyntax>();
if (invocation is null)
return;

context.RegisterCodeFix(
XunitCodeAction.Create(
c => UseCheck(context.Document, invocation, c),
Key_UseAlternateAssert,
"Use DoesNotContain"
),
context.Diagnostics
);
}

static async Task<Document> 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();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
using System.Threading.Tasks;
using Xunit;
using Verify = CSharpVerifier<Xunit.Analyzers.AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheck>;

public class AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheckTests
{
public static TheoryData<string, string> 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<string> 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);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
using System.Threading.Tasks;
using Xunit;
using Xunit.Analyzers.Fixes;
using Verify = CSharpVerifier<Xunit.Analyzers.AssertEmptyShouldNotBeUsedForCollectionDoesNotContainCheck>;

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);
}
}
9 changes: 8 additions & 1 deletion src/xunit.analyzers/Utility/Descriptors.xUnit2xxx.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -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<TSource>(System.Collections.Generic.IEnumerable<TSource>, System.Func<TSource, bool>)";

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)
)
)
);
}
}

0 comments on commit a67a4c7

Please sign in to comment.