From b365fd8b9032b9ffea1da91dc85c00c8633391e3 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Tue, 16 Jan 2024 11:14:57 -0600 Subject: [PATCH 1/5] AK2001: harden so rule only runs for Akka.NET v1.5.15 and later --- .../Akka.Analyzers.Tests.csproj | 3 +-- ...loseOverSenderWhenUsingPipeToAnalyzerSpecs.cs | 4 ++-- ...ledMessagesInMessageExtractorAnalyzerSpecs.cs | 5 +++++ .../Utility/ReferenceAssembliesHelper.cs | 2 +- ...dledMessagesInsideMessageExtractorAnalyzer.cs | 16 ++++++++++------ 5 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/Akka.Analyzers.Tests/Akka.Analyzers.Tests.csproj b/src/Akka.Analyzers.Tests/Akka.Analyzers.Tests.csproj index 2db5c04..fa35657 100644 --- a/src/Akka.Analyzers.Tests/Akka.Analyzers.Tests.csproj +++ b/src/Akka.Analyzers.Tests/Akka.Analyzers.Tests.csproj @@ -11,9 +11,8 @@ - - + diff --git a/src/Akka.Analyzers.Tests/Analyzers/AK1000/MustCloseOverSenderWhenUsingPipeToAnalyzerSpecs.cs b/src/Akka.Analyzers.Tests/Analyzers/AK1000/MustCloseOverSenderWhenUsingPipeToAnalyzerSpecs.cs index 8b64ff0..6ebf389 100644 --- a/src/Akka.Analyzers.Tests/Analyzers/AK1000/MustCloseOverSenderWhenUsingPipeToAnalyzerSpecs.cs +++ b/src/Akka.Analyzers.Tests/Analyzers/AK1000/MustCloseOverSenderWhenUsingPipeToAnalyzerSpecs.cs @@ -184,7 +184,7 @@ public async Task SuccessCase(string testCode) [Theory] [MemberData(nameof(FailureCases))] - public async Task FailureCase( + public Task FailureCase( (string testCode, (int startLine, int startColumn, int endLine, int endColumn) spanData) d) { var expected = Verify.Diagnostic() @@ -192,6 +192,6 @@ public async Task FailureCase( .WithArguments("Sender") .WithSeverity(DiagnosticSeverity.Error); - await Verify.VerifyAnalyzer(d.testCode, expected).ConfigureAwait(true); + return Verify.VerifyAnalyzer(d.testCode, expected); } } \ No newline at end of file diff --git a/src/Akka.Analyzers.Tests/Analyzers/AK2000/MustNotHandleAutomaticallyHandledMessagesInMessageExtractorAnalyzerSpecs.cs b/src/Akka.Analyzers.Tests/Analyzers/AK2000/MustNotHandleAutomaticallyHandledMessagesInMessageExtractorAnalyzerSpecs.cs index d0631b6..2e389ef 100644 --- a/src/Akka.Analyzers.Tests/Analyzers/AK2000/MustNotHandleAutomaticallyHandledMessagesInMessageExtractorAnalyzerSpecs.cs +++ b/src/Akka.Analyzers.Tests/Analyzers/AK2000/MustNotHandleAutomaticallyHandledMessagesInMessageExtractorAnalyzerSpecs.cs @@ -294,6 +294,11 @@ public string ShardId(object message) { return Random.Shared.Next(0,10).ToString(); } + + public string ShardId(string entityId, object message) + { + return Random.Shared.Next(0,10).ToString(); + } } """, new[] { diff --git a/src/Akka.Analyzers.Tests/Utility/ReferenceAssembliesHelper.cs b/src/Akka.Analyzers.Tests/Utility/ReferenceAssembliesHelper.cs index 0a14ebf..7104438 100644 --- a/src/Akka.Analyzers.Tests/Utility/ReferenceAssembliesHelper.cs +++ b/src/Akka.Analyzers.Tests/Utility/ReferenceAssembliesHelper.cs @@ -33,7 +33,7 @@ static ReferenceAssembliesHelper() // TODO: does this bring all other transitive dependencies? CurrentAkka = defaultAssemblies.AddPackages( - [new PackageIdentity("Akka", "1.5.14"), new PackageIdentity("Akka.Cluster.Sharding", "1.5.14")] + [new PackageIdentity("Akka.Cluster.Sharding", "1.5.15")] ); } } \ No newline at end of file diff --git a/src/Akka.Analyzers/AK2000/MustNotUseAutomaticallyHandledMessagesInsideMessageExtractorAnalyzer.cs b/src/Akka.Analyzers/AK2000/MustNotUseAutomaticallyHandledMessagesInsideMessageExtractorAnalyzer.cs index f496a0b..49c01cd 100644 --- a/src/Akka.Analyzers/AK2000/MustNotUseAutomaticallyHandledMessagesInsideMessageExtractorAnalyzer.cs +++ b/src/Akka.Analyzers/AK2000/MustNotUseAutomaticallyHandledMessagesInsideMessageExtractorAnalyzer.cs @@ -15,6 +15,16 @@ namespace Akka.Analyzers; public class MustNotUseAutomaticallyHandledMessagesInsideMessageExtractorAnalyzer() : AkkaDiagnosticAnalyzer(RuleDescriptors.Ak2001DoNotUseAutomaticallyHandledMessagesInShardMessageExtractor) { + private static readonly Version RelevantVersion = new(1, 5, 15, 0); + + protected override bool ShouldAnalyze(AkkaContext akkaContext) + { + Guard.AssertIsNotNull(akkaContext); + + // Akka.Cluster.Sharding has to be installed and the version of it used has to be greater than or equal to v1.5.15 + return akkaContext.HasAkkaClusterShardingInstalled && akkaContext.AkkaClusterSharding.Version >= RelevantVersion; + } + public override void AnalyzeCompilation(CompilationStartAnalysisContext context, AkkaContext akkaContext) { Guard.AssertIsNotNull(context); @@ -22,17 +32,11 @@ public override void AnalyzeCompilation(CompilationStartAnalysisContext context, context.RegisterSyntaxNodeAction(ctx => { - if (akkaContext.HasAkkaClusterShardingInstalled == false) - return; // exit early if we don't have Akka.Cluster.Sharding installed - AnalyzeMethodDeclaration(ctx, akkaContext); }, SyntaxKind.MethodDeclaration); context.RegisterSyntaxNodeAction(ctx => { - if (akkaContext.HasAkkaClusterShardingInstalled == false) - return; // exit early if we don't have Akka.Cluster.Sharding installed - var invocationExpr = (InvocationExpressionSyntax)ctx.Node; var semanticModel = ctx.SemanticModel; if (semanticModel.GetSymbolInfo(invocationExpr).Symbol is not IMethodSymbol methodSymbol) From 830717160fc614ef3e7d5b65c8b5e9bcba2d003b Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Tue, 16 Jan 2024 11:17:18 -0600 Subject: [PATCH 2/5] fixed package warning --- src/Akka.Analyzers.Tests/Akka.Analyzers.Tests.csproj | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Akka.Analyzers.Tests/Akka.Analyzers.Tests.csproj b/src/Akka.Analyzers.Tests/Akka.Analyzers.Tests.csproj index fa35657..36c5380 100644 --- a/src/Akka.Analyzers.Tests/Akka.Analyzers.Tests.csproj +++ b/src/Akka.Analyzers.Tests/Akka.Analyzers.Tests.csproj @@ -10,8 +10,6 @@ - - From a197186db6104796ab31003c05ebe458da77a5e7 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Tue, 16 Jan 2024 11:25:04 -0600 Subject: [PATCH 3/5] added failure cases for #52 --- ...eOverSenderWhenUsingPipeToAnalyzerSpecs.cs | 29 +++++++++++ ...loseOverSenderWhenUsingPipeToFixerSpecs.cs | 50 +++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/src/Akka.Analyzers.Tests/Analyzers/AK1000/MustCloseOverSenderWhenUsingPipeToAnalyzerSpecs.cs b/src/Akka.Analyzers.Tests/Analyzers/AK1000/MustCloseOverSenderWhenUsingPipeToAnalyzerSpecs.cs index 6ebf389..eddaefe 100644 --- a/src/Akka.Analyzers.Tests/Analyzers/AK1000/MustCloseOverSenderWhenUsingPipeToAnalyzerSpecs.cs +++ b/src/Akka.Analyzers.Tests/Analyzers/AK1000/MustCloseOverSenderWhenUsingPipeToAnalyzerSpecs.cs @@ -194,4 +194,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..23a33f9 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; + } + + // incorrect use of closures + var sender = this.Sender; + 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 From 0a873f59ee55356f21c434c2ded4a2d7ba25d73c Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Tue, 16 Jan 2024 11:37:32 -0600 Subject: [PATCH 4/5] added fix to Analyzer --- ...stCloseOverSenderWhenUsingPipeToAnalyzerSpecs.cs | 13 ++++++++++++- .../MustCloseOverSenderWhenUsingPipeToAnalyzer.cs | 9 ++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/Akka.Analyzers.Tests/Analyzers/AK1000/MustCloseOverSenderWhenUsingPipeToAnalyzerSpecs.cs b/src/Akka.Analyzers.Tests/Analyzers/AK1000/MustCloseOverSenderWhenUsingPipeToAnalyzerSpecs.cs index eddaefe..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 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 From af6a1c141388de444bf7df2edbe356c19eb3f01a Mon Sep 17 00:00:00 2001 From: Gregorius Soedharmo Date: Wed, 17 Jan 2024 04:39:24 +0700 Subject: [PATCH 5/5] Fix bad expected value --- .../AK1000/MustCloseOverSenderWhenUsingPipeToFixerSpecs.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Akka.Analyzers.Tests/Fixes/AK1000/MustCloseOverSenderWhenUsingPipeToFixerSpecs.cs b/src/Akka.Analyzers.Tests/Fixes/AK1000/MustCloseOverSenderWhenUsingPipeToFixerSpecs.cs index 23a33f9..9c8d22f 100644 --- a/src/Akka.Analyzers.Tests/Fixes/AK1000/MustCloseOverSenderWhenUsingPipeToFixerSpecs.cs +++ b/src/Akka.Analyzers.Tests/Fixes/AK1000/MustCloseOverSenderWhenUsingPipeToFixerSpecs.cs @@ -294,9 +294,9 @@ async Task LocalFunction(){ await Task.Delay(10); return message.ToString().Length; } + var sender = this.Sender; // incorrect use of closures - var sender = this.Sender; LocalFunction().PipeTo(sender); } }