Add JoinableTaskContext.CreateNoOpContext() method

The `JoinableTaskContext` constructor unfortunately has a poor design where folks passing in `null` for the `SynchronizationContext` may mistakenly believe that they're getting a no-op instance when they aren't, if `SynchronizationContext.Current != null`.

In this change, I add a method that *definitely* does what they expect.

I also add an analyzer and a code fix provider to help identify code that was likely written with the incorrect expectation to help them switch to the new method which matches their expectation, or suppress the warning with a more clear syntax.

Author: AArnott

doc/analyzers/VSTHRD115.md

@@ -0,0 +1,38 @@
+# VSTHRD115 Avoid creating a JoinableTaskContext with an explicit `null` `SynchronizationContext`
+
+Constructing a `JoinableTaskContext` with an explicit `null` `SynchronizationContext` is not recommended as a means to construct an instance for use in unit tests or processes without a main thread.
+This is because the constructor will automatically use `SynchronizationContext.Current` in lieu of a non-`null` argument.
+If `SynchronizationContext.Current` happens to be non-`null`, the constructor may unexpectedly configure the new instance as if a main thread were present.
+
+## Examples of patterns that are flagged by this analyzer
+
+```csharp
+void SetupJTC() {
+    this.jtc = new JoinableTaskContext(null, null);
+}
+```
+
+This code *appears* to configure the `JoinableTaskContext` to not be associated with any `SynchronizationContext`.
+But in fact it will be associated with the current `SynchronizationContext` if one is present.
+
+## Solution
+
+If you intended to inherit `SynchronizationContext.Current` to initialize with a main thread,
+provide that value explicitly as the second argument to suppress the warning:
+
+```cs
+void SetupJTC() {
+    this.jtc = new JoinableTaskContext(null, SynchronizationContext.Current);
+}
+```
+
+If you intended to create a `JoinableTaskContext` for use in a unit test or in a process without a main thread,
+call `JoinableTaskContext.CreateNoOpContext()` instead:
+
+```cs
+void SetupJTC() {
+    this.jtc = JoinableTaskContext.CreateNoOpContext();
+}
+```
+
+Code fixes are offered to update code to either of the above patterns.

doc/analyzers/index.md

@@ -28,6 +28,7 @@ ID | Title | Severity | Supports | Default diagnostic severity
 [VSTHRD112](VSTHRD112.md) | Implement `System.IAsyncDisposable` | Advisory | | Info
 [VSTHRD113](VSTHRD113.md) | Check for `System.IAsyncDisposable` | Advisory | | Info
 [VSTHRD114](VSTHRD114.md) | Avoid returning null from a `Task`-returning method. | Advisory | | Warning
+[VSTHRD115](VSTHRD115.md) | Avoid creating a JoinableTaskContext with an explicit `null` `SynchronizationContext` | Advisory | | Warning
 [VSTHRD200](VSTHRD200.md) | Use `Async` naming convention | Guideline | [VSTHRD103](VSTHRD103.md) | Warning
 
 ## Severity descriptions

src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/FixUtils.cs

@@ -12,6 +12,9 @@
 using Microsoft.CodeAnalysis.FindSymbols;
 using Microsoft.CodeAnalysis.Rename;
 using Microsoft.CodeAnalysis.Simplification;
+using CSSyntax = Microsoft.CodeAnalysis.CSharp.Syntax;
+using VB = Microsoft.CodeAnalysis.VisualBasic;
+using VBSyntax = Microsoft.CodeAnalysis.VisualBasic.Syntax;
 
 namespace Microsoft.VisualStudio.Threading.Analyzers;
 
@@ -330,6 +333,22 @@ internal static NameSyntax QualifyName(IReadOnlyList<string> qualifiers, SimpleN
 
     internal static async Task<SyntaxNode> GetSyntaxRootOrThrowAsync(this Document document, CancellationToken cancellationToken) => await document.GetSyntaxRootAsync(cancellationToken) ?? throw new InvalidOperationException("No syntax root could be obtained from the document.");
 
+    internal static (SyntaxNode? Creation, SyntaxNode[]? Arguments) FindObjectCreationSyntax(SyntaxNode startFrom)
+    {
+        if (startFrom is CSharpSyntaxNode && startFrom.FirstAncestorOrSelf<CSSyntax.ObjectCreationExpressionSyntax>() is { } csCreation)
+        {
+            return (csCreation, csCreation.ArgumentList?.Arguments.ToArray());
+        }
+        else if (startFrom is VB.VisualBasicSyntaxNode && startFrom.FirstAncestorOrSelf<VBSyntax.ObjectCreationExpressionSyntax>() is { } vbCreation)
+        {
+            return (vbCreation, vbCreation.ArgumentList?.Arguments.ToArray());
+        }
+        else
+        {
+            return (null, null);
+        }
+    }
+
     private static CSharpSyntaxNode UpdateStatementsForAsyncMethod(CSharpSyntaxNode body, SemanticModel? semanticModel, bool hasResultValue, CancellationToken cancellationToken)
     {
         var blockBody = body as BlockSyntax;

src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD115AvoidJoinableTaskContextCtorWithNullArgsCodeFix.cs

@@ -0,0 +1,131 @@
+// 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 System.Threading;
+using System.Threading.Tasks;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CodeActions;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.Editing;
+
+namespace Microsoft.VisualStudio.Threading.Analyzers;
+
+[ExportCodeFixProvider(LanguageNames.CSharp)]
+public class VSTHRD115AvoidJoinableTaskContextCtorWithNullArgsCodeFix : CodeFixProvider
+{
+    public const string SuppressWarningEquivalenceKey = "SuppressWarning";
+
+    public const string UseFactoryMethodEquivalenceKey = "UseFactoryMethod";
+
+    private static readonly ImmutableArray<string> ReusableFixableDiagnosticIds = ImmutableArray.Create(
+        VSTHRD115AvoidJoinableTaskContextCtorWithNullArgsAnalyzer.Id);
+
+    /// <inheritdoc />
+    public override ImmutableArray<string> FixableDiagnosticIds => ReusableFixableDiagnosticIds;
+
+    /// <inheritdoc />
+    public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;
+
+    public override async Task RegisterCodeFixesAsync(CodeFixContext context)
+    {
+        foreach (Diagnostic diagnostic in context.Diagnostics)
+        {
+            SyntaxNode? root = await context.Document.GetSyntaxRootAsync(context.CancellationToken);
+            if (root is null)
+            {
+                continue;
+            }
+
+            if (!diagnostic.Properties.TryGetValue(VSTHRD115AvoidJoinableTaskContextCtorWithNullArgsAnalyzer.NodeTypePropertyName, out string? nodeType) || nodeType is null)
+            {
+                continue;
+            }
+
+            context.RegisterCodeFix(CodeAction.Create(Strings.VSTHRD115_CodeFix_Suppress_Title, ct => this.SuppressDiagnostic(context, root, nodeType, diagnostic, ct), SuppressWarningEquivalenceKey), diagnostic);
+
+            if (diagnostic.Properties.TryGetValue(VSTHRD115AvoidJoinableTaskContextCtorWithNullArgsAnalyzer.UsesDefaultThreadPropertyName, out string? usesDefaultThreadString) && usesDefaultThreadString is "true")
+            {
+                context.RegisterCodeFix(CodeAction.Create(Strings.VSTHRD115_CodeFix_UseFactory_Title, ct => this.SwitchToFactory(context, root, nodeType, diagnostic, ct), UseFactoryMethodEquivalenceKey), diagnostic);
+            }
+        }
+    }
+
+    private async Task<Document> SuppressDiagnostic(CodeFixContext context, SyntaxNode root, string nodeType, Diagnostic diagnostic, CancellationToken cancellationToken)
+    {
+        SyntaxGenerator generator = SyntaxGenerator.GetGenerator(context.Document);
+        SyntaxNode targetNode = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true);
+
+        Compilation? compilation = await context.Document.Project.GetCompilationAsync(cancellationToken);
+        if (compilation is null)
+        {
+            return context.Document;
+        }
+
+        ITypeSymbol? syncContext = compilation.GetTypeByMetadataName("System.Threading.SynchronizationContext");
+        if (syncContext is null)
+        {
+            return context.Document;
+        }
+
+        ITypeSymbol? jtc = compilation.GetTypeByMetadataName(Types.JoinableTaskContext.FullName);
+        if (jtc is null)
+        {
+            return context.Document;
+        }
+
+        SyntaxNode syncContextCurrent = generator.MemberAccessExpression(generator.TypeExpression(syncContext, addImport: true), nameof(SynchronizationContext.Current));
+        switch (nodeType)
+        {
+            case VSTHRD115AvoidJoinableTaskContextCtorWithNullArgsAnalyzer.NodeTypeArgument:
+                root = root.ReplaceNode(targetNode, syncContextCurrent);
+                break;
+            case VSTHRD115AvoidJoinableTaskContextCtorWithNullArgsAnalyzer.NodeTypeCreation:
+                (SyntaxNode? creationNode, SyntaxNode[]? args) = FixUtils.FindObjectCreationSyntax(targetNode);
+                if (creationNode is null || args is null)
+                {
+                    return context.Document;
+                }
+
+                SyntaxNode threadArg = args.Length >= 1 ? args[0] : generator.Argument(generator.NullLiteralExpression());
+                SyntaxNode syncContextArg = generator.Argument(syncContextCurrent);
+
+                root = root.ReplaceNode(creationNode, generator.ObjectCreationExpression(jtc, threadArg, syncContextArg));
+                break;
+        }
+
+        Document modifiedDocument = context.Document.WithSyntaxRoot(root);
+        return modifiedDocument;
+    }
+
+    private async Task<Document> SwitchToFactory(CodeFixContext context, SyntaxNode root, string nodeType, Diagnostic diagnostic, CancellationToken cancellationToken)
+    {
+        SyntaxGenerator generator = SyntaxGenerator.GetGenerator(context.Document);
+        SyntaxNode targetNode = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true);
+
+        Compilation? compilation = await context.Document.Project.GetCompilationAsync(cancellationToken);
+        if (compilation is null)
+        {
+            return context.Document;
+        }
+
+        (SyntaxNode? creationExpression, _) = FixUtils.FindObjectCreationSyntax(targetNode);
+        if (creationExpression is null)
+        {
+            return context.Document;
+        }
+
+        ITypeSymbol? jtc = compilation.GetTypeByMetadataName(Types.JoinableTaskContext.FullName);
+        if (jtc is null)
+        {
+            return context.Document;
+        }
+
+        SyntaxNode factoryExpression = generator.InvocationExpression(generator.MemberAccessExpression(generator.TypeExpression(jtc, addImport: true), Types.JoinableTaskContext.CreateNoOpContext));
+
+        root = root.ReplaceNode(creationExpression, factoryExpression);
+
+        Document modifiedDocument = context.Document.WithSyntaxRoot(root);
+        return modifiedDocument;
+    }
+}

src/Microsoft.VisualStudio.Threading.Analyzers/Strings.Designer.cs

@@ -341,4 +341,16 @@ Start the work within this context, or use JoinableTaskFactory.RunAsync to start
     <value>Use 'Task.FromResult' instead</value>
     <comment>"Task.FromResult" should not be translated.</comment>
   </data>
+  <data name="VSTHRD115_Title" xml:space="preserve">
+    <value>Avoid creating JoinableTaskContext with null SynchronizationContext</value>
+  </data>
+  <data name="VSTHRD115_MessageFormat" xml:space="preserve">
+    <value>Avoid creating JoinableTaskContext with 'null' as the value for the SynchronizationContext because behavior varies by the value of SynchronizationContext.Current</value>
+  </data>
+  <data name="VSTHRD115_CodeFix_Suppress_Title" xml:space="preserve">
+    <value>Specify 'SynchronizationContext.Current' explicitly</value>
+  </data>
+  <data name="VSTHRD115_CodeFix_UseFactory_Title" xml:space="preserve">
+    <value>Use 'JoinableTaskContext.CreateNoOpContext' instead.</value>
+  </data>
 </root>

src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx

@@ -11,6 +11,8 @@ namespace Microsoft.VisualStudio.Threading.Analyzers;
 /// </summary>
 internal static class Types
 {
+    private const string VSThreadingNamespace = "Microsoft.VisualStudio.Threading";
+
     internal static class BclAsyncDisposable
     {
         internal const string FullName = "System.IAsyncDisposable";
@@ -131,6 +133,10 @@ internal static class JoinableTaskCollection
     internal static class JoinableTaskContext
     {
         internal const string TypeName = "JoinableTaskContext";
+
+        internal const string FullName = $"{VSThreadingNamespace}.{TypeName}";
+
+        internal const string CreateNoOpContext = "CreateNoOpContext";
     }
 
     internal static class JoinableTask

src/Microsoft.VisualStudio.Threading.Analyzers/Types.cs

@@ -0,0 +1,95 @@
+// 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;
+using System.Collections.Immutable;
+using System.Linq;
+using System.Threading;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+
+namespace Microsoft.VisualStudio.Threading.Analyzers;
+
+/// <summary>
+/// Flags the use of <c>new JoinableTaskContext(null, null)</c> and advises using <c>JoinableTaskContext.CreateNoOpContext</c> instead.
+/// </summary>
+[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
+public class VSTHRD115AvoidJoinableTaskContextCtorWithNullArgsAnalyzer : DiagnosticAnalyzer
+{
+    public const string Id = "VSTHRD115";
+
+    internal const string UsesDefaultThreadPropertyName = "UsesDefaultThread";
+
+    internal const string NodeTypePropertyName = "NodeType";
+
+    internal const string NodeTypeArgument = "Argument";
+
+    internal const string NodeTypeCreation = "Creation";
+
+    internal static readonly DiagnosticDescriptor Descriptor = new DiagnosticDescriptor(
+        id: Id,
+        title: new LocalizableResourceString(nameof(Strings.VSTHRD115_Title), Strings.ResourceManager, typeof(Strings)),
+        messageFormat: new LocalizableResourceString(nameof(Strings.VSTHRD115_MessageFormat), Strings.ResourceManager, typeof(Strings)),
+        description: null,
+        helpLinkUri: Utils.GetHelpLink(Id),
+        category: "Usage",
+        defaultSeverity: DiagnosticSeverity.Warning,
+        isEnabledByDefault: true);
+
+    /// <inheritdoc />
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Descriptor);
+
+    public override void Initialize(AnalysisContext context)
+    {
+        context.EnableConcurrentExecution();
+        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze);
+
+        context.RegisterCompilationStartAction(startCompilation =>
+        {
+            INamedTypeSymbol? joinableTaskContextType = startCompilation.Compilation.GetTypeByMetadataName(Types.JoinableTaskContext.FullName);
+            if (joinableTaskContextType is not null)
+            {
+                IMethodSymbol? problematicCtor = joinableTaskContextType.InstanceConstructors.SingleOrDefault(ctor => ctor.Parameters.Length == 2 && ctor.Parameters[0].Type.Name == nameof(Thread) && ctor.Parameters[1].Type.Name == nameof(SynchronizationContext));
+
+                if (problematicCtor is not null && joinableTaskContextType.GetMembers(Types.JoinableTaskContext.CreateNoOpContext).Length > 0)
+                {
+                    startCompilation.RegisterOperationAction(Utils.DebuggableWrapper(c => this.AnalyzeObjectCreation(c, joinableTaskContextType, problematicCtor)), OperationKind.ObjectCreation);
+                }
+            }
+        });
+    }
+
+    private void AnalyzeObjectCreation(OperationAnalysisContext context, INamedTypeSymbol joinableTaskContextType, IMethodSymbol problematicCtor)
+    {
+        IObjectCreationOperation creation = (IObjectCreationOperation)context.Operation;
+        if (SymbolEqualityComparer.Default.Equals(creation.Constructor, problematicCtor))
+        {
+            // Only flag if "null" is passed in as the constructor's second argument (explicitly or implicitly).
+            if (creation.Arguments.Length == 2)
+            {
+                IOperation arg2 = creation.Arguments[1].Value;
+                if (arg2 is IConversionOperation { Operand: ILiteralOperation { ConstantValue: { HasValue: true, Value: null } } literal })
+                {
+                    context.ReportDiagnostic(Diagnostic.Create(Descriptor, literal.Syntax.GetLocation(), CreateProperties(NodeTypeArgument)));
+                }
+                else if (arg2 is IDefaultValueOperation { ConstantValue: { HasValue: true, Value: null } })
+                {
+                    context.ReportDiagnostic(Diagnostic.Create(Descriptor, creation.Syntax.GetLocation(), CreateProperties(NodeTypeCreation)));
+                }
+
+                ImmutableDictionary<string, string?> CreateProperties(string nodeType)
+                {
+                    // The caller is using the default thread if they omit the argument, pass in "null", or pass in "Thread.CurrentThread".
+                    // At the moment, we are not testing for the Thread.CurrentThread case.
+                    bool usesDefaultThread = creation.Arguments[0].Value is IDefaultValueOperation { ConstantValue: { HasValue: true, Value: null } }
+                        or IConversionOperation { Operand: ILiteralOperation { ConstantValue: { HasValue: true, Value: null } } };
+
+                    return ImmutableDictionary.Create<string, string?>()
+                        .Add(UsesDefaultThreadPropertyName, usesDefaultThread ? "true" : "false")
+                        .Add(NodeTypePropertyName, nodeType);
+                }
+            }
+        }
+    }
+}