Address review comments

doc/analyzers/VSTHRD112.md

@@ -0,0 +1,31 @@
+# VSTHRD112 Avoid returning a null Task
+
+Returning `null` from a non-async `Task`/`Task<T>` method will cause a `NullReferenceException` at runtime. This problem can be avoided by returning `Task.CompletedTask`, `Task.FromResult<T>(null)` or `Task.FromResult(default(T))` instead.
+
+## Examples of patterns that are flagged by this analyzer
+
+Any non-async `Task` returning method with an explicit `return null;` will be flagged.
+
+```csharp
+Task DoAsync() {
+    return null;
+}
+
+Task<object> GetSomethingAsync() {
+    return null;
+}
+```
+
+## Solution
+
+Return a task like `Task.CompletedTask` or `Task.FromResult`.
+
+```csharp
+Task DoAsync() {
+    return Task.CompletedTask;
+}
+
+Task<object> GetSomethingAsync() {
+    return Task.FromResult<object>(null);
+}
+```

doc/analyzers/index.md

@@ -24,6 +24,7 @@ ID | Title | Severity | Supports | Default diagnostic severity
 [VSTHRD109](VSTHRD109.md) | Switch instead of assert in async methods | Advisory | [1st rule](../threading_rules.md#Rule1) | Error
 [VSTHRD110](VSTHRD110.md) | Observe result of async calls | Advisory | | Warning
 [VSTHRD111](VSTHRD111.md) | Use `.ConfigureAwait(bool)` | Advisory | | Hidden
+[VSTHRD112](VSTHRD112.md) | Avoid returning null from a `Task`-returning method. | Advisory | | Warning
 [VSTHRD200](VSTHRD200.md) | Use `Async` naming convention | Guideline | [VSTHRD103](VSTHRD103.md) | Warning
 
 ## Severity descriptions

src/Microsoft.VisualStudio.Threading.Analyzers.Tests/MultiAnalyzerTests.cs

@@ -6,10 +6,8 @@
     using System.Linq;
     using System.Threading.Tasks;
     using Microsoft.CodeAnalysis;
-    using Microsoft.CodeAnalysis.CSharp.Testing;
     using Microsoft.CodeAnalysis.Diagnostics;
     using Microsoft.CodeAnalysis.Testing;
-    using Microsoft.CodeAnalysis.Testing.Verifiers;
     using Xunit;
     using Verify = MultiAnalyzerTests.Verifier;
 
@@ -32,7 +30,7 @@ class Test {
         return Task.FromResult(1);
     }
 
-    Task BarAsync() => null; // VSTHRD112
+    Task BarAsync() => Task.CompletedTask;
 
     static void SetTaskSourceIfCompleted<T>(Task<T> task, TaskCompletionSource<T> tcs) {
         if (task.IsCompleted) {
@@ -45,7 +43,6 @@ class Test {
             {
                 Verify.Diagnostic(VSTHRD103UseAsyncOptionAnalyzer.DescriptorNoAlternativeMethod).WithSpan(10, 24, 10, 33).WithArguments("GetResult"),
                 Verify.Diagnostic(VSTHRD103UseAsyncOptionAnalyzer.Descriptor).WithSpan(11, 13, 11, 16).WithArguments("Run", "RunAsync"),
-                Verify.Diagnostic(VSTHRD112AvoidNullReturnInNonAsyncTaskMethodAnalyzer.Descriptor).WithSpan(15, 24, 15, 28),
                 Verify.Diagnostic(VSTHRD002UseJtfRunAnalyzer.Descriptor).WithSpan(19, 32, 19, 38),
             };
 
@@ -172,7 +169,7 @@ public class A {
         E().ToString();
         E()();
         string v = nameof(E);
-        {|VSTHRD112:return null;|}
+        return Task.CompletedTask;
     }
 
     internal Task CAsync() {
@@ -182,7 +179,7 @@ public class A {
         E().ToString();
         E()();
         string v = nameof(E);
-        {|VSTHRD112:return null;|}
+        return Task.CompletedTask;
     }
 
     private void D<T>() { }

src/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD112AvoidNullReturnInNonAsyncTaskMethodAnalyzerTests.cs

@@ -23,7 +23,7 @@ class Test
             await new Verify.Test
             {
                 TestCode = test,
-                ExpectedDiagnostics = { Verify.Diagnostic().WithSpan(8, 9, 8, 21), },
+                ExpectedDiagnostics = { Verify.Diagnostic().WithSpan(8, 16, 8, 20), },
             }.RunAsync();
         }
 
@@ -37,14 +37,13 @@ class Test
 {
     public Task GetTask()
     {
-        return null;
+        return [|null|];
     }
 }
 ";
             await new Verify.Test
             {
                 TestCode = test,
-                ExpectedDiagnostics = { Verify.Diagnostic().WithSpan(8, 9, 8, 21), },
             }.RunAsync();
         }
 
@@ -56,13 +55,12 @@ using System.Threading.Tasks;
 
 class Test
 {
-    public Task GetTask() => null;
+    public Task GetTask() => [|null|];
 }
 ";
             await new Verify.Test
             {
                 TestCode = test,
-                ExpectedDiagnostics = { Verify.Diagnostic().WithSpan(6, 30, 6, 34), },
             }.RunAsync();
         }
 
@@ -139,10 +137,10 @@ class Test
     {
         if (string.IsNullOrEmpty(s))
         {
-            {|VSTHRD112:return null;|}
+            return [|null|];
         }
 
-        {|VSTHRD112:return null;|}
+        return [|null|];
     }
 }
 ";

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

@@ -496,7 +496,7 @@ namespace Microsoft.VisualStudio.Threading.Analyzers {
         }
         
         /// <summary>
-        ///   Looks up a localized string similar to Avoid returning null from a non-async awaitable method.
+        ///   Looks up a localized string similar to Avoid returning null from a Task-returning method..
         /// </summary>
         internal static string VSTHRD112_MessageFormat {
             get {
@@ -505,7 +505,7 @@ namespace Microsoft.VisualStudio.Threading.Analyzers {
         }
         
         /// <summary>
-        ///   Looks up a localized string similar to Avoid returning null from a non-async awaitable method.
+        ///   Looks up a localized string similar to Avoid returning a null Task.
         /// </summary>
         internal static string VSTHRD112_Title {
             get {

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

@@ -310,9 +310,9 @@ Use AsyncLazy&lt;T&gt; instead.</value>
     <comment>"ThrowIfCancellationRequested" is a method name and should not be translated.</comment>
   </data>
   <data name="VSTHRD112_MessageFormat" xml:space="preserve">
-    <value>Avoid returning null from a non-async awaitable method</value>
+    <value>Avoid returning null from a Task-returning method.</value>
   </data>
   <data name="VSTHRD112_Title" xml:space="preserve">
-    <value>Avoid returning null from a non-async awaitable method</value>
+    <value>Avoid returning a null Task</value>
   </data>
 </root>

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

@@ -116,15 +116,20 @@ namespace Microsoft.VisualStudio.Threading.Analyzers
                 catch (Exception ex) when (LaunchDebuggerExceptionFilter())
                 {
                     var messageBuilder = new StringBuilder();
-                    messageBuilder.AppendLine("Analyzer failure while processing syntax at");
+                    messageBuilder.Append("Analyzer failure while processing syntax(es) at ");
 
-                    foreach (var operation in ctxt.OperationBlocks)
+                    for (int i = 0; i < ctxt.OperationBlocks.Length; i++)
                     {
+                        var operation = ctxt.OperationBlocks[i];
                         var lineSpan = operation.Syntax.GetLocation()?.GetLineSpan();
-                        messageBuilder.AppendLine($"- {operation.Syntax.SyntaxTree.FilePath}({lineSpan?.StartLinePosition.Line + 1},{lineSpan?.StartLinePosition.Character + 1}). Syntax: {operation.Syntax}");
+                        if (i > 0)
+                        {
+                            messageBuilder.Append(", ");
+                        }
+                        messageBuilder.Append($"{operation.Syntax.SyntaxTree.FilePath}({lineSpan?.StartLinePosition.Line + 1},{lineSpan?.StartLinePosition.Character + 1}). Syntax: {operation.Syntax}.");
                     }
 
-                    messageBuilder.AppendLine($"{ex.GetType()} {ex.Message}");
+                    messageBuilder.Append($". {ex.GetType()} {ex.Message}");
 
                     throw new Exception(messageBuilder.ToString(), ex);
                 }

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

@@ -10,7 +10,7 @@
     /// Finds await expressions on <see cref="Task"/> that do not use <see cref="Task.ConfigureAwait(bool)"/>.
     /// Also works on <see cref="ValueTask"/>.
     /// </summary>
-    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    [DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
     public class VSTHRD112AvoidNullReturnInNonAsyncTaskMethodAnalyzer : DiagnosticAnalyzer
     {
         public const string Id = "VSTHRD112";
@@ -54,7 +54,7 @@
                 returnOperation.ReturnedValue.ConstantValue.HasValue &&
                 returnOperation.ReturnedValue.ConstantValue.Value == null)
             {
-                context.ReportDiagnostic(Diagnostic.Create(Descriptor, returnOperation.Syntax.GetLocation()));
+                context.ReportDiagnostic(Diagnostic.Create(Descriptor, returnOperation.ReturnedValue.Syntax.GetLocation()));
             }
         }
     }

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

@@ -250,13 +250,13 @@ Použijte místo toho AsyncLazy&lt;T&gt;.</target>
         <note from="MultilingualBuild" annotates="source" priority="2">"ConfigureAwait(bool)" is a reference and should NOT be translated.</note>
       </trans-unit>
       <trans-unit id="VSTHRD112_MessageFormat">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning null from a Task-returning method.</source>
+        <target state="new">Avoid returning null from a Task-returning method.</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD112_Title">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning a null Task</source>
+        <target state="new">Avoid returning a null Task</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD200_AddAsync_MessageFormat" translate="yes" xml:space="preserve">

src/Microsoft.VisualStudio.Threading.Analyzers/xlf/Strings.de.xlf

@@ -250,13 +250,13 @@ Verwenden Sie stattdessen "AsyncLazy&lt;T&gt;".</target>
         <note from="MultilingualBuild" annotates="source" priority="2">"ConfigureAwait(bool)" is a reference and should NOT be translated.</note>
       </trans-unit>
       <trans-unit id="VSTHRD112_MessageFormat">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning null from a Task-returning method.</source>
+        <target state="new">Avoid returning null from a Task-returning method.</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD112_Title">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning a null Task</source>
+        <target state="new">Avoid returning a null Task</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD200_AddAsync_MessageFormat" translate="yes" xml:space="preserve">

src/Microsoft.VisualStudio.Threading.Analyzers/xlf/Strings.es.xlf

@@ -250,13 +250,13 @@ Use AsyncLazy&lt;T&gt; en su lugar.</target>
         <note from="MultilingualBuild" annotates="source" priority="2">"ConfigureAwait(bool)" is a reference and should NOT be translated.</note>
       </trans-unit>
       <trans-unit id="VSTHRD112_MessageFormat">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning null from a Task-returning method.</source>
+        <target state="new">Avoid returning null from a Task-returning method.</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD112_Title">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning a null Task</source>
+        <target state="new">Avoid returning a null Task</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD200_AddAsync_MessageFormat" translate="yes" xml:space="preserve">

src/Microsoft.VisualStudio.Threading.Analyzers/xlf/Strings.fr.xlf

@@ -250,13 +250,13 @@ Utilisez AsyncLazy&lt;T&gt; à la place.</target>
         <note from="MultilingualBuild" annotates="source" priority="2">"ConfigureAwait(bool)" is a reference and should NOT be translated.</note>
       </trans-unit>
       <trans-unit id="VSTHRD112_MessageFormat">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning null from a Task-returning method.</source>
+        <target state="new">Avoid returning null from a Task-returning method.</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD112_Title">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning a null Task</source>
+        <target state="new">Avoid returning a null Task</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD200_AddAsync_MessageFormat" translate="yes" xml:space="preserve">

src/Microsoft.VisualStudio.Threading.Analyzers/xlf/Strings.it.xlf

@@ -250,13 +250,13 @@ In alternativa, usare AsyncLazy&lt;T&gt;.</target>
         <note from="MultilingualBuild" annotates="source" priority="2">"ConfigureAwait(bool)" is a reference and should NOT be translated.</note>
       </trans-unit>
       <trans-unit id="VSTHRD112_MessageFormat">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning null from a Task-returning method.</source>
+        <target state="new">Avoid returning null from a Task-returning method.</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD112_Title">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning a null Task</source>
+        <target state="new">Avoid returning a null Task</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD200_AddAsync_MessageFormat" translate="yes" xml:space="preserve">

src/Microsoft.VisualStudio.Threading.Analyzers/xlf/Strings.ja.xlf

@@ -250,13 +250,13 @@ Use AsyncLazy&lt;T&gt; instead.</source>
         <note from="MultilingualBuild" annotates="source" priority="2">"ConfigureAwait(bool)" is a reference and should NOT be translated.</note>
       </trans-unit>
       <trans-unit id="VSTHRD112_MessageFormat">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning null from a Task-returning method.</source>
+        <target state="new">Avoid returning null from a Task-returning method.</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD112_Title">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning a null Task</source>
+        <target state="new">Avoid returning a null Task</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD200_AddAsync_MessageFormat" translate="yes" xml:space="preserve">

src/Microsoft.VisualStudio.Threading.Analyzers/xlf/Strings.ko.xlf

@@ -250,13 +250,13 @@ Use AsyncLazy&lt;T&gt; instead.</source>
         <note from="MultilingualBuild" annotates="source" priority="2">"ConfigureAwait(bool)" is a reference and should NOT be translated.</note>
       </trans-unit>
       <trans-unit id="VSTHRD112_MessageFormat">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning null from a Task-returning method.</source>
+        <target state="new">Avoid returning null from a Task-returning method.</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD112_Title">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning a null Task</source>
+        <target state="new">Avoid returning a null Task</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD200_AddAsync_MessageFormat" translate="yes" xml:space="preserve">

src/Microsoft.VisualStudio.Threading.Analyzers/xlf/Strings.pl.xlf

@@ -250,13 +250,13 @@ Zamiast tego użyj elementu AsyncLazy&lt;T&gt;.</target>
         <note from="MultilingualBuild" annotates="source" priority="2">"ConfigureAwait(bool)" is a reference and should NOT be translated.</note>
       </trans-unit>
       <trans-unit id="VSTHRD112_MessageFormat">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning null from a Task-returning method.</source>
+        <target state="new">Avoid returning null from a Task-returning method.</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD112_Title">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning a null Task</source>
+        <target state="new">Avoid returning a null Task</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD200_AddAsync_MessageFormat" translate="yes" xml:space="preserve">

src/Microsoft.VisualStudio.Threading.Analyzers/xlf/Strings.pt-BR.xlf

@@ -250,13 +250,13 @@ Nesse caso, use AsyncLazy&lt;T&gt;.</target>
         <note from="MultilingualBuild" annotates="source" priority="2">"ConfigureAwait(bool)" is a reference and should NOT be translated.</note>
       </trans-unit>
       <trans-unit id="VSTHRD112_MessageFormat">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning null from a Task-returning method.</source>
+        <target state="new">Avoid returning null from a Task-returning method.</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD112_Title">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning a null Task</source>
+        <target state="new">Avoid returning a null Task</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD200_AddAsync_MessageFormat" translate="yes" xml:space="preserve">

src/Microsoft.VisualStudio.Threading.Analyzers/xlf/Strings.ru.xlf

@@ -250,13 +250,13 @@ Use AsyncLazy&lt;T&gt; instead.</source>
         <note from="MultilingualBuild" annotates="source" priority="2">"ConfigureAwait(bool)" is a reference and should NOT be translated.</note>
       </trans-unit>
       <trans-unit id="VSTHRD112_MessageFormat">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning null from a Task-returning method.</source>
+        <target state="new">Avoid returning null from a Task-returning method.</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD112_Title">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning a null Task</source>
+        <target state="new">Avoid returning a null Task</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD200_AddAsync_MessageFormat" translate="yes" xml:space="preserve">

src/Microsoft.VisualStudio.Threading.Analyzers/xlf/Strings.tr.xlf

@@ -250,13 +250,13 @@ Bunun yerine AsyncLazy&lt;T&gt; kullanın.</target>
         <note from="MultilingualBuild" annotates="source" priority="2">"ConfigureAwait(bool)" is a reference and should NOT be translated.</note>
       </trans-unit>
       <trans-unit id="VSTHRD112_MessageFormat">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning null from a Task-returning method.</source>
+        <target state="new">Avoid returning null from a Task-returning method.</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD112_Title">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning a null Task</source>
+        <target state="new">Avoid returning a null Task</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD200_AddAsync_MessageFormat" translate="yes" xml:space="preserve">

src/Microsoft.VisualStudio.Threading.Analyzers/xlf/Strings.zh-Hans.xlf

@@ -250,13 +250,13 @@ Use AsyncLazy&lt;T&gt; instead.</source>
         <note from="MultilingualBuild" annotates="source" priority="2">"ConfigureAwait(bool)" is a reference and should NOT be translated.</note>
       </trans-unit>
       <trans-unit id="VSTHRD112_MessageFormat">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning null from a Task-returning method.</source>
+        <target state="new">Avoid returning null from a Task-returning method.</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD112_Title">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning a null Task</source>
+        <target state="new">Avoid returning a null Task</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD200_AddAsync_MessageFormat" translate="yes" xml:space="preserve">

src/Microsoft.VisualStudio.Threading.Analyzers/xlf/Strings.zh-Hant.xlf

@@ -250,13 +250,13 @@ Use AsyncLazy&lt;T&gt; instead.</source>
         <note from="MultilingualBuild" annotates="source" priority="2">"ConfigureAwait(bool)" is a reference and should NOT be translated.</note>
       </trans-unit>
       <trans-unit id="VSTHRD112_MessageFormat">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning null from a Task-returning method.</source>
+        <target state="new">Avoid returning null from a Task-returning method.</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD112_Title">
-        <source>Avoid returning null from a non-async awaitable method</source>
-        <target state="new">Avoid returning null from a non-async awaitable method</target>
+        <source>Avoid returning a null Task</source>
+        <target state="new">Avoid returning a null Task</target>
         <note />
       </trans-unit>
       <trans-unit id="VSTHRD200_AddAsync_MessageFormat" translate="yes" xml:space="preserve">