Skip to content

Commit

Permalink
Merge pull request #714 from CommunityToolkit/dev/async-void-command-…
Browse files Browse the repository at this point in the history
…analyzer

Add new analyzer/fixer for async void [RelayCommand] methods
  • Loading branch information
Sergio0694 committed Jun 8, 2023
2 parents 7c3473a + c6eedee commit d6eb5cc
Show file tree
Hide file tree
Showing 9 changed files with 304 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Collections.Immutable;
using System.Composition;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.Simplification;
using Microsoft.CodeAnalysis.Text;
using static CommunityToolkit.Mvvm.SourceGenerators.Diagnostics.DiagnosticDescriptors;

namespace CommunityToolkit.Mvvm.CodeFixers;

/// <summary>
/// A code fixer that automatically updates the return type of <see langword="async"/> <see cref="void"/> methods using <c>[RelayCommand]</c> to return a <see cref="Task"/> instead.
/// </summary>
[ExportCodeFixProvider(LanguageNames.CSharp)]
[Shared]
public sealed class AsyncVoidReturningRelayCommandMethodCodeFixer : CodeFixProvider
{
/// <inheritdoc/>
public override ImmutableArray<string> FixableDiagnosticIds { get; } = ImmutableArray.Create(AsyncVoidReturningRelayCommandMethodId);

/// <inheritdoc/>
public override FixAllProvider? GetFixAllProvider()
{
return WellKnownFixAllProviders.BatchFixer;
}

/// <inheritdoc/>
public override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
Diagnostic diagnostic = context.Diagnostics[0];
TextSpan diagnosticSpan = context.Span;

SyntaxNode? root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);

// Get the method declaration from the target diagnostic
if (root!.FindNode(diagnosticSpan) is MethodDeclarationSyntax methodDeclaration)
{
// Register the code fix to update the return type to be Task instead
context.RegisterCodeFix(
CodeAction.Create(
title: "Change return type to Task",
createChangedDocument: token => ChangeReturnType(context.Document, root, methodDeclaration, token),
equivalenceKey: "Change return type to Task"),
diagnostic);
}
}

/// <summary>
/// Applies the code fix to a target method declaration and returns an updated document.
/// </summary>
/// <param name="document">The original document being fixed.</param>
/// <param name="root">The original tree root belonging to the current document.</param>
/// <param name="methodDeclaration">The <see cref="MethodDeclarationSyntax"/> to update.</param>
/// <param name="cancellationToken">The cancellation token for the operation.</param>
/// <returns>An updated document with the applied code fix, and the return type of the method being <see cref="Task"/>.</returns>
private static async Task<Document> ChangeReturnType(Document document, SyntaxNode root, MethodDeclarationSyntax methodDeclaration, CancellationToken cancellationToken)
{
// Get the semantic model (bail if it's not available)
if (await document.GetSemanticModelAsync(cancellationToken) is not SemanticModel semanticModel)
{
return document;
}

// Also bail if we can't resolve the Task symbol (this should really never happen)
if (semanticModel.Compilation.GetTypeByMetadataName("System.Threading.Tasks.Task") is not INamedTypeSymbol taskSymbol)
{
return document;
}

// Create the new syntax node for the return, and configure it to automatically add "using System.Threading.Tasks;" if needed
SyntaxNode typeSyntax = SyntaxGenerator.GetGenerator(document).TypeExpression(taskSymbol).WithAdditionalAnnotations(Simplifier.AddImportsAnnotation);

// Replace the void return type with the newly created Task type expression
return document.WithSyntaxRoot(root.ReplaceNode(methodDeclaration.ReturnType, typeSyntax));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ public sealed class ClassUsingAttributeInsteadOfInheritanceCodeFixer : CodeFixPr
public override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
Diagnostic diagnostic = context.Diagnostics[0];
TextSpan diagnosticSpan = diagnostic.Location.SourceSpan;
TextSpan diagnosticSpan = context.Span;

// Retrieve the property passed by the analyzer
// Retrieve the properties passed by the analyzer
if (diagnostic.Properties[ClassUsingAttributeInsteadOfInheritanceAnalyzer.TypeNameKey] is not string typeName ||
diagnostic.Properties[ClassUsingAttributeInsteadOfInheritanceAnalyzer.AttributeTypeNameKey] is not string attributeTypeName)
{
Expand All @@ -59,11 +59,9 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
context.RegisterCodeFix(
CodeAction.Create(
title: "Inherit from ObservableObject",
createChangedDocument: token => UpdateReference(context.Document, root, classDeclaration, attributeTypeName),
createChangedDocument: token => RemoveAttribute(context.Document, root, classDeclaration, attributeTypeName),
equivalenceKey: "Inherit from ObservableObject"),
diagnostic);

return;
}
}

Expand All @@ -75,7 +73,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
/// <param name="classDeclaration">The <see cref="ClassDeclarationSyntax"/> to update.</param>
/// <param name="attributeTypeName">The name of the attribute that should be removed.</param>
/// <returns>An updated document with the applied code fix, and <paramref name="classDeclaration"/> inheriting from <c>ObservableObject</c>.</returns>
private static Task<Document> UpdateReference(Document document, SyntaxNode root, ClassDeclarationSyntax classDeclaration, string attributeTypeName)
private static Task<Document> RemoveAttribute(Document document, SyntaxNode root, ClassDeclarationSyntax classDeclaration, string attributeTypeName)
{
// Insert ObservableObject always in first position in the base list. The type might have
// some interfaces in the base list, so we just copy them back after ObservableObject.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public sealed class FieldReferenceForObservablePropertyFieldCodeFixer : CodeFixP
public override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
Diagnostic diagnostic = context.Diagnostics[0];
TextSpan diagnosticSpan = diagnostic.Location.SourceSpan;
TextSpan diagnosticSpan = context.Span;

// Retrieve the properties passed by the analyzer
if (diagnostic.Properties[FieldReferenceForObservablePropertyFieldAnalyzer.FieldNameKey] is not string fieldName ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,4 @@ Rule ID | Category | Severity | Notes
--------|----------|----------|-------
MVVMTK0037 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Error | See https://aka.ms/mvvmtoolkit/errors/mvvmtk0037
MVVMTK0038 | CommunityToolkit.Mvvm.SourceGenerators.RelayCommandGenerator | Error | See https://aka.ms/mvvmtoolkit/errors/mvvmtk0038
MVVMTK0039 | CommunityToolkit.Mvvm.SourceGenerators.RelayCommandGenerator | Warning | See https://aka.ms/mvvmtoolkit/errors/mvvmtk0039
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
<Compile Include="$(MSBuildThisFileDirectory)ComponentModel\ObservableValidatorValidateAllPropertiesGenerator.Execute.cs" />
<Compile Include="$(MSBuildThisFileDirectory)ComponentModel\TransitiveMembersGenerator.cs" />
<Compile Include="$(MSBuildThisFileDirectory)ComponentModel\TransitiveMembersGenerator.Execute.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\AsyncVoidReturningRelayCommandMethodAnalyzer.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\InvalidClassLevelNotifyDataErrorInfoAttributeAnalyzer.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\InvalidClassLevelNotifyPropertyChangedRecipientsAttributeAnalyzer.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\ClassUsingAttributeInsteadOfInheritanceAnalyzer.cs" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Collections.Immutable;
using System.Linq;
using CommunityToolkit.Mvvm.SourceGenerators.Extensions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using static CommunityToolkit.Mvvm.SourceGenerators.Diagnostics.DiagnosticDescriptors;

namespace CommunityToolkit.Mvvm.SourceGenerators;

/// <summary>
/// A diagnostic analyzer that generates a warning when using <c>[RelayCommand]</c> over an <see langword="async"/> <see cref="void"/> method.
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class AsyncVoidReturningRelayCommandMethodAnalyzer : DiagnosticAnalyzer
{
/// <inheritdoc/>
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(AsyncVoidReturningRelayCommandMethod);

/// <inheritdoc/>
public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
context.EnableConcurrentExecution();

context.RegisterCompilationStartAction(static context =>
{
// Get the symbol for [RelayCommand]
if (context.Compilation.GetTypeByMetadataName("CommunityToolkit.Mvvm.Input.RelayCommandAttribute") is not INamedTypeSymbol relayCommandSymbol)
{
return;
}
context.RegisterSymbolAction(context =>
{
// We're only looking for async void methods
if (context.Symbol is not IMethodSymbol { IsAsync: true, ReturnsVoid: true } methodSymbol)
{
return;
}
// We only care about methods annotated with [RelayCommand]
if (!methodSymbol.HasAttributeWithType(relayCommandSymbol))
{
return;
}
// Warn on async void methods using [RelayCommand] (they should return a Task instead)
context.ReportDiagnostic(Diagnostic.Create(
AsyncVoidReturningRelayCommandMethod,
context.Symbol.Locations.FirstOrDefault(),
context.Symbol));
}, SymbolKind.Method);
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ internal static class DiagnosticDescriptors
/// </summary>
public const string FieldReferenceForObservablePropertyFieldId = "MVVMTK0034";

/// <summary>
/// The diagnostic id for <see cref="AsyncVoidReturningRelayCommandMethod"/>.
/// </summary>
public const string AsyncVoidReturningRelayCommandMethodId = "MVVMTK0039";

/// <summary>
/// Gets a <see cref="DiagnosticDescriptor"/> indicating when a duplicate declaration of <see cref="INotifyPropertyChanged"/> would happen.
/// <para>
Expand Down Expand Up @@ -637,4 +642,20 @@ internal static class DiagnosticDescriptors
isEnabledByDefault: true,
description: "All attributes targeting the generated field or property for a method annotated with [RelayCommand] must be using valid expressions.",
helpLinkUri: "https://aka.ms/mvvmtoolkit/errors/mvvmtk0038");

/// <summary>
/// Gets a <see cref="DiagnosticDescriptor"/> indicating when a method with <c>[RelayCommand]</c> is async void.
/// <para>
/// Format: <c>"The method {0} annotated with [RelayCommand] is async void (make sure to return a Task type instead)"</c>.
/// </para>
/// </summary>
public static readonly DiagnosticDescriptor AsyncVoidReturningRelayCommandMethod = new DiagnosticDescriptor(
id: AsyncVoidReturningRelayCommandMethodId,
title: "Async void returning method annotated with RelayCommand",
messageFormat: "The method {0} annotated with [RelayCommand] is async void (make sure to return a Task type instead)",
category: typeof(RelayCommandGenerator).FullName,
defaultSeverity: DiagnosticSeverity.Warning,
isEnabledByDefault: true,
description: "All asynchronous methods annotated with [RelayCommand] should return a Task type, to benefit from the additional support provided by AsyncRelayCommand and AsyncRelayCommand<T>.",
helpLinkUri: "https://aka.ms/mvvmtoolkit/errors/mvvmtk0039");
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Threading.Tasks;
using CommunityToolkit.Mvvm.Input;
using Microsoft.CodeAnalysis.Testing;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using CSharpCodeFixTest = Microsoft.CodeAnalysis.CSharp.Testing.CSharpCodeFixTest<
CommunityToolkit.Mvvm.SourceGenerators.AsyncVoidReturningRelayCommandMethodAnalyzer,
CommunityToolkit.Mvvm.CodeFixers.AsyncVoidReturningRelayCommandMethodCodeFixer,
Microsoft.CodeAnalysis.Testing.Verifiers.MSTestVerifier>;
using CSharpCodeFixVerifier = Microsoft.CodeAnalysis.CSharp.Testing.CSharpCodeFixVerifier<
CommunityToolkit.Mvvm.SourceGenerators.AsyncVoidReturningRelayCommandMethodAnalyzer,
CommunityToolkit.Mvvm.CodeFixers.AsyncVoidReturningRelayCommandMethodCodeFixer,
Microsoft.CodeAnalysis.Testing.Verifiers.MSTestVerifier>;

namespace CommunityToolkit.Mvvm.SourceGenerators.Roslyn401.UnitTests;

[TestClass]
public class Test_AsyncVoidReturningRelayCommandMethodCodeFixer
{
[TestMethod]
public async Task AsyncVoidMethod_FileContainsSystemThreadingTasksUsingDirective()
{
string original = """
using System.Threading.Tasks;
using CommunityToolkit.Mvvm.Input;

partial class C
{
[RelayCommand]
private async void Foo()
{
}
}
""";

string @fixed = """
using System.Threading.Tasks;
using CommunityToolkit.Mvvm.Input;

partial class C
{
[RelayCommand]
private async Task Foo()
{
}
}
""";

CSharpCodeFixTest test = new()
{
TestCode = original,
FixedCode = @fixed,
ReferenceAssemblies = ReferenceAssemblies.Net.Net60
};

test.TestState.AdditionalReferences.Add(typeof(RelayCommand).Assembly);
test.ExpectedDiagnostics.AddRange(new[]
{
// /0/Test0.cs(7,24): error MVVMTK0039: The method C.Foo() annotated with [RelayCommand] is async void (make sure to return a Task type instead)
CSharpCodeFixVerifier.Diagnostic().WithSpan(7, 24, 7, 27).WithArguments("C.Foo()")
});

await test.RunAsync();
}

[TestMethod]
public async Task AsyncVoidMethod_FileDoesNotContainSystemThreadingTasksUsingDirective()
{
string original = """
using CommunityToolkit.Mvvm.Input;

partial class C
{
[RelayCommand]
private async void Foo()
{
}
}
""";

string @fixed = """
using System.Threading.Tasks;
using CommunityToolkit.Mvvm.Input;

partial class C
{
[RelayCommand]
private async Task Foo()
{
}
}
""";

CSharpCodeFixTest test = new()
{
TestCode = original,
FixedCode = @fixed,
ReferenceAssemblies = ReferenceAssemblies.Net.Net60
};

test.TestState.AdditionalReferences.Add(typeof(RelayCommand).Assembly);
test.ExpectedDiagnostics.AddRange(new[]
{
// /0/Test0.cs(7,24): error MVVMTK0039: The method C.Foo() annotated with [RelayCommand] is async void (make sure to return a Task type instead)
CSharpCodeFixVerifier.Diagnostic().WithSpan(6, 24, 6, 27).WithArguments("C.Foo()")
});

await test.RunAsync();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1801,6 +1801,25 @@ public class TestAttribute : Attribute
VerifyGeneratedDiagnostics<RelayCommandGenerator>(source, "MVVMTK0038");
}

[TestMethod]
public async Task AsyncVoidReturningRelayCommandMethodAnalyzer()
{
string source = """
using System;
using CommunityToolkit.Mvvm.Input;

public partial class MyViewModel
{
[RelayCommand]
private async void {|MVVMTK0039:TestAsync|}()
{
}
}
""";

await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<AsyncVoidReturningRelayCommandMethodAnalyzer>(source, LanguageVersion.CSharp8);
}

/// <summary>
/// Verifies the diagnostic errors for a given analyzer, and that all available source generators can run successfully with the input source (including subsequent compilation).
/// </summary>
Expand Down

0 comments on commit d6eb5cc

Please sign in to comment.