Skip to content

Commit

Permalink
EC84 avoid async void method (#29)
Browse files Browse the repository at this point in the history
  • Loading branch information
Djoums authored Apr 16, 2024
1 parent 2d81ecb commit fdc390a
Show file tree
Hide file tree
Showing 13 changed files with 261 additions and 43 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using Microsoft.CodeAnalysis.Operations;
using System.Linq;

namespace EcoCode.Analyzers;

Expand Down Expand Up @@ -34,7 +33,7 @@ public override void Initialize(AnalysisContext context)
}

private static IFieldSymbol GetStringEmptySymbol(Compilation compilation) =>
(IFieldSymbol)compilation.GetTypeByMetadataName("System.String")!.GetMembers("Empty").First();
(IFieldSymbol)compilation.GetTypeByMetadataName("System.String")!.GetMembers("Empty")[0];

private static bool UsesConstantFormat(object? format) =>
format is null || format is string str && (str.Length == 0 || str is "g" or "G" or "f" or "F");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
namespace EcoCode.Analyzers;

/// <summary>Analyzer for avoid async void methods.</summary>
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class AvoidAsyncVoidMethodsAnalyzer : DiagnosticAnalyzer
{
/// <summary>The diagnostic descriptor.</summary>
public static DiagnosticDescriptor Descriptor { get; } = new(
Rule.Ids.EC84_AvoidAsyncVoidMethods,
title: "Avoid async void methods",
messageFormat: "Avoid async void methods",
Rule.Categories.Design,
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
description: null,
helpLinkUri: Rule.GetHelpUri(Rule.Ids.EC84_AvoidAsyncVoidMethods));

/// <inheritdoc/>
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [Descriptor];

/// <inheritdoc/>
public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.RegisterSyntaxNodeAction(static context => AnalyzeMethod(context), SyntaxKind.MethodDeclaration);
}

private static void AnalyzeMethod(SyntaxNodeAnalysisContext context)
{
var methodDeclaration = (MethodDeclarationSyntax)context.Node;
if (methodDeclaration.Modifiers.Any(SyntaxKind.AsyncKeyword) &&
methodDeclaration.ReturnType is PredefinedTypeSyntax returnType &&
returnType.Keyword.IsKind(SyntaxKind.VoidKeyword))
{
context.ReportDiagnostic(Diagnostic.Create(Descriptor, methodDeclaration.Identifier.GetLocation()));
}
}
}
1 change: 1 addition & 0 deletions src/EcoCode.Analyzers/Rule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,6 @@ public static class Ids
public const string EC81_UseStructLayout = "EC81";
public const string EC82_VariableCanBeMadeConstant = "EC82";
public const string EC83_ReplaceEnumToStringWithNameOf = "EC83";
public const string EC84_AvoidAsyncVoidMethods = "EC84";
}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,4 @@
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.Simplification;
using System.Threading;

namespace EcoCode.CodeFixes;
namespace EcoCode.CodeFixes;

/// <summary>The code fix provider for use struct layout.</summary>
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(SpecifyStructLayoutCodeFixProvider)), Shared]
Expand All @@ -19,6 +13,8 @@ public sealed class SpecifyStructLayoutCodeFixProvider : CodeFixProvider
/// <inheritdoc/>
public override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
if (context.Diagnostics.Length == 0) return;

var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
var node = root?.FindNode(context.Span, getInnermostNodeForTie: true);
if (node is not TypeDeclarationSyntax nodeToFix) return;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Formatting;
using Microsoft.CodeAnalysis.Simplification;
using System.Threading;
using Microsoft.CodeAnalysis.Formatting;

namespace EcoCode.CodeFixes;

Expand All @@ -26,20 +21,22 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
var root = await document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
if (root is null) return;

var diagnostic = context.Diagnostics[0];
var parent = root.FindToken(diagnostic.Location.SourceSpan.Start).Parent;
if (parent is null) return;

foreach (var node in parent.AncestorsAndSelf())
foreach (var diagnostic in context.Diagnostics)
{
if (node is not LocalDeclarationStatementSyntax declaration) continue;
context.RegisterCodeFix(
CodeAction.Create(
title: "Make variable constant",
createChangedDocument: token => RefactorAsync(document, declaration, token),
equivalenceKey: "Make variable constant"),
diagnostic);
break;
var parent = root.FindToken(diagnostic.Location.SourceSpan.Start).Parent;
if (parent is null) return;

foreach (var node in parent.AncestorsAndSelf())
{
if (node is not LocalDeclarationStatementSyntax declaration) continue;
context.RegisterCodeFix(
CodeAction.Create(
title: "Make variable constant",
createChangedDocument: token => RefactorAsync(document, declaration, token),
equivalenceKey: "Make variable constant"),
diagnostic);
break;
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.Operations;
using System.Threading;
using Microsoft.CodeAnalysis.Operations;

namespace EcoCode.CodeFixes;

Expand Down Expand Up @@ -32,7 +27,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
title: "Use nameof",
createChangedDocument: token => RefactorAsync(document, nodeToFix, token),
equivalenceKey: "Use nameof"),
context.Diagnostics[0]);
context.Diagnostics);
}

private static async Task<Document> RefactorAsync(Document document, SyntaxNode node, CancellationToken token)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
namespace EcoCode.CodeFixes;

/// <summary>The code fix provider for avoid async void methods.</summary>
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(AvoidAsyncVoidMethodsCodeFixProvider)), Shared]
public sealed class AvoidAsyncVoidMethodsCodeFixProvider : CodeFixProvider
{
/// <inheritdoc/>
public override ImmutableArray<string> FixableDiagnosticIds => [AvoidAsyncVoidMethodsAnalyzer.Descriptor.Id];

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

/// <inheritdoc/>
public override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
if (context.Diagnostics.Length == 0) return;

var document = context.Document;
var root = await document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
if (root is null) return;

foreach (var diagnostic in context.Diagnostics)
{
var parent = root.FindToken(diagnostic.Location.SourceSpan.Start).Parent;
if (parent is null) continue;

foreach (var node in parent.AncestorsAndSelf())
{
if (node is not MethodDeclarationSyntax declaration) continue;
context.RegisterCodeFix(
CodeAction.Create(
title: "Convert to async Task",
createChangedDocument: token => RefactorAsync(document, declaration, token),
equivalenceKey: "Convert to async Task"),
diagnostic);
break;
}
}
}

private static async Task<Document> RefactorAsync(Document document, MethodDeclarationSyntax methodDecl, CancellationToken token)
{
// Note : it may seem a good idea to add the System.Thread.Tasks using directive if it isn't present yet
// However it isn't properly doable because :
// - It could be added as a global using in a different file, but Roslyn doesn't give easy access to those
// - The user could have enabled the ImplicitUsings option, which makes the using directives both global and invisible to the analyzer
// So as a result, we simply don't handle it

var root = await document.GetSyntaxRootAsync(token).ConfigureAwait(false);
return root is null ? document : document.WithSyntaxRoot(
root.ReplaceNode(methodDecl, methodDecl.WithReturnType(
SyntaxFactory.IdentifierName("Task") // Change the return type of the method to Task
.WithLeadingTrivia(methodDecl.ReturnType.GetLeadingTrivia())
.WithTrailingTrivia(methodDecl.ReturnType.GetTrailingTrivia()))));
}
}
6 changes: 6 additions & 0 deletions src/EcoCode.CodeFixes/GlobalUsings.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
global using EcoCode.Analyzers;
global using EcoCode.Shared;
global using Microsoft.CodeAnalysis;
global using Microsoft.CodeAnalysis.CodeActions;
global using Microsoft.CodeAnalysis.CodeFixes;
global using Microsoft.CodeAnalysis.CSharp;
global using Microsoft.CodeAnalysis.CSharp.Syntax;
global using Microsoft.CodeAnalysis.Editing;
global using Microsoft.CodeAnalysis.Simplification;
global using System.Collections.Immutable;
global using System.Composition;
global using System.Runtime.InteropServices;
global using System.Threading;
global using System.Threading.Tasks;
17 changes: 12 additions & 5 deletions src/EcoCode.Shared/Extensions/SymbolExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,10 @@ public static bool IsDeclaredOutsideLoop(this ISymbol symbol, SyntaxNode loopNod
switch (symbol)
{
case ILocalSymbol localSymbol:
if (localSymbol.DeclaringSyntaxReferences.FirstOrDefault() is SyntaxReference syntaxRef)
var localRefs = localSymbol.DeclaringSyntaxReferences;
if (localRefs.Length != 0)
{
for (var node = syntaxRef.GetSyntax(); node is not null; node = node.Parent)
for (var node = localRefs[0].GetSyntax(); node is not null; node = node.Parent)
{
if (node is BlockSyntax or MethodDeclarationSyntax or CompilationUnitSyntax)
return node;
Expand All @@ -69,13 +70,19 @@ public static bool IsDeclaredOutsideLoop(this ISymbol symbol, SyntaxNode loopNod
break;

case IFieldSymbol fieldSymbol:
return fieldSymbol.ContainingType.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax();
var fieldRefs = fieldSymbol.ContainingType.DeclaringSyntaxReferences;
if (fieldRefs.Length != 0) return fieldRefs[0].GetSyntax();
break;

case IPropertySymbol propertySymbol:
return propertySymbol.ContainingType.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax();
var propRefs = propertySymbol.ContainingType.DeclaringSyntaxReferences;
if (propRefs.Length != 0) return propRefs[0].GetSyntax();
break;

case IParameterSymbol parameterSymbol:
return parameterSymbol.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax();
var paramRefs = parameterSymbol.DeclaringSyntaxReferences;
if (paramRefs.Length != 0) return paramRefs[0].GetSyntax();
break;
}
return null;
}
Expand Down
34 changes: 33 additions & 1 deletion src/EcoCode.Shared/Extensions/SyntaxNodeExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
namespace EcoCode.Shared;
using Microsoft.CodeAnalysis.CSharp;

namespace EcoCode.Shared;

/// <summary>Extensions methods for <see cref="SyntaxNode"/>.</summary>
public static class SyntaxNodeExtensions
Expand Down Expand Up @@ -37,4 +39,34 @@ public static SyntaxList<StatementSyntax> GetLoopStatements(this SyntaxNode node
_ => default,
};
}

/// <summary>Returns whether a node has a given using directive, use with a document root.</summary>
/// <param name="node">The node.</param>
/// <param name="namespace">The namespace of the using directive.</param>
/// <returns>True if the node has the given using, false otherwise.</returns>
public static bool HasUsingDirective(this SyntaxNode node, string @namespace)
{
foreach (var descendant in node.DescendantNodes())
{
if ((descendant as UsingDirectiveSyntax)?.Name?.ToString() == @namespace)
return true;
}
return false;
}

/// <summary>Adds a using directive to the given node, use with a document root.</summary>
/// <param name="node">The node.</param>
/// <param name="namespace">The namespace of the using directive.</param>
/// <returns>The updated node with the using directive.</returns>
public static SyntaxNode AddUsingDirective(this SyntaxNode node, string @namespace)
{
var usingDirective = SyntaxFactory.UsingDirective(SyntaxFactory.ParseName(@namespace)).NormalizeWhitespace();

foreach (var descendant in node.DescendantNodes())
{
if (descendant is NamespaceDeclarationSyntax namespaceDeclaration)
return node.ReplaceNode(namespaceDeclaration, namespaceDeclaration.AddUsings(usingDirective));
}
return ((CompilationUnitSyntax)node).AddUsings(usingDirective); // Add the using directive at the top of the file
}
}
1 change: 0 additions & 1 deletion src/EcoCode.Shared/GlobalUsings.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
global using Microsoft.CodeAnalysis;
global using Microsoft.CodeAnalysis.CSharp.Syntax;
global using System.Linq;
91 changes: 91 additions & 0 deletions src/EcoCode.Tests/Tests/EC84_AvoidAsyncVoidMethodsUnitTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
namespace EcoCode.Tests;

[TestClass]
public class AvoidAsyncVoidMethodsUnitTests
{
private static readonly VerifyDlg VerifyAsync = CodeFixVerifier.VerifyAsync<
AvoidAsyncVoidMethodsAnalyzer,
AvoidAsyncVoidMethodsCodeFixProvider>;

[TestMethod]
public async Task EmptyCodeAsync() => await VerifyAsync("").ConfigureAwait(false);

[TestMethod]
public async Task AvoidAsyncVoidMethodAsync() => await VerifyAsync("""
using System;
using System.Threading.Tasks;
public static class Program
{
public static async void [|Main|]()
{
await Task.Delay(1000).ConfigureAwait(false);
Console.WriteLine();
}
}
""",
fixedSource: """
using System;
using System.Threading.Tasks;
public static class Program
{
public static async Task Main()
{
await Task.Delay(1000).ConfigureAwait(false);
Console.WriteLine();
}
}
""").ConfigureAwait(false);

[TestMethod]
public async Task AvoidAsyncVoidMethodWithMissingUsingAsync() => await VerifyAsync("""
using System;
using System.Net.Http;
public static class Program
{
public static async void [|Main|]()
{
using var httpClient = new HttpClient();
_ = await httpClient.GetAsync(new Uri("URL")).ConfigureAwait(false);
}
}
""",
fixedSource: """
using System;
using System.Net.Http;
public static class Program
{
public static async {|CS0246:Task|} {|CS0161:Main|}()
{
using var httpClient = new HttpClient();
_ = await httpClient.GetAsync(new Uri("URL")).ConfigureAwait(false);
}
}
""").ConfigureAwait(false);

[TestMethod]
public async Task AsyncTaskMethodIsOkAsync() => await VerifyAsync("""
using System;
using System.Threading.Tasks;
public static class Program
{
public static async Task Main()
{
await Task.Delay(1000).ConfigureAwait(false);
Console.WriteLine();
}
}
""").ConfigureAwait(false);

[TestMethod]
public async Task AsyncGenericTaskMethodIsOkAsync() => await VerifyAsync("""
using System.Threading.Tasks;
public static class Program
{
public static async Task<int> Main()
{
await Task.Delay(1000).ConfigureAwait(false);
return 1;
}
}
""").ConfigureAwait(false);
}
Loading

0 comments on commit fdc390a

Please sign in to comment.