feat(analyzers): suppress VSTHRD200 on test and hook methods

[thomhurst]

What

Adds Vsthrd200AsyncSuffixSuppressor, a Roslyn DiagnosticSuppressor that silences VSTHRD200 ("Use Async suffix for async methods") on:

• Test methods — any method decorated with an attribute deriving from TUnit.Core.BaseTestAttribute (covers [Test], [DynamicTestBuilder], and any future subclass).
• Hook methods — [Before] / [After] / [BeforeEvery] / [AfterEvery].

VSTHRD200 is useful for production code but noisy on tests: [Test] async Task MyTest() shouldn't be forced to MyTestAsync. This mirrors the convention MSTest and xUnit already ship.

How

• New suppressor follows the existing MarkMethodStaticSuppressor (CA1822) pattern.
• New MethodExtensions.HasTestAttribute walks the attribute inheritance chain via the existing GetSelfAndBaseTypes() helper. Left the existing exact-match IsTestMethod untouched to avoid changing behavior of the ~14 analyzers that call it.
• Test project references Microsoft.VisualStudio.Threading.Analyzers (analyzer dll, Analyzer="false") so the VSTHRD200 analyzer type is loadable in suppressor tests.

No AnalyzerReleases.*.md change — suppressors are not tracked there.

Tests

Vsthrd200AsyncSuffixSuppressorTests — VSTHRD200 suppressed on [Test] + 4 hook variants; not suppressed on a plain async non-test method. All pass.

Closes #6121

[claude]

Review: Vsthrd200AsyncSuffixSuppressor

This is a clean, well-motivated addition. The implementation is consistent with existing suppressors and the tests cover all the important cases. A few observations and suggestions below.

---

Asymmetry with MarkMethodStaticSuppressor (potential coverage gap)

MarkMethodStaticSuppressor explicitly limits its hook-method suppression to IsStandardHookMethod where hookLevel is HookLevel.Test — it does not suppress CA1822 for [BeforeEvery] / [AfterEvery] hooks. The new Vsthrd200AsyncSuffixSuppressor uses IsHookMethod (which covers both standard and "every" hooks), so it suppresses VSTHRD200 on all hook levels.

That is probably the right call for VSTHRD200 — there is no reason any hook method should require an Async suffix — but it is worth noting explicitly so the intentional difference is documented and does not get "fixed" toward the narrower behaviour later.

If the intent is "all hooks", consider a short comment in ReportSuppressions like:

// All hook levels (Before/After/BeforeEvery/AfterEvery) are suppressed —
// hook methods intentionally omit the Async suffix regardless of scope.

---

HasTestAttribute vs IsTestMethod — diverging helpers with similar names

IsTestMethod checks for exact TUnit.Core.TestAttribute, while HasTestAttribute walks the inheritance chain up from BaseTestAttribute. Both exist on MethodExtensions now, with the doc comment on HasTestAttribute explaining the distinction. However:

1. The name gap is subtle. A future contributor may reach for IsTestMethod when they actually want the broader HasTestAttribute check. Consider whether HasTestAttribute should supersede IsTestMethod by having IsTestMethod delegate to it, or at least adding an XML <remarks> to IsTestMethod pointing to HasTestAttribute:

/// <remarks>
/// This performs an exact-match on <c>TUnit.Core.TestAttribute</c>.
/// Use <see cref="HasTestAttribute"/> if you need to match custom subclasses
/// (e.g. <c>[DynamicTestBuilder]</c>).
/// </remarks>
public static bool IsTestMethod(this IMethodSymbol methodSymbol, Compilation compilation)

2. Is IsTestMethod correct for the existing 14 callers? They all use it for validation logic that presumably should also apply to [DynamicTestBuilder] tests. If so, they have a latent bug that this PR's new helper exposes but does not fix. That can be a separate issue/PR, but calling it out here is worthwhile.

---

SupportedSuppressions.First(...) in Suppress — unnecessary allocation per call

This is a pattern copied from the existing suppressors, so it is not introduced by this PR. But since this PR adds another instance, it is worth noting: SupportedSuppressions.First(s => s.SuppressedDiagnosticId == diagnostic.Id) performs a linear scan every time a suppression is reported. For a suppressor that only ever suppresses one diagnostic ID (VSTHRD200), a direct reference is simpler and allocation-free:

private void Suppress(SuppressionAnalysisContext context, Diagnostic diagnostic)
    => context.ReportSuppression(Suppression.Create(SupportedSuppressions[0], diagnostic));

Or, since CreateDescriptor is private and only called once, just capture the descriptor in a static readonly field alongside SupportedSuppressions. Again — this is a pre-existing pattern issue, but this PR is a good opportunity to fix it in the new class at least.

---

Test: [Before(Test)] hook is missing from MarkMethodStaticSuppressorTests — intentional?

MarkMethodStaticSuppressorTests tests [BeforeEvery(Test)] and [AfterEvery(Test)]... actually looking again, it tests [Before(Test)] and [After(Test)] only. The new Vsthrd200AsyncSuffixSuppressorTests additionally tests [BeforeEvery(Test)] and [AfterEvery(Test)], which makes sense given the broader IsHookMethod check. This is a good incremental improvement.

---

Minor: #if NET8_0_OR_GREATER guard

The test file is correctly gated, matching the pattern in MarkMethodStaticSuppressorTests. No issue here — just confirming the guard is appropriate given the NET8_0_OR_GREATER requirement for the VSTHRD analyzer type used in tests.

---

Summary

Overall this is a well-executed feature: it follows existing patterns, has the right scope (test + all hook methods), and includes both positive and negative test cases. The main actionable suggestion is adding a <remarks> to IsTestMethod to signal its narrow semantics compared to HasTestAttribute, which will prevent future confusion as both helpers coexist.

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 9 complexity

Metric	Results
Complexity	9

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]

Review: Vsthrd200AsyncSuffixSuppressor

This is a clean, well-motivated addition. The implementation is consistent with existing suppressors and the tests cover all important cases. Below are observations ranging from minor to more significant.

---

SupportedSuppressions[0] — improvement already applied

The new suppressor correctly uses SupportedSuppressions[0] rather than the .First(s => s.SuppressedDiagnosticId == diagnostic.Id) scan seen in MarkMethodStaticSuppressor and DynamicTestAwaitExpressionSuppressor. Since this suppressor handles exactly one diagnostic ID, the direct index access is both more efficient and clearer. This is a good practice improvement over the pre-existing pattern.

---

Latent bug: IsTestClass also relies on exact-match IsTestMethod

TypeExtensions.IsTestClass (line 42–47 in TypeExtensions.cs) determines whether a class is a test class by checking whether any of its methods pass IsTestMethod. Since IsTestMethod performs an exact match on TUnit.Core.TestAttribute, a class that only contains [DynamicTestBuilder] methods is not recognized as a test class by IsTestClass. This flows into at least 10 callsites (e.g. MatrixAnalyzer, TestDataAnalyzer, ClassParametersAnalyzer, etc.) that may quietly skip analysis for [DynamicTestBuilder]-only classes.

This is a pre-existing issue exposed by the documentation contrast between IsTestMethod (narrow) and HasTestAttribute (broad) that this PR introduces. It is worth creating a follow-up issue so it is not forgotten.

---

HasTestAttribute — allocation in the hot path

HasTestAttribute calls GetSelfAndBaseTypes() which yields types lazily, then passes the sequence to Contains(...) with an IEqualityComparer. The Contains extension from LINQ forces enumeration of the entire chain up to System.Object even after a match is found on the first type in the chain (because .Contains on IEnumerable<T> is a linear scan).

A more allocation-efficient pattern:

return methodSymbol.GetAttributes().Any(attribute =>
{
    var cls = attribute.AttributeClass;
    while (cls is not null && cls.SpecialType != SpecialType.System_Object)
    {
        if (SymbolEqualityComparer.Default.Equals(cls, baseTestAttribute))
            return true;
        cls = cls.BaseType;
    }
    return false;
});

This is a minor concern for an analyzer (not a hot production path), but since GetSelfAndBaseTypes() is already implemented as a yield-based iterator, simply replacing the LINQ .Contains call with .Any(t => SymbolEqualityComparer.Default.Equals(t, baseTestAttribute)) would at least short-circuit correctly:

return methodSymbol.GetAttributes().Any(attribute =>
    attribute.AttributeClass?.GetSelfAndBaseTypes()
        .Any(t => SymbolEqualityComparer.Default.Equals(t, baseTestAttribute)) == true);

The current Contains(baseTestAttribute, SymbolEqualityComparer.Default) calls the IEnumerable<T>.Contains overload which does not short-circuit the iterator — it walks to the end. Using .Any(...) instead would short-circuit on first match.

