Add MSTEST0067 analyzer to flag Thread.Sleep/Task.Wait/Task<T>.Result in tests

src/Analyzers/MSTest.Analyzers/AnalyzerReleases.Unshipped.md

@@ -8,3 +8,4 @@ Rule ID | Category | Severity | Notes
 MSTEST0064 | Usage | Info | PreferAsyncAssertionAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0064)
 MSTEST0065 | Usage | Warning | AvoidAssertAreEqualOnCollectionsAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0065)
 MSTEST0066 | Design | Info | IgnoreShouldHaveJustificationAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0066)
+MSTEST0067 | Usage | Info | AvoidThreadSleepAndTaskWaitInTestsAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0067)

src/Analyzers/MSTest.Analyzers/AvoidThreadSleepAndTaskWaitInTestsAnalyzer.cs

@@ -0,0 +1,210 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System.Collections.Immutable;
+
+using Analyzer.Utilities.Extensions;
+
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+
+using MSTest.Analyzers.Helpers;
+
+namespace MSTest.Analyzers;
+
+/// <summary>
+/// MSTEST0067: <inheritdoc cref="Resources.AvoidThreadSleepAndTaskWaitInTestsTitle"/>.
+/// </summary>
+[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
+public sealed class AvoidThreadSleepAndTaskWaitInTestsAnalyzer : DiagnosticAnalyzer
+{
+    private static readonly LocalizableResourceString Title = new(nameof(Resources.AvoidThreadSleepAndTaskWaitInTestsTitle), Resources.ResourceManager, typeof(Resources));
+    private static readonly LocalizableResourceString MessageFormat = new(nameof(Resources.AvoidThreadSleepAndTaskWaitInTestsMessageFormat), Resources.ResourceManager, typeof(Resources));
+    private static readonly LocalizableResourceString Description = new(nameof(Resources.AvoidThreadSleepAndTaskWaitInTestsDescription), Resources.ResourceManager, typeof(Resources));
+
+    internal static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorHelper.Create(
+        DiagnosticIds.AvoidThreadSleepAndTaskWaitInTestsRuleId,
+        Title,
+        MessageFormat,
+        Description,
+        Category.Usage,
+        DiagnosticSeverity.Info,
+        isEnabledByDefault: true);
+
+    /// <inheritdoc />
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; }
+        = ImmutableArray.Create(Rule);
+
+    /// <inheritdoc />
+    public override void Initialize(AnalysisContext context)
+    {
+        context.EnableConcurrentExecution();
+        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+
+        context.RegisterCompilationStartAction(context =>
+        {
+            Compilation compilation = context.Compilation;
+            INamedTypeSymbol? threadSymbol = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemThreadingThread);
+            INamedTypeSymbol? taskSymbol = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemThreadingTasksTask);
+            INamedTypeSymbol? taskOfTSymbol = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemThreadingTasksTask1);
+
+            // Collect the set of attribute symbols that mark a method as "test code".
+            ImmutableHashSet<INamedTypeSymbol> testRelatedAttributeSymbols = GetTestRelatedAttributeSymbols(compilation);
+            INamedTypeSymbol? testMethodAttributeSymbol = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingTestMethodAttribute);
+
+            // If no test attribute is available in this compilation, there is nothing to analyze.
+            if (testRelatedAttributeSymbols.IsEmpty && testMethodAttributeSymbol is null)
+            {
+                return;
+            }
+
+            if (threadSymbol is not null || taskSymbol is not null || taskOfTSymbol is not null)
+            {
+                context.RegisterOperationAction(
+                    context => AnalyzeInvocation(context, threadSymbol, taskSymbol, taskOfTSymbol, testRelatedAttributeSymbols, testMethodAttributeSymbol),
+                    OperationKind.Invocation);
+            }
+
+            if (taskOfTSymbol is not null)
+            {
+                context.RegisterOperationAction(
+                    context => AnalyzePropertyReference(context, taskOfTSymbol, testRelatedAttributeSymbols, testMethodAttributeSymbol),
+                    OperationKind.PropertyReference);
+            }
+        });
+    }
+
+    private static void AnalyzeInvocation(
+        OperationAnalysisContext context,
+        INamedTypeSymbol? threadSymbol,
+        INamedTypeSymbol? taskSymbol,
+        INamedTypeSymbol? taskOfTSymbol,
+        ImmutableHashSet<INamedTypeSymbol> testRelatedAttributeSymbols,
+        INamedTypeSymbol? testMethodAttributeSymbol)
+    {
+        var invocation = (IInvocationOperation)context.Operation;
+        IMethodSymbol targetMethod = invocation.TargetMethod;
+
+        string? offendingApi = null;
+        if (threadSymbol is not null
+            && targetMethod is { Name: "Sleep", IsStatic: true }
+            && SymbolEqualityComparer.Default.Equals(targetMethod.ContainingType, threadSymbol))
+        {
+            offendingApi = "Thread.Sleep";
+        }
+        else if (targetMethod is { Name: "Wait", IsStatic: false }
+            && (SymbolEqualityComparer.Default.Equals(targetMethod.ContainingType, taskSymbol)
+                || (taskOfTSymbol is not null && IsConstructedFrom(targetMethod.ContainingType, taskOfTSymbol))))
+        {
+            offendingApi = "Task.Wait";
+        }
+        else if (taskSymbol is not null
+            && targetMethod is { Name: "WaitAll" or "WaitAny", IsStatic: true }
+            && SymbolEqualityComparer.Default.Equals(targetMethod.ContainingType, taskSymbol))
+        {
+            offendingApi = $"Task.{targetMethod.Name}";
+        }
+
+        if (offendingApi is null)
+        {
+            return;
+        }
+
+        if (!IsInsideTestCode(context.ContainingSymbol, testRelatedAttributeSymbols, testMethodAttributeSymbol))
+        {
+            return;
+        }
+
+        context.ReportDiagnostic(invocation.CreateDiagnostic(Rule, offendingApi));
+    }
+
+    private static void AnalyzePropertyReference(
+        OperationAnalysisContext context,
+        INamedTypeSymbol taskOfTSymbol,
+        ImmutableHashSet<INamedTypeSymbol> testRelatedAttributeSymbols,
+        INamedTypeSymbol? testMethodAttributeSymbol)
+    {
+        var propertyReference = (IPropertyReferenceOperation)context.Operation;
+        IPropertySymbol property = propertyReference.Property;
+
+        if (property is not { Name: "Result", IsStatic: false }
+            || !IsConstructedFrom(property.ContainingType, taskOfTSymbol))
+        {
+            return;
+        }
+
+        if (!IsInsideTestCode(context.ContainingSymbol, testRelatedAttributeSymbols, testMethodAttributeSymbol))
+        {
+            return;
+        }
+
+        context.ReportDiagnostic(propertyReference.CreateDiagnostic(Rule, "Task<TResult>.Result"));
+    }
+
+    private static bool IsConstructedFrom(INamedTypeSymbol? typeSymbol, INamedTypeSymbol genericDefinition)
+        => typeSymbol is not null && SymbolEqualityComparer.Default.Equals(typeSymbol.OriginalDefinition, genericDefinition);
+
+    private static bool IsInsideTestCode(
+        ISymbol? containingSymbol,
+        ImmutableHashSet<INamedTypeSymbol> testRelatedAttributeSymbols,
+        INamedTypeSymbol? testMethodAttributeSymbol)
+    {
+        // Walk up through local functions / lambdas to find the enclosing user-declared method.
+        ISymbol? current = containingSymbol;
+        while (current is IMethodSymbol method)
+        {
+            foreach (AttributeData attribute in method.GetAttributes())
+            {
+                INamedTypeSymbol? attributeClass = attribute.AttributeClass;
+                if (attributeClass is null)
+                {
+                    continue;
+                }
+
+                if (testRelatedAttributeSymbols.Contains(attributeClass))
+                {
+                    return true;
+                }
+
+                if (testMethodAttributeSymbol is not null && attributeClass.Inherits(testMethodAttributeSymbol))
+                {
+                    return true;
+                }
+            }
+
+            // Continue walking only when the symbol is synthesized from a local function / lambda body.
+            if (method.MethodKind is MethodKind.LocalFunction or MethodKind.AnonymousFunction)
+            {
+                current = method.ContainingSymbol;
+                continue;
+            }
+
+            return false;
+        }
+
+        return false;
+    }
+
+    private static ImmutableHashSet<INamedTypeSymbol> GetTestRelatedAttributeSymbols(Compilation compilation)
+    {
+        ImmutableHashSet<INamedTypeSymbol>.Builder builder = ImmutableHashSet.CreateBuilder<INamedTypeSymbol>(SymbolEqualityComparer.Default);
+        AddIfPresent(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingTestInitializeAttribute);
+        AddIfPresent(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingTestCleanupAttribute);
+        AddIfPresent(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingClassInitializeAttribute);
+        AddIfPresent(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingClassCleanupAttribute);
+        AddIfPresent(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingAssemblyInitializeAttribute);
+        AddIfPresent(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingAssemblyCleanupAttribute);
+        AddIfPresent(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingGlobalTestInitializeAttribute);
+        AddIfPresent(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingGlobalTestCleanupAttribute);
+        return builder.ToImmutable();
+
+        void AddIfPresent(string metadataName)
+        {
+            if (compilation.GetOrCreateTypeByMetadataName(metadataName) is { } symbol)
+            {
+                builder.Add(symbol);
+            }
+        }
+    }
+}

