diff --git a/doc/analyzers/VSTHRD115.md b/doc/analyzers/VSTHRD115.md new file mode 100644 index 000000000..5e1560911 --- /dev/null +++ b/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. diff --git a/doc/analyzers/index.md b/doc/analyzers/index.md index fcbbb5ab6..bf20d0337 100644 --- a/doc/analyzers/index.md +++ b/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 diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/FixUtils.cs b/src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/FixUtils.cs index 8d9fac560..9639738b9 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/FixUtils.cs +++ b/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 qualifiers, SimpleN internal static async Task 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() is { } csCreation) + { + return (csCreation, csCreation.ArgumentList?.Arguments.ToArray()); + } + else if (startFrom is VB.VisualBasicSyntaxNode && startFrom.FirstAncestorOrSelf() 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; diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD115AvoidJoinableTaskContextCtorWithNullArgsCodeFix.cs b/src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD115AvoidJoinableTaskContextCtorWithNullArgsCodeFix.cs new file mode 100644 index 000000000..dbbd61137 --- /dev/null +++ b/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 ReusableFixableDiagnosticIds = ImmutableArray.Create( + VSTHRD115AvoidJoinableTaskContextCtorWithNullArgsAnalyzer.Id); + + /// + public override ImmutableArray FixableDiagnosticIds => ReusableFixableDiagnosticIds; + + /// + 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 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 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; + } +} diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers/Strings.Designer.cs b/src/Microsoft.VisualStudio.Threading.Analyzers/Strings.Designer.cs index 0e0d5f400..b16b44432 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers/Strings.Designer.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers/Strings.Designer.cs @@ -583,6 +583,42 @@ internal static string VSTHRD114_Title { } } + /// + /// Looks up a localized string similar to Specify 'SynchronizationContext.Current' explicitly. + /// + internal static string VSTHRD115_CodeFix_Suppress_Title { + get { + return ResourceManager.GetString("VSTHRD115_CodeFix_Suppress_Title", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Use 'JoinableTaskContext.CreateNoOpContext' instead.. + /// + internal static string VSTHRD115_CodeFix_UseFactory_Title { + get { + return ResourceManager.GetString("VSTHRD115_CodeFix_UseFactory_Title", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Avoid creating JoinableTaskContext with 'null' as the value for the SynchronizationContext because behavior varies by the value of SynchronizationContext.Current. + /// + internal static string VSTHRD115_MessageFormat { + get { + return ResourceManager.GetString("VSTHRD115_MessageFormat", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Avoid creating JoinableTaskContext with null SynchronizationContext. + /// + internal static string VSTHRD115_Title { + get { + return ResourceManager.GetString("VSTHRD115_Title", resourceCulture); + } + } + /// /// Looks up a localized string similar to Use "Async" suffix in names of methods that return an awaitable type. /// diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx b/src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx index 40b76e27f..97e248e4d 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx +++ b/src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx @@ -341,4 +341,16 @@ Start the work within this context, or use JoinableTaskFactory.RunAsync to start Use 'Task.FromResult' instead "Task.FromResult" should not be translated. + + Avoid creating JoinableTaskContext with null SynchronizationContext + + + Avoid creating JoinableTaskContext with 'null' as the value for the SynchronizationContext because behavior varies by the value of SynchronizationContext.Current + + + Specify 'SynchronizationContext.Current' explicitly + + + Use 'JoinableTaskContext.CreateNoOpContext' instead. + \ No newline at end of file diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers/Types.cs b/src/Microsoft.VisualStudio.Threading.Analyzers/Types.cs index 54ec2acdc..8762d3a01 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers/Types.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers/Types.cs @@ -11,6 +11,8 @@ namespace Microsoft.VisualStudio.Threading.Analyzers; /// 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 diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers/VSTHRD115AvoidJoinableTaskContextCtorWithNullArgsAnalyzer.cs b/src/Microsoft.VisualStudio.Threading.Analyzers/VSTHRD115AvoidJoinableTaskContextCtorWithNullArgsAnalyzer.cs new file mode 100644 index 000000000..92360240b --- /dev/null +++ b/src/Microsoft.VisualStudio.Threading.Analyzers/VSTHRD115AvoidJoinableTaskContextCtorWithNullArgsAnalyzer.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; + +/// +/// Flags the use of new JoinableTaskContext(null, null) and advises using JoinableTaskContext.CreateNoOpContext instead. +/// +[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); + + /// + public override ImmutableArray 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 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() + .Add(UsesDefaultThreadPropertyName, usesDefaultThread ? "true" : "false") + .Add(NodeTypePropertyName, nodeType); + } + } + } + } +} diff --git a/src/Microsoft.VisualStudio.Threading/JoinableTaskContext.cs b/src/Microsoft.VisualStudio.Threading/JoinableTaskContext.cs index 3eb58fbcb..332f8945f 100644 --- a/src/Microsoft.VisualStudio.Threading/JoinableTaskContext.cs +++ b/src/Microsoft.VisualStudio.Threading/JoinableTaskContext.cs @@ -153,6 +153,17 @@ public partial class JoinableTaskContext : IDisposable /// will provide the means to switch /// to the main thread from another thread. /// + /// + /// + /// When is at the time this constructor is invoked, + /// requests to switch to the main thread using + /// will not result in any thread switch. + /// This is appropriate for unit test environments where there is no main thread to switch to or processes + /// which otherwise do not define a main thread. + /// Thread safety concern: When configured without a synchronization context, code that requests the main thread + /// as a means of avoiding concurrency may malfunction due to data race conditions. + /// + /// public JoinableTaskContext() : this(Thread.CurrentThread, SynchronizationContext.Current) { @@ -163,10 +174,19 @@ public JoinableTaskContext() /// /// /// The thread to switch to in . - /// If omitted, the current thread will be assumed to be the main thread. + /// If , the current thread will be assumed to be the main thread. /// /// - /// The synchronization context to use to switch to the main thread. + /// The synchronization context to use to switch to the main thread. + /// + /// If is specified (or the argument is omitted), the current synchronization context will be used. + /// If is also , + /// requests to switch to the main thread using will not result in any thread switch. + /// This is appropriate for unit test environments where there is no main thread to switch to or processes + /// which otherwise do not define a main thread. + /// Thread safety concern: When configured without a synchronization context, code that requests the main thread + /// as a means of avoiding concurrency may malfunction due to data race conditions. + /// /// public JoinableTaskContext(Thread? mainThread = null, SynchronizationContext? synchronizationContext = null) { @@ -282,6 +302,41 @@ protected internal virtual SynchronizationContext NoMessagePumpSynchronizationCo } } + /// + /// Initializes a new instance of the class + /// that is configured to no-op on calls to . + /// + /// A new instance of . + /// + /// + /// This method is equivalent to calling the constructor + /// with the property first set to . + /// This entry point however will have the same behavior regardless of the value of . + /// + /// + /// The caller's thread will still be captured for use by such properties as + /// and . + /// These properties generally have no effect except as used by application-specific code beyond this library. + /// + /// + /// This method is useful for creating a in a unit test environment + /// or in a process that does not have a main thread, but which includes code that requires an instance + /// of or . + /// Such code can receive the instance returned by this method and use it in a normal way but no main thread switches will be honored. + /// + /// + /// Thread safety concern: Because main thread switches will not be honored, code that requests the main thread + /// as a means of avoiding concurrency may malfunction due to data race conditions. + /// + /// + public static JoinableTaskContext CreateNoOpContext() + { + using (((SynchronizationContext?)null).Apply()) + { + return new JoinableTaskContext(); + } + } + /// /// Conceals any JoinableTask the caller is associated with until the returned value is disposed. /// diff --git a/src/Microsoft.VisualStudio.Threading/JoinableTaskFactory.cs b/src/Microsoft.VisualStudio.Threading/JoinableTaskFactory.cs index a93f5fe87..018f4c4bc 100644 --- a/src/Microsoft.VisualStudio.Threading/JoinableTaskFactory.cs +++ b/src/Microsoft.VisualStudio.Threading/JoinableTaskFactory.cs @@ -130,7 +130,7 @@ protected SynchronizationContext? UnderlyingSynchronizationContext } /// - /// Gets an awaitable whose continuations execute on the synchronization context that this instance was initialized with, + /// Gets an awaitable whose continuations execute on the main thread, /// in such a way as to mitigate both deadlocks and reentrancy. /// /// @@ -159,6 +159,10 @@ protected SynchronizationContext? UnderlyingSynchronizationContext /// } /// /// + /// + /// When the owning is created with a , + /// this method has no effect and the caller will continue execution on its original thread. + /// /// public MainThreadAwaitable SwitchToMainThreadAsync(CancellationToken cancellationToken = default(CancellationToken)) { diff --git a/src/Microsoft.VisualStudio.Threading/net472/PublicAPI.Unshipped.txt b/src/Microsoft.VisualStudio.Threading/net472/PublicAPI.Unshipped.txt index bb9b5f944..0e00ec898 100644 --- a/src/Microsoft.VisualStudio.Threading/net472/PublicAPI.Unshipped.txt +++ b/src/Microsoft.VisualStudio.Threading/net472/PublicAPI.Unshipped.txt @@ -1 +1,2 @@ -Microsoft.VisualStudio.Threading.AsyncBarrier.SignalAndWait(System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask \ No newline at end of file +Microsoft.VisualStudio.Threading.AsyncBarrier.SignalAndWait(System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask +static Microsoft.VisualStudio.Threading.JoinableTaskContext.CreateNoOpContext() -> Microsoft.VisualStudio.Threading.JoinableTaskContext! \ No newline at end of file diff --git a/src/Microsoft.VisualStudio.Threading/net6.0-windows/PublicAPI.Unshipped.txt b/src/Microsoft.VisualStudio.Threading/net6.0-windows/PublicAPI.Unshipped.txt index bb9b5f944..0e00ec898 100644 --- a/src/Microsoft.VisualStudio.Threading/net6.0-windows/PublicAPI.Unshipped.txt +++ b/src/Microsoft.VisualStudio.Threading/net6.0-windows/PublicAPI.Unshipped.txt @@ -1 +1,2 @@ -Microsoft.VisualStudio.Threading.AsyncBarrier.SignalAndWait(System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask \ No newline at end of file +Microsoft.VisualStudio.Threading.AsyncBarrier.SignalAndWait(System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask +static Microsoft.VisualStudio.Threading.JoinableTaskContext.CreateNoOpContext() -> Microsoft.VisualStudio.Threading.JoinableTaskContext! \ No newline at end of file diff --git a/src/Microsoft.VisualStudio.Threading/net6.0/PublicAPI.Unshipped.txt b/src/Microsoft.VisualStudio.Threading/net6.0/PublicAPI.Unshipped.txt index bb9b5f944..0e00ec898 100644 --- a/src/Microsoft.VisualStudio.Threading/net6.0/PublicAPI.Unshipped.txt +++ b/src/Microsoft.VisualStudio.Threading/net6.0/PublicAPI.Unshipped.txt @@ -1 +1,2 @@ -Microsoft.VisualStudio.Threading.AsyncBarrier.SignalAndWait(System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask \ No newline at end of file +Microsoft.VisualStudio.Threading.AsyncBarrier.SignalAndWait(System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask +static Microsoft.VisualStudio.Threading.JoinableTaskContext.CreateNoOpContext() -> Microsoft.VisualStudio.Threading.JoinableTaskContext! \ No newline at end of file diff --git a/src/Microsoft.VisualStudio.Threading/netstandard2.0/PublicAPI.Unshipped.txt b/src/Microsoft.VisualStudio.Threading/netstandard2.0/PublicAPI.Unshipped.txt index bb9b5f944..0e00ec898 100644 --- a/src/Microsoft.VisualStudio.Threading/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/Microsoft.VisualStudio.Threading/netstandard2.0/PublicAPI.Unshipped.txt @@ -1 +1,2 @@ -Microsoft.VisualStudio.Threading.AsyncBarrier.SignalAndWait(System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask \ No newline at end of file +Microsoft.VisualStudio.Threading.AsyncBarrier.SignalAndWait(System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask +static Microsoft.VisualStudio.Threading.JoinableTaskContext.CreateNoOpContext() -> Microsoft.VisualStudio.Threading.JoinableTaskContext! \ No newline at end of file diff --git a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD115AvoidJoinableTaskContextCtorWithNullArgTests.cs b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD115AvoidJoinableTaskContextCtorWithNullArgTests.cs new file mode 100644 index 000000000..8603eb8c9 --- /dev/null +++ b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD115AvoidJoinableTaskContextCtorWithNullArgTests.cs @@ -0,0 +1,147 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using CSVerify = Microsoft.VisualStudio.Threading.Analyzers.Tests.CSharpCodeFixVerifier; +using VBVerify = Microsoft.VisualStudio.Threading.Analyzers.Tests.VisualBasicCodeFixVerifier; + +public class VSTHRD115AvoidJoinableTaskContextCtorWithNullArgTests +{ + private const string CSPreamble = """ + using System.Threading; + using Microsoft.VisualStudio.Threading; + + """; + + private const string VBPreamble = """ + Imports System.Threading + Imports Microsoft.VisualStudio.Threading + + """; + + [Fact] + public async Task ConstructorWithImplicitNullSyncContext_SuppressWarning_CS() + { + var test = CSPreamble + """ + class Test + { + void Create1() => [|new JoinableTaskContext(null)|]; + void Create2() => [|new JoinableTaskContext(Thread.CurrentThread)|]; + } + """; + + var withFix = CSPreamble + """ + class Test + { + void Create1() => new JoinableTaskContext(null, SynchronizationContext.Current); + void Create2() => new JoinableTaskContext(Thread.CurrentThread, SynchronizationContext.Current); + } + """; + + await new CSVerify.Test + { + TestCode = test, + FixedCode = withFix, + CodeActionEquivalenceKey = VSTHRD115AvoidJoinableTaskContextCtorWithNullArgsCodeFix.SuppressWarningEquivalenceKey, + }.RunAsync(); + } + + [Fact] + public async Task ConstructorWithExplicitNullSyncContext_SuppressWarning() + { + var test = CSPreamble + """ + class Test + { + void Create1() => new JoinableTaskContext(null, [|null|]); + void Create2() => new JoinableTaskContext(Thread.CurrentThread, [|null|]); + } + """; + + var withFix = CSPreamble + """ + class Test + { + void Create1() => new JoinableTaskContext(null, SynchronizationContext.Current); + void Create2() => new JoinableTaskContext(Thread.CurrentThread, SynchronizationContext.Current); + } + """; + + await new CSVerify.Test + { + TestCode = test, + FixedCode = withFix, + CodeActionEquivalenceKey = VSTHRD115AvoidJoinableTaskContextCtorWithNullArgsCodeFix.SuppressWarningEquivalenceKey, + }.RunAsync(); + } + + [Fact] + public async Task ConstructorWithNonDefaultThread_SuppressAction() + { + var test = CSPreamble + """ + class Test + { + void Create2(Thread thread) => [|new JoinableTaskContext(thread)|]; + void Create1(Thread thread) => new JoinableTaskContext(thread, [|null|]); + } + """; + + var withFix = CSPreamble + """ + class Test + { + void Create2(Thread thread) => new JoinableTaskContext(thread, SynchronizationContext.Current); + void Create1(Thread thread) => new JoinableTaskContext(thread, SynchronizationContext.Current); + } + """; + + await new CSVerify.Test + { + TestCode = test, + FixedCode = withFix, + CodeActionEquivalenceKey = VSTHRD115AvoidJoinableTaskContextCtorWithNullArgsCodeFix.SuppressWarningEquivalenceKey, + }.RunAsync(); + } + + [Fact] + public async Task ConstructorWithNonDefaultThread_GetsNoFactoryAction() + { + var test = CSPreamble + """ + class Test + { + void Create2(Thread thread) => [|new JoinableTaskContext(thread)|]; + void Create1(Thread thread) => new JoinableTaskContext(thread, [|null|]); + } + """; + + await new CSVerify.Test + { + TestCode = test, + FixedCode = test, // no fix offered + CodeActionEquivalenceKey = VSTHRD115AvoidJoinableTaskContextCtorWithNullArgsCodeFix.UseFactoryMethodEquivalenceKey, + }.RunAsync(); + } + + [Fact] + public async Task ConstructorWithNullSyncContext_UseFactoryMethod() + { + var test = CSPreamble + """ + class Test + { + void Create1() => [|new JoinableTaskContext(null)|]; + void Create2() => new JoinableTaskContext(null, [|null|]); + } + """; + + var withFix = CSPreamble + """ + class Test + { + void Create1() => JoinableTaskContext.CreateNoOpContext(); + void Create2() => JoinableTaskContext.CreateNoOpContext(); + } + """; + + await new CSVerify.Test + { + TestCode = test, + FixedCode = withFix, + CodeActionEquivalenceKey = VSTHRD115AvoidJoinableTaskContextCtorWithNullArgsCodeFix.UseFactoryMethodEquivalenceKey, + }.RunAsync(); + } +} diff --git a/test/Microsoft.VisualStudio.Threading.Tests/JoinableTaskContextTests.cs b/test/Microsoft.VisualStudio.Threading.Tests/JoinableTaskContextTests.cs index e50d64078..93dfffa59 100644 --- a/test/Microsoft.VisualStudio.Threading.Tests/JoinableTaskContextTests.cs +++ b/test/Microsoft.VisualStudio.Threading.Tests/JoinableTaskContextTests.cs @@ -7,6 +7,7 @@ using System.Threading; using System.Threading.Tasks; using System.Xml.Linq; +using Microsoft; using Microsoft.VisualStudio.Threading; using Xunit; using Xunit.Abstractions; @@ -718,6 +719,58 @@ public void Disposable() disposable.Dispose(); } + [Fact] + public void Ctor_ExplicitNullSyncContext() + { + this.SimulateUIThread(async delegate + { + Thread mainThread = Thread.CurrentThread; + Assumes.NotNull(SynchronizationContext.Current); + JoinableTaskContext jtc = JoinableTaskContext.CreateNoOpContext(); + await TaskScheduler.Default.SwitchTo(alwaysYield: true); // Get off the main thread. + Assert.NotSame(mainThread, Thread.CurrentThread); + + // Verify that switching to the main thread is a no-op. + Thread threadpoolThread = Thread.CurrentThread; + await jtc.Factory.SwitchToMainThreadAsync(this.TimeoutToken); + Assert.Same(threadpoolThread, Thread.CurrentThread); + }); + } + + [Fact] + public void Ctor_NullSyncContextArg_AmbientSyncContext() + { + this.SimulateUIThread(async delegate + { + Thread mainThread = Thread.CurrentThread; + Assumes.NotNull(SynchronizationContext.Current); + JoinableTaskContext jtc = new(null, null); + await TaskScheduler.Default.SwitchTo(alwaysYield: true); // Get off the main thread. + Assert.NotSame(mainThread, Thread.CurrentThread); + + // Verify that switching to the main thread works. + await jtc.Factory.SwitchToMainThreadAsync(this.TimeoutToken); + Assert.Same(mainThread, Thread.CurrentThread); + }); + } + + [Fact] + public void Ctor_Default() + { + this.SimulateUIThread(async delegate + { + Thread mainThread = Thread.CurrentThread; + Assumes.NotNull(SynchronizationContext.Current); + JoinableTaskContext jtc = new(); + await TaskScheduler.Default.SwitchTo(alwaysYield: true); // Get off the main thread. + Assert.NotSame(mainThread, Thread.CurrentThread); + + // Verify that switching to the main thread works. + await jtc.Factory.SwitchToMainThreadAsync(this.TimeoutToken); + Assert.Same(mainThread, Thread.CurrentThread); + }); + } + protected override JoinableTaskContext CreateJoinableTaskContext() { return new JoinableTaskContextDerived();