---

MarkMethodStaticSuppressor still uses narrow IsTestMethod

CA1822 ("mark member as static") is suppressed for [Test] methods but not for [DynamicTestBuilder] methods via the static suppressor. If a [DynamicTestBuilder] method is an instance method (the common case — it accesses test context), CA1822 would fire and not be suppressed. This is the same kind of oversight that VSTHRD200 had before this PR. This is not a new problem introduced here, but it is a natural companion fix.

---

Test coverage: missing [DynamicTestBuilder] case for HasTestAttribute

The WarningsOnTestAndHookMethodsAreSuppressed test covers [Test] and all four hook variants, but not [DynamicTestBuilder] — which is precisely the motivation for introducing HasTestAttribute over IsTestMethod. Adding a [Arguments("DynamicTestBuilder")] case would close this gap:

[Arguments("Test")]
[Arguments("DynamicTestBuilder")]   // <-- add this
[Arguments("Before(Test)")]
...

---

Minor: SupportedSuppressions as instance property backed by ImmutableArray.Create

SupportedSuppressions is declared as an instance property initialized with ImmutableArray.Create(...). Since the array does not vary per-instance, it could be static readonly:

public override ImmutableArray<SuppressionDescriptor> SupportedSuppressions { get; } =
    ImmutableArray.Create(CreateDescriptor("VSTHRD200"));

This matches the pattern in MarkMethodStaticSuppressor, so it is already consistent — just noting that this is actually static state stored per-instance. A private static readonly backing field and a property returning it would be marginally more efficient (no per-instance allocation), but this is a nit given how infrequently suppressor instances are created.

---

Summary

The core feature is correct and well-structured. The main actionable items are:

1. Add a [DynamicTestBuilder] test case to WarningsOnTestAndHookMethodsAreSuppressed to validate the primary motivating scenario for HasTestAttribute.
2. Use .Any(...) instead of .Contains(...) in HasTestAttribute to ensure short-circuit evaluation.
3. File a follow-up issue tracking the IsTestClass / IsTestMethod gap for [DynamicTestBuilder]-only classes.
4. Consider a companion fix for MarkMethodStaticSuppressor using HasTestAttribute to cover [DynamicTestBuilder].

Items 3 and 4 are out-of-scope for this PR and can be addressed in follow-ups.

[claude]

Code Review: feat(analyzers): suppress VSTHRD200 on test and hook methods

This is a well-structured addition that follows the existing suppressor pattern. The design is sound and the core logic is correct. A few issues worth addressing:

---

Bug: — use ID-aware lookup instead

File: TUnit.Analyzers/Vsthrd200AsyncSuffixSuppressor.cs, line 45

// Current (fragile):
private void Suppress(SuppressionAnalysisContext context, Diagnostic diagnostic)
    => context.ReportSuppression(Suppression.Create(SupportedSuppressions[0], diagnostic));

// MarkMethodStaticSuppressor does this (correct pattern):
var suppression = SupportedSuppressions.First(s => s.SuppressedDiagnosticId == diagnostic.Id);
context.ReportSuppression(Suppression.Create(suppression, diagnostic));

MarkMethodStaticSuppressor (line 41) uses SupportedSuppressions.First(s => s.SuppressedDiagnosticId == diagnostic.Id). The new suppressor hard-codes index [0] instead. Today this is harmless since there is only one descriptor — but it is inconsistent with the established codebase pattern and becomes a latent bug the moment a second descriptor is added to SupportedSuppressions: the wrong descriptor would be applied to diagnostics matching the second ID, silently mis-attributing the suppression.

---

Behavioral asymmetry: is suppressed for VSTHRD200 but not for CA1822

File: TUnit.Analyzers/Vsthrd200AsyncSuffixSuppressor.cs, line 34 vs MarkMethodStaticSuppressor.cs line 27

The new suppressor uses HasTestAttribute (inheritance-aware, covers [DynamicTestBuilder]). MarkMethodStaticSuppressor uses IsTestMethod (exact match, [DynamicTestBuilder] not covered). The result: the same [DynamicTestBuilder] async non-static method will have its VSTHRD200 suppressed by this PR, but will still get a CA1822 "make static" warning. This inconsistency will confuse users.

