Skip to content

Commit

Permalink
Reenable xUnit1051 inside of Assert or Record lambdas
Browse files Browse the repository at this point in the history
  • Loading branch information
bradwilson committed Dec 22, 2024
1 parent b55799f commit abfeef7
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -162,4 +162,25 @@ async Task InnerMethod() {

await Verify.VerifyAnalyzerV3(LanguageVersion.CSharp7, source);
}

[Fact]
public async Task InsideAssertionLambda_Triggers()
{
var source = /* lang=c#-test */ """
using System;
using System.Threading;
using System.Threading.Tasks;
using Xunit;
class TestClass {
[Fact]
public async ValueTask TestMethod() {
await Assert.ThrowsAsync<Exception>(() => [|Task.Delay(1)|]);
await Record.ExceptionAsync(() => [|Task.Delay(1)|]);
}
}
""";

await Verify.VerifyAnalyzerV3(LanguageVersion.CSharp7, source);
}
}
38 changes: 31 additions & 7 deletions src/xunit.analyzers/Utility/CodeAnalysisExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,30 +68,49 @@ public static ImmutableArray<AttributeData> GetAttributesWithInheritance(
return result.Values.SelectMany(x => x).ToImmutableArray();
}

public static bool IsInTestMethod(
public static (bool isInTestMethod, IOperation? lambdaOwner) IsInTestMethod(
this IOperation operation,
XunitContext xunitContext)
{
Guard.ArgumentNotNull(operation);
Guard.ArgumentNotNull(xunitContext);

if (xunitContext.Core.FactAttributeType is null || xunitContext.Core.TheoryAttributeType is null)
return false;
return (false, null);

var semanticModel = operation.SemanticModel;
if (semanticModel is null)
return false;
return (false, null);

IOperation? lambdaOwner = null;

for (var parent = operation.Parent; parent is not null; parent = parent.Parent)
{
if (parent is IAnonymousFunctionOperation or ILocalFunctionOperation)
return false;
if (parent is IAnonymousFunctionOperation)
{
if (lambdaOwner is null)
{
lambdaOwner = parent;

if (parent.Parent is IDelegateCreationOperation &&
parent.Parent.Parent is IArgumentOperation &&
parent.Parent.Parent.Parent is IInvocationOperation invocationOperation)
lambdaOwner = invocationOperation;
}

continue;
};
if (parent is ILocalFunctionOperation)
{
lambdaOwner = parent;
continue;
}
if (parent is not IMethodBodyOperation methodBodyOperation)
continue;
if (methodBodyOperation.Syntax is not MethodDeclarationSyntax methodSyntax)
continue;

return methodSyntax.AttributeLists.SelectMany(list => list.Attributes).Any(attr =>
var insideTestMethod = methodSyntax.AttributeLists.SelectMany(list => list.Attributes).Any(attr =>
{
var typeInfo = semanticModel.GetTypeInfo(attr);
if (typeInfo.Type is null)
Expand All @@ -101,9 +120,14 @@ public static bool IsInTestMethod(
SymbolEqualityComparer.Default.Equals(typeInfo.Type, xunitContext.Core.FactAttributeType) ||
SymbolEqualityComparer.Default.Equals(typeInfo.Type, xunitContext.Core.TheoryAttributeType);
});

if (!insideTestMethod)
return (false, null);

return (true, lambdaOwner);
}

return false;
return (false, null);
}

public static bool IsTestClass(
Expand Down
1 change: 1 addition & 0 deletions src/xunit.analyzers/Utility/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ public static class Xunit
public const string LongLivedMarshalByRefObject_Execution_V2 = "Xunit.LongLivedMarshalByRefObject";
public const string LongLivedMarshalByRefObject_RunnerUtility = "Xunit.Sdk.LongLivedMarshalByRefObject";
public const string MemberDataAttribute = "Xunit.MemberDataAttribute";
public const string Record = "Xunit.Record";
public const string RegisterXunitSerializerAttribute_V3 = "Xunit.Sdk.RegisterXunitSerializerAttribute";
public const string TestContext_V3 = "Xunit.TestContext";
public const string TheoryAttribute = "Xunit.TheoryAttribute";
Expand Down
3 changes: 3 additions & 0 deletions src/xunit.analyzers/Utility/TypeSymbolFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,9 @@ public static IArrayTypeSymbol ObjectArray(Compilation compilation) =>
public static INamedTypeSymbol? OptionalAttribute(Compilation compilation) =>
Guard.ArgumentNotNull(compilation).GetTypeByMetadataName("System.Runtime.InteropServices.OptionalAttribute");

public static INamedTypeSymbol? Record(Compilation compilation) =>
Guard.ArgumentNotNull(compilation).GetTypeByMetadataName(Constants.Types.Xunit.Record);

public static INamedTypeSymbol? RegisterXunitSerializerAttribute_V3(Compilation compilation) =>
Guard.ArgumentNotNull(compilation).GetTypeByMetadataName(Constants.Types.Xunit.RegisterXunitSerializerAttribute_V3);

Expand Down
9 changes: 5 additions & 4 deletions src/xunit.analyzers/X1000/DoNotUseBlockingTaskOperations.cs
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,12 @@ static bool FindSymbol(
return false;

// Only trigger when you're inside a test method
var foundSymbol = operation.IsInTestMethod(xunitContext);
if (foundSymbol)
foundSymbolName = symbol.Name;
var (foundSymbol, lambdaOwner) = operation.IsInTestMethod(xunitContext);
if (!foundSymbol || lambdaOwner is not null)
return false;

return foundSymbol;
foundSymbolName = symbol.Name;
return true;
}

static bool TaskIsKnownToBeCompleted(
Expand Down
3 changes: 2 additions & 1 deletion src/xunit.analyzers/X1000/DoNotUseConfigureAwait.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ public override void AnalyzeCompilation(
if (!match)
return;

if (!invocation.IsInTestMethod(xunitContext))
var (foundSymbol, lambdaOwner) = invocation.IsInTestMethod(xunitContext);
if (!foundSymbol || lambdaOwner is not null)
return;

// invocation should be two nodes: "(some other code).ConfigureAwait" and the arguments (like "(false)")
Expand Down
16 changes: 15 additions & 1 deletion src/xunit.analyzers/X1000/UseCancellationToken.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,28 @@ public override void AnalyzeCompilation(
if (cancellationTokenType is null)
return;

var xunitContainerTypes = new[]
{
TypeSymbolFactory.Assert(context.Compilation),
TypeSymbolFactory.Record(context.Compilation),
}.WhereNotNull().ToImmutableHashSet(SymbolEqualityComparer.Default);

context.RegisterOperationAction(context =>
{
if (context.Operation is not IInvocationOperation invocationOperation)
return;

if (!invocationOperation.IsInTestMethod(xunitContext))
var (foundSymbol, lambdaOwner) = invocationOperation.IsInTestMethod(xunitContext);
if (!foundSymbol || lambdaOwner is ILocalFunctionOperation or IAnonymousFunctionOperation)
return;

// We want to try to catch anything that's a lambda from Assert or Record, but
// ignore all other lambdas, because we don't want to catch false positives from
// things like mocking libraries.
if (lambdaOwner is IInvocationOperation lambdaOwnerInvocation)
if (!xunitContainerTypes.Contains(lambdaOwnerInvocation.TargetMethod.ContainingType))
return;

var invokedMethod = invocationOperation.TargetMethod;
var parameters = invokedMethod.Parameters;

Expand Down

0 comments on commit abfeef7

Please sign in to comment.