Ak1008 take3

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

@@ -0,0 +1,304 @@
+// -----------------------------------------------------------------------
+//  <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 Microsoft.CodeAnalysis.Testing;
+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
+        {
+            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!;
+        }
+        """,
+
+        // Stash calls inside 2 different code branch (switch statements)
+        """
+        // 05
+        using Akka.Actor;
+
+        public sealed class MyActor : ReceiveActor, IWithStash
+        {
+            public MyActor(int n)
+            {
+                Receive<string>(str =>
+                {
+                    switch(str){
+                        case null:
+                            Stash!.Stash();
+                            break;
+                        default:
+                            Stash!.Stash(); // should not flag this
+                            break;
+                    }
+                });
+            }
+
+            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!;
+                }
+                """, [
+                    (12, 13, 12, 26),
+                    (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),
+                    (14, 13, 14, 26)]),
+
+            // UntypedActor invoking Stash() twice without branching
+            (
+                """
+                // 03
+                using Akka.Actor;
+
+                public class MyUntypedActor : UntypedActor, IWithStash
+                {
+                    protected override void OnReceive(object message)
+                    {
+                        Stash.Stash();
+                        Stash.Stash(); // Error
+                    }
+
+                    public IStash Stash { get; set; } = null!;
+                }
+                """, [(8, 9, 8, 22),
+                    (9, 9, 9, 22)]),
+            // UntypedActor invoking Stash() twice with a switch, but one is outside
+            (
+                """
+                // 04
+                using Akka.Actor;
+
+                public class MyUntypedActor : UntypedActor, IWithStash
+                {
+                    protected override void OnReceive(object message)
+                    {
+                        Stash.Stash();
+
+                        switch(message)
+                        {
+                            case string s:
+                                Stash.Stash(); // Error
+                                break;
+                        }
+                    }
+
+                    public IStash Stash { get; set; } = null!;
+                }
+                """, [(8, 9, 8, 22),
+                    (13, 17, 13, 30)]),
+
+            // UntypedActor invoking Stash() twice with a switch, but one is outside
+            (
+                """
+                // 05
+                using Akka.Actor;
+
+                public class MyUntypedActor : UntypedActor, IWithStash
+                {
+                    protected override void OnReceive(object message)
+                    {
+                        Stash.Stash();
+
+                        if(message is string s)
+                        {
+                            Stash.Stash(); // Error
+                        }
+                    }
+
+                    public IStash Stash { get; set; } = null!;
+                }
+                """, [(8, 9, 8, 22),
+                    (12, 17, 12, 30)]),
+        };
+
+    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)
+    {
+        List<DiagnosticResult> expectedResults = new();
+
+        foreach(var (startLine, startColumn, endLine, endColumn) in d.spanData)
+        {
+            var expected = Verify.Diagnostic().WithSpan(startLine, startColumn, endLine, endColumn).WithSeverity(DiagnosticSeverity.Error);
+            expectedResults.Add(expected);
+        }
+
+        return Verify.VerifyAnalyzer(d.testCode, expectedResults.ToArray());
+    }
+}

src/Akka.Analyzers/AK1000/MustNotInvokeStashMoreThanOnceAnalyzer.cs

@@ -0,0 +1,84 @@
+// -----------------------------------------------------------------------
+//  <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.CSharp.Syntax;
+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);
+
+        // For lambdas, namely Receive<T> and ReceiveAny
+        context.RegisterSyntaxNodeAction(ctx =>
+        {
+            var node = ctx.Node;
+            var semanticModel = ctx.SemanticModel;
+            var akkaCore = akkaContext.AkkaCore;
+            var stashMethod = akkaCore.Actor.IStash.Stash!;
+
+            // First: need to check if this method / lambda is declared in an ActorBase subclass
+            var symbol = semanticModel.GetSymbolInfo(node).Symbol;
+            if (symbol is null || !symbol.ContainingType.IsActorBaseSubclass(akkaContext.AkkaCore))
+                return;
+
+            // Find all stash calls within the method or lambda
+            var stashCalls = node.DescendantNodes()
+                .OfType<InvocationExpressionSyntax>()
+                .Where(c => IsStashInvocation(semanticModel, c, stashMethod))
+                .ToList();
+
+            if (stashCalls.Count < 2)
+                return;
+
+            // Flag all stash calls if they are in the same block without branching
+            var groupedByBlock = stashCalls
+                .GroupBy(GetContainingBlock)
+                .Where(group => group.Key != null && group.Count() > 1)
+                .SelectMany(group => group)
+                .ToList();
+
+            // Report all calls in the same block
+            foreach (var call in groupedByBlock)
+            {
+                ReportDiagnostic(call, ctx);
+            }
+        }, SyntaxKind.MethodDeclaration, SyntaxKind.InvocationExpression);
+        return;
+
+        static void ReportDiagnostic(InvocationExpressionSyntax call, SyntaxNodeAnalysisContext ctx)
+        {
+            var diagnostic = Diagnostic.Create(
+                descriptor: RuleDescriptors.Ak1008MustNotInvokeStashMoreThanOnce,
+                location: call.GetLocation());
+            ctx.ReportDiagnostic(diagnostic);
+        }
+    }
+
+    private static bool IsStashInvocation(SemanticModel model, InvocationExpressionSyntax invocation,
+        IMethodSymbol stashMethod)
+    {
+        var symbol = model.GetSymbolInfo(invocation).Symbol;
+        return SymbolEqualityComparer.Default.Equals(symbol, stashMethod);
+    }
+
+    private static SyntaxNode? GetContainingBlock(SyntaxNode node)
+    {
+        return node.AncestorsAndSelf()
+            .FirstOrDefault(n => n is BlockSyntax || n is SwitchSectionSyntax || n is MethodDeclarationSyntax);
+    }
+}