Skip to content

Commit

Permalink
AK1001: must close over Sender when using PipeTo (#5)
Browse files Browse the repository at this point in the history
* AK1001: must close over `Sender` when using `PipeTo`

Not 100% sure that this is an issue in newer versions of Akka.NET, but it's still a good idea to enforce this.

* added success cases

* added failure case

* fixed analyzer being referenced

* riding the strugglebus

* have first unhappy path passing

* parameterized span counts for failure cases
  • Loading branch information
Aaronontheweb authored Dec 28, 2023
1 parent ff49171 commit c12bd30
Show file tree
Hide file tree
Showing 4 changed files with 174 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
using Microsoft.CodeAnalysis;
using Verify = Akka.Analyzers.Tests.Utility.AkkaVerifier<Akka.Analyzers.MustCloseOverSenderWhenUsingPipeToAnalyzer>;

namespace Akka.Analyzers.Tests.Analyzers.AK1000;

public class MustCloseOverSenderWhenUsingPipeToSpecs
{
public static readonly TheoryData<string> SuccessCases = new()
{
// ReceiveActor using PipeTo with a closure inside a Receive<string> block
@"using Akka.Actor;
using System.Threading.Tasks;
public sealed class MyActor : ReceiveActor{
public MyActor(){
Receive<string>(str => {
async Task<int> LocalFunction(){
await Task.Delay(10);
return str.Length;
}
// correct use of closure
var sender = Sender;
LocalFunction().PipeTo(sender);
});
}
}",

// ReceiveActor using PipeTo calling a method from within a Receive<T> block
@"using Akka.Actor;
using System.Threading.Tasks;
public sealed class MyActor : ReceiveActor{
public MyActor(){
Receive<string>(str => {
Execute(str);
});
}
private void Execute(string str){
async Task<int> LocalFunction(){
await Task.Delay(10);
return str.Length;
}
// correct use of closure
var sender = Sender;
LocalFunction().PipeTo(sender);
}
};",

// Actor doing message handling in a Receive<T> block without PipeTo at all
@"using Akka.Actor;
public sealed class MyActor : ReceiveActor{
public MyActor(){
Receive<string>(str => {
Sender.Tell(str); // shouldn't flag this
});
}
}"
};

[Theory]
[MemberData(nameof(SuccessCases))]
public async Task SuccessCase(string testCode)
{
await Verify.VerifyAnalyzer(testCode).ConfigureAwait(true);
}

public static readonly TheoryData<(string testData, (int startLine, int startColumn, int endLine, int endColumn) spanData)> FailureCases = new()
{
// Receive actor using PipeTo without a closure inside a Receive<T> block
(@"using Akka.Actor;
using System.Threading.Tasks;
public sealed class MyActor : ReceiveActor{
public MyActor(){
Receive<string>(str => {
async Task<int> LocalFunction(){
await Task.Delay(10);
return str.Length;
}
// incorrect use of closure
LocalFunction().PipeTo(Sender);
});
}
}", (14, 37, 14, 43)),

// UntypedActor using PipeTo without a closure inside a OnReceive block
(@"using Akka.Actor;
using System.Threading.Tasks;
public sealed class MyActor : UntypedActor{
protected override void OnReceive(object message){
async Task<int> LocalFunction(){
await Task.Delay(10);
return message.ToString().Length;
}
// incorrect use of closure
LocalFunction().PipeTo(Sender);
}
}", (13, 33, 13, 39)),
};

[Theory]
[MemberData(nameof(FailureCases))]
public async 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)
.WithArguments("Sender")
.WithSeverity(DiagnosticSeverity.Error);

await Verify.VerifyAnalyzer(d.testCode, expected).ConfigureAwait(true);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@
// </copyright>
// -----------------------------------------------------------------------

using Akka.Analyzers.Tests.Utility;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Testing;
using Microsoft.CodeAnalysis.Testing;
using static Akka.Analyzers.RuleDescriptors;
using Verify = Akka.Analyzers.Tests.Utility.AkkaVerifier<Akka.Analyzers.MustNotUseNewKeywordOnActorsAnalyzer>;

namespace Akka.Analyzers.Tests.Analyzers.AK1000;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;

namespace Akka.Analyzers;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class MustCloseOverSenderWhenUsingPipeToAnalyzer()
: AkkaDiagnosticAnalyzer(RuleDescriptors.Ak1001CloseOverSenderUsingPipeTo)
{
public override void AnalyzeCompilation(CompilationStartAnalysisContext context, AkkaContext akkaContext)
{
Guard.AssertIsNotNull(context);
Guard.AssertIsNotNull(akkaContext);

context.RegisterSyntaxNodeAction(ctx =>
{
var invocationExpr = (InvocationExpressionSyntax)ctx.Node;

// Check if it's a PipeTo method call
if (invocationExpr.Expression is MemberAccessExpressionSyntax { Name.Identifier.ValueText: "PipeTo" } memberAccessExpr)
{
// Check if the containing type is an Akka.NET actor
var containingType = ctx.SemanticModel.GetEnclosingSymbol(invocationExpr.SpanStart)?.ContainingType;
if (containingType != null && containingType.IsActorBaseSubclass(akkaContext))
{
// Check if 'this.Sender' is used in the arguments
foreach (var arg in invocationExpr.ArgumentList.Arguments)
{
var symbol = ctx.SemanticModel.GetSymbolInfo(arg.Expression).Symbol;
if (IsThisSenderSymbol(symbol, akkaContext))
{
var diagnostic = Diagnostic.Create(RuleDescriptors.Ak1001CloseOverSenderUsingPipeTo, memberAccessExpr.Name.GetLocation());
ctx.ReportDiagnostic(diagnostic);
break; // Report only once per invocation
}
}
}
}
}, SyntaxKind.InvocationExpression);
}

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);
}
}
2 changes: 1 addition & 1 deletion src/Akka.Analyzers/Utility/CodeAnalysisExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public static bool IsActorBaseSubclass(this INamedTypeSymbol typeSymbol, AkkaCon
if(akkaContext.AkkaCore.ActorBaseType is null)
return false;

var currentBaseType = typeSymbol.BaseType;
var currentBaseType = typeSymbol;
while (currentBaseType != null)
{
if (SymbolEqualityComparer.Default.Equals(currentBaseType, akkaContext.AkkaCore.ActorBaseType))
Expand Down

0 comments on commit c12bd30

Please sign in to comment.