From da529cdb47b2981f25e617f8a92b592d5bdfbba1 Mon Sep 17 00:00:00 2001 From: Gregorius Soedharmo Date: Wed, 24 Jan 2024 04:48:56 +0700 Subject: [PATCH 1/3] Add ReceiveAsync with sync lambda analyzer and code fix --- ...tUseReceiveAsyncWithoutAsyncLambdaFixer.cs | 113 +++++ ...iveAsyncWithoutAsyncLambdaAnalyzerSpecs.cs | 346 +++++++++++++++ ...eceiveAsyncWithoutAsyncLambdaFixerSpecs.cs | 404 ++++++++++++++++++ ...eReceiveAsyncWithoutAsyncLambdaAnalyzer.cs | 86 ++++ src/Akka.Analyzers/Utility/RuleDescriptors.cs | 8 + 5 files changed, 957 insertions(+) create mode 100644 src/Akka.Analyzers.Fixes/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixer.cs create mode 100644 src/Akka.Analyzers.Tests/Analyzers/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaAnalyzerSpecs.cs create mode 100644 src/Akka.Analyzers.Tests/Fixes/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixerSpecs.cs create mode 100644 src/Akka.Analyzers/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaAnalyzer.cs diff --git a/src/Akka.Analyzers.Fixes/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixer.cs b/src/Akka.Analyzers.Fixes/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixer.cs new file mode 100644 index 0000000..ac1172a --- /dev/null +++ b/src/Akka.Analyzers.Fixes/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixer.cs @@ -0,0 +1,113 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2013-2024 .NET Foundation +// +// ----------------------------------------------------------------------- + +using System.Composition; +using System.Runtime.CompilerServices; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Editing; + +namespace Akka.Analyzers.Fixes; + +[ExportCodeFixProvider(LanguageNames.CSharp)] +[Shared] +public class ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixer() + : BatchedCodeFixProvider(RuleDescriptors.Ak1003ShouldNotUseReceiveAsyncSynchronously.Id) +{ + public const string Key_FixReceiveAsyncWithoutAsync = "AK1003_FixReceiveAsyncWithoutAsync"; + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + if (root is null) + return; + + var diagnostic = context.Diagnostics.FirstOrDefault(); + if (diagnostic is null) + return; + var diagnosticSpan = diagnostic.Location.SourceSpan; + + // Find the ReceiveAsync() invocation identified by the diagnostic + var invocationExpr = root.FindToken(diagnosticSpan.Start).Parent?.AncestorsAndSelf().OfType().First(); + if (invocationExpr is null) + return; + + context.RegisterCodeFix( + CodeAction.Create( + title: "Replace ReceiveAsync or ReceiveAnyAsync with Receive or ReceiveAny", + createChangedDocument: c => ReplaceReceiveAsyncWithReceiveAsync(context.Document, invocationExpr, c), + equivalenceKey: Key_FixReceiveAsyncWithoutAsync), + diagnostic); + } + + private static async Task ReplaceReceiveAsyncWithReceiveAsync(Document document, + InvocationExpressionSyntax invocationExpr, CancellationToken cancellationToken) + { + var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); + if (semanticModel is null) + return document; + + // Get the method argument that matches a lambda expression of Func + var lambdaArg = invocationExpr.ArgumentList.Arguments + .FirstOrDefault(arg => + arg.Expression is LambdaExpressionSyntax expr + && IsFuncOfTTask(semanticModel.GetTypeInfo(expr).ConvertedType)); + if(lambdaArg is null) + return document; + + var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); + + // Remove "Async" from the method name + var newMethodName = invocationExpr.Expression.ToString().Replace("Async", ""); + var newInvocationExpr = invocationExpr + .WithExpression(SyntaxFactory.ParseExpression(newMethodName)) + .WithTriviaFrom(invocationExpr); + + var lambdaExpr = (LambdaExpressionSyntax)lambdaArg.Expression; + + // Remove 'async' keyword + var newLambdaExpr = lambdaExpr.WithAsyncKeyword(default); + + // Remove 'return Task.CompletedTask;' + if (lambdaExpr.Body is BlockSyntax blockSyntax) + { + var newBlockStatements = blockSyntax.Statements + .Where(stmt => + stmt is not ReturnStatementSyntax returnStmt + || returnStmt.Expression?.ToString() is not "Task.CompletedTask"); + var newBlock = SyntaxFactory.Block(newBlockStatements).WithTriviaFrom(blockSyntax); + newLambdaExpr = newLambdaExpr.WithBlock(newBlock); + } + + // replace old lambda argument expression content with the new one + var newLambdaArg = lambdaArg.WithExpression(newLambdaExpr); + var newArgumentList = SyntaxFactory.ArgumentList(invocationExpr.ArgumentList.Arguments.Replace(lambdaArg, newLambdaArg)); + + // replace original method invocation lambda argument with the modified one + newInvocationExpr = newInvocationExpr.WithArgumentList(newArgumentList); + + // replace original method invocation with the modified one + editor.ReplaceNode(invocationExpr, newInvocationExpr); + + return editor.GetChangedDocument(); + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static bool IsFuncOfTTask(ITypeSymbol? typeSymbol) + { + if (typeSymbol is not INamedTypeSymbol { DelegateInvokeMethod: not null } namedTypeSymbol) + return false; + + var delegateReturnType = namedTypeSymbol.DelegateInvokeMethod.ReturnType; + var delegateParameters = namedTypeSymbol.DelegateInvokeMethod.Parameters; + + return delegateReturnType is INamedTypeSymbol { Name: nameof(Task) } + && delegateParameters.Length == 1; + } +} \ No newline at end of file diff --git a/src/Akka.Analyzers.Tests/Analyzers/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaAnalyzerSpecs.cs b/src/Akka.Analyzers.Tests/Analyzers/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaAnalyzerSpecs.cs new file mode 100644 index 0000000..fe7e09f --- /dev/null +++ b/src/Akka.Analyzers.Tests/Analyzers/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaAnalyzerSpecs.cs @@ -0,0 +1,346 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2013-2024 .NET Foundation +// +// ----------------------------------------------------------------------- + +using Microsoft.CodeAnalysis; +using Verify = Akka.Analyzers.Tests.Utility.AkkaVerifier; + +namespace Akka.Analyzers.Tests.Analyzers.AK1000; + +public class ShouldNotUseReceiveAsyncWithoutAsyncLambdaAnalyzerSpecs +{ + public static readonly TheoryData SuccessCases = new() + { + // ReceiveActor using ReceiveAsync with async keyword on the async lambda expression +""" +// 1 +using Akka.Actor; +using System.Threading.Tasks; + +public sealed class MyActor : ReceiveActor +{ + public MyActor() + { + ReceiveAsync(async str => { + var sender = Sender; + sender.Tell(await LocalFunction()); + return; + + async Task LocalFunction() + { + await Task.Delay(10); + return str.Length; + } + }); + } +} +""", + +// ReceiveActor using ReceiveAsync with async keyword on the async lambda expression, alternate version +""" +// 2 +using Akka.Actor; +using System.Threading.Tasks; + +public sealed class MyActor : ReceiveActor +{ + public MyActor() + { + ReceiveAsync(str => true, async str => { + var sender = Sender; + sender.Tell(await LocalFunction()); + return; + + async Task LocalFunction() + { + await Task.Delay(10); + return str.Length; + } + }); + } +} +""", + + // ReceiveActor using ReceiveAsync with async keyword on the async lambda expression +""" +// 3 +using Akka.Actor; +using System.Threading.Tasks; + +public sealed class MyActor : ReceiveActor +{ + public MyActor(){ + ReceiveAsync(async str => { + await Execute(str, Sender); + }); + } + + private async Task Execute(string str, IActorRef sender){ + async Task LocalFunction(){ + await Task.Delay(10); + return str.Length; + } + + sender.Tell(await LocalFunction()); + } +} +""", + + // ReceiveActor using ReceiveAsync with async keyword on the async lambda expression, alternate version +""" +// 4 +using Akka.Actor; +using System.Threading.Tasks; + +public sealed class MyActor : ReceiveActor +{ + public MyActor(){ + ReceiveAsync(str => true, async str => { + await Execute(str, Sender); + }); + } + + private async Task Execute(string str, IActorRef sender){ + async Task LocalFunction(){ + await Task.Delay(10); + return str.Length; + } + + sender.Tell(await LocalFunction()); + } +} +""", + + // ReceiveActor using ReceiveAsync with async keyword on the one-liner async lambda expression +""" +// 5 +using Akka.Actor; +using System.Threading.Tasks; + +public sealed class MyActor : ReceiveActor +{ + public MyActor() + { + ReceiveAsync(async str => await Execute(str, Sender)); + } + + private async Task Execute(string str, IActorRef sender){ + async Task LocalFunction(){ + await Task.Delay(10); + return str.Length; + } + + sender.Tell(await LocalFunction()); + } +} +""", + + // ReceiveActor using ReceiveAsync with async keyword on the one-liner async lambda expression, alternate version +""" +// 6 +using Akka.Actor; +using System.Threading.Tasks; + +public sealed class MyActor : ReceiveActor +{ + public MyActor() + { + ReceiveAsync(str => true, async str => await Execute(str, Sender)); + } + + private async Task Execute(string str, IActorRef sender){ + async Task LocalFunction(){ + await Task.Delay(10); + return str.Length; + } + + sender.Tell(await LocalFunction()); + } +} +""", + + // ReceiveActor using ReceiveAsync with async keyword on the one-liner async lambda expression +""" +// 7 +using Akka.Actor; +using System.Threading.Tasks; + +public sealed class MyActor : ReceiveActor +{ + public MyActor() + { + ReceiveAsync(async str => Sender.Tell(await Execute(str))); + } + + private async Task Execute(string str){ + return str.Length; + } +} +""", + + // ReceiveActor using ReceiveAsync with async keyword on the one-liner async lambda expression, alternate version +""" +// 8 +using Akka.Actor; +using System.Threading.Tasks; + +public sealed class MyActor : ReceiveActor +{ + public MyActor() + { + ReceiveAsync(str => true, async str => Sender.Tell(await Execute(str))); + } + + private async Task Execute(string str){ + return str.Length; + } +} +""", + + // Identical ReceiveAsync and ReceiveAnyAsync method fingerprint in non-ReceiveActor class +""" +// 9 +using System; +using Akka.Actor; +using System.Threading.Tasks; + +public sealed class MyActor : UntypedActor +{ + public MyActor() + { + ReceiveAnyAsync(async o => Self.Tell(o)); + ReceiveAsync(async s => Self.Tell(s)); + } + + protected override void OnReceive(object message) { } + + protected void ReceiveAsync(Func handler, Predicate? shouldHandle = null) { } + protected void ReceiveAnyAsync(Func handler) { } +} +""", + + // ReceiveActor using ReceiveAsync with async method delegate, this needs to be handled by a different analyzer +""" +// 10 +using Akka.Actor; +using System.Threading.Tasks; + +public sealed class MyActor : ReceiveActor +{ + public MyActor() + { + ReceiveAsync(Handler); + } + + private async Task Handler(string s) { } +} +""" + }; + + public static readonly + TheoryData<(string testData, (int startLine, int startColumn, int endLine, int endColumn) spanData)> + FailureCases = new() + { + // ReceiveActor using ReceiveAsync without async keyword on the async lambda expression + ( +""" +// 1 +using Akka.Actor; +using System.Threading.Tasks; + +public sealed class MyActor : ReceiveActor +{ + public MyActor() + { + ReceiveAsync(str => + { + Sender.Tell(str); + return Task.CompletedTask; + }); + } +} +""", (9, 9, 13, 11)), + + // ReceiveActor using ReceiveAsync without async keyword on the async lambda expression, alternate version + ( +""" +// 2 +using Akka.Actor; +using System.Threading.Tasks; + +public sealed class MyActor : ReceiveActor +{ + public MyActor() + { + ReceiveAsync(str => true, str => + { + Sender.Tell(str); + return Task.CompletedTask; + }); + } +} +""", (9, 9, 13, 11)), + + // ReceiveActor using ReceiveAsync with async keyword on the async lambda expression + // but without any awaited code + ( +""" +// 3 +using Akka.Actor; +using System.Threading.Tasks; + +public sealed class MyActor : ReceiveActor +{ + public MyActor() + { + ReceiveAsync(async str => + { + Sender.Tell(str); + }); + } +} +""", (9, 9, 12, 11)), + + // ReceiveActor using ReceiveAsync with async keyword on the async lambda expression + // but without any awaited code, alternate version + ( +""" +// 4 +using Akka.Actor; +using System.Threading.Tasks; + +public sealed class MyActor : ReceiveActor +{ + public MyActor() + { + ReceiveAsync(str => true, async str => + { + Sender.Tell(str); + }); + } +} +""", (9, 9, 12, 11)), + }; + + [Theory] + [MemberData(nameof(SuccessCases))] + public async Task SuccessCase(string testCode) + { + await Verify.VerifyAnalyzer(testCode).ConfigureAwait(true); + } + + [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) + .WithArguments("lambda expression") + .WithSeverity(DiagnosticSeverity.Warning); + + return Verify.VerifyAnalyzer(d.testCode, expected); + } +} \ No newline at end of file diff --git a/src/Akka.Analyzers.Tests/Fixes/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixerSpecs.cs b/src/Akka.Analyzers.Tests/Fixes/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixerSpecs.cs new file mode 100644 index 0000000..0b25794 --- /dev/null +++ b/src/Akka.Analyzers.Tests/Fixes/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixerSpecs.cs @@ -0,0 +1,404 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2013-2024 .NET Foundation +// +// ----------------------------------------------------------------------- + +using Akka.Analyzers.Fixes; +using Microsoft.CodeAnalysis; +using Verify = Akka.Analyzers.Tests.Utility.AkkaVerifier; + +namespace Akka.Analyzers.Tests.Fixes.AK1000; + +public class ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixerSpecs +{ + [Fact] + public Task ConvertReceiveAsyncToReceive() + { + const string before = +""" +using Akka.Actor; +using System.Threading.Tasks; + +public sealed class MyActor : ReceiveActor +{ + public MyActor() + { + ReceiveAsync(str => + { + Sender.Tell(str); + return Task.CompletedTask; + }); + } +} +"""; + + const string after = +""" +using Akka.Actor; +using System.Threading.Tasks; + +public sealed class MyActor : ReceiveActor +{ + public MyActor() + { + Receive(str => + { + Sender.Tell(str); + }); + } +} +"""; + + var expectedDiagnostic = Verify.Diagnostic() + .WithSpan(8, 9, 12, 11) + .WithArguments("lambda expression"); + + return Verify.VerifyCodeFix(before, after, ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixer.Key_FixReceiveAsyncWithoutAsync, + expectedDiagnostic); + } + + [Fact] + public Task ConvertReceiveAsyncToReceiveAlternate() + { + const string before = + """ + using Akka.Actor; + using System.Threading.Tasks; + + public sealed class MyActor : ReceiveActor + { + public MyActor() + { + ReceiveAsync(str => true, str => + { + Sender.Tell(str); + return Task.CompletedTask; + }); + } + } + """; + + const string after = + """ + using Akka.Actor; + using System.Threading.Tasks; + + public sealed class MyActor : ReceiveActor + { + public MyActor() + { + Receive(str => true, str => + { + Sender.Tell(str); + }); + } + } + """; + + var expectedDiagnostic = Verify.Diagnostic() + .WithSpan(8, 9, 12, 11) + .WithArguments("lambda expression"); + + return Verify.VerifyCodeFix(before, after, ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixer.Key_FixReceiveAsyncWithoutAsync, + expectedDiagnostic); + } + + [Fact] + public Task ConvertReceiveAnyAsyncToReceiveAny() + { + const string before = + """ + using Akka.Actor; + using System.Threading.Tasks; + + public sealed class MyActor : ReceiveActor + { + public MyActor() + { + ReceiveAnyAsync(o => + { + Sender.Tell(o); + return Task.CompletedTask; + }); + } + } + """; + + const string after = + """ + using Akka.Actor; + using System.Threading.Tasks; + + public sealed class MyActor : ReceiveActor + { + public MyActor() + { + ReceiveAny(o => + { + Sender.Tell(o); + }); + } + } + """; + + var expectedDiagnostic = Verify.Diagnostic() + .WithSpan(8, 9, 12, 11) + .WithArguments("lambda expression"); + + return Verify.VerifyCodeFix(before, after, ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixer.Key_FixReceiveAsyncWithoutAsync, + expectedDiagnostic); + } + + [Fact] + public Task ConvertReceiveAsyncToReceiveAndRemoveAsync() + { + const string before = +""" +using Akka.Actor; +using System.Threading.Tasks; + +public sealed class MyActor : ReceiveActor +{ + public MyActor() + { + ReceiveAsync(async str => + { + Sender.Tell(str); + }); + } +} +"""; + + const string after = +""" +using Akka.Actor; +using System.Threading.Tasks; + +public sealed class MyActor : ReceiveActor +{ + public MyActor() + { + Receive(str => + { + Sender.Tell(str); + }); + } +} +"""; + + var expectedDiagnostic = Verify.Diagnostic() + .WithSpan(8, 9, 11, 11) + .WithArguments("lambda expression"); + + return Verify.VerifyCodeFix(before, after, ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixer.Key_FixReceiveAsyncWithoutAsync, + expectedDiagnostic); + } + + [Fact] + public Task ConvertReceiveAsyncToReceiveAndRemoveAsyncAlternate() + { + const string before = + """ + using Akka.Actor; + using System.Threading.Tasks; + + public sealed class MyActor : ReceiveActor + { + public MyActor() + { + ReceiveAsync(str => true, async str => + { + Sender.Tell(str); + }); + } + } + """; + + const string after = + """ + using Akka.Actor; + using System.Threading.Tasks; + + public sealed class MyActor : ReceiveActor + { + public MyActor() + { + Receive(str => true, str => + { + Sender.Tell(str); + }); + } + } + """; + + var expectedDiagnostic = Verify.Diagnostic() + .WithSpan(8, 9, 11, 11) + .WithArguments("lambda expression"); + + return Verify.VerifyCodeFix(before, after, ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixer.Key_FixReceiveAsyncWithoutAsync, + expectedDiagnostic); + } + + [Fact] + public Task ConvertReceiveAnyAsyncToReceiveAnyAndRemoveAsync() + { + const string before = + """ + using Akka.Actor; + using System.Threading.Tasks; + + public sealed class MyActor : ReceiveActor + { + public MyActor() + { + ReceiveAnyAsync(async o => + { + Sender.Tell(o); + }); + } + } + """; + + const string after = + """ + using Akka.Actor; + using System.Threading.Tasks; + + public sealed class MyActor : ReceiveActor + { + public MyActor() + { + ReceiveAny(o => + { + Sender.Tell(o); + }); + } + } + """; + + var expectedDiagnostic = Verify.Diagnostic() + .WithSpan(8, 9, 11, 11) + .WithArguments("lambda expression"); + + return Verify.VerifyCodeFix(before, after, ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixer.Key_FixReceiveAsyncWithoutAsync, + expectedDiagnostic); + } + + [Fact] + public Task ConvertBlocklessReceiveAsyncToReceiveAndRemoveAsync() + { + const string before = +""" +using Akka.Actor; +using System.Threading.Tasks; + +public sealed class MyActor : ReceiveActor +{ + public MyActor() + { + ReceiveAsync(async str => Sender.Tell(str)); + } +} +"""; + + const string after = +""" +using Akka.Actor; +using System.Threading.Tasks; + +public sealed class MyActor : ReceiveActor +{ + public MyActor() + { + Receive(str => Sender.Tell(str)); + } +} +"""; + + var expectedDiagnostic = Verify.Diagnostic() + .WithSpan(8, 9, 8, 60) + .WithArguments("lambda expression"); + + return Verify.VerifyCodeFix(before, after, ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixer.Key_FixReceiveAsyncWithoutAsync, + expectedDiagnostic); + } + + [Fact] + public Task ConvertBlocklessReceiveAsyncToReceiveAndRemoveAsyncAlternate() + { + const string before = + """ + using Akka.Actor; + using System.Threading.Tasks; + + public sealed class MyActor : ReceiveActor + { + public MyActor() + { + ReceiveAsync(str => true, async str => Sender.Tell(str)); + } + } + """; + + const string after = + """ + using Akka.Actor; + using System.Threading.Tasks; + + public sealed class MyActor : ReceiveActor + { + public MyActor() + { + Receive(str => true, str => Sender.Tell(str)); + } + } + """; + + var expectedDiagnostic = Verify.Diagnostic() + .WithSpan(8, 9, 8, 73) + .WithArguments("lambda expression"); + + return Verify.VerifyCodeFix(before, after, ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixer.Key_FixReceiveAsyncWithoutAsync, + expectedDiagnostic); + } + + [Fact] + public Task ConvertBlocklessReceiveAnyAsyncToReceiveAnyAndRemoveAsync() + { + const string before = + """ + using Akka.Actor; + using System.Threading.Tasks; + + public sealed class MyActor : ReceiveActor + { + public MyActor() + { + ReceiveAnyAsync(async o => Sender.Tell(o)); + } + } + """; + + const string after = + """ + using Akka.Actor; + using System.Threading.Tasks; + + public sealed class MyActor : ReceiveActor + { + public MyActor() + { + ReceiveAny(o => Sender.Tell(o)); + } + } + """; + + var expectedDiagnostic = Verify.Diagnostic() + .WithSpan(8, 9, 8, 51) + .WithArguments("lambda expression"); + + return Verify.VerifyCodeFix(before, after, ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixer.Key_FixReceiveAsyncWithoutAsync, + expectedDiagnostic); + } +} \ No newline at end of file diff --git a/src/Akka.Analyzers/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaAnalyzer.cs b/src/Akka.Analyzers/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaAnalyzer.cs new file mode 100644 index 0000000..48e67ee --- /dev/null +++ b/src/Akka.Analyzers/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaAnalyzer.cs @@ -0,0 +1,86 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2013-2024 .NET Foundation +// +// ----------------------------------------------------------------------- + +using System.Runtime.CompilerServices; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Akka.Analyzers; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class ShouldNotUseReceiveAsyncWithoutAsyncLambdaAnalyzer(): AkkaDiagnosticAnalyzer(RuleDescriptors.Ak1003ShouldNotUseReceiveAsyncSynchronously) +{ + public override void AnalyzeCompilation(CompilationStartAnalysisContext context, AkkaContext akkaContext) + { + Guard.AssertIsNotNull(context); + Guard.AssertIsNotNull(akkaContext); + + context.RegisterSyntaxNodeAction(ctx => + { + var invocationExpr = (InvocationExpressionSyntax)ctx.Node; + var semanticModel = ctx.SemanticModel; + + // check that the invocation is a valid ReceiveAsync or ReceiveAnyAsync method + if(!invocationExpr.IsReceiveAsyncInvocation(semanticModel, akkaContext)) + return; + + // Get the method argument that matches a lambda expression of Func + var lambdaArg = invocationExpr.ArgumentList.Arguments + .FirstOrDefault(arg => + arg.Expression is LambdaExpressionSyntax expr + && IsFuncOfTTask(semanticModel.GetTypeInfo(expr).ConvertedType)); + + if (lambdaArg is null) + return; + + var lambdaExpr = (LambdaExpressionSyntax)lambdaArg.Expression; + + // first case, lambda expression is not prefixed with the `async` keyword + if (!lambdaExpr.AsyncKeyword.IsKind(SyntaxKind.AsyncKeyword)) + { + ReportDiagnostic(); + return; + } + + // second case, lambda expression contains a block and one of its child is awaited, OK + if(lambdaExpr.Body.DescendantNodes().OfType().Any()) + return; + + // third case, lambda expression is a one-liner await expression, OK + if (lambdaExpr.Body is AwaitExpressionSyntax) + return; + + // fourth case, there are no await expression inside the lambda expression body + ReportDiagnostic(); + return; + + void ReportDiagnostic() + { + var diagnostic = Diagnostic.Create( + descriptor: RuleDescriptors.Ak1003ShouldNotUseReceiveAsyncSynchronously, + location: invocationExpr.GetLocation(), + "lambda expression"); + ctx.ReportDiagnostic(diagnostic); + } + }, SyntaxKind.InvocationExpression); + + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static bool IsFuncOfTTask(ITypeSymbol? typeSymbol) + { + if (typeSymbol is not INamedTypeSymbol { DelegateInvokeMethod: not null } namedTypeSymbol) + return false; + + var delegateReturnType = namedTypeSymbol.DelegateInvokeMethod.ReturnType; + var delegateParameters = namedTypeSymbol.DelegateInvokeMethod.Parameters; + + return delegateReturnType is INamedTypeSymbol { Name: nameof(Task) } + && delegateParameters.Length == 1; + } +} \ No newline at end of file diff --git a/src/Akka.Analyzers/Utility/RuleDescriptors.cs b/src/Akka.Analyzers/Utility/RuleDescriptors.cs index 9b6cbd6..b5f3a84 100644 --- a/src/Akka.Analyzers/Utility/RuleDescriptors.cs +++ b/src/Akka.Analyzers/Utility/RuleDescriptors.cs @@ -42,6 +42,14 @@ private static DiagnosticDescriptor Rule( messageFormat: "Do not await on `Self.GracefulStop()` inside `ReceiveAsync()` because this will lead into " + "a deadlock inside the `ReceiveAsync()` and the actor will never receive the `PoisonPill` message sent by `GracefulStop` while it's `await`-ing."); + public static DiagnosticDescriptor Ak1003ShouldNotUseReceiveAsyncSynchronously { get; } = Rule( + id: "AK1003", + title: "ReceiveAsync() or ReceiveAnyAsync() message handler without async lambda body", + category: AnalysisCategory.ActorDesign, + defaultSeverity: DiagnosticSeverity.Warning, + messageFormat: "ReceiveAsync() or ReceiveAnyAsync() message handler with synchronous code body or " + + "block is less performant compared to Receive() or ReceiveAny(). " + + "Consider changing this message handler to Receive() or ReceiveAny() instead."); #endregion #region AK2000 Rules From 01384f0bee330f0d6e7a3f10ef6ea92a3dfc3660 Mon Sep 17 00:00:00 2001 From: Gregorius Soedharmo Date: Wed, 24 Jan 2024 05:03:07 +0700 Subject: [PATCH 2/3] Fix typos --- src/Akka.Analyzers/Utility/RuleDescriptors.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Akka.Analyzers/Utility/RuleDescriptors.cs b/src/Akka.Analyzers/Utility/RuleDescriptors.cs index b5f3a84..4c7e56e 100644 --- a/src/Akka.Analyzers/Utility/RuleDescriptors.cs +++ b/src/Akka.Analyzers/Utility/RuleDescriptors.cs @@ -47,9 +47,9 @@ private static DiagnosticDescriptor Rule( title: "ReceiveAsync() or ReceiveAnyAsync() message handler without async lambda body", category: AnalysisCategory.ActorDesign, defaultSeverity: DiagnosticSeverity.Warning, - messageFormat: "ReceiveAsync() or ReceiveAnyAsync() message handler with synchronous code body or " + - "block is less performant compared to Receive() or ReceiveAny(). " + - "Consider changing this message handler to Receive() or ReceiveAny() instead."); + messageFormat: "ReceiveAsync() or ReceiveAnyAsync() message handler with synchronous code body or " + + "block is less performant compared to Receive() or ReceiveAny(). " + + "Consider changing this message handler to Receive() or ReceiveAny() instead."); #endregion #region AK2000 Rules From c54c095feed707264b45f3c8c68ddc62053aedb5 Mon Sep 17 00:00:00 2001 From: Gregorius Soedharmo Date: Thu, 25 Jan 2024 22:43:48 +0700 Subject: [PATCH 3/3] Make analyzer ignore lambda expression without async keyword --- ...iveAsyncWithoutAsyncLambdaAnalyzerSpecs.cs | 39 +++-- ...eceiveAsyncWithoutAsyncLambdaFixerSpecs.cs | 138 ------------------ ...eReceiveAsyncWithoutAsyncLambdaAnalyzer.cs | 6 +- 3 files changed, 21 insertions(+), 162 deletions(-) diff --git a/src/Akka.Analyzers.Tests/Analyzers/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaAnalyzerSpecs.cs b/src/Akka.Analyzers.Tests/Analyzers/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaAnalyzerSpecs.cs index fe7e09f..36b04e2 100644 --- a/src/Akka.Analyzers.Tests/Analyzers/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaAnalyzerSpecs.cs +++ b/src/Akka.Analyzers.Tests/Analyzers/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaAnalyzerSpecs.cs @@ -236,17 +236,13 @@ public MyActor() private async Task Handler(string s) { } } -""" - }; +""", - public static readonly - TheoryData<(string testData, (int startLine, int startColumn, int endLine, int endColumn) spanData)> - FailureCases = new() - { - // ReceiveActor using ReceiveAsync without async keyword on the async lambda expression - ( -""" -// 1 + // ReceiveActor using ReceiveAsync without async keyword on the async lambda expression. + // Note that this is actually a non-performant code, we're opting not to detect this because to make sure + // that we're capturing an actual sync over async code, we'd need to write an actual static analyzer + """ +// 11 using Akka.Actor; using System.Threading.Tasks; @@ -261,12 +257,10 @@ public MyActor() }); } } -""", (9, 9, 13, 11)), - - // ReceiveActor using ReceiveAsync without async keyword on the async lambda expression, alternate version - ( -""" -// 2 +""", + // ReceiveActor using ReceiveAsync without async keyword on the async lambda expression, alternate version + """ +// 12 using Akka.Actor; using System.Threading.Tasks; @@ -281,13 +275,18 @@ public MyActor() }); } } -""", (9, 9, 13, 11)), - +""", + }; + + public static readonly + TheoryData<(string testData, (int startLine, int startColumn, int endLine, int endColumn) spanData)> + FailureCases = new() + { // ReceiveActor using ReceiveAsync with async keyword on the async lambda expression // but without any awaited code ( """ -// 3 +// 1 using Akka.Actor; using System.Threading.Tasks; @@ -307,7 +306,7 @@ public MyActor() // but without any awaited code, alternate version ( """ -// 4 +// 2 using Akka.Actor; using System.Threading.Tasks; diff --git a/src/Akka.Analyzers.Tests/Fixes/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixerSpecs.cs b/src/Akka.Analyzers.Tests/Fixes/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixerSpecs.cs index 0b25794..9fb3feb 100644 --- a/src/Akka.Analyzers.Tests/Fixes/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixerSpecs.cs +++ b/src/Akka.Analyzers.Tests/Fixes/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixerSpecs.cs @@ -12,144 +12,6 @@ namespace Akka.Analyzers.Tests.Fixes.AK1000; public class ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixerSpecs { - [Fact] - public Task ConvertReceiveAsyncToReceive() - { - const string before = -""" -using Akka.Actor; -using System.Threading.Tasks; - -public sealed class MyActor : ReceiveActor -{ - public MyActor() - { - ReceiveAsync(str => - { - Sender.Tell(str); - return Task.CompletedTask; - }); - } -} -"""; - - const string after = -""" -using Akka.Actor; -using System.Threading.Tasks; - -public sealed class MyActor : ReceiveActor -{ - public MyActor() - { - Receive(str => - { - Sender.Tell(str); - }); - } -} -"""; - - var expectedDiagnostic = Verify.Diagnostic() - .WithSpan(8, 9, 12, 11) - .WithArguments("lambda expression"); - - return Verify.VerifyCodeFix(before, after, ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixer.Key_FixReceiveAsyncWithoutAsync, - expectedDiagnostic); - } - - [Fact] - public Task ConvertReceiveAsyncToReceiveAlternate() - { - const string before = - """ - using Akka.Actor; - using System.Threading.Tasks; - - public sealed class MyActor : ReceiveActor - { - public MyActor() - { - ReceiveAsync(str => true, str => - { - Sender.Tell(str); - return Task.CompletedTask; - }); - } - } - """; - - const string after = - """ - using Akka.Actor; - using System.Threading.Tasks; - - public sealed class MyActor : ReceiveActor - { - public MyActor() - { - Receive(str => true, str => - { - Sender.Tell(str); - }); - } - } - """; - - var expectedDiagnostic = Verify.Diagnostic() - .WithSpan(8, 9, 12, 11) - .WithArguments("lambda expression"); - - return Verify.VerifyCodeFix(before, after, ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixer.Key_FixReceiveAsyncWithoutAsync, - expectedDiagnostic); - } - - [Fact] - public Task ConvertReceiveAnyAsyncToReceiveAny() - { - const string before = - """ - using Akka.Actor; - using System.Threading.Tasks; - - public sealed class MyActor : ReceiveActor - { - public MyActor() - { - ReceiveAnyAsync(o => - { - Sender.Tell(o); - return Task.CompletedTask; - }); - } - } - """; - - const string after = - """ - using Akka.Actor; - using System.Threading.Tasks; - - public sealed class MyActor : ReceiveActor - { - public MyActor() - { - ReceiveAny(o => - { - Sender.Tell(o); - }); - } - } - """; - - var expectedDiagnostic = Verify.Diagnostic() - .WithSpan(8, 9, 12, 11) - .WithArguments("lambda expression"); - - return Verify.VerifyCodeFix(before, after, ShouldNotUseReceiveAsyncWithoutAsyncLambdaFixer.Key_FixReceiveAsyncWithoutAsync, - expectedDiagnostic); - } - [Fact] public Task ConvertReceiveAsyncToReceiveAndRemoveAsync() { diff --git a/src/Akka.Analyzers/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaAnalyzer.cs b/src/Akka.Analyzers/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaAnalyzer.cs index 48e67ee..4060b42 100644 --- a/src/Akka.Analyzers/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaAnalyzer.cs +++ b/src/Akka.Analyzers/AK1000/ShouldNotUseReceiveAsyncWithoutAsyncLambdaAnalyzer.cs @@ -40,12 +40,10 @@ arg.Expression is LambdaExpressionSyntax expr var lambdaExpr = (LambdaExpressionSyntax)lambdaArg.Expression; - // first case, lambda expression is not prefixed with the `async` keyword + // first case, lambda expression is not prefixed with the `async` keyword. + // We will assume that the user is doing a proper thing and not just returning a `Task.CompletedTask` if (!lambdaExpr.AsyncKeyword.IsKind(SyntaxKind.AsyncKeyword)) - { - ReportDiagnostic(); return; - } // second case, lambda expression contains a block and one of its child is awaited, OK if(lambdaExpr.Body.DescendantNodes().OfType().Any())