Add AK1008 - Check for multiple Stash.Stash() invocation

src/Akka.Analyzers.Tests/Analyzers/AK1000/MustNotInvokeStashMoreThanOnceInsideABlockSpecs.cs

@@ -0,0 +1,205 @@
+// -----------------------------------------------------------------------
+//  <copyright file="MustNotInvokeStashMoreThanOnceInsideABlockSpecs.cs" company="Akka.NET Project">
+//      Copyright (C) 2013-2024 .NET Foundation <https://github.com/akkadotnet/akka.net>
+//  </copyright>
+// -----------------------------------------------------------------------
+
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Xunit.Abstractions;
+using Verify = Akka.Analyzers.Tests.Utility.AkkaVerifier<Akka.Analyzers.MustNotInvokeStashMoreThanOnceAnalyzer>;
+
+namespace Akka.Analyzers.Tests.Analyzers.AK1000;
+
+public class MustNotInvokeStashMoreThanOnceInsideABlockSpecs
+{
+    public static readonly TheoryData<string> SuccessCases = new()
+    {
+        // ReceiveActor with single Stash() invocation
+"""
+// 01
+using Akka.Actor;
+using System.Threading.Tasks;
+
+public sealed class MyActor : ReceiveActor, IWithStash
+{
+    public MyActor()
+    {
+        Receive<string>(str => {
+            Sender.Tell(str);
+            Stash.Stash(); // should not flag this
+        });
+    }
+
+    public void Handler()
+    {
+        Stash.Stash();
+    }
+
+    public IStash Stash { get; set; }
+}
+""",
+
+        // Non-Actor class that has Stash() methods, we're not responsible for this.
+"""
+// 02
+public interface INonAkkaStash
+{
+    public void Stash();
+}
+
+public class NonAkkaStash : INonAkkaStash
+{
+    public void Stash() { }
+}
+
+public sealed class MyActor
+{
+    public MyActor()
+    {
+        Stash = new NonAkkaStash();
+    }
+
+    public void Test()
+    {
+        Stash.Stash();
+        Stash.Stash(); // should not flag this
+    }
+
+    public INonAkkaStash Stash { get; }
+}
+""",
+
+        // Non-Actor class that uses Stash(),
+        // we're only responsible for checking usage inside ActorBase class and its descendants.
+"""
+// 03
+using System;
+using Akka.Actor;
+
+public class MyActor: IWithStash
+{
+    public MyActor(IStash stash)
+    {
+        Stash = stash;
+    }
+
+    public void Test()
+    {
+        Stash.Stash();
+        Stash.Stash(); // should not flag this
+    }
+
+    public IStash Stash { get; set; }
+}
+""",
+        // Stash calls inside 2 different code branch
+"""
+// 04
+using Akka.Actor;
+
+public sealed class MyActor : ReceiveActor, IWithStash
+{
+    public MyActor(int n)
+    {
+        Receive<string>(str =>
+        {
+            if(n < 0)
+            {
+                Stash!.Stash();
+            }
+            else
+            {
+                Stash!.Stash(); // should not flag this
+            }
+        });
+    }
+
+    public IStash Stash { get; set; } = null!;
+}
+""",
+    };
+
+    public static readonly
+        TheoryData<(string testData, (int startLine, int startColumn, int endLine, int endColumn) spanData)>
+        FailureCases = new()
+        {
+            // Receive actor invoking Stash()
+            (
+"""
+// 01
+using System;
+using Akka.Actor;
+using System.Threading.Tasks;
+
+public sealed class MyActor : ReceiveActor, IWithStash
+{
+    public MyActor()
+    {
+        Receive<string>(str => 
+        {
+            Stash.Stash();
+            Stash.Stash(); // Error
+        });
+    }
+
+    public IStash Stash { get; set; } = null!;
+}
+""", (13, 13, 13, 26)),
+
+            // Receive actor invoking Stash() inside and outside of a code branch
+            (
+"""
+// 02
+using System;
+using Akka.Actor;
+using System.Threading.Tasks;
+
+public sealed class MyActor : ReceiveActor, IWithStash
+{
+    public MyActor(int n)
+    {
+        Receive<string>(str =>
+        {
+            if(n < 0)
+            {
+                Stash!.Stash();
+            }
+
+            Stash.Stash(); // Error
+        });
+    }
+
+    public IStash Stash { get; set; } = null!;
+}
+""", (12, 13, 12, 105)),
+            };
+
+    private readonly ITestOutputHelper _output;
+
+    public MustNotInvokeStashMoreThanOnceInsideABlockSpecs(ITestOutputHelper output)
+    {
+        _output = output;
+    }
+
+    [Theory]
+    [MemberData(nameof(SuccessCases))]
+    public Task SuccessCase(string testCode)
+    {
+        return Verify.VerifyAnalyzer(testCode);
+    }
+
+    [Theory]
+    [MemberData(nameof(FailureCases))]
+    public Task FailureCase(
+        (string testCode, (int startLine, int startColumn, int endLine, int endColumn) spanData) d)
+    {
+        var expected = Verify.Diagnostic()
+            .WithSpan(d.spanData.startLine, d.spanData.startColumn, d.spanData.endLine, d.spanData.endColumn)
+            .WithSeverity(DiagnosticSeverity.Error);
+
+        return Verify.VerifyAnalyzer(d.testCode, expected);
+    }
+
+}
+

