Check for TimeSpan.Zero on Ask and Ask<T> operations (#17)

* Check for `TimeSpan.Zero` on `Ask` and `Ask<T>` operationrs close #11 close akkadotnet/akka.net#6131 * working on specs * sdfsdf * have tests finally starting to pass * cleaning up tests and unused code * disabled variable scanning - not supported yet
akkadotnet · Dec 29, 2023 · 4258d1c · 4258d1c
1 parent 5482a51
commit 4258d1c
Show file tree

Hide file tree

Showing 6 changed files with 213 additions and 3 deletions.
diff --git a/src/Akka.Analyzers.Tests/Akka.Analyzers.Tests.csproj b/src/Akka.Analyzers.Tests/Akka.Analyzers.Tests.csproj
@@ -31,8 +31,10 @@
     </ItemGroup>
 
     <ItemGroup>
-      <ProjectReference Include="..\Akka.Analyzers.Fixes\Akka.Analyzers.Fixes.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="true"/>
-      <ProjectReference Include="..\Akka.Analyzers\Akka.Analyzers.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="true" />
+      <ProjectReference Include="..\Akka.Analyzers.Fixes\Akka.Analyzers.Fixes.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="true"
+                      SetTargetFramework="TargetFramework=netstandard2.0"/>
+      <ProjectReference Include="..\Akka.Analyzers\Akka.Analyzers.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="true"
+                      SetTargetFramework="TargetFramework=netstandard2.0"/>
     </ItemGroup>
 
 </Project>
diff --git a/src/Akka.Analyzers.Tests/Analyzers/AK2000/MustNotUseTimeSpanZeroWithAskAnalyzerSpecs.cs b/src/Akka.Analyzers.Tests/Analyzers/AK2000/MustNotUseTimeSpanZeroWithAskAnalyzerSpecs.cs
@@ -0,0 +1,132 @@
+// -----------------------------------------------------------------------
+//  <copyright file="MustNotUseTimeSpanZeroWithAskAnalyzerSpecs.cs" company="Akka.NET Project">
+//      Copyright (C) 2015-2023 .NET Petabridge, LLC
+//  </copyright>
+// -----------------------------------------------------------------------
+
+using Verify = Akka.Analyzers.Tests.Utility.AkkaVerifier<Akka.Analyzers.AK2000.MustNotUseTimeSpanZeroWithAskAnalyzer>;
+
+namespace Akka.Analyzers.Tests.Analyzers.AK2000;
+
+public class MustNotUseTimeSpanZeroWithAskAnalyzerSpecs
+{
+    public static readonly TheoryData<string> SuccessCases = new()
+    {
+        // Explicit non-zero TimeSpan
+        @"using Akka.Actor;
+        using System.Threading.Tasks;
+        using System;
+
+        public static class MyActorCaller{
+            public static Task<string> Call(IActorRef actor){
+                return actor.Ask<string>(""hello"", TimeSpan.FromSeconds(1));
+            }
+        }",
+
+        // Implicit timeout - handled by Akka.NET default timeout
+        @"using Akka.Actor;
+        using System.Threading.Tasks;
+        using System;
+
+        public static class MyActorCaller{
+            public static Task<string> Call(IActorRef actor){
+                return actor.Ask<string>(""hello"");
+            }
+        }"
+    };
+
+    [Theory]
+    [MemberData(nameof(SuccessCases))]
+    public Task SuccessCase(string code)
+    {
+        return Verify.VerifyAnalyzer(code);
+    }
+
+    [Fact]
+    public Task FailureCaseExplicitZeroTimeSpanGeneric()
+    {
+        var code =
+            @"using Akka.Actor;
+ using System.Threading.Tasks;
+ using System;
+
+ public static class MyActorCaller{
+     public static Task<string> Call(IActorRef actor){
+         return actor.Ask<string>(""hello"", TimeSpan.Zero);
+     }
+ }";
+        var expectedDiagnostic = Verify.Diagnostic().WithSpan(7, 44, 7, 57); // Adjust the line and character positions
+        return Verify.VerifyAnalyzer(code, expectedDiagnostic);
+    }
+
+    [Fact]
+    public Task FailureCaseExplicitZeroTimeSpanNonGeneric()
+    {
+        var code =
+            @"using Akka.Actor;
+ using System.Threading.Tasks;
+ using System;
+
+ public static class MyActorCaller{
+     public static Task Call(IActorRef actor){
+         return actor.Ask(""hello"", TimeSpan.Zero);
+     }
+ }";
+        var expectedDiagnostic = Verify.Diagnostic().WithSpan(7, 36, 7, 49); // Adjust the line and character positions
+        return Verify.VerifyAnalyzer(code, expectedDiagnostic);
+    }
+
+
+    [Fact]
+    public Task FailureCaseExplicitZeroTimeSpanWithNamedArgumentAndOtherArguments()
+    {
+        var code =
+            @"using Akka.Actor;
+ using System.Threading.Tasks;
+ using System;
+
+ public static class MyActorCaller{
+    public static Task<string> Call(IActorRef actor, string message){
+         return actor.Ask<string>(message, timeout: TimeSpan.Zero, cancellationToken: default);
+    }
+}";
+        var expectedDiagnostic = Verify.Diagnostic().WithSpan(7, 44, 7, 66);
+        return Verify.VerifyAnalyzer(code, expectedDiagnostic);
+    }
+
+    [Fact(Skip = "Variable analysis not yet implemented")]
+    public Task FailureCaseTimeSpanVariableSetToDefault()
+    {
+        var code =
+@"using Akka.Actor;
+using System.Threading.Tasks;
+using System;
+
+public static class MyActorCaller{
+ public static Task<string> Call(IActorRef actor){
+     TimeSpan myTs = default;
+     return actor.Ask<string>(""hello"", timeout: myTs);
+ }
+}";
+        var expectedDiagnostic = Verify.Diagnostic().WithLocation(7, 19); // Adjust accordingly
+        return Verify.VerifyAnalyzer(code, expectedDiagnostic);
+    }
+
+    [Fact(Skip = "Variable analysis not yet implemented")]
+    public Task FailureCaseTimeSpanVariableSetToTimeSpanZero()
+    {
+        var code =
+            @"using Akka.Actor;
+using System.Threading.Tasks;
+using System;
+
+public static class MyActorCaller{
+ public static Task<string> Call(IActorRef actor){
+     TimeSpan myTs = TimeSpan.Zero;
+     return actor.Ask<string>(""hello"", timeout: myTs);
+ }
+}";
+        var expectedDiagnostic = Verify.Diagnostic().WithLocation(7, 19); // Adjust accordingly
+        return Verify.VerifyAnalyzer(code, expectedDiagnostic);
+    }
+}
diff --git a/src/Akka.Analyzers/AK2000/MustNotUseTimeSpanZeroWithAskAnalyzer.cs b/src/Akka.Analyzers/AK2000/MustNotUseTimeSpanZeroWithAskAnalyzer.cs
@@ -0,0 +1,59 @@
+// -----------------------------------------------------------------------
+//  <copyright file="MustNotUseTimeSpanZeroWithAskAnalyzer.cs" company="Akka.NET Project">
+//      Copyright (C) 2015-2023 .NET Petabridge, LLC
+//  </copyright>
+// -----------------------------------------------------------------------
+
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+
+namespace Akka.Analyzers.AK2000;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public class MustNotUseTimeSpanZeroWithAskAnalyzer()
+    : AkkaDiagnosticAnalyzer(RuleDescriptors.Ak2000DoNotUseZeroTimeoutWithAsk)
+{
+    public override void AnalyzeCompilation(CompilationStartAnalysisContext context, AkkaContext akkaContext)
+    {
+        Guard.AssertIsNotNull(context);
+        Guard.AssertIsNotNull(akkaContext);
+
+        context.RegisterSyntaxNodeAction(ctx =>
+        {
+            var invocationExpr = (InvocationExpressionSyntax)ctx.Node;
+
+            if (ctx.SemanticModel.GetSymbolInfo(invocationExpr).Symbol is IMethodSymbol { Name: "Ask" } methodSymbol &&
+                methodSymbol.Parameters.Any(p => p.Type.ToString() == "System.TimeSpan?"))
+            {
+                foreach (var argument in invocationExpr.ArgumentList.Arguments)
+                {
+                    if (ctx.SemanticModel.GetTypeInfo(argument.Expression).Type is INamedTypeSymbol argType &&
+                        argType.ConstructedFrom.ToString() == "System.TimeSpan")
+                    {
+                        if (argument.Expression is MemberAccessExpressionSyntax { Name.Identifier.ValueText: "Zero" })
+                        {
+                            var diagnostic = Diagnostic.Create(RuleDescriptors.Ak2000DoNotUseZeroTimeoutWithAsk,
+                                argument.GetLocation());
+                            ctx.ReportDiagnostic(diagnostic);
+                        }
+                        else if (argument.Expression is IdentifierNameSyntax identifierName)
+                        {
+                            var symbol = ctx.SemanticModel.GetSymbolInfo(identifierName).Symbol;
+                            if (symbol is ILocalSymbol
+                                {
+                                    HasConstantValue: true, ConstantValue: not null
+                                } localSymbol && localSymbol.ConstantValue.Equals(TimeSpan.Zero))
+                            {
+                                var diagnostic = Diagnostic.Create(RuleDescriptors.Ak2000DoNotUseZeroTimeoutWithAsk,
+                                    argument.GetLocation());
+                                ctx.ReportDiagnostic(diagnostic);
+                            }
+                        }
+                    }
+                }
+            }
+        }, SyntaxKind.InvocationExpression);
+    }
+}
diff --git a/src/Akka.Analyzers/Akka.Analyzers.csproj.DotSettings b/src/Akka.Analyzers/Akka.Analyzers.csproj.DotSettings
@@ -1,3 +1,4 @@
 <wpf:ResourceDictionary xml:space="preserve" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib" xmlns:ss="urn:shemas-jetbrains-com:settings-storage-xaml" xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation">
 	<s:Boolean x:Key="/Default/CodeInspection/NamespaceProvider/NamespaceFoldersToSkip/=ak1000/@EntryIndexedValue">True</s:Boolean>
+	<s:Boolean x:Key="/Default/CodeInspection/NamespaceProvider/NamespaceFoldersToSkip/=ak2000/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/CodeInspection/NamespaceProvider/NamespaceFoldersToSkip/=utility/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary>
diff --git a/src/Akka.Analyzers/Utility/AnalysisCategory.cs b/src/Akka.Analyzers/Utility/AnalysisCategory.cs
@@ -12,4 +12,8 @@ public enum AnalysisCategory
     /// 1xxx
     /// </summary>
     ActorDesign,
+    /// <summary>
+    /// 2xxx
+    /// </summary>
+    ApiUsage,
 }
diff --git a/src/Akka.Analyzers/Utility/RuleDescriptors.cs b/src/Akka.Analyzers/Utility/RuleDescriptors.cs
@@ -18,12 +18,24 @@ private static DiagnosticDescriptor Rule(
             isEnabledByDefault: true, helpLinkUri: helpLink);
     }
 
+    #region AK1000 Rules
+
     public static DiagnosticDescriptor Ak1000DoNotNewActors { get; } = Rule("Akka1000",
         "Do not use `new` to create actors", AnalysisCategory.ActorDesign, DiagnosticSeverity.Error,
         "Actors must be instantiated using `ActorOf` or `ActorOfAsTestActorRef` via a `Props` class.");
 
-    public static DiagnosticDescriptor Ak1001CloseOverSenderUsingPipeTo { get; } = Rule("Akka1002",
+    public static DiagnosticDescriptor Ak1001CloseOverSenderUsingPipeTo { get; } = Rule("Akka1001",
         "Should always close over `Sender` when using `PipeTo`", AnalysisCategory.ActorDesign, DiagnosticSeverity.Error,
         "When using `PipeTo`, you must always close over `Sender` to ensure that the actor's `Sender` property " +
         "is captured at the time you're scheduling the `PipeTo`, as this value may change asynchronously.");
+
+    #endregion
+
+    #region AK2000 Rules
+
+    public static DiagnosticDescriptor Ak2000DoNotUseZeroTimeoutWithAsk { get; } = Rule("Akka2000",
+        "Do not use `Ask` with `TimeSpan.Zero` for timeout.", AnalysisCategory.ApiUsage, DiagnosticSeverity.Error,
+        "When using `Ask`, you must always specify a timeout value greater than `TimeSpan.Zero`.");
+
+    #endregion
 }