diff --git a/README.md b/README.md index a78bd32d..a0a027a5 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ Both the EcoCode NuGet package and Visual Studio extension target .Net Standard |[EC69](https://github.com/green-code-initiative/ecoCode/blob/main/ecocode-rules-specifications/src/main/rules/EC69/csharp/EC69.asciidoc)|Don’t call loop invariant functions in loop conditions|⚠️|❌| |[EC72](https://github.com/green-code-initiative/ecoCode/blob/main/ecocode-rules-specifications/src/main/rules/EC72/csharp/EC72.asciidoc)|Don’t execute SQL queries in loops|⚠️|❌| |[EC75](https://github.com/green-code-initiative/ecoCode/blob/main/ecocode-rules-specifications/src/main/rules/EC75/csharp/EC75.asciidoc)|Don’t concatenate `strings` in loops|⚠️|❌| -|[EC81](https://github.com/green-code-initiative/ecoCode/blob/main/ecocode-rules-specifications/src/main/rules/EC81/csharp/EC81.asciidoc)|Specify struct layouts|⚠️|✔️| +|[EC81](https://github.com/green-code-initiative/ecoCode/blob/main/ecocode-rules-specifications/src/main/rules/EC81/csharp/EC81.asciidoc)|Specify `struct` layouts|⚠️|✔️| |[EC82](https://github.com/green-code-initiative/ecoCode/blob/main/ecocode-rules-specifications/src/main/rules/EC82/csharp/EC82.asciidoc)|Variable can be made constant|ℹ️|✔️| |[EC83](https://github.com/green-code-initiative/ecoCode/blob/main/ecocode-rules-specifications/src/main/rules/EC83/csharp/EC83.asciidoc)|Replace Enum `ToString()` with `nameof`|⚠️|✔️| |[EC84](https://github.com/green-code-initiative/ecoCode/blob/main/ecocode-rules-specifications/src/main/rules/EC84/csharp/EC84.asciidoc)|Avoid `async void` methods|⚠️|✔️| @@ -45,7 +45,8 @@ Both the EcoCode NuGet package and Visual Studio extension target .Net Standard |[EC87](https://github.com/green-code-initiative/ecoCode/blob/main/ecocode-rules-specifications/src/main/rules/EC87/csharp/EC87.asciidoc)|Use collection indexer|⚠️|✔️| |[EC88](https://github.com/green-code-initiative/ecoCode/blob/main/ecocode-rules-specifications/src/main/rules/EC88/csharp/EC88.asciidoc)|Dispose resource asynchronously|⚠️|✔️| |[EC91](https://github.com/green-code-initiative/ecoCode/blob/main/ecocode-rules-specifications/src/main/rules/EC91/csharp/EC91.asciidoc)|Use `Where` before `OrderBy`|⚠️|✔️| -|[EC92](https://github.com/green-code-initiative/ecoCode/blob/main/ecocode-rules-specifications/src/main/rules/EC92/csharp/EC92.asciidoc)|Use `Length` to test empty strings|⚠️|✔️| +|[EC92](https://github.com/green-code-initiative/ecoCode/blob/main/ecocode-rules-specifications/src/main/rules/EC92/csharp/EC92.asciidoc)|Use `Length` to test empty `strings`|⚠️|✔️| +|[EC93](https://github.com/green-code-initiative/ecoCode/blob/main/ecocode-rules-specifications/src/main/rules/EC93/csharp/EC93.asciidoc)|Return `Task` directly|ℹ️|✔️| 🌿 Roslyn Rules ------------------- diff --git a/src/EcoCode.Core/Analyzers/EC93.ReturnTaskDirectly.Fixer.cs b/src/EcoCode.Core/Analyzers/EC93.ReturnTaskDirectly.Fixer.cs new file mode 100644 index 00000000..d70eb8ed --- /dev/null +++ b/src/EcoCode.Core/Analyzers/EC93.ReturnTaskDirectly.Fixer.cs @@ -0,0 +1,123 @@ +namespace EcoCode.Analyzers; + +/// EC93 fixer: Return Task directly. +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(ReturnTaskDirectly)), Shared] +public sealed class ReturnTaskDirectlyFixer : CodeFixProvider +{ + /// + public override ImmutableArray FixableDiagnosticIds => _fixableDiagnosticIds; + private static readonly ImmutableArray _fixableDiagnosticIds = [ReturnTaskDirectly.Descriptor.Id]; + + /// + [ExcludeFromCodeCoverage] + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + /// + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + if (await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false) is not { } root) + return; + + foreach (var diagnostic in context.Diagnostics) + { + if (root.FindToken(diagnostic.Location.SourceSpan.Start).Parent is not { } parent) + continue; + + foreach (var node in parent.AncestorsAndSelf()) + { + if (node is not MethodDeclarationSyntax methodDecl) continue; + + int asyncIndex = methodDecl.Modifiers.IndexOf(SyntaxKind.AsyncKeyword); + if (asyncIndex == -1) continue; + + if (methodDecl.ExpressionBody is { Expression: AwaitExpressionSyntax awaitExpr1 }) + { + context.RegisterCodeFix( // Expression body + CodeAction.Create( + title: "Dispose resource asynchronously", + createChangedDocument: _ => ReturnTaskDirectlyWithExpressionAsync(context.Document, methodDecl, awaitExpr1, asyncIndex), + equivalenceKey: "Dispose resource asynchronously"), + diagnostic); + break; + } + + var statement = methodDecl.Body?.Statements.SingleOrDefaultNoThrow(); + if (statement is ExpressionStatementSyntax { Expression: AwaitExpressionSyntax awaitExpr2 }) + { + context.RegisterCodeFix( // Body with 'await' statement + CodeAction.Create( + title: "Dispose resource asynchronously", + createChangedDocument: _ => ReturnTaskDirectlyWithBodyAwaitAsync(context.Document, methodDecl, awaitExpr2, asyncIndex), + equivalenceKey: "Dispose resource asynchronously"), + diagnostic); + break; + } + if (statement is ReturnStatementSyntax { Expression: AwaitExpressionSyntax awaitExpr3 } returnStmt) + { + context.RegisterCodeFix( // Body with 'return await' statement + CodeAction.Create( + title: "Dispose resource asynchronously", + createChangedDocument: _ => ReturnTaskDirectlyWithBodyReturnAwaitAsync(context.Document, methodDecl, returnStmt, awaitExpr3, asyncIndex), + equivalenceKey: "Dispose resource asynchronously"), + diagnostic); + break; + } + } + } + } + + private static async Task ReturnTaskDirectlyWithExpressionAsync( + Document document, + MethodDeclarationSyntax methodDecl, + AwaitExpressionSyntax awaitExpr, + int asyncIndex) + { + var newReturnStmt = SyntaxFactory.ReturnStatement(awaitExpr.Expression); + + var newBody = SyntaxFactory.ArrowExpressionClause(newReturnStmt.Expression!.WithTriviaFrom(awaitExpr)) + .WithTriviaFrom(methodDecl.ExpressionBody!); + + return await document.WithUpdatedRoot(methodDecl, methodDecl + .WithModifiers(methodDecl.Modifiers.RemoveAt(asyncIndex)) + .WithExpressionBody(newBody)); + } + + private static async Task ReturnTaskDirectlyWithBodyAwaitAsync( + Document document, + MethodDeclarationSyntax methodDecl, + AwaitExpressionSyntax awaitExpr, + int asyncIndex) + { + var newReturnStmt = SyntaxFactory.ReturnStatement(awaitExpr.Expression) + .WithLeadingTrivia(awaitExpr.GetLeadingTrivia()) + .WithTrailingTrivia(((ExpressionStatementSyntax)awaitExpr.Parent!).SemicolonToken.TrailingTrivia); + + var newBody = SyntaxFactory.Block(newReturnStmt) + .WithOpenBraceToken(methodDecl.Body!.OpenBraceToken) + .WithCloseBraceToken(methodDecl.Body.CloseBraceToken) + .WithTriviaFrom(methodDecl.Body); + + return await document.WithUpdatedRoot(methodDecl, methodDecl + .WithModifiers(methodDecl.Modifiers.RemoveAt(asyncIndex)) + .WithBody(newBody)); + } + + private static async Task ReturnTaskDirectlyWithBodyReturnAwaitAsync( + Document document, + MethodDeclarationSyntax methodDecl, + ReturnStatementSyntax returnStmt, + AwaitExpressionSyntax awaitExpr, + int asyncIndex) + { + var newReturnStmt = returnStmt.WithExpression(awaitExpr.Expression); + + var newBody = SyntaxFactory.Block(newReturnStmt) + .WithOpenBraceToken(methodDecl.Body!.OpenBraceToken) + .WithCloseBraceToken(methodDecl.Body.CloseBraceToken) + .WithTriviaFrom(methodDecl.Body); + + return await document.WithUpdatedRoot(methodDecl, methodDecl + .WithModifiers(methodDecl.Modifiers.RemoveAt(asyncIndex)) + .WithBody(newBody)); + } +} diff --git a/src/EcoCode.Core/Analyzers/EC93.ReturnTaskDirectly.cs b/src/EcoCode.Core/Analyzers/EC93.ReturnTaskDirectly.cs new file mode 100644 index 00000000..a152318d --- /dev/null +++ b/src/EcoCode.Core/Analyzers/EC93.ReturnTaskDirectly.cs @@ -0,0 +1,54 @@ +namespace EcoCode.Analyzers; + +/// EC93: Return Task directly. +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class ReturnTaskDirectly : DiagnosticAnalyzer +{ + private static readonly ImmutableArray MethodDeclarations = [SyntaxKind.MethodDeclaration]; + + /// The diagnostic descriptor. + public static DiagnosticDescriptor Descriptor { get; } = Rule.CreateDescriptor( + id: Rule.Ids.EC93_ReturnTaskDirectly, + title: "Consider returning Task directly", + message: "Consider returning a Task directly instead of awaiting a single statement", + category: Rule.Categories.Performance, + severity: DiagnosticSeverity.Info, + description: "Consider returning a Task directly instead of awaiting a single statement, as this can save performance."); + + /// + public override ImmutableArray SupportedDiagnostics => _supportedDiagnostics; + private static readonly ImmutableArray _supportedDiagnostics = [Descriptor]; + + /// + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.RegisterSyntaxNodeAction(static context => AnalyzeSyntaxNode(context), MethodDeclarations); + } + + private static void AnalyzeSyntaxNode(SyntaxNodeAnalysisContext context) + { + // Check if the method is async + var methodDeclaration = (MethodDeclarationSyntax)context.Node; + int asyncIndex = methodDeclaration.Modifiers.IndexOf(SyntaxKind.AsyncKeyword); + if (asyncIndex == -1) return; + + // Check if the method contains a single await statement + var awaitExpr = methodDeclaration.ExpressionBody?.Expression as AwaitExpressionSyntax; + if (awaitExpr is null && methodDeclaration.Body?.Statements.SingleOrDefaultNoThrow() is { } statement) + { + if (statement is ExpressionStatementSyntax expressionStmt) // Is it an 'await' statement + awaitExpr = expressionStmt.Expression as AwaitExpressionSyntax; + else if (statement is ReturnStatementSyntax returnStmt) // Is it a 'return await' statement + awaitExpr = returnStmt.Expression as AwaitExpressionSyntax; + } + if (awaitExpr is null) return; + + // Check if the await statement has any nested await statement (like parameters) + foreach (var node in awaitExpr.DescendantNodes()) + if (node is AwaitExpressionSyntax) return; + + context.ReportDiagnostic(Diagnostic.Create(Descriptor, methodDeclaration.Modifiers[asyncIndex].GetLocation())); + } +} diff --git a/src/EcoCode.Core/Extensions/SyntaxListExtensions.cs b/src/EcoCode.Core/Extensions/SyntaxListExtensions.cs new file mode 100644 index 00000000..b00ce0fe --- /dev/null +++ b/src/EcoCode.Core/Extensions/SyntaxListExtensions.cs @@ -0,0 +1,12 @@ +namespace EcoCode.Extensions; + +/// Extensions methods for . +public static class SyntaxListExtensions +{ + /// Returns the single node of the list, default if empty or more than one node is contained. + /// The node type. + /// The syntax list. + /// The single node of the list, default if empty or more than one node is contained. + public static TNode? SingleOrDefaultNoThrow(this SyntaxList list) + where TNode : SyntaxNode => list.Count == 1 ? list[0] : default; +} diff --git a/src/EcoCode.Core/Models/Rule.cs b/src/EcoCode.Core/Models/Rule.cs index a46e37ca..75ca6e15 100644 --- a/src/EcoCode.Core/Models/Rule.cs +++ b/src/EcoCode.Core/Models/Rule.cs @@ -25,6 +25,7 @@ public static class Ids public const string EC89_DoNotPassMutableStructAsRefReadonly = "EC89"; public const string EC91_UseWhereBeforeOrderBy = "EC91"; public const string EC92_UseStringEmptyLength = "EC92"; + public const string EC93_ReturnTaskDirectly = "EC93"; } /// Creates a diagnostic descriptor. diff --git a/src/EcoCode.Tests/GlobalUsings.cs b/src/EcoCode.Tests/GlobalUsings.cs index f96c6020..49828dc8 100644 --- a/src/EcoCode.Tests/GlobalUsings.cs +++ b/src/EcoCode.Tests/GlobalUsings.cs @@ -1,5 +1,6 @@ global using EcoCode.Analyzers; global using Microsoft.CodeAnalysis.CodeFixes; +global using Microsoft.CodeAnalysis.CSharp; global using Microsoft.CodeAnalysis.CSharp.Testing; global using Microsoft.CodeAnalysis.Diagnostics; global using Microsoft.CodeAnalysis.Testing.Verifiers; diff --git a/src/EcoCode.Tests/TestRunner.cs b/src/EcoCode.Tests/TestRunner.cs index 0305c561..1e470084 100644 --- a/src/EcoCode.Tests/TestRunner.cs +++ b/src/EcoCode.Tests/TestRunner.cs @@ -1,5 +1,4 @@ -using Microsoft.CodeAnalysis.CSharp; -using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis; namespace EcoCode.Tests; diff --git a/src/EcoCode.Tests/Tests/EC87.UseListIndexer.Tests.cs b/src/EcoCode.Tests/Tests/EC87.UseListIndexer.Tests.cs index 5706c306..0598a308 100644 --- a/src/EcoCode.Tests/Tests/EC87.UseListIndexer.Tests.cs +++ b/src/EcoCode.Tests/Tests/EC87.UseListIndexer.Tests.cs @@ -1,6 +1,4 @@ -using Microsoft.CodeAnalysis.CSharp; - -namespace EcoCode.Tests; +namespace EcoCode.Tests; [TestClass] public sealed class UseListIndexerTests diff --git a/src/EcoCode.Tests/Tests/EC93.ReturnTaskDirectly.Tests.cs b/src/EcoCode.Tests/Tests/EC93.ReturnTaskDirectly.Tests.cs new file mode 100644 index 00000000..bf523c27 --- /dev/null +++ b/src/EcoCode.Tests/Tests/EC93.ReturnTaskDirectly.Tests.cs @@ -0,0 +1,232 @@ +namespace EcoCode.Tests; + +[TestClass] +public sealed class ReturnTaskDirectlyTests +{ + private static readonly CodeFixerDlg VerifyAsync = TestRunner.VerifyAsync; + + [TestMethod] + public async Task EmptyCodeAsync() => await VerifyAsync("").ConfigureAwait(false); + + [TestMethod] + public Task DontWarnWhenReturningTask1Async() => VerifyAsync(""" + using System.Threading.Tasks; + public static class Test + { + public static Task Run() => Task.Delay(0); + } + """); + + [TestMethod] + public Task DontWarnWhenReturningTask2Async() => VerifyAsync(""" + using System.Threading.Tasks; + public static class Test + { + public static Task Run() + { + return Task.Delay(0); + } + } + """); + + [TestMethod] + public Task DontWarnWhenReturningTask3Async() => VerifyAsync(""" + using System.Threading.Tasks; + public static class Test + { + public static Task Run() + { + System.Console.WriteLine(); + return Task.Delay(0); + } + } + """); + + [TestMethod] + public Task DontWarnWithMultipleStatements1Async() => VerifyAsync(""" + using System.Threading.Tasks; + public static class Test + { + public static async Task Run() + { + System.Console.WriteLine(); + await Task.Delay(0); + } + } + """); + + [TestMethod] + public Task DontWarnWithMultipleStatements2Async() => VerifyAsync(""" + using System.Threading.Tasks; + public static class Test + { + public static async Task Run() + { + await Task.Delay(0); + await Task.Delay(0); + } + } + """); + + [TestMethod] + public Task WarnOnSingleAwaitExpressionAsync() => VerifyAsync(""" + using System.Threading.Tasks; + public static class Test + { + public static [|async|] Task Run() => await Task.Delay(0); + } + """, """ + using System.Threading.Tasks; + public static class Test + { + public static Task Run() => Task.Delay(0); + } + """); + + [TestMethod] + public Task WarnOnSingleAwaitWithTrivia1ExpressionAsync() => VerifyAsync(""" + using System.Threading.Tasks; + public static class Test + { + public static [|async|] Task Run() => await Task.Delay(0); // Comment + } + """, """ + using System.Threading.Tasks; + public static class Test + { + public static Task Run() => Task.Delay(0); // Comment + } + """); + + [TestMethod] + public Task WarnOnSingleAwaitWithTrivia2ExpressionAsync() => VerifyAsync(""" + using System.Threading.Tasks; + public static class Test + { + public static [|async|] Task Run() => + // Comment + await Task.Delay(0); + } + """, """ + using System.Threading.Tasks; + public static class Test + { + public static Task Run() => + // Comment + Task.Delay(0); + } + """); + + [TestMethod] + public Task WarnOnSingleAwaitBody1Async() => VerifyAsync(""" + using System.Threading.Tasks; + public static class Test + { + public static [|async|] Task Run() + { + await Task.Delay(0); + } + } + """, """ + using System.Threading.Tasks; + public static class Test + { + public static Task Run() + { + return Task.Delay(0); + } + } + """); + + [TestMethod] + public Task WarnOnSingleAwaitBody2Async() => VerifyAsync(""" + using System.Threading.Tasks; + public static class Test + { + public static [|async|] Task Run() + { + return await Task.FromResult(0); + } + } + """, """ + using System.Threading.Tasks; + public static class Test + { + public static Task Run() + { + return Task.FromResult(0); + } + } + """); + + [TestMethod] + public Task WarnOnSingleAwaitBodyWithTrivia1Async() => VerifyAsync(""" + using System.Threading.Tasks; + public static class Test + { + public static [|async|] Task Run() + { + // Comment 0 + await Task.Delay(0); // Comment 1 + // Comment 2 + } + } + """, """ + using System.Threading.Tasks; + public static class Test + { + public static Task Run() + { + // Comment 0 + return Task.Delay(0); // Comment 1 + // Comment 2 + } + } + """); + + [TestMethod] + public Task WarnOnSingleAwaitBodyWithTrivia2Async() => VerifyAsync(""" + using System.Threading.Tasks; + public static class Test + { + public static [|async|] Task Run() + { + // Comment 0 + return await Task.FromResult(0); // Comment 1 + // Comment 2 + } + } + """, """ + using System.Threading.Tasks; + public static class Test + { + public static Task Run() + { + // Comment 0 + return Task.FromResult(0); // Comment 1 + // Comment 2 + } + } + """); + + [TestMethod] + public Task DontWarnOnNestedAwaitExpressionAsync() => VerifyAsync(""" + using System.Threading.Tasks; + public static class Test + { + public static async Task Run() => await Task.Delay(await Task.FromResult(0)); + } + """); + + [TestMethod] + public Task DontWarnOnNestedAwaitBodyAsync() => VerifyAsync(""" + using System.Threading.Tasks; + public static class Test + { + public static async Task Run() + { + await Task.Delay(await Task.FromResult(0)); + } + } + """); +}