PH_S035: Blocking Async Method Invocation in Constructor (#102)

* Implemented analyzer for blocking async access in constructors

* Updated app version to v3.7.0

* Added PH_S035 to the release notes

* Add documentation for PH_S035

* Fixed the analysis title

Author: camrein

doc/analyzers/PH_S035.md

@@ -0,0 +1,48 @@
+# PH_S034 - Blocking Async Method Invocation in Constructor
+
+## Problem
+
+Constructors cannot be asynchronous. Therefore, developers tend to call asynchronous APIs from the constructor blockingly, either using `Wait()` or `Result`. 
+
+```cs
+class FileResource {
+  public string Content { get; }
+
+  public FileResource(string filePath) {
+    Content = File.ReadAllTextAsync(filePath).Result;
+  }
+}
+```
+
+Invoking async methods to await their completion blockingly is generally discouraged, as stated by [PH_P005](PH_P005.md).
+
+
+## Solution
+
+Refactor the constructor to an `async` factory method that the caller can await. Alternatively, the invocation of the blocking counterpart might be an acceptable solution here.
+
+```cs
+// Solution #1 - Refactor to factory method (recommended)
+class FileResource {
+  public string Content { get; }
+
+  private FileResource(string content) {
+    Content = content;
+  }
+
+  public static async Task<FileResource> CreateAsync(string filePath) {
+    return new FileResource(
+      await File.ReadAllTextAsync(filePath)
+    );
+  }
+}
+
+// Solution #2 - Call the synchronous API
+class FileResource {
+  public string Content { get; }
+
+  public FileResource(string filePath) {
+    Content = File.ReadAllText(filePath);
+  }
+}
+```

src/ParallelHelper.Plugin/ReleaseNotes.txt

@@ -1,4 +1,11 @@
-v3.6.0
+v3.7.0
+
+- New Analyzer: PH_S035 - Blocking Async Method Invocation in Constructor
+
+
+------------------
+
+v3.6.0
 
 - New Analyzer: PH_S034 - Async Lambda Inferred to Async Void
 

src/ParallelHelper.Plugin/source.extension.vsixmanifest

@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="utf-8"?>
 <PackageManifest Version="2.0.0" xmlns="http://schemas.microsoft.com/developer/vsx-schema/2011" xmlns:d="http://schemas.microsoft.com/developer/vsx-schema-design/2011">
     <Metadata>
-        <Identity Id="ParallelHelper.f833642b-cd24-49ac-addc-e5328ce8fe88" Version="3.6.0" Language="en-US" Publisher="Christoph Amrein" />
+        <Identity Id="ParallelHelper.f833642b-cd24-49ac-addc-e5328ce8fe88" Version="3.7.0" Language="en-US" Publisher="Christoph Amrein" />
         <DisplayName>ParallelHelper</DisplayName>
         <Description xml:space="preserve">ParallelHelper is a static code analyzer that helps to identify concurrency related issues. Moreover, it provides hints to improve the robustness of code with concurrency in mind.</Description>
         <MoreInfo>https://github.com/Concurrency-Lab/ParallelHelper</MoreInfo>

src/ParallelHelper.Test/Analyzer/Smells/BlockingAsyncMethodInvocationInConstructorAnalyzerTest.cs

@@ -0,0 +1,70 @@
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using ParallelHelper.Analyzer.Smells;
+
+namespace ParallelHelper.Test.Analyzer.Smells {
+  [TestClass]
+  public class BlockingAsyncMethodInvocationInConstructorAnalyzerTest : AnalyzerTestBase<BlockingAsyncMethodInvocationInConstructorAnalyzer> {
+    [TestMethod]
+    public void ReportsTaskRunWaitInvocationInConstructor() {
+      const string source = @"
+using System.Threading.Tasks;
+
+class Test {
+  public Test() {
+    Task.Run(() => {}).Wait();
+  }
+}";
+      VerifyDiagnostic(source, new DiagnosticResultLocation(5, 5));
+    }
+
+    [TestMethod]
+    public void ReportsWebClientDownloadResultAccessInConstructor() {
+      const string source = @"
+using System;
+using System.Net;
+
+class Test {
+  private readonly string content;
+
+  public Test(Uri address) {
+    using(var client = new WebClient()) {
+      content = client.DownloadStringTaskAsync(address).Result;
+    }
+  }
+}";
+      VerifyDiagnostic(source, new DiagnosticResultLocation(9, 17));
+    }
+
+    [TestMethod]
+    public void DoesNotReportTaskRunWaitInvocationInDifferentActivationFrameInConstructor() {
+      const string source = @"
+using System;
+using System.Threading.Tasks;
+
+class Test {
+  public Test() {
+    Action action = () => Task.Run(() => {}).Wait();
+  }
+}";
+      VerifyDiagnostic(source);
+    }
+
+    [TestMethod]
+    public void DoesNotReportWebClientDownloadResultAccessInMethod() {
+      const string source = @"
+using System;
+using System.Net;
+
+class Test {
+  private string content;
+
+  public void Refresh(Uri address) {
+    using(var client = new WebClient()) {
+      content = client.DownloadStringTaskAsync(address).Result;
+    }
+  }
+}";
+      VerifyDiagnostic(source);
+    }
+  }
+}

src/ParallelHelper/Analyzer/Smells/BlockingAsyncMethodInvocationInConstructorAnalyzer.cs

@@ -0,0 +1,82 @@
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+using ParallelHelper.Extensions;
+using ParallelHelper.Util;
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.Linq;
+
+namespace ParallelHelper.Analyzer.Smells {
+  /// <summary>
+  /// Analyzer that analyzes sources for blocking invocations of async methods in constructors.
+  /// 
+  /// <example>Illustrates a class with a method that invokes an async method within a constructor and blocks for the result.
+  /// <code>
+  /// class Sample {
+  ///   private readonly string content;
+  /// 
+  ///   public Sample(string filePath) {
+  ///     content = File.ReadAllTextAsync(filePath).Result;
+  ///   }
+  /// }
+  /// </code>
+  /// </example>
+  /// </summary>
+  [DiagnosticAnalyzer(LanguageNames.CSharp)]
+  public class BlockingAsyncMethodInvocationInConstructorAnalyzer : DiagnosticAnalyzer {
+    public const string DiagnosticId = "PH_S035";
+
+    private const string Category = "Concurrency";
+
+    private static readonly LocalizableString Title = "Blocking Async Method Invocation in Constructor";
+    private static readonly LocalizableString MessageFormat = "The blocking invocation of async methods within the constructor is discouraged since they cannot be awaited. Use a factory method instead.";
+    private static readonly LocalizableString Description = "";
+
+    private static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor(
+      DiagnosticId, Title, MessageFormat, Category, DiagnosticSeverity.Warning,
+      isEnabledByDefault: true, description: Description, helpLinkUri: HelpLinkFactory.CreateUri(DiagnosticId)
+    );
+
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
+
+    public override void Initialize(AnalysisContext context) {
+      context.EnableConcurrentExecution();
+      context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+      context.RegisterSyntaxNodeAction(AnalyzeConstructor, SyntaxKind.ConstructorDeclaration);
+    }
+
+    private static void AnalyzeConstructor(SyntaxNodeAnalysisContext context) {
+      new Analyzer(context).Analyze();
+    }
+
+    private class Analyzer : InternalAnalyzerBase<ConstructorDeclarationSyntax> {
+      private readonly TaskAnalysis _taskAnalysis;
+
+      public Analyzer(SyntaxNodeAnalysisContext context) : base(new SyntaxNodeAnalysisContextWrapper(context)) {
+        _taskAnalysis = new TaskAnalysis(context.SemanticModel, context.CancellationToken);
+      }
+
+      public override void Analyze() {
+        foreach(var blockingAccess in GetBlockingTaskAccesses()) {
+          Context.ReportDiagnostic(Diagnostic.Create(Rule, blockingAccess.GetLocation()));
+        }
+      }
+
+      private IEnumerable<SyntaxNode> GetBlockingTaskAccesses() {
+        return Root.DescendantNodesInSameActivationFrame()
+          .WithCancellation(CancellationToken)
+          .Where(IsBlockingTaskAccess);
+      }
+
+      private bool IsBlockingTaskAccess(SyntaxNode node) {
+        return node switch {
+          InvocationExpressionSyntax invocation => _taskAnalysis.IsBlockingMethodInvocation(invocation),
+          MemberAccessExpressionSyntax memberAccess => _taskAnalysis.IsBlockingPropertyAccess(memberAccess),
+          _ => false
+        };
+      }
+    }
+  }
+}

src/ParallelHelper/ParallelHelper.csproj

@@ -10,13 +10,13 @@
 
   <PropertyGroup>
     <PackageId>ParallelHelper</PackageId>
-    <Version>3.6.0</Version>
+    <Version>3.7.0</Version>
     <Authors>Christoph Amrein</Authors>
     <PackageProjectUrl>https://github.com/Concurrency-Lab/ParallelHelper</PackageProjectUrl>
     <RepositoryUrl>https://github.com/Concurrency-Lab/ParallelHelper</RepositoryUrl>
     <RepositoryType>git</RepositoryType>
     <Description>ParallelHelper is a static code analyzer that helps to identify concurrency related issues. Moreover, it provides hints to improve the robustness of code with concurrency in mind.</Description>
-    <PackageReleaseNotes>- New Analyzer: PH_S034 - Async Lambda Inferred to Async Void</PackageReleaseNotes>
+    <PackageReleaseNotes>- New Analyzer: PH_S035 - Blocking Async Method Invocation in Constructor</PackageReleaseNotes>
     <Copyright>Copyright (C) 2022  Christoph Amrein</Copyright>
     <PackageTags>C#, Parallel, Asynchronous, Concurrency, Bugs, Best Practices, TPL, Task, QC, Static Analysis, async, await</PackageTags>
     <NoPackageAnalysis>true</NoPackageAnalysis>