Merge pull request #97 from Concurrency-Lab/ph-s020-refactor-to-taska…

…nalysis PH_S020: Refactor to TaskAnalysis
Concurrency-Lab · Nov 29, 2021 · 717fb51 · 717fb51
2 parents b16a1e5 + d0e37f3
commit 717fb51
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 13 deletions.
diff --git a/src/ParallelHelper.Test/Util/TaskAnalysisTest.cs b/src/ParallelHelper.Test/Util/TaskAnalysisTest.cs
@@ -861,5 +861,65 @@ public void DoIt() {
       var analysis = new TaskAnalysis(semanticModel, default);
       Assert.IsFalse(analysis.IsFromResultInvocation(invocation));
     }
+
+
+
+    [TestMethod]
+    public void IsCompletedTaskAccessReturnsTrueForAccessingCompletedTask() {
+      const string source = @"
+using System.Threading.Tasks;
+
+public class Test {
+  public void DoIt() {
+    var task = Task.CompletedTask;
+  }
+}";
+      var semanticModel = CompilationFactory.GetSemanticModel(source);
+      var memberAccess = semanticModel.SyntaxTree.GetRoot()
+        .DescendantNodes()
+        .OfType<MemberAccessExpressionSyntax>()
+        .Single();
+      var analysis = new TaskAnalysis(semanticModel, default);
+      Assert.IsTrue(analysis.IsCompletedTaskAccess(memberAccess));
+    }
+
+    [TestMethod]
+    public void IsCompletedTaskAccessReturnsFalseForAccessingUnknownTaskMember() {
+      const string source = @"
+using System.Threading.Tasks;
+
+public class Test {
+  public void DoIt() {
+    var task = Task.SomeNotExistantProperty;
+  }
+}";
+      var semanticModel = CompilationFactory.GetSemanticModel(source);
+      var memberAccess = semanticModel.SyntaxTree.GetRoot()
+        .DescendantNodes()
+        .OfType<MemberAccessExpressionSyntax>()
+        .Single();
+      var analysis = new TaskAnalysis(semanticModel, default);
+      Assert.IsFalse(analysis.IsCompletedTaskAccess(memberAccess));
+    }
+
+    [TestMethod]
+    public void IsCompletedTaskAccessReturnsFalseForAccessingDifferentMember() {
+      const string source = @"
+using System;
+using System.Threading.Tasks;
+
+public class Test {
+  public void DoIt() {
+    var task = Task.Factory;
+  }
+}";
+      var semanticModel = CompilationFactory.GetSemanticModel(source);
+      var memberAccess = semanticModel.SyntaxTree.GetRoot()
+        .DescendantNodes()
+        .OfType<MemberAccessExpressionSyntax>()
+        .Single();
+      var analysis = new TaskAnalysis(semanticModel, default);
+      Assert.IsFalse(analysis.IsCompletedTaskAccess(memberAccess));
+    }
   }
 }
diff --git a/src/ParallelHelper/Analyzer/Smells/AwaitSynchronousTaskCompletionAnalyzer.cs b/src/ParallelHelper/Analyzer/Smells/AwaitSynchronousTaskCompletionAnalyzer.cs
@@ -2,7 +2,6 @@
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Diagnostics;
-using ParallelHelper.Extensions;
 using ParallelHelper.Util;
 using System.Collections.Immutable;
 
@@ -36,10 +35,6 @@ public class AwaitSynchronousTaskCompletionAnalyzer : DiagnosticAnalyzer {
       isEnabledByDefault: true, description: Description, helpLinkUri: HelpLinkFactory.CreateUri(DiagnosticId)
     );
 
-    private const string TaskType = "System.Threading.Tasks.Task";
-    private const string FromResultMethod = "FromResult";
-    private const string CompletedTaskProperty = "CompletedTask";
-
     public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
 
     public override void Initialize(AnalysisContext context) {
@@ -53,7 +48,11 @@ private static void AnalyzeAwaitExpression(SyntaxNodeAnalysisContext context) {
     }
 
     private class Analyzer : InternalAnalyzerBase<AwaitExpressionSyntax> {
-      public Analyzer(SyntaxNodeAnalysisContext context) : base(new SyntaxNodeAnalysisContextWrapper(context)) { }
+      private readonly TaskAnalysis _taskAnalysis;
+
+      public Analyzer(SyntaxNodeAnalysisContext context) : base(new SyntaxNodeAnalysisContextWrapper(context)) {
+        _taskAnalysis = new TaskAnalysis(context.SemanticModel, context.CancellationToken);
+      }
 
       public override void Analyze() {
         if(!AwaitsSynchronouslyCompletedTask()) {
@@ -69,16 +68,12 @@ private bool AwaitsSynchronouslyCompletedTask() {
 
       private bool IsTaskFromResult(ExpressionSyntax expression) {
         return expression is InvocationExpressionSyntax invocation
-          && SemanticModel.GetSymbolInfo(invocation, CancellationToken).Symbol is IMethodSymbol method
-          && method.Name.Equals(FromResultMethod)
-          && SemanticModel.IsEqualType(method.ContainingType, TaskType);
+          && _taskAnalysis.IsFromResultInvocation(invocation);
       }
 
       private bool IsCompletedTask(ExpressionSyntax expression) {
         return expression is MemberAccessExpressionSyntax memberAccess
-          && SemanticModel.GetSymbolInfo(memberAccess, CancellationToken).Symbol is IPropertySymbol property
-          && property.Name.Equals(CompletedTaskProperty)
-          && SemanticModel.IsEqualType(property.ContainingType, TaskType);
+          && _taskAnalysis.IsCompletedTaskAccess(memberAccess);
       }
     }
   }

diff --git a/src/ParallelHelper/Util/TaskAnalysis.cs b/src/ParallelHelper/Util/TaskAnalysis.cs
@@ -36,6 +36,7 @@ public class TaskAnalysis {
     );
 
     private const string FromResultMethod = "FromResult";
+    private const string CompletedTaskProperty = "CompletedTask";
 
     private readonly SemanticModel _semanticModel;
     private readonly CancellationToken _cancellationToken;
@@ -130,7 +131,16 @@ public bool IsFromResultInvocation(InvocationExpressionSyntax invocation) {
     }
 
     /// <summary>
-    /// CHecks if the given node is a method or function (e.g. lambda) returns a task.
+    /// Checks if the given member access is accessing <see cref="System.Threading.Tasks.Task.CompletedTask"/>.
+    /// </summary>
+    /// <param name="memberAccess">The member access to check.</param>
+    /// <returns><c>true</c> if it's the member access of CompletedTask, <c>false</c> otherwise.</returns>
+    public bool IsCompletedTaskAccess(MemberAccessExpressionSyntax memberAccess) {
+      return IsTaskMemberAccess(memberAccess, CompletedTaskProperty);
+    }
+
+    /// <summary>
+    /// Checks if the given node is a method or function (e.g. lambda) returns a task.
     /// </summary>
     /// <param name="node">The node to check.</param>
     /// <returns><c>true</c> if the underlying method or function returns a task, <c>false</c> otherwise.</returns>
@@ -155,6 +165,12 @@ private bool IsTaskMethodInvocation(InvocationExpressionSyntax invocation, strin
         && IsTaskMember(method, taskMember);
     }
 
+    private bool IsTaskMemberAccess(MemberAccessExpressionSyntax invocation, string taskMember) {
+      var symbol = _semanticModel.GetSymbolInfo(invocation, _cancellationToken).Symbol;
+      return symbol != null
+        && IsTaskMember(symbol, taskMember);
+    }
+
     private bool IsTaskMember(ISymbol member, string taskMember) {
       return member.Name == taskMember && IsTaskType(member.ContainingType);
     }