fix(analyzers): scope TUnit0031 async-void rule to tests and hooks (#6190)

[thomhurst]

Problem

Discussion #6190: TUnit0031 (AsyncVoidAnalyzer) flags every async void method and lambda in any project that references TUnit — with no test/hook scoping. Because the rule's severity is Error (Rules.cs), legitimate async void code becomes a build break.

Reported case: a System.Timers.Timer.Elapsed event handler (which requires a void signature) made async → flagged as an error, despite having nothing to do with testing. The thread's "fix" (move tests to a separate project) is only a workaround — the rule ships transitively, so any TUnit-referencing project keeps hitting it.

Fix

Scope the analyzer to its intended target:

• Methods — only flag async void when the method carries a TUnit test attribute (HasTestAttribute, covers [Test]/[DynamicTestBuilder]) or a hook attribute (IsHookMethod, Before/After).
• Lambdas — only flag async void lambdas declared inside such a method (walks out of nested lambdas via ContainingSymbol).

Event handlers and other async void code elsewhere in a test project are left alone.

Tests

Existing tests still pass (async void test method / lambda inside [Test] still error). Added:

• non-test async void event-handler method → no diagnostic
• async void event-handler lambda in a non-test method → no diagnostic
• async void hook ([Before(HookType.Test)]) → still errors

All 10 AsyncVoidAnalyzerTests pass (net10.0).

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 8 complexity

Metric	Results
Complexity	8

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[claude]

Code Review: fix(analyzers): scope TUnit0031 async-void rule to tests and hooks (#6190)

Overall: Correct fix, well-structured, and properly tested. Approved with one edge-case note.

---

What's Good

The root cause is correctly identified and the fix is well-targeted. Previously AsyncVoidAnalyzer raised TUnit0031 on any async void in the compilation, which punished legitimate event-handler patterns (e.g. System.Timers.Timer.Elapsed) just for referencing TUnit. The new guard—IsTestOrHookMethod—restricts the diagnostic to where it actually matters.

The refactoring to early-return style in both AnalyzeMethod and AnalyzeLambda is cleaner than the original nested if shape.

The three new test cases cover all important scenarios:

• Non-test async void method → no diagnostic ✓
• async void lambda inside a non-test method → no diagnostic ✓
• async void on a hook method → diagnostic ✓

Existing tests (lines 52–189) confirm the positive cases inside [Test] methods continue to be flagged.

---

One Edge Case: Local Functions as an Unintentional Escape Hatch

GetEnclosingMethod stops the walk at the first non-AnonymousFunction symbol:

if (method.MethodKind != MethodKind.AnonymousFunction)
{
    return method;
}

MethodKind.LocalFunction is not AnonymousFunction, so a local function inside a [Test] method acts as an invisible barrier:

[Test]
public void MyTest()
{
    void Helper()
    {
        Action a = async () => { await Task.Delay(1); }; // NOT flagged — could surprise devs
    }
}

The lambda walks up to Helper (a local function), IsTestOrHookMethod(Helper) returns false, and the diagnostic is suppressed. In practice this is uncommon and the test-writer probably intended the lambda there intentionally, so this is a low-priority gap. Worth a // Note: local functions are treated as a boundary... comment or a tracked follow-up rather than a blocker here.

---

Minor Note: IsHookMethod Out-Param Discard

private static bool IsTestOrHookMethod(IMethodSymbol methodSymbol, Compilation compilation)
{
    return methodSymbol.HasTestAttribute(compilation)
           || methodSymbol.IsHookMethod(compilation, out _, out _, out _);
}

Three discards is slightly awkward at the call site. The awkward signature is pre-existing, not introduced here, but if IsHookMethod ever gets a boolean-only overload this helper could simplify to a one-liner without the discards. Not a blocker.

---

Summary

The fix is correct, the scope is appropriate, tests are thorough, and the control-flow refactor is an improvement. The local-function edge case is noted but doesn't warrant holding this up.

[claude]

Code Review: fix(analyzers): scope TUnit0031 async-void rule to tests and hooks (#6190)

Overall: Previous review concern addressed. Correct, complete, and well-tested. Approved.

---

Previous Review Concern: Resolved ✓

The earlier review (from the first pass at this PR) flagged that GetEnclosingMethod would treat LocalFunction as an invisible boundary, letting async void lambdas inside local functions of test methods escape the diagnostic:

// Previous version — LocalFunction stopped the walk
if (method.MethodKind != MethodKind.AnonymousFunction)
{
    return method;
}

The updated implementation correctly includes LocalFunction in the "keep walking" condition:

// Current — both AnonymousFunction and LocalFunction are transparent
if (method.MethodKind is not (MethodKind.AnonymousFunction or MethodKind.LocalFunction))
{
    return method;
}

And the new Async_Void_Lambda_In_Local_Function_In_Test_Raises_Error test case proves exactly this scenario is now flagged. Good catch and good fix.

---

Correctness Analysis

GetEnclosingMethod walk logic — The walk correctly handles all nesting levels. Starting from the lambda symbol (AnonymousFunction), it steps through any enclosing local functions and lambdas, stopping only at a real method (constructor, regular method, operator, etc.). If it reaches a containing symbol that isn't an IMethodSymbol (e.g., a class), the while loop exits and returns null, which is correctly treated as "not in a test" and suppresses the diagnostic. ✓

HasTestAttribute uses BaseTestAttribute inheritance — The extension checks the full inheritance chain (GetSelfAndBaseTypes()), so custom test attributes deriving from BaseTestAttribute (e.g., [DynamicTestBuilder]) are correctly covered without any extra wiring. ✓

IsHookMethod covers both standard and "every" hook variants — The implementation delegates to IsStandardHookMethod and IsEveryHookMethod, so all TUnit hook attribute families are included. ✓

---

Test Coverage: Comprehensive

Scenario	Expected	Covered
async void regular method (no attributes)	No diagnostic	Async_Void_NonTest_Method_Raises_No_Error ✓
async void lambda in non-test method	No diagnostic	Async_Void_EventHandler_Lambda_In_NonTest_Method_Raises_No_Error ✓
async void lambda inside local function inside [Test]	Diagnostic	Async_Void_Lambda_In_Local_Function_In_Test_Raises_Error ✓
async void hook method	Diagnostic	Async_Void_Hook_Raises_Error ✓
async void [Test] method	Diagnostic	Covered by existing tests ✓
async void lambda inside [Test] method	Diagnostic	Covered by existing tests ✓

No gaps.

---

Negligible Notes (not blockers)

• The BOM removal on the first line is a harmless cleanup.
• The IsHookMethod(compilation, out _, out _, out _) triple-discard remains slightly awkward but is pre-existing API design, not introduced here. A boolean-only overload of IsHookMethod would be a nice follow-up if that surface ever gets refactored.

---

The fix correctly narrows TUnit0031's scope from "all async void in the compilation" to "async void in TUnit test methods and hooks", which is exactly what was intended. Implementation, tests, and edge-case handling are all solid.