Merge pull request #65 from Concurrency-Lab/ph-s019-exclude-methods

PH_S019: Add support for method exclusions

Author: camrein

doc/analyzers/PH_S019.md

@@ -7,3 +7,15 @@ An asynchronous method makes use of a blocking method, although there exists an
 ## Solution
 
 Replace the blocking method with its asynchronous counterpart.
+
+## Additional Note
+
+The methods `AddAsync` and `AddRangeAsync` of entity framework's `DbSet` and `DbContext` are explicitely excluded from this analysis by default. The [documentation](https://docs.microsoft.com/en-us/dotnet/api/microsoft.entityframeworkcore.dbcontext.addasync?view=efcore-5.0) recommends using the non-async versions instead.
+
+## Options
+
+```
+# A white-space separated list of methods to exclude when searching for async counterparts
+# Format: <type-specifier>:<method1>,<method2>
+dotnet_diagnostic.PH_P019.exclusions = Microsoft.EntityFrameworkCore.DbContext:Add,AddRange Microsoft.EntityFrameworkCore.DbSet`1:Add,AddRange
+```

reportall.editorconfig

@@ -36,6 +36,7 @@ dotnet_diagnostic.PH_S016.severity = warning
 dotnet_diagnostic.PH_S017.severity = warning
 dotnet_diagnostic.PH_S018.severity = warning
 dotnet_diagnostic.PH_S019.severity = warning
+dotnet_diagnostic.PH_S019.exclusions = 
 dotnet_diagnostic.PH_S020.severity = warning
 dotnet_diagnostic.PH_S021.severity = warning
 dotnet_diagnostic.PH_S022.severity = warning

src/ParallelHelper.Plugin/ReleaseNotes.txt

@@ -3,6 +3,7 @@
 - New Analyzer: PH_B015 - Disposed Task Instead of Value
 - Improved Analyzer: PH_S007 - Now respects activation frames
 - Improved Analyzer: PH_P007 - Now reports default expressions where a token is available
+- Improved Analyzer: PH_S019 - Now ignores EF Core's Add and AddRange of DbSet and DbContext by default
 - Improved Analyzer: PH_S025 - Now supports lambda expressions and respects activation frames
 
 

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

@@ -108,6 +108,28 @@ public Task DoItAsync() {
       VerifyDiagnostic(source, new DiagnosticResultLocation(7, 5));
     }
 
+    [TestMethod]
+    public void ReportsReadOfTypeWithReadAsyncInAsyncMethodIfExclusionEntryIsInvalid() {
+      const string source = @"
+using System.IO;
+using System.Net.Sockets;
+using System.Threading.Tasks;
+
+class Test {
+  public async Task DoWorkAsync() {
+    using(var client = new TcpClient())
+    using(var reader = new StreamReader(client.GetStream())) {
+      var buffer = new char[1024];
+      reader.Read(buffer, 0, buffer.Length);
+    }
+  }
+}";
+      CreateAnalyzerCompilationBuilder()
+        .AddSourceTexts(source)
+        .AddAnalyzerOption("dotnet_diagnostic.PH_S019.exclusions", "System.IO.StreamReader;ReadLine,Read,ReadBlock")
+        .VerifyDiagnostic(new DiagnosticResultLocation(10, 7));
+    }
+
     [TestMethod]
     public void DoesNotReportReadOfTypeWithReadAsyncInNonAsyncMethod() {
       const string source = @"
@@ -278,5 +300,74 @@ public Task DoItAsync() {
 }";
       VerifyDiagnostic(source);
     }
+
+    [TestMethod]
+    public void DoesNotReportReadOfTypeWithReadAsyncInAsyncMethodIfExcludedAsSingleTypeAndMethodEntry() {
+      const string source = @"
+using System.IO;
+using System.Net.Sockets;
+using System.Threading.Tasks;
+
+class Test {
+  public async Task DoWorkAsync() {
+    using(var client = new TcpClient())
+    using(var reader = new StreamReader(client.GetStream())) {
+      var buffer = new char[1024];
+      reader.Read(buffer, 0, buffer.Length);
+    }
+  }
+}";
+      CreateAnalyzerCompilationBuilder()
+        .AddSourceTexts(source)
+        .AddAnalyzerOption("dotnet_diagnostic.PH_S019.exclusions", "System.IO.StreamReader:Read")
+        .VerifyDiagnostic();
+    }
+
+    [TestMethod]
+    public void DoesNotReportReadOfTypeWithReadAsyncInAsyncMethodIfExcludedAsSingleTypeEntry() {
+      const string source = @"
+using System.IO;
+using System.Net.Sockets;
+using System.Threading.Tasks;
+
+class Test {
+  public async Task DoWorkAsync() {
+    using(var client = new TcpClient())
+    using(var reader = new StreamReader(client.GetStream())) {
+      var buffer = new char[1024];
+      reader.Read(buffer, 0, buffer.Length);
+    }
+  }
+}";
+      CreateAnalyzerCompilationBuilder()
+        .AddSourceTexts(source)
+        .AddAnalyzerOption("dotnet_diagnostic.PH_S019.exclusions", "System.IO.StreamReader:ReadLine,Read,ReadBlock")
+        .VerifyDiagnostic();
+    }
+
+    [TestMethod]
+    public void DoesNotReportReadOfTypeWithReadAsyncInAsyncMethodIfExcludedAsMultipleTypeEntry() {
+      const string source = @"
+using System.IO;
+using System.Net.Sockets;
+using System.Threading.Tasks;
+
+class Test {
+  public async Task DoWorkAsync() {
+    using(var client = new TcpClient())
+    using(var reader = new StreamReader(client.GetStream())) {
+      var buffer = new char[1024];
+      reader.Read(buffer, 0, buffer.Length);
+    }
+  }
+}";
+      const string exclusions = @"Microsoft.EntityFrameworkCore.DbContext:Add,AddRange
+Microsoft.EntityFrameworkCore.DbSet`1:Add,AddRange
+System.IO.StreamReader:ReadLine,Read,ReadBlock";
+      CreateAnalyzerCompilationBuilder()
+        .AddSourceTexts(source)
+        .AddAnalyzerOption("dotnet_diagnostic.PH_S019.exclusions", exclusions)
+        .VerifyDiagnostic();
+    }
   }
 }

src/ParallelHelper/Analyzer/Smells/BlockingMethodWithAsyncCounterpartInAsyncMethodAnalyzer.cs

@@ -47,6 +47,9 @@ public class BlockingMethodWithAsyncCounterpartInAsyncMethodAnalyzer : Diagnosti
 
     public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
 
+    private const string DefaultExcludedMethods = @"Microsoft.EntityFrameworkCore.DbContext:Add,AddRange
+Microsoft.EntityFrameworkCore.DbSet`1:Add,AddRange";
+
     public override void Initialize(AnalysisContext context) {
       context.EnableConcurrentExecution();
       context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
@@ -74,20 +77,36 @@ public override void Analyze() {
         if(!IsAsyncMethod && !IsAsyncAnonymousFunction) {
           return;
         }
+        var excludedMethods = GetExcludedMethods().ToArray();
         foreach(var invocation in GetAllInvocations()) {
-          AnalyzeInvocation(invocation);
+          AnalyzeInvocation(invocation, excludedMethods);
         }
       }
 
+      private IEnumerable<MethodDescriptor> GetExcludedMethods() {
+        return Context.Options.GetConfig(Rule, "exclusions", DefaultExcludedMethods).Split()
+          .WithCancellation(CancellationToken)
+          .Select(ToMethodDescriptor)
+          .IsNotNull();
+      }
+
+      private static MethodDescriptor? ToMethodDescriptor(string config) {
+        var splitByTypeAndMethods = config.Split(':');
+        return splitByTypeAndMethods.Length != 2
+          ? null
+          : new MethodDescriptor(splitByTypeAndMethods[0], splitByTypeAndMethods[1].Split(','));
+      }
+
       private IEnumerable<InvocationExpressionSyntax> GetAllInvocations() {
         return Root.DescendantNodesInSameActivationFrame()
           .WithCancellation(CancellationToken)
           .OfType<InvocationExpressionSyntax>();
       }
 
-      private void AnalyzeInvocation(InvocationExpressionSyntax invocation) {
+      private void AnalyzeInvocation(InvocationExpressionSyntax invocation, IReadOnlyCollection<MethodDescriptor> excludedMethods) {
         if(SemanticModel.GetSymbolInfo(invocation, CancellationToken).Symbol is IMethodSymbol method 
-            && !IsPotentiallyAsyncMethod(method) && TryGetAsyncCounterpart(method, out var asyncName)) {
+            && !IsPotentiallyAsyncMethod(method) && TryGetAsyncCounterpart(method, out var asyncName)
+            && !IsExcludedMethod(method, excludedMethods)) {
           Context.ReportDiagnostic(Diagnostic.Create(Rule, invocation.GetLocation(), method.Name, asyncName));
         }
       }
@@ -105,6 +124,26 @@ private static bool TryGetAsyncCounterpart(IMethodSymbol method, out string asyn
         asyncName = "";
         return false;
       }
+
+      private bool IsExcludedMethod(IMethodSymbol method, IReadOnlyCollection<MethodDescriptor> excludedMethods) {
+        return excludedMethods.WithCancellation(CancellationToken)
+          .Any(excludedMethod => IsAnyMethodOf(method, excludedMethod));
+      }
+
+      private bool IsAnyMethodOf(IMethodSymbol method, MethodDescriptor descriptor) {
+        return SemanticModel.IsEqualType(method.ContainingType, descriptor.Type)
+          && descriptor.Methods.Contains(method.Name);
+      }
+    }
+
+    private class MethodDescriptor {
+      public string Type { get; }
+      public IImmutableSet<string> Methods { get; }
+
+      public MethodDescriptor(string type, params string[] methods) {
+        Type = type;
+        Methods = methods.ToImmutableHashSet();
+      }
     }
   }
 }

src/ParallelHelper/ParallelHelper.csproj

@@ -20,6 +20,7 @@
     <PackageReleaseNotes>- New Analyzer: PH_B015 - Disposed Task Instead of Value
 - Improved Analyzer: PH_S007 - Now respects activation frames
 - Improved Analyzer: PH_P007 - Now reports default expressions where a token is available
+- Improved Analyzer: PH_S019 - Now ignores EF Core's Add and AddRange of DbSet and DbContext by default
 - Improved Analyzer: PH_S025 - Now supports lambda expressions and respects activation frames</PackageReleaseNotes>
     <Copyright>Copyright (C) 2019 - 2021  Christoph Amrein, Concurrency Lab, Eastern Switzerland University of Applied Sciences, Switzerland</Copyright>
     <PackageTags>C#, Parallel, Asynchronous, Concurrency, Bugs, Best Practices, TPL, Task, QC, Static Analysis, async, await</PackageTags>