Either update MarkMethodStaticSuppressor to also use HasTestAttribute, or document why the two suppressors intentionally have different coverage.

---

Wrong code comment: Contains vs Any

File: TUnit.Analyzers/Extensions/MethodExtensions.cs, lines 36–37

// Use Any(...) rather than Contains(...): the latter walks the whole base-type chain even
// after a match, whereas Any short-circuits on the first matching type.

This comment is factually incorrect. Enumerable.Contains<T> (LINQ extension on IEnumerable<T>) short-circuits on the first match — it is equivalent to Any(x => Equals(x, value)). There is no performance difference. A future maintainer reading this comment will draw false conclusions about LINQ semantics. The comment should either be removed or corrected to explain the actual reason Any was chosen (e.g., the predicate form is clearer, or there was a specific compat concern).

---

Minor: without CancellationToken

File: TUnit.Analyzers/Vsthrd200AsyncSuffixSuppressor.cs, line 19

SuppressionAnalysisContext exposes context.CancellationToken. SyntaxTree.GetRoot(CancellationToken) cooperates with IDE cancellation. The tokenless overload is used here (and in MarkMethodStaticSuppressor/DynamicTestAwaitExpressionSuppressor — pre-existing), but since the new PR touches this path, it would be a good opportunity to pass context.CancellationToken consistently.

---

Test gap: Assembly/Class-scope hooks not tested

File: TUnit.Analyzers.Tests/Vsthrd200AsyncSuffixSuppressorTests.cs, line 13

The parameterized test covers Before(Test), After(Test), BeforeEvery(Test), AfterEvery(Test). The suppressor comment says suppression is deliberate for all hook levels. Before(Assembly), Before(Class), and their every-variants are not covered. A future narrowing of IsHookMethod to only Test-level scope (matching MarkMethodStaticSuppressor's pattern) would silently regress Assembly/Class hooks with no test catching it. Worth adding a few extra [Arguments] entries.

---

Overall

The core design — using HasTestAttribute for inheritance-aware detection and delegating to IsHookMethod for hooks — is correct and clean. The SupportedSuppressions[0] → ID-aware lookup fix is the most important change since it aligns with the pattern used everywhere else in the codebase. The [DynamicTestBuilder] CA1822 asymmetry is worth a conscious decision one way or the other.

[claude]

Code Review: feat(analyzers): suppress VSTHRD200 on test and hook methods

This is a well-structured addition that follows the existing suppressor pattern. The design is sound and the core logic is correct. A few issues worth addressing:

---

Bug: SupportedSuppressions[0] — use ID-aware lookup instead

File: TUnit.Analyzers/Vsthrd200AsyncSuffixSuppressor.cs, line 45

MarkMethodStaticSuppressor (line 41) uses SupportedSuppressions.First(s => s.SuppressedDiagnosticId == diagnostic.Id). The new suppressor hard-codes index [0] instead. Today this is harmless since there is only one descriptor, but it is inconsistent with the established codebase pattern and becomes a latent bug the moment a second descriptor is added: the wrong descriptor would be applied to diagnostics matching the second ID, silently mis-attributing the suppression.

Recommended fix — match the existing pattern:

private void Suppress(SuppressionAnalysisContext context, Diagnostic diagnostic)
{
    var descriptor = SupportedSuppressions.First(s => s.SuppressedDiagnosticId == diagnostic.Id);
    context.ReportSuppression(Suppression.Create(descriptor, diagnostic));
}

---

Behavioral asymmetry: [DynamicTestBuilder] suppressed for VSTHRD200 but not CA1822

File: TUnit.Analyzers/Vsthrd200AsyncSuffixSuppressor.cs line 34 vs MarkMethodStaticSuppressor.cs line 27

The new suppressor uses HasTestAttribute (inheritance-aware, covers [DynamicTestBuilder]). MarkMethodStaticSuppressor uses IsTestMethod (exact match — [DynamicTestBuilder] not covered). The result: a [DynamicTestBuilder] async non-static method will have VSTHRD200 suppressed but will still get a CA1822 "make static" warning from the other suppressor. This inconsistency will confuse users who annotate methods with [DynamicTestBuilder].

Either update MarkMethodStaticSuppressor to also use HasTestAttribute, or add a comment explaining why the two suppressors intentionally have different coverage.

---

Wrong code comment: Contains vs Any

File: TUnit.Analyzers/Extensions/MethodExtensions.cs, lines 36–37

The comment reads: "the latter walks the whole base-type chain even after a match, whereas Any short-circuits on the first matching type."

This is factually incorrect. Enumerable.Contains<T> (LINQ extension on IEnumerable<T>) short-circuits on the first match — it is functionally equivalent to Any(x => Equals(x, value)). There is no performance difference between the two. A future maintainer will draw false conclusions about LINQ semantics from this comment. Remove it or replace it with the actual reason Any was chosen (e.g., the predicate form is structurally necessary because the equality check is non-trivial).

---

Minor: GetRoot() without CancellationToken

File: TUnit.Analyzers/Vsthrd200AsyncSuffixSuppressor.cs, line 19

SuppressionAnalysisContext exposes context.CancellationToken. SyntaxTree.GetRoot(CancellationToken) cooperates with IDE cancellation. The tokenless overload is used here and in the two other pre-existing suppressors, but since this PR adds new code to the same pattern, it would be a good opportunity to use diagnostic.Location.SourceTree?.GetRoot(context.CancellationToken) consistently.

---

Test gap: Assembly/Class-scope hooks not covered

File: TUnit.Analyzers.Tests/Vsthrd200AsyncSuffixSuppressorTests.cs, line 13

The parameterized test covers Before(Test), After(Test), BeforeEvery(Test), AfterEvery(Test). The suppressor comment claims all hook scopes are covered, but Before(Assembly), Before(Class), and their every-variants are not tested. A future narrowing of IsHookMethod to only Test-level scope (paralleling MarkMethodStaticSuppressor) would silently stop suppressing Assembly/Class hooks with no failing test.

---

Overall

The core design is correct and clean. The SupportedSuppressions[0] → ID-aware lookup fix and the [DynamicTestBuilder] / CA1822 asymmetry are the most important things to address — both for consistency with the existing codebase and to avoid surprising users.

[claude]

Code Review

This is a well-motivated, cleanly executed PR. Suppressing VSTHRD200 on test/hook methods is the right call — the convention that async test methods should be named MyTestAsync is pure noise, and this mirrors what MSTest and xUnit already do. The implementation follows the existing suppressor pattern faithfully and is in good shape overall.

Good things worth calling out

• HasTestAttribute is a cleaner, more reusable primitive than IsTestMethod and the PR correctly leaves IsTestMethod alone to avoid touching 14 call sites. The new method handles the BaseTestAttribute subclass case ([DynamicTestBuilder], custom user attributes) that the old one misses.
• Using GetRoot(context.CancellationToken) is a real improvement over the existing suppressors (MarkMethodStaticSuppressor, DynamicTestAwaitExpressionSuppressor) which both call GetRoot() without the cancellation token. This is the more correct Roslyn pattern.
• The decision to use IsHookMethod for all hook levels (rather than just HookLevel.Test as MarkMethodStaticSuppressor does) is sound — the async-naming rule is irrelevant at every hook scope, not just test-level hooks. The inline comment explaining this divergence is helpful.
• Test coverage for all 10 hook attribute variants is thorough.

Issues

1. Missing test for custom BaseTestAttribute subclass (most significant gap)

The whole point of HasTestAttribute over IsTestMethod is inheritance-aware detection. But the tests only exercise built-in TUnit attributes. A test that defines:

[AttributeUsage(AttributeTargets.Method)]
public class MyCustomTestAttribute : TUnit.Core.BaseTestAttribute { }

public class MyTests
{
    [MyCustomTest]
    public async Task {|#0:Foo|}() { await Task.CompletedTask; }
}

...and asserts VSTHRD200 is suppressed would validate the core new behavior. Without it, a regression that broke inheritance walking would go undetected.

2. Suppress() should be private static

Suppress() doesn't access this, so it should be private static void Suppress(...). The same issue exists in MarkMethodStaticSuppressor and DynamicTestAwaitExpressionSuppressor — but it's worth fixing consistently here since this is new code.

// current
private void Suppress(SuppressionAnalysisContext context, Diagnostic diagnostic) { ... }

// better
private static void Suppress(SuppressionAnalysisContext context, Diagnostic diagnostic) { ... }

3. SupportedSuppressions.First() should use index access

SupportedSuppressions is an ImmutableArray<SuppressionDescriptor> with exactly one element, constructed at class instantiation. Inside the suppression hot path:

// current — allocates an enumerator every call
var descriptor = SupportedSuppressions.First(s => s.SuppressedDiagnosticId == diagnostic.Id);

// better — direct index access, no allocation
var descriptor = SupportedSuppressions[0];

Given only one descriptor is registered and it's already been matched to diagnostic.Id by the Roslyn framework before this method is called, the First() predicate is redundant. (Same issue exists in MarkMethodStaticSuppressor.)

4. Pre-existing: IsTestMethod null-forgiveness vs. HasTestAttribute null guard

IsTestMethod uses ! null-forgiveness on compilation.GetTypeByMetadataName(...)! while the new HasTestAttribute correctly null-guards it. This inconsistency pre-exists and is out of scope here, but worth tracking — if TUnit.Core isn't referenced in the compilation, IsTestMethod will throw.

Summary

The core feature and design are solid. The main actionable items are: add a test for a custom BaseTestAttribute subclass (validates the actual new behavior), make Suppress() static, and use [0] instead of .First(). Items 2 and 3 are minor polish.

[thomhurst]

Addressed in the latest commit:

• Suppress() static: hoisted the single descriptor into a static readonly field so Suppress is now static and references it directly.
• [0] vs First(): settled on direct index access with a documenting comment (this oscillated between review passes — the two are equivalent given one descriptor; [0] avoids the per-call scan).

Declined:

• Custom BaseTestAttribute subclass test: not viable — BaseTestAttribute's constructor is internal, so external code cannot derive from it. The inheritance-aware HasTestAttribute path is already validated by the [DynamicTestBuilder] case, which is a genuine BaseTestAttribute subclass distinct from the exact [Test].
• IsTestMethod null-forgiveness (item 4): pre-existing and out of scope, as noted.

[claude]

Review: feat(analyzers): suppress VSTHRD200 on test and hook methods

Overall this is a clean, well-motivated addition that follows established patterns — and in a few places improves on them. A couple of things worth discussing:

---

Good improvements over the existing pattern

Null-guard in HasTestAttribute (MethodExtensions.cs)
The existing IsTestMethod uses ! (null-forgiving) after GetTypeByMetadataName. HasTestAttribute does the right thing instead:

if (baseTestAttribute is null) return false;

Strictly better, and worth noting as the preferred pattern going forward.

CancellationToken passed to GetRoot (Vsthrd200AsyncSuffixSuppressor.cs:20)

diagnostic.Location.SourceTree?.GetRoot(context.CancellationToken)

MarkMethodStaticSuppressor and DynamicTestAwaitExpressionSuppressor both call GetRoot() without a token — this is a quiet improvement. No action needed here, but it highlights a minor gap in the existing suppressors.

---

Minor: Contains with SymbolEqualityComparer.Default — type inference subtlety

GetSelfAndBaseTypes() returns IEnumerable<INamedTypeSymbol>, and SymbolEqualityComparer.Default is IEqualityComparer<ISymbol>. These are compatible only because IEnumerable<T> is covariant, so the compiler resolves TSource as ISymbol. It compiles and behaves correctly, but it's a subtle inference.

The Any form from an earlier commit is slightly more explicit about what's happening:

.Any(t => SymbolEqualityComparer.Default.Equals(t, baseTestAttribute))

Both short-circuit identically. The commit notes say Contains "already short-circuits" and reverted from Any — this is correct. It's a style preference, not a correctness issue.

---

Design note: intentional asymmetry with MarkMethodStaticSuppressor

The PR comment documents this well: MarkMethodStaticSuppressor (CA1822) uses exact-match IsTestMethod + only Test-level hooks, while this suppressor uses inheritance-aware HasTestAttribute + all hook levels. The reasoning is sound — CA1822 "make static" has narrower applicability than "drop Async suffix". The comment in the code explains this clearly enough to not be surprising to future maintainers.

---

Test coverage

Comprehensive — covers all hook scopes (Test/Class/Assembly), [DynamicTestBuilder], and verifies non-test methods are not suppressed. The #if NET8_0_OR_GREATER guard correctly includes all targeted frameworks. The package reference setup with Analyzer="false" is the right approach to make the VSTHRD200 analyzer type loadable without applying it to the test project itself.

---

Verdict: Ready to merge. The only real discussion point is Contains vs Any on the IEnumerable<INamedTypeSymbol>, which is purely stylistic — both are correct and both short-circuit. Everything else is solid.