From 715ba29b09de4dd99d7ad357913d9e0e152d0da0 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Sat, 22 Sep 2018 17:57:54 -0700 Subject: [PATCH] Add VSTHRD002 check for ValueTask sync blocking methods --- doc/analyzers/VSTHRD002.md | 2 +- .../VSTHRD002UseJtfRunAnalyzerTests.cs | 62 ++++++++++++++++++- .../CommonInterest.cs | 2 + 3 files changed, 63 insertions(+), 3 deletions(-) diff --git a/doc/analyzers/VSTHRD002.md b/doc/analyzers/VSTHRD002.md index 40659f61..676a919b 100644 --- a/doc/analyzers/VSTHRD002.md +++ b/doc/analyzers/VSTHRD002.md @@ -1,6 +1,6 @@ # VSTHRD002 Avoid problematic synchronous waits -Synchronously waiting on `Task` objects or awaiters is dangerous and may cause dead locks. +Synchronously waiting on `Task`, `ValueTask`, or awaiters is dangerous and may cause dead locks. ## Examples of patterns that are flagged by this analyzer diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD002UseJtfRunAnalyzerTests.cs b/src/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD002UseJtfRunAnalyzerTests.cs index 373646cb..ecd54294 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD002UseJtfRunAnalyzerTests.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD002UseJtfRunAnalyzerTests.cs @@ -61,7 +61,7 @@ class Test { } [Fact] - public async Task TaskResultShouldReportWarning() + public async Task Task_Result_ShouldReportWarning() { var test = @" using System; @@ -89,6 +89,35 @@ class Test { await Verify.VerifyCodeFixAsync(test, expected, withFix); } + [Fact] + public async Task ValueTask_Result_ShouldReportWarning() + { + var test = @" +using System; +using System.Threading.Tasks; + +class Test { + void F() { + ValueTask task = default; + var result = task.Result; + } +} +"; + var withFix = @" +using System; +using System.Threading.Tasks; + +class Test { + async Task FAsync() { + ValueTask task = default; + var result = await task; + } +} +"; + var expected = Verify.Diagnostic().WithSpan(8, 27, 8, 33); + await Verify.VerifyCodeFixAsync(test, expected, withFix); + } + [Fact] public async Task TaskResultShouldReportWarning_WithinAnonymousDelegate() { @@ -142,7 +171,7 @@ class Test { } [Fact] - public async Task AwaiterGetResultShouldReportWarning() + public async Task Task_GetAwaiter_GetResult_ShouldReportWarning() { var test = @" using System; @@ -170,6 +199,35 @@ class Test { await Verify.VerifyCodeFixAsync(test, expected, withFix); } + [Fact] + public async Task ValueTask_GetAwaiter_GetResult_ShouldReportWarning() + { + var test = @" +using System; +using System.Threading.Tasks; + +class Test { + void F() { + ValueTask task = default; + task.GetAwaiter().GetResult(); + } +} +"; + var withFix = @" +using System; +using System.Threading.Tasks; + +class Test { + async Task FAsync() { + ValueTask task = default; + await task; + } +} +"; + var expected = Verify.Diagnostic().WithSpan(8, 27, 8, 36); + await Verify.VerifyCodeFixAsync(test, expected, withFix); + } + [Fact] public async Task TaskResult_FixUpdatesCallers() { diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs b/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs index 5d56cf8d..99f255fc 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs @@ -32,6 +32,7 @@ namespace Microsoft.VisualStudio.Threading.Analyzers { new SyncBlockingMethod(new QualifiedMember(new QualifiedType(Namespaces.SystemThreadingTasks, nameof(Task)), nameof(Task.Wait)), null), new SyncBlockingMethod(new QualifiedMember(new QualifiedType(Namespaces.SystemRuntimeCompilerServices, nameof(TaskAwaiter)), nameof(TaskAwaiter.GetResult)), null), + new SyncBlockingMethod(new QualifiedMember(new QualifiedType(Namespaces.SystemRuntimeCompilerServices, nameof(ValueTaskAwaiter)), nameof(ValueTaskAwaiter.GetResult)), null), }; internal static readonly IEnumerable SyncBlockingMethods = JTFSyncBlockers.Concat(ProblematicSyncBlockingMethods).Concat(new[] @@ -55,6 +56,7 @@ namespace Microsoft.VisualStudio.Threading.Analyzers internal static readonly IReadOnlyList SyncBlockingProperties = new[] { new SyncBlockingMethod(new QualifiedMember(new QualifiedType(Namespaces.SystemThreadingTasks, nameof(Task)), nameof(Task.Result)), null), + new SyncBlockingMethod(new QualifiedMember(new QualifiedType(Namespaces.SystemThreadingTasks, nameof(ValueTask)), nameof(ValueTask.Result)), null), }; internal static readonly IEnumerable ThreadAffinityTestingMethods = new[]