src/Analyzers/MSTest.Analyzers/Helpers/DiagnosticIds.cs

@@ -71,4 +71,5 @@ internal static class DiagnosticIds
     public const string PreferAsyncAssertionRuleId = "MSTEST0064";
     public const string AvoidAssertAreEqualOnCollectionsRuleId = "MSTEST0065";
     public const string IgnoreShouldHaveJustificationRuleId = "MSTEST0066";
+    public const string AvoidThreadSleepAndTaskWaitInTestsRuleId = "MSTEST0067";
 }

src/Analyzers/MSTest.Analyzers/Helpers/WellKnownTypeNames.cs

@@ -63,4 +63,5 @@ internal static class WellKnownTypeNames
     public const string SystemThreadingTasksTask1 = "System.Threading.Tasks.Task`1";
     public const string SystemThreadingTasksValueTask = "System.Threading.Tasks.ValueTask";
     public const string SystemThreadingTasksValueTask1 = "System.Threading.Tasks.ValueTask`1";
+    public const string SystemThreadingThread = "System.Threading.Thread";
 }

src/Analyzers/MSTest.Analyzers/Resources.resx

@@ -756,6 +756,15 @@ The type declaring these methods should also respect the following rules:
   <data name="TestClassConstructorShouldBeValidDescription" xml:space="preserve">
     <value>Test classes must have a public constructor that is either parameterless or accepts a single TestContext parameter. This allows the test framework to instantiate the test class properly.</value>
   </data>
+  <data name="AvoidThreadSleepAndTaskWaitInTestsTitle" xml:space="preserve">
+    <value>Avoid synchronously blocking calls such as 'Thread.Sleep', 'Task.Wait', 'Task.WaitAll', 'Task.WaitAny' or 'Task&lt;TResult&gt;.Result' in test code</value>
+  </data>
+  <data name="AvoidThreadSleepAndTaskWaitInTestsMessageFormat" xml:space="preserve">
+    <value>Avoid '{0}' in test code as it can cause test flakiness; consider an asynchronous alternative such as 'await Task.Delay' or 'await' the task</value>
+  </data>
+  <data name="AvoidThreadSleepAndTaskWaitInTestsDescription" xml:space="preserve">
+    <value>Synchronously blocking the current thread with 'Thread.Sleep', 'Task.Wait', 'Task.WaitAll', 'Task.WaitAny' or 'Task&lt;TResult&gt;.Result' inside test methods or test fixtures is a common source of flakiness and can also deadlock when the test framework runs tests on a SynchronizationContext that requires cooperative scheduling. Prefer 'await Task.Delay' for time-based waits and 'await' the task to observe its result.</value>
+  </data>
   <data name="PreferAsyncAssertionTitle" xml:space="preserve">
     <value>Prefer async assertion methods</value>
   </data>

src/Analyzers/MSTest.Analyzers/xlf/Resources.cs.xlf

@@ -157,6 +157,21 @@ Typ deklarující tyto metody by měl také respektovat následující pravidla:
         <target state="translated">Vyhněte se předávání explicitního typu DynamicDataSourceType a použijte výchozí chování automatického zjišťování.</target>
         <note />
       </trans-unit>
+      <trans-unit id="AvoidThreadSleepAndTaskWaitInTestsDescription">
+        <source>Synchronously blocking the current thread with 'Thread.Sleep', 'Task.Wait', 'Task.WaitAll', 'Task.WaitAny' or 'Task&lt;TResult&gt;.Result' inside test methods or test fixtures is a common source of flakiness and can also deadlock when the test framework runs tests on a SynchronizationContext that requires cooperative scheduling. Prefer 'await Task.Delay' for time-based waits and 'await' the task to observe its result.</source>
+        <target state="new">Synchronously blocking the current thread with 'Thread.Sleep', 'Task.Wait', 'Task.WaitAll', 'Task.WaitAny' or 'Task&lt;TResult&gt;.Result' inside test methods or test fixtures is a common source of flakiness and can also deadlock when the test framework runs tests on a SynchronizationContext that requires cooperative scheduling. Prefer 'await Task.Delay' for time-based waits and 'await' the task to observe its result.</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="AvoidThreadSleepAndTaskWaitInTestsMessageFormat">
+        <source>Avoid '{0}' in test code as it can cause test flakiness; consider an asynchronous alternative such as 'await Task.Delay' or 'await' the task</source>
+        <target state="new">Avoid '{0}' in test code as it can cause test flakiness; consider an asynchronous alternative such as 'await Task.Delay' or 'await' the task</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="AvoidThreadSleepAndTaskWaitInTestsTitle">
+        <source>Avoid synchronously blocking calls such as 'Thread.Sleep', 'Task.Wait', 'Task.WaitAll', 'Task.WaitAny' or 'Task&lt;TResult&gt;.Result' in test code</source>
+        <target state="new">Avoid synchronously blocking calls such as 'Thread.Sleep', 'Task.Wait', 'Task.WaitAll', 'Task.WaitAny' or 'Task&lt;TResult&gt;.Result' in test code</target>
+        <note />
+      </trans-unit>
       <trans-unit id="AvoidUsingAssertsInAsyncVoidContextDescription">
         <source>Do not assert inside 'async void' methods, local functions, or lambdas. Exceptions that are thrown in this context will be unhandled exceptions. When using VSTest under .NET Framework, they will be silently swallowed. When using Microsoft.Testing.Platform or VSTest under modern .NET, they may crash the process.</source>
         <target state="translated">Nepoužívejte výraz uvnitř metod async void, místních funkcí nebo výrazů lambda. Výjimky vyvolané v tomto kontextu budou neošetřené výjimky. Když používáte VSTest v .NET Framework, budou se bezobslužně používat. Když používáte Microsoft.Testing.Platform nebo VSTest v moderním rozhraní .NET, může dojít k chybovému ukončení procesu.</target>

src/Analyzers/MSTest.Analyzers/xlf/Resources.de.xlf

@@ -157,6 +157,21 @@ Der Typ, der diese Methoden deklariert, sollte auch die folgenden Regeln beachte
         <target state="translated">Vermeiden Sie die explizite Übergabe von „DynamicDataSourceType” und verwenden Sie das standardmäßige automatische Erkennungsverhalten.</target>
         <note />
       </trans-unit>
+      <trans-unit id="AvoidThreadSleepAndTaskWaitInTestsDescription">
+        <source>Synchronously blocking the current thread with 'Thread.Sleep', 'Task.Wait', 'Task.WaitAll', 'Task.WaitAny' or 'Task&lt;TResult&gt;.Result' inside test methods or test fixtures is a common source of flakiness and can also deadlock when the test framework runs tests on a SynchronizationContext that requires cooperative scheduling. Prefer 'await Task.Delay' for time-based waits and 'await' the task to observe its result.</source>
+        <target state="new">Synchronously blocking the current thread with 'Thread.Sleep', 'Task.Wait', 'Task.WaitAll', 'Task.WaitAny' or 'Task&lt;TResult&gt;.Result' inside test methods or test fixtures is a common source of flakiness and can also deadlock when the test framework runs tests on a SynchronizationContext that requires cooperative scheduling. Prefer 'await Task.Delay' for time-based waits and 'await' the task to observe its result.</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="AvoidThreadSleepAndTaskWaitInTestsMessageFormat">
+        <source>Avoid '{0}' in test code as it can cause test flakiness; consider an asynchronous alternative such as 'await Task.Delay' or 'await' the task</source>
+        <target state="new">Avoid '{0}' in test code as it can cause test flakiness; consider an asynchronous alternative such as 'await Task.Delay' or 'await' the task</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="AvoidThreadSleepAndTaskWaitInTestsTitle">
+        <source>Avoid synchronously blocking calls such as 'Thread.Sleep', 'Task.Wait', 'Task.WaitAll', 'Task.WaitAny' or 'Task&lt;TResult&gt;.Result' in test code</source>
+        <target state="new">Avoid synchronously blocking calls such as 'Thread.Sleep', 'Task.Wait', 'Task.WaitAll', 'Task.WaitAny' or 'Task&lt;TResult&gt;.Result' in test code</target>
+        <note />
+      </trans-unit>
       <trans-unit id="AvoidUsingAssertsInAsyncVoidContextDescription">
         <source>Do not assert inside 'async void' methods, local functions, or lambdas. Exceptions that are thrown in this context will be unhandled exceptions. When using VSTest under .NET Framework, they will be silently swallowed. When using Microsoft.Testing.Platform or VSTest under modern .NET, they may crash the process.</source>
         <target state="translated">Nicht innerhalb von "async void"-Methoden, lokalen Funktionen oder Lambdafunktionen bestätigen. Ausnahmen, die in diesem Kontext ausgelöst werden, werden nicht behandelt. Bei Verwendung von VSTest unter .NET Framework werden diese im Hintergrund verschlungen. Wenn Sie Microsoft.Testing.Platform oder VSTest unter modernem .NET verwenden, kann der Prozess abgestürzt werden.</target>