From e601f33e2643c69b147c14b4c5f8096ffdd1c43f Mon Sep 17 00:00:00 2001 From: Gregorius Soedharmo Date: Wed, 17 Jan 2024 04:55:12 +0700 Subject: [PATCH] Ak1001 `Context.Sender` fix (#54) * AK2001: harden so rule only runs for Akka.NET v1.5.15 and later * fixed package warning * added failure cases for #52 * added fix to Analyzer * Fix bad expected value --------- Co-authored-by: Aaron Stannard --- ...eOverSenderWhenUsingPipeToAnalyzerSpecs.cs | 42 +++++++++++++++- ...loseOverSenderWhenUsingPipeToFixerSpecs.cs | 50 +++++++++++++++++++ ...tCloseOverSenderWhenUsingPipeToAnalyzer.cs | 9 ++-- 3 files changed, 97 insertions(+), 4 deletions(-) diff --git a/src/Akka.Analyzers.Tests/Analyzers/AK1000/MustCloseOverSenderWhenUsingPipeToAnalyzerSpecs.cs b/src/Akka.Analyzers.Tests/Analyzers/AK1000/MustCloseOverSenderWhenUsingPipeToAnalyzerSpecs.cs index 6ebf389..2989ff5 100644 --- a/src/Akka.Analyzers.Tests/Analyzers/AK1000/MustCloseOverSenderWhenUsingPipeToAnalyzerSpecs.cs +++ b/src/Akka.Analyzers.Tests/Analyzers/AK1000/MustCloseOverSenderWhenUsingPipeToAnalyzerSpecs.cs @@ -94,7 +94,18 @@ async Task LocalFunction(){ LocalFunction().PipeTo(Sender); } } - """ + """, + // Replying to Sender using Context.Sender + @"using Akka.Actor; + + public sealed class MyActor : ReceiveActor{ + + public MyActor(){ + Receive(str => { + Context.Sender.Tell(str); // shouldn't flag this + }); + } + }", }; public static readonly @@ -194,4 +205,33 @@ public Task FailureCase( return Verify.VerifyAnalyzer(d.testCode, expected); } + + [Fact(DisplayName = "Should detect missing closure when using Context.Sender instead of this.Sender")] + public Task FailureCaseWithContextSender() + { + var code = """ + using Akka.Actor; + using System.Threading.Tasks; + + public sealed class MyActor : UntypedActor{ + + protected override void OnReceive(object message){ + async Task LocalFunction(){ + await Task.Delay(10); + return message.ToString().Length; + } + + // incorrect use of closures + LocalFunction().PipeTo(Context.Sender); + } + } + """; + + var expected = Verify.Diagnostic() + .WithSpan(13, 25, 13, 31) + .WithArguments("Context.Sender") + .WithSeverity(DiagnosticSeverity.Error); + + return Verify.VerifyAnalyzer(code, expected); + } } \ No newline at end of file diff --git a/src/Akka.Analyzers.Tests/Fixes/AK1000/MustCloseOverSenderWhenUsingPipeToFixerSpecs.cs b/src/Akka.Analyzers.Tests/Fixes/AK1000/MustCloseOverSenderWhenUsingPipeToFixerSpecs.cs index 621da46..9c8d22f 100644 --- a/src/Akka.Analyzers.Tests/Fixes/AK1000/MustCloseOverSenderWhenUsingPipeToFixerSpecs.cs +++ b/src/Akka.Analyzers.Tests/Fixes/AK1000/MustCloseOverSenderWhenUsingPipeToFixerSpecs.cs @@ -5,6 +5,7 @@ // ----------------------------------------------------------------------- using Akka.Analyzers.Fixes; +using Microsoft.CodeAnalysis; using Verify = Akka.Analyzers.Tests.Utility.AkkaVerifier; namespace Akka.Analyzers.Tests.Fixes.AK1000; @@ -260,4 +261,53 @@ async Task LocalFunction(){ return Verify.VerifyCodeFix(before, after, MustCloseOverSenderWhenUsingPipeToFixer.Key_FixPipeToSender, expectedDiagnostic); } + + [Fact(DisplayName = "Should fix missing closure when using Context.Sender instead of this.Sender")] + public Task FailureCaseWithContextSender() + { + var before = """ + using Akka.Actor; + using System.Threading.Tasks; + + public sealed class MyActor : UntypedActor{ + + protected override void OnReceive(object message){ + async Task LocalFunction(){ + await Task.Delay(10); + return message.ToString().Length; + } + + // incorrect use of closures + LocalFunction().PipeTo(Context.Sender); + } + } + """; + + var after = """ + using Akka.Actor; + using System.Threading.Tasks; + + public sealed class MyActor : UntypedActor{ + + protected override void OnReceive(object message){ + async Task LocalFunction(){ + await Task.Delay(10); + return message.ToString().Length; + } + var sender = this.Sender; + + // incorrect use of closures + LocalFunction().PipeTo(sender); + } + } + """; + + var expected = Verify.Diagnostic() + .WithSpan(13, 25, 13, 31) + .WithArguments("Context.Sender") + .WithSeverity(DiagnosticSeverity.Error); + + return Verify.VerifyCodeFix(before, after, MustCloseOverSenderWhenUsingPipeToFixer.Key_FixPipeToSender, + expected); + } } \ No newline at end of file diff --git a/src/Akka.Analyzers/AK1000/MustCloseOverSenderWhenUsingPipeToAnalyzer.cs b/src/Akka.Analyzers/AK1000/MustCloseOverSenderWhenUsingPipeToAnalyzer.cs index 803385a..c907b2f 100644 --- a/src/Akka.Analyzers/AK1000/MustCloseOverSenderWhenUsingPipeToAnalyzer.cs +++ b/src/Akka.Analyzers/AK1000/MustCloseOverSenderWhenUsingPipeToAnalyzer.cs @@ -51,8 +51,11 @@ public override void AnalyzeCompilation(CompilationStartAnalysisContext context, private static bool IsThisSenderSymbol(ISymbol? symbol, AkkaContext akkaContext) { - // Check if the symbol is 'this.Sender' - return symbol is { Name: "Sender", ContainingType.BaseType: not null } && - symbol.ContainingType.IsActorBaseSubclass(akkaContext); + // Check if the symbol is 'this.Sender' or 'Context.Sender' + return (symbol is { Name: "Sender", ContainingType.BaseType: not null } && + symbol.ContainingType.IsActorBaseSubclass(akkaContext)) || + (symbol is IPropertySymbol propertySymbol && + propertySymbol.Name == "Sender" && + SymbolEqualityComparer.Default.Equals(propertySymbol.ContainingType, akkaContext.AkkaCore.ActorContextType)); } } \ No newline at end of file