Skip to content

Commit

Permalink
[EC93] Return Task directly (#66)
Browse files Browse the repository at this point in the history
  • Loading branch information
Djoums authored Jun 8, 2024
1 parent 8404032 commit db1ed27
Show file tree
Hide file tree
Showing 9 changed files with 428 additions and 7 deletions.
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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|⚠️|✔️|
Expand All @@ -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
-------------------
Expand Down
123 changes: 123 additions & 0 deletions src/EcoCode.Core/Analyzers/EC93.ReturnTaskDirectly.Fixer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
namespace EcoCode.Analyzers;

/// <summary>EC93 fixer: Return Task directly.</summary>
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(ReturnTaskDirectly)), Shared]
public sealed class ReturnTaskDirectlyFixer : CodeFixProvider
{
/// <inheritdoc/>
public override ImmutableArray<string> FixableDiagnosticIds => _fixableDiagnosticIds;
private static readonly ImmutableArray<string> _fixableDiagnosticIds = [ReturnTaskDirectly.Descriptor.Id];

/// <inheritdoc/>
[ExcludeFromCodeCoverage]
public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;

/// <inheritdoc/>
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<Document> 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<Document> 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<Document> 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));
}
}
54 changes: 54 additions & 0 deletions src/EcoCode.Core/Analyzers/EC93.ReturnTaskDirectly.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
namespace EcoCode.Analyzers;

/// <summary>EC93: Return Task directly.</summary>
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class ReturnTaskDirectly : DiagnosticAnalyzer
{
private static readonly ImmutableArray<SyntaxKind> MethodDeclarations = [SyntaxKind.MethodDeclaration];

/// <summary>The diagnostic descriptor.</summary>
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.");

/// <inheritdoc/>
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => _supportedDiagnostics;
private static readonly ImmutableArray<DiagnosticDescriptor> _supportedDiagnostics = [Descriptor];

/// <inheritdoc/>
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()));
}
}
12 changes: 12 additions & 0 deletions src/EcoCode.Core/Extensions/SyntaxListExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
namespace EcoCode.Extensions;

/// <summary>Extensions methods for <see cref="SyntaxList{TNode}"/>.</summary>
public static class SyntaxListExtensions
{
/// <summary>Returns the single node of the list, default if empty or more than one node is contained.</summary>
/// <typeparam name="TNode">The node type.</typeparam>
/// <param name="list">The syntax list.</param>
/// <returns>The single node of the list, default if empty or more than one node is contained.</returns>
public static TNode? SingleOrDefaultNoThrow<TNode>(this SyntaxList<TNode> list)
where TNode : SyntaxNode => list.Count == 1 ? list[0] : default;
}
1 change: 1 addition & 0 deletions src/EcoCode.Core/Models/Rule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}

/// <summary>Creates a diagnostic descriptor.</summary>
Expand Down
1 change: 1 addition & 0 deletions src/EcoCode.Tests/GlobalUsings.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
3 changes: 1 addition & 2 deletions src/EcoCode.Tests/TestRunner.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis;

namespace EcoCode.Tests;

Expand Down
4 changes: 1 addition & 3 deletions src/EcoCode.Tests/Tests/EC87.UseListIndexer.Tests.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
using Microsoft.CodeAnalysis.CSharp;

namespace EcoCode.Tests;
namespace EcoCode.Tests;

[TestClass]
public sealed class UseListIndexerTests
Expand Down
Loading

0 comments on commit db1ed27

Please sign in to comment.