From 1fe31ea2f2e305eb61ec3a3da19b642328039d43 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Fri, 29 May 2026 23:35:47 +0100 Subject: [PATCH 1/5] feat(analyzers): suppress VSTHRD200 on test and hook methods Adds a DiagnosticSuppressor that silences VSTHRD200 ("Use Async suffix for async methods") on TUnit test methods (any attribute deriving from BaseTestAttribute) and hook methods, matching the convention used by MSTest and xUnit. Closes #6121 --- Directory.Packages.props | 1 + .../TUnit.Analyzers.Tests.csproj | 7 +++ .../Vsthrd200AsyncSuffixSuppressorTests.cs | 62 +++++++++++++++++++ .../Extensions/MethodExtensions.cs | 18 ++++++ .../Vsthrd200AsyncSuffixSuppressor.cs | 60 ++++++++++++++++++ 5 files changed, 148 insertions(+) create mode 100644 TUnit.Analyzers.Tests/Vsthrd200AsyncSuffixSuppressorTests.cs create mode 100644 TUnit.Analyzers/Vsthrd200AsyncSuffixSuppressor.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index e4b23376fb..ab9fba2d99 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -35,6 +35,7 @@ + diff --git a/TUnit.Analyzers.Tests/TUnit.Analyzers.Tests.csproj b/TUnit.Analyzers.Tests/TUnit.Analyzers.Tests.csproj index bcabd116ef..1be1335763 100644 --- a/TUnit.Analyzers.Tests/TUnit.Analyzers.Tests.csproj +++ b/TUnit.Analyzers.Tests/TUnit.Analyzers.Tests.csproj @@ -49,6 +49,13 @@ Link="Shared\AnalyzerTestCompatibility.cs" /> + + + + + + diff --git a/TUnit.Analyzers.Tests/Vsthrd200AsyncSuffixSuppressorTests.cs b/TUnit.Analyzers.Tests/Vsthrd200AsyncSuffixSuppressorTests.cs new file mode 100644 index 0000000000..601c237fdd --- /dev/null +++ b/TUnit.Analyzers.Tests/Vsthrd200AsyncSuffixSuppressorTests.cs @@ -0,0 +1,62 @@ +#if NET8_0_OR_GREATER +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Testing; +using Microsoft.VisualStudio.Threading.Analyzers; + +namespace TUnit.Analyzers.Tests; + +public class Vsthrd200AsyncSuffixSuppressorTests +{ + private static readonly DiagnosticResult VSTHRD200 = new("VSTHRD200", DiagnosticSeverity.Warning); + + [Test] + [Arguments("Test")] + [Arguments("Before(Test)")] + [Arguments("After(Test)")] + [Arguments("BeforeEvery(Test)")] + [Arguments("AfterEvery(Test)")] + public async Task WarningsOnTestAndHookMethodsAreSuppressed(string attribute) => + await AnalyzerTestHelpers + .CreateSuppressorTest( + $$""" + using System.Threading.Tasks; + using TUnit.Core; + using static TUnit.Core.HookType; + + public class MyTests + { + [{{attribute}}] + public async Task {|#0:Foo|}() + { + await Task.CompletedTask; + } + } + """ + ) + .WithAnalyzer() + .WithSpecificDiagnostics(VSTHRD200) + .WithExpectedDiagnosticsResults(VSTHRD200.WithLocation(0).WithIsSuppressed(true)) + .RunAsync(); + + [Test] + public async Task WarningsAllowedElsewhere() => + await AnalyzerTestHelpers + .CreateSuppressorTest( + """ + using System.Threading.Tasks; + + public class MyTests + { + public async Task {|#0:DoSomething|}() + { + await Task.CompletedTask; + } + } + """ + ) + .WithAnalyzer() + .WithSpecificDiagnostics(VSTHRD200) + .WithExpectedDiagnosticsResults(VSTHRD200.WithLocation(0).WithIsSuppressed(false)) + .RunAsync(); +} +#endif diff --git a/TUnit.Analyzers/Extensions/MethodExtensions.cs b/TUnit.Analyzers/Extensions/MethodExtensions.cs index 4f5d305eee..b02072a7e5 100644 --- a/TUnit.Analyzers/Extensions/MethodExtensions.cs +++ b/TUnit.Analyzers/Extensions/MethodExtensions.cs @@ -12,6 +12,24 @@ public static bool IsTestMethod(this IMethodSymbol methodSymbol, Compilation com return methodSymbol.GetAttributes().Any(x => SymbolEqualityComparer.Default.Equals(x.AttributeClass, testAttribute)); } + /// + /// Returns true if the method is decorated with any attribute deriving from + /// TUnit.Core.BaseTestAttribute (e.g. [Test] or [DynamicTestBuilder]). + /// + public static bool HasTestAttribute(this IMethodSymbol methodSymbol, Compilation compilation) + { + var baseTestAttribute = compilation.GetTypeByMetadataName("TUnit.Core.BaseTestAttribute"); + + if (baseTestAttribute is null) + { + return false; + } + + return methodSymbol.GetAttributes().Any(attribute => + attribute.AttributeClass?.GetSelfAndBaseTypes() + .Contains(baseTestAttribute, SymbolEqualityComparer.Default) == true); + } + public static bool IsHookMethod(this IMethodSymbol methodSymbol, Compilation compilation, [NotNullWhen(true)] out INamedTypeSymbol? type, [NotNullWhen(true)] out HookLevel? hookLevel, [NotNullWhen(true)] out HookType? hookType) { return IsStandardHookMethod(methodSymbol, compilation, out type, out hookLevel, out hookType) || IsEveryHookMethod(methodSymbol, compilation, out type, out hookLevel, out hookType); diff --git a/TUnit.Analyzers/Vsthrd200AsyncSuffixSuppressor.cs b/TUnit.Analyzers/Vsthrd200AsyncSuffixSuppressor.cs new file mode 100644 index 0000000000..e27e5e6178 --- /dev/null +++ b/TUnit.Analyzers/Vsthrd200AsyncSuffixSuppressor.cs @@ -0,0 +1,60 @@ +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using TUnit.Analyzers.Extensions; + +namespace TUnit.Analyzers; + +/// +/// Suppresses VSTHRD200 ("Use 'Async' suffix for async methods") on TUnit test +/// methods and hook methods, which intentionally do not follow the Async naming convention. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class Vsthrd200AsyncSuffixSuppressor : DiagnosticSuppressor +{ + public override void ReportSuppressions(SuppressionAnalysisContext context) + { + foreach (var diagnostic in context.ReportedDiagnostics) + { + if (diagnostic.Location.SourceTree?.GetRoot().FindNode(diagnostic.Location.SourceSpan) is not { } node) + { + continue; + } + + var semanticModel = context.GetSemanticModel(diagnostic.Location.SourceTree); + + if (semanticModel.GetDeclaredSymbol(node) is not IMethodSymbol methodSymbol) + { + continue; + } + + if (methodSymbol.HasTestAttribute(context.Compilation) + || methodSymbol.IsHookMethod(context.Compilation, out _, out _, out _)) + { + Suppress(context, diagnostic); + } + } + } + + private void Suppress(SuppressionAnalysisContext context, Diagnostic diagnostic) + { + var suppression = SupportedSuppressions.First(s => s.SuppressedDiagnosticId == diagnostic.Id); + + context.ReportSuppression( + Suppression.Create( + suppression, + diagnostic + ) + ); + } + + public override ImmutableArray SupportedSuppressions { get; } = + ImmutableArray.Create(CreateDescriptor("VSTHRD200")); + + private static SuppressionDescriptor CreateDescriptor(string id) + => new( + id: $"{id}Suppression", + suppressedDiagnosticId: id, + justification: "TUnit test and hook methods do not require the 'Async' suffix." + ); +} From 72fd176ca5e95503ede95bdb23b972d7be746103 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Sat, 30 May 2026 00:40:18 +0100 Subject: [PATCH 2/5] docs(analyzers): clarify IsTestMethod semantics; tidy VSTHRD200 suppressor - Add to IsTestMethod noting it is an exact-match check vs the inheritance-aware HasTestAttribute, preventing future confusion. - Document the intentional all-hook-levels suppression in the VSTHRD200 suppressor and reference the single descriptor directly instead of a per-diagnostic linear scan. --- TUnit.Analyzers/Extensions/MethodExtensions.cs | 8 ++++++++ .../Vsthrd200AsyncSuffixSuppressor.cs | 16 ++++++---------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/TUnit.Analyzers/Extensions/MethodExtensions.cs b/TUnit.Analyzers/Extensions/MethodExtensions.cs index b02072a7e5..660f43463e 100644 --- a/TUnit.Analyzers/Extensions/MethodExtensions.cs +++ b/TUnit.Analyzers/Extensions/MethodExtensions.cs @@ -6,6 +6,14 @@ namespace TUnit.Analyzers.Extensions; public static class MethodExtensions { + /// + /// Returns true if the method is decorated with the exact TUnit.Core.TestAttribute. + /// + /// + /// This is an exact-match check — it does not match subclasses of BaseTestAttribute + /// (e.g. [DynamicTestBuilder]). Use for the broader, + /// inheritance-aware check. + /// public static bool IsTestMethod(this IMethodSymbol methodSymbol, Compilation compilation) { var testAttribute = compilation.GetTypeByMetadataName("TUnit.Core.TestAttribute")!; diff --git a/TUnit.Analyzers/Vsthrd200AsyncSuffixSuppressor.cs b/TUnit.Analyzers/Vsthrd200AsyncSuffixSuppressor.cs index e27e5e6178..cbb526fdf1 100644 --- a/TUnit.Analyzers/Vsthrd200AsyncSuffixSuppressor.cs +++ b/TUnit.Analyzers/Vsthrd200AsyncSuffixSuppressor.cs @@ -28,6 +28,9 @@ public override void ReportSuppressions(SuppressionAnalysisContext context) continue; } + // IsHookMethod covers all hook levels (Before/After/BeforeEvery/AfterEvery) intentionally — + // no hook method requires the 'Async' suffix regardless of scope. (This is deliberately + // broader than MarkMethodStaticSuppressor, which only narrows CA1822 to Test-level hooks.) if (methodSymbol.HasTestAttribute(context.Compilation) || methodSymbol.IsHookMethod(context.Compilation, out _, out _, out _)) { @@ -36,17 +39,10 @@ public override void ReportSuppressions(SuppressionAnalysisContext context) } } + // This suppressor only ever handles VSTHRD200, so reference the single descriptor directly + // rather than scanning SupportedSuppressions on every reported diagnostic. private void Suppress(SuppressionAnalysisContext context, Diagnostic diagnostic) - { - var suppression = SupportedSuppressions.First(s => s.SuppressedDiagnosticId == diagnostic.Id); - - context.ReportSuppression( - Suppression.Create( - suppression, - diagnostic - ) - ); - } + => context.ReportSuppression(Suppression.Create(SupportedSuppressions[0], diagnostic)); public override ImmutableArray SupportedSuppressions { get; } = ImmutableArray.Create(CreateDescriptor("VSTHRD200")); From 509844cd5e8daa6d1cb8649139ac3d6679c504a2 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Sat, 30 May 2026 00:56:53 +0100 Subject: [PATCH 3/5] test(analyzers): cover [DynamicTestBuilder]; short-circuit HasTestAttribute MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add a [DynamicTestBuilder] case to the VSTHRD200 suppressor tests — the motivating scenario for HasTestAttribute over the exact-match IsTestMethod. - Replace Contains(...) with Any(...) in HasTestAttribute so the base-type chain walk short-circuits on first match. --- TUnit.Analyzers.Tests/Vsthrd200AsyncSuffixSuppressorTests.cs | 1 + TUnit.Analyzers/Extensions/MethodExtensions.cs | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/TUnit.Analyzers.Tests/Vsthrd200AsyncSuffixSuppressorTests.cs b/TUnit.Analyzers.Tests/Vsthrd200AsyncSuffixSuppressorTests.cs index 601c237fdd..36df24323a 100644 --- a/TUnit.Analyzers.Tests/Vsthrd200AsyncSuffixSuppressorTests.cs +++ b/TUnit.Analyzers.Tests/Vsthrd200AsyncSuffixSuppressorTests.cs @@ -11,6 +11,7 @@ public class Vsthrd200AsyncSuffixSuppressorTests [Test] [Arguments("Test")] + [Arguments("DynamicTestBuilder")] [Arguments("Before(Test)")] [Arguments("After(Test)")] [Arguments("BeforeEvery(Test)")] diff --git a/TUnit.Analyzers/Extensions/MethodExtensions.cs b/TUnit.Analyzers/Extensions/MethodExtensions.cs index 660f43463e..3f6161325c 100644 --- a/TUnit.Analyzers/Extensions/MethodExtensions.cs +++ b/TUnit.Analyzers/Extensions/MethodExtensions.cs @@ -33,9 +33,11 @@ public static bool HasTestAttribute(this IMethodSymbol methodSymbol, Compilation return false; } + // 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. return methodSymbol.GetAttributes().Any(attribute => attribute.AttributeClass?.GetSelfAndBaseTypes() - .Contains(baseTestAttribute, SymbolEqualityComparer.Default) == true); + .Any(t => SymbolEqualityComparer.Default.Equals(t, baseTestAttribute)) == true); } public static bool IsHookMethod(this IMethodSymbol methodSymbol, Compilation compilation, [NotNullWhen(true)] out INamedTypeSymbol? type, [NotNullWhen(true)] out HookLevel? hookLevel, [NotNullWhen(true)] out HookType? hookType) From 5bcf5353d10f3403fd1d1502df4738beb65b4cc9 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Sat, 30 May 2026 01:24:38 +0100 Subject: [PATCH 4/5] review(analyzers): address VSTHRD200 suppressor follow-up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Suppress(): use ID-aware SupportedSuppressions.First lookup, matching the established suppressor pattern. - Pass context.CancellationToken to GetRoot for IDE cancellation cooperation. - Expand the asymmetry comment: HasTestAttribute covers [DynamicTestBuilder] while CA1822's IsTestMethod does not — intentional, documented. - Revert HasTestAttribute to Contains (LINQ Contains already short-circuits; the prior comment was incorrect) and drop the misleading comment. - Cover Class/Assembly-scope hooks in the suppressor tests. --- .../Vsthrd200AsyncSuffixSuppressorTests.cs | 4 ++++ TUnit.Analyzers/Extensions/MethodExtensions.cs | 4 +--- .../Vsthrd200AsyncSuffixSuppressor.cs | 18 +++++++++++------- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/TUnit.Analyzers.Tests/Vsthrd200AsyncSuffixSuppressorTests.cs b/TUnit.Analyzers.Tests/Vsthrd200AsyncSuffixSuppressorTests.cs index 36df24323a..774e3d745b 100644 --- a/TUnit.Analyzers.Tests/Vsthrd200AsyncSuffixSuppressorTests.cs +++ b/TUnit.Analyzers.Tests/Vsthrd200AsyncSuffixSuppressorTests.cs @@ -16,6 +16,10 @@ public class Vsthrd200AsyncSuffixSuppressorTests [Arguments("After(Test)")] [Arguments("BeforeEvery(Test)")] [Arguments("AfterEvery(Test)")] + [Arguments("Before(Class)")] + [Arguments("After(Class)")] + [Arguments("Before(Assembly)")] + [Arguments("After(Assembly)")] public async Task WarningsOnTestAndHookMethodsAreSuppressed(string attribute) => await AnalyzerTestHelpers .CreateSuppressorTest( diff --git a/TUnit.Analyzers/Extensions/MethodExtensions.cs b/TUnit.Analyzers/Extensions/MethodExtensions.cs index 3f6161325c..660f43463e 100644 --- a/TUnit.Analyzers/Extensions/MethodExtensions.cs +++ b/TUnit.Analyzers/Extensions/MethodExtensions.cs @@ -33,11 +33,9 @@ public static bool HasTestAttribute(this IMethodSymbol methodSymbol, Compilation return false; } - // 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. return methodSymbol.GetAttributes().Any(attribute => attribute.AttributeClass?.GetSelfAndBaseTypes() - .Any(t => SymbolEqualityComparer.Default.Equals(t, baseTestAttribute)) == true); + .Contains(baseTestAttribute, SymbolEqualityComparer.Default) == true); } public static bool IsHookMethod(this IMethodSymbol methodSymbol, Compilation compilation, [NotNullWhen(true)] out INamedTypeSymbol? type, [NotNullWhen(true)] out HookLevel? hookLevel, [NotNullWhen(true)] out HookType? hookType) diff --git a/TUnit.Analyzers/Vsthrd200AsyncSuffixSuppressor.cs b/TUnit.Analyzers/Vsthrd200AsyncSuffixSuppressor.cs index cbb526fdf1..dba9130e13 100644 --- a/TUnit.Analyzers/Vsthrd200AsyncSuffixSuppressor.cs +++ b/TUnit.Analyzers/Vsthrd200AsyncSuffixSuppressor.cs @@ -16,7 +16,7 @@ public override void ReportSuppressions(SuppressionAnalysisContext context) { foreach (var diagnostic in context.ReportedDiagnostics) { - if (diagnostic.Location.SourceTree?.GetRoot().FindNode(diagnostic.Location.SourceSpan) is not { } node) + if (diagnostic.Location.SourceTree?.GetRoot(context.CancellationToken).FindNode(diagnostic.Location.SourceSpan) is not { } node) { continue; } @@ -28,9 +28,12 @@ public override void ReportSuppressions(SuppressionAnalysisContext context) continue; } - // IsHookMethod covers all hook levels (Before/After/BeforeEvery/AfterEvery) intentionally — - // no hook method requires the 'Async' suffix regardless of scope. (This is deliberately - // broader than MarkMethodStaticSuppressor, which only narrows CA1822 to Test-level hooks.) + // HasTestAttribute is inheritance-aware (covers [Test], [DynamicTestBuilder], etc.) and + // IsHookMethod covers all hook levels (Before/After/BeforeEvery/AfterEvery). This is + // deliberately broader than MarkMethodStaticSuppressor (CA1822), which uses the exact-match + // IsTestMethod and only narrows to Test-level hooks — so a [DynamicTestBuilder] method may + // still see CA1822. That divergence is intentional: VSTHRD200 (async naming) never applies + // to any test/hook method, whereas CA1822 (make-static) has narrower, scope-specific intent. if (methodSymbol.HasTestAttribute(context.Compilation) || methodSymbol.IsHookMethod(context.Compilation, out _, out _, out _)) { @@ -39,10 +42,11 @@ public override void ReportSuppressions(SuppressionAnalysisContext context) } } - // This suppressor only ever handles VSTHRD200, so reference the single descriptor directly - // rather than scanning SupportedSuppressions on every reported diagnostic. private void Suppress(SuppressionAnalysisContext context, Diagnostic diagnostic) - => context.ReportSuppression(Suppression.Create(SupportedSuppressions[0], diagnostic)); + { + var descriptor = SupportedSuppressions.First(s => s.SuppressedDiagnosticId == diagnostic.Id); + context.ReportSuppression(Suppression.Create(descriptor, diagnostic)); + } public override ImmutableArray SupportedSuppressions { get; } = ImmutableArray.Create(CreateDescriptor("VSTHRD200")); From c048bd993b086cada7a9c81d9f1ffb58f2d259c7 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Sat, 30 May 2026 02:00:47 +0100 Subject: [PATCH 5/5] review(analyzers): make Suppress static via descriptor field - Hoist the single VSTHRD200 descriptor into a static readonly field so Suppress() can be static and reference it directly (no per-call scan). - Settled the [0] vs First() churn with a documenting comment. Did not add the suggested 'custom BaseTestAttribute subclass' test: its constructor is internal, so external code cannot derive from it. The inheritance-aware HasTestAttribute path is already covered by the [DynamicTestBuilder] case (a real BaseTestAttribute subclass). --- TUnit.Analyzers/Vsthrd200AsyncSuffixSuppressor.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/TUnit.Analyzers/Vsthrd200AsyncSuffixSuppressor.cs b/TUnit.Analyzers/Vsthrd200AsyncSuffixSuppressor.cs index dba9130e13..c6a6c70dbe 100644 --- a/TUnit.Analyzers/Vsthrd200AsyncSuffixSuppressor.cs +++ b/TUnit.Analyzers/Vsthrd200AsyncSuffixSuppressor.cs @@ -42,14 +42,16 @@ public override void ReportSuppressions(SuppressionAnalysisContext context) } } - private void Suppress(SuppressionAnalysisContext context, Diagnostic diagnostic) - { - var descriptor = SupportedSuppressions.First(s => s.SuppressedDiagnosticId == diagnostic.Id); - context.ReportSuppression(Suppression.Create(descriptor, diagnostic)); - } + // Exactly one descriptor is registered (VSTHRD200), and Roslyn only routes diagnostics whose id + // matches it here, so referencing the single descriptor directly is correct and avoids a + // per-call predicate scan. + private static void Suppress(SuppressionAnalysisContext context, Diagnostic diagnostic) + => context.ReportSuppression(Suppression.Create(Vsthrd200Descriptor, diagnostic)); + + private static readonly SuppressionDescriptor Vsthrd200Descriptor = CreateDescriptor("VSTHRD200"); public override ImmutableArray SupportedSuppressions { get; } = - ImmutableArray.Create(CreateDescriptor("VSTHRD200")); + ImmutableArray.Create(Vsthrd200Descriptor); private static SuppressionDescriptor CreateDescriptor(string id) => new(