src/Akka.Analyzers/AK1000/MustNotInvokeStashMoreThanOnceAnalyzer.cs

@@ -0,0 +1,95 @@
+// -----------------------------------------------------------------------
+//  <copyright file="MustNotInvokeStashMoreThanOnce.cs" company="Akka.NET Project">
+//      Copyright (C) 2013-2024 .NET Foundation <https://github.com/akkadotnet/akka.net>
+//  </copyright>
+// -----------------------------------------------------------------------
+
+using Akka.Analyzers.Context;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.FlowAnalysis;
+using Microsoft.CodeAnalysis.Operations;
+
+namespace Akka.Analyzers;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public class MustNotInvokeStashMoreThanOnceAnalyzer()
+    : AkkaDiagnosticAnalyzer(RuleDescriptors.Ak1008MustNotInvokeStashMoreThanOnce)
+{
+    public override void AnalyzeCompilation(CompilationStartAnalysisContext context, AkkaContext akkaContext)
+    {
+        Guard.AssertIsNotNull(context);
+        Guard.AssertIsNotNull(akkaContext);
+
+        context.RegisterSyntaxNodeAction(ctx => AnalyzeMethod(ctx, akkaContext), SyntaxKind.MethodDeclaration, SyntaxKind.ConstructorDeclaration);
+    }
+
+    private static void AnalyzeMethod(SyntaxNodeAnalysisContext context, AkkaContext akkaContext)
+    {
+        var semanticModel = context.SemanticModel;
+
+        // TODO: ControlFlowGraph does not recurse into local functions and lambda anonymous functions, how to grab those? 
+        var controlFlowGraph = ControlFlowGraph.Create(context.Node, semanticModel);
+
+        if (controlFlowGraph == null)
+            return;
+
+        var stashMethod = akkaContext.AkkaCore.Actor.IStash.Stash!;
+        var stashInvocations = new Dictionary<BasicBlock, int>();
+
+        // Track Stash.Stash() calls inside each blocks
+        foreach (var block in controlFlowGraph.Blocks)
+        {
+            AnalyzeBlock(block, stashMethod, stashInvocations);
+        }
+
+        var entryBlock = controlFlowGraph.Blocks.First(b => b.Kind == BasicBlockKind.Entry);
+        RecurseBlocks(entryBlock, stashInvocations, 0);
+    }
+
+    private static void AnalyzeBlock(BasicBlock block, IMethodSymbol stashMethod, Dictionary<BasicBlock, int> stashInvocations)
+    {
+        var stashInvocationCount = 0;
+
+        foreach (var operation in block.Descendants())
+        {
+            switch (operation)
+            {
+                case IInvocationOperation invocation:
+                    if(SymbolEqualityComparer.Default.Equals(invocation.TargetMethod, stashMethod))
+                        stashInvocationCount++;
+                    break;
+
+                case IFlowAnonymousFunctionOperation flow:
+                    // TODO: check for flow anonymous lambda function invocation
+                    break;
+
+                // TODO: check for local function invocation
+            }
+        }
+
+        if(stashInvocationCount > 0)
+            stashInvocations.Add(block, stashInvocationCount);
+    }
+
+    private static void RecurseBlocks(BasicBlock block, Dictionary<BasicBlock, int> stashInvocations, int totalInvocations)
+    {
+        if (stashInvocations.TryGetValue(block, out var blockInvocation))
+        {
+            totalInvocations += blockInvocation;
+        }
+
+        if (totalInvocations > 1)
+        {
+            // TODO: report diagnostic
+        }
+
+        if(block.ConditionalSuccessor is { Destination: not null })
+            RecurseBlocks(block.ConditionalSuccessor.Destination, stashInvocations, totalInvocations);
+
+        if(block.FallThroughSuccessor is { Destination: not null })
+            RecurseBlocks(block.FallThroughSuccessor.Destination, stashInvocations, totalInvocations);
+    }
+}
+

src/Akka.Analyzers/Context/Core/Actor/ActorSymbolFactory.cs

@@ -51,4 +51,8 @@ public static class ActorSymbolFactory
     public static INamedTypeSymbol? TimerSchedulerInterface(Compilation compilation)
         => Guard.AssertIsNotNull(compilation)
             .GetTypeByMetadataName($"{AkkaActorNamespace}.ITimerScheduler");
+
+    public static INamedTypeSymbol? StashInterface(Compilation compilation)
+        => Guard.AssertIsNotNull(compilation)
+            .GetTypeByMetadataName($"{AkkaActorNamespace}.IStash");
 }

src/Akka.Analyzers/Context/Core/Actor/AkkaCoreActorContext.cs

@@ -23,6 +23,7 @@ private EmptyAkkaCoreActorContext() { }
     public INamedTypeSymbol? ITellSchedulerType => null;
     public INamedTypeSymbol? ActorRefsType => null;
     public INamedTypeSymbol? ITimerSchedulerType => null;
+    public INamedTypeSymbol? IStashType => null;
 
     public IGracefulStopSupportContext GracefulStopSupportSupport => EmptyGracefulStopSupportContext.Instance;
     public IIndirectActorProducerContext IIndirectActorProducer => EmptyIndirectActorProducerContext.Instance;
@@ -33,6 +34,7 @@ private EmptyAkkaCoreActorContext() { }
     public ITellSchedulerInterfaceContext ITellScheduler => EmptyTellSchedulerInterfaceContext.Instance;
     public IActorRefsContext ActorRefs => EmptyActorRefsContext.Empty;
     public ITimerSchedulerContext ITimerScheduler => EmptyTimerSchedulerContext.Instance;
+    public IStashContext IStash => EmptyStashContext.Instance;
 }
 
 public sealed class AkkaCoreActorContext : IAkkaCoreActorContext
@@ -47,6 +49,7 @@ public sealed class AkkaCoreActorContext : IAkkaCoreActorContext
     private readonly Lazy<INamedTypeSymbol?> _lazyTellSchedulerInterface;
     private readonly Lazy<INamedTypeSymbol?> _lazyActorRefsType;
     private readonly Lazy<INamedTypeSymbol?> _lazyITimerSchedulerType;
+    private readonly Lazy<INamedTypeSymbol?> _lazyIStashType;
 
     private readonly Lazy<IGracefulStopSupportContext> _lazyGracefulStopSupport;
     private readonly Lazy<IIndirectActorProducerContext> _lazyIIndirectActorProducer;
@@ -67,6 +70,7 @@ private AkkaCoreActorContext(Compilation compilation)
         _lazyTellSchedulerInterface = new Lazy<INamedTypeSymbol?>(() => ActorSymbolFactory.TellSchedulerInterface(compilation));
         _lazyActorRefsType = new Lazy<INamedTypeSymbol?>(() => ActorSymbolFactory.ActorRefs(compilation));
         _lazyITimerSchedulerType = new Lazy<INamedTypeSymbol?>(() => ActorSymbolFactory.TimerSchedulerInterface(compilation));
+        _lazyIStashType = new Lazy<INamedTypeSymbol?>(() => ActorSymbolFactory.StashInterface(compilation));
 
         _lazyGracefulStopSupport = new Lazy<IGracefulStopSupportContext>(() => GracefulStopSupportContext.Get(this));
         _lazyIIndirectActorProducer = new Lazy<IIndirectActorProducerContext>(() => IndirectActorProducerContext.Get(this));
@@ -77,6 +81,7 @@ private AkkaCoreActorContext(Compilation compilation)
         ITellScheduler = TellSchedulerInterfaceContext.Get(compilation);
         ActorRefs = ActorRefsContext.Get(this);
         ITimerScheduler = TimerSchedulerContext.Get(this);
+        IStash = StashContext.Get(this);
     }
 
     public INamedTypeSymbol? ActorBaseType => _lazyActorBaseType.Value;
@@ -89,6 +94,7 @@ private AkkaCoreActorContext(Compilation compilation)
     public INamedTypeSymbol? ActorRefsType => _lazyActorRefsType.Value;
     public INamedTypeSymbol? GracefulStopSupportType => _lazyGracefulStopSupportType.Value;
     public INamedTypeSymbol? ITimerSchedulerType => _lazyITimerSchedulerType.Value;
+    public INamedTypeSymbol? IStashType => _lazyIStashType.Value;
 
     public IGracefulStopSupportContext GracefulStopSupportSupport => _lazyGracefulStopSupport.Value;
     public IIndirectActorProducerContext IIndirectActorProducer => _lazyIIndirectActorProducer.Value;
@@ -99,6 +105,7 @@ private AkkaCoreActorContext(Compilation compilation)
     public ITellSchedulerInterfaceContext ITellScheduler { get; }
     public IActorRefsContext ActorRefs { get; }
     public ITimerSchedulerContext ITimerScheduler { get; }
+    public IStashContext IStash { get; }
 
     public static IAkkaCoreActorContext Get(Compilation compilation)
         => new AkkaCoreActorContext(compilation);

src/Akka.Analyzers/Context/Core/Actor/IAkkaCoreActorContext.cs

@@ -22,6 +22,7 @@ public interface IAkkaCoreActorContext
     public INamedTypeSymbol? ITellSchedulerType { get; }
     public INamedTypeSymbol? ActorRefsType { get; }
     public INamedTypeSymbol? ITimerSchedulerType { get; } 
+    public INamedTypeSymbol? IStashType { get; }
 
     public IGracefulStopSupportContext GracefulStopSupportSupport { get; }
     public IIndirectActorProducerContext IIndirectActorProducer { get; }
@@ -32,4 +33,5 @@ public interface IAkkaCoreActorContext
     public ITellSchedulerInterfaceContext ITellScheduler { get; }
     public IActorRefsContext ActorRefs { get; }
     public ITimerSchedulerContext ITimerScheduler { get; }
+    public IStashContext IStash { get; }
 }