Skip to content

Commit d6eb5cc

Browse files
Sergio0694Sergio0694
authored
Merge pull request #714 from CommunityToolkit/dev/async-void-command-analyzer
Add new analyzer/fixer for async void [RelayCommand] methods
2 parents 7c3473a + c6eedee commit d6eb5cc

File tree

9 files changed

+304
-7
lines changed

9 files changed

+304
-7
lines changed

‎src/CommunityToolkit.Mvvm.CodeFixers/AsyncVoidReturningRelayCommandMethodCodeFixer.cs‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System.Collections.Immutable;
6+
using System.Composition;
7+
using System.Threading;
8+
using System.Threading.Tasks;
9+
using Microsoft.CodeAnalysis;
10+
using Microsoft.CodeAnalysis.CodeActions;
11+
using Microsoft.CodeAnalysis.CodeFixes;
12+
using Microsoft.CodeAnalysis.CSharp.Syntax;
13+
using Microsoft.CodeAnalysis.Editing;
14+
using Microsoft.CodeAnalysis.Simplification;
15+
using Microsoft.CodeAnalysis.Text;
16+
using static CommunityToolkit.Mvvm.SourceGenerators.Diagnostics.DiagnosticDescriptors;
17+
18+
namespace CommunityToolkit.Mvvm.CodeFixers;
19+
20+
/// <summary>
21+
/// 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.
22+
/// </summary>
23+
[ExportCodeFixProvider(LanguageNames.CSharp)]
24+
[Shared]
25+
public sealed class AsyncVoidReturningRelayCommandMethodCodeFixer : CodeFixProvider
26+
{
27+
/// <inheritdoc/>
28+
public override ImmutableArray<string> FixableDiagnosticIds { get; } = ImmutableArray.Create(AsyncVoidReturningRelayCommandMethodId);
29+
30+
/// <inheritdoc/>
31+
public override FixAllProvider? GetFixAllProvider()
32+
{
33+
return WellKnownFixAllProviders.BatchFixer;
34+
}
35+
36+
/// <inheritdoc/>
37+
public override async Task RegisterCodeFixesAsync(CodeFixContext context)
38+
{
39+
Diagnostic diagnostic = context.Diagnostics[0];
40+
TextSpan diagnosticSpan = context.Span;
41+
42+
SyntaxNode? root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
43+
44+
// Get the method declaration from the target diagnostic
45+
if (root!.FindNode(diagnosticSpan) is MethodDeclarationSyntax methodDeclaration)
46+
{
47+
// Register the code fix to update the return type to be Task instead
48+
context.RegisterCodeFix(
49+
CodeAction.Create(
50+
title: "Change return type to Task",
51+
createChangedDocument: token => ChangeReturnType(context.Document, root, methodDeclaration, token),
52+
equivalenceKey: "Change return type to Task"),
53+
diagnostic);
54+
}
55+
}
56+
57+
/// <summary>
58+
/// Applies the code fix to a target method declaration and returns an updated document.
59+
/// </summary>
60+
/// <param name="document">The original document being fixed.</param>
61+
/// <param name="root">The original tree root belonging to the current document.</param>
62+
/// <param name="methodDeclaration">The <see cref="MethodDeclarationSyntax"/> to update.</param>
63+
/// <param name="cancellationToken">The cancellation token for the operation.</param>
64+
/// <returns>An updated document with the applied code fix, and the return type of the method being <see cref="Task"/>.</returns>
65+
private static async Task<Document> ChangeReturnType(Document document, SyntaxNode root, MethodDeclarationSyntax methodDeclaration, CancellationToken cancellationToken)
66+
{
67+
// Get the semantic model (bail if it's not available)
68+
if (await document.GetSemanticModelAsync(cancellationToken) is not SemanticModel semanticModel)
69+
{
70+
return document;
71+
}
72+
73+
// Also bail if we can't resolve the Task symbol (this should really never happen)
74+
if (semanticModel.Compilation.GetTypeByMetadataName("System.Threading.Tasks.Task") is not INamedTypeSymbol taskSymbol)
75+
{
76+
return document;
77+
}
78+
79+
// Create the new syntax node for the return, and configure it to automatically add "using System.Threading.Tasks;" if needed
80+
SyntaxNode typeSyntax = SyntaxGenerator.GetGenerator(document).TypeExpression(taskSymbol).WithAdditionalAnnotations(Simplifier.AddImportsAnnotation);
81+
82+
// Replace the void return type with the newly created Task type expression
83+
return document.WithSyntaxRoot(root.ReplaceNode(methodDeclaration.ReturnType, typeSyntax));
84+
}
85+
}

‎src/CommunityToolkit.Mvvm.CodeFixers/ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,9 @@ public sealed class ClassUsingAttributeInsteadOfInheritanceCodeFixer : CodeFixPr
4040
public override async Task RegisterCodeFixesAsync(CodeFixContext context)
4141
{
4242
Diagnostic diagnostic = context.Diagnostics[0];
43-
TextSpan diagnosticSpan = diagnostic.Location.SourceSpan;
43+
TextSpan diagnosticSpan = context.Span;
4444

45-
// Retrieve the property passed by the analyzer
45+
// Retrieve the properties passed by the analyzer
4646
if (diagnostic.Properties[ClassUsingAttributeInsteadOfInheritanceAnalyzer.TypeNameKey] is not string typeName ||
4747
diagnostic.Properties[ClassUsingAttributeInsteadOfInheritanceAnalyzer.AttributeTypeNameKey] is not string attributeTypeName)
4848
{
@@ -59,11 +59,9 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
5959
context.RegisterCodeFix(
6060
CodeAction.Create(
6161
title: "Inherit from ObservableObject",
62-
createChangedDocument: token => UpdateReference(context.Document, root, classDeclaration, attributeTypeName),
62+
createChangedDocument: token => RemoveAttribute(context.Document, root, classDeclaration, attributeTypeName),
6363
equivalenceKey: "Inherit from ObservableObject"),
6464
diagnostic);
65-
66-
return;
6765
}
6866
}
6967

@@ -75,7 +73,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
7573
/// <param name="classDeclaration">The <see cref="ClassDeclarationSyntax"/> to update.</param>
7674
/// <param name="attributeTypeName">The name of the attribute that should be removed.</param>
7775
/// <returns>An updated document with the applied code fix, and <paramref name="classDeclaration"/> inheriting from <c>ObservableObject</c>.</returns>
78-
private static Task<Document> UpdateReference(Document document, SyntaxNode root, ClassDeclarationSyntax classDeclaration, string attributeTypeName)
76+
private static Task<Document> RemoveAttribute(Document document, SyntaxNode root, ClassDeclarationSyntax classDeclaration, string attributeTypeName)
7977
{
8078
// Insert ObservableObject always in first position in the base list. The type might have
8179
// some interfaces in the base list, so we just copy them back after ObservableObject.

‎src/CommunityToolkit.Mvvm.CodeFixers/FieldReferenceForObservablePropertyFieldCodeFixer.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public sealed class FieldReferenceForObservablePropertyFieldCodeFixer : CodeFixP
3737
public override async Task RegisterCodeFixesAsync(CodeFixContext context)
3838
{
3939
Diagnostic diagnostic = context.Diagnostics[0];
40-
TextSpan diagnosticSpan = diagnostic.Location.SourceSpan;
40+
TextSpan diagnosticSpan = context.Span;
4141

4242
// Retrieve the properties passed by the analyzer
4343
if (diagnostic.Properties[FieldReferenceForObservablePropertyFieldAnalyzer.FieldNameKey] is not string fieldName ||

‎src/CommunityToolkit.Mvvm.SourceGenerators/AnalyzerReleases.Shipped.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,3 +66,4 @@ Rule ID | Category | Severity | Notes
6666
--------|----------|----------|-------
6767
MVVMTK0037 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Error | See https://aka.ms/mvvmtoolkit/errors/mvvmtk0037
6868
MVVMTK0038 | CommunityToolkit.Mvvm.SourceGenerators.RelayCommandGenerator | Error | See https://aka.ms/mvvmtoolkit/errors/mvvmtk0038
69+
MVVMTK0039 | CommunityToolkit.Mvvm.SourceGenerators.RelayCommandGenerator | Warning | See https://aka.ms/mvvmtoolkit/errors/mvvmtk0039

‎src/CommunityToolkit.Mvvm.SourceGenerators/CommunityToolkit.Mvvm.SourceGenerators.projitems‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
<Compile Include="$(MSBuildThisFileDirectory)ComponentModel\ObservableValidatorValidateAllPropertiesGenerator.Execute.cs" />
4040
<Compile Include="$(MSBuildThisFileDirectory)ComponentModel\TransitiveMembersGenerator.cs" />
4141
<Compile Include="$(MSBuildThisFileDirectory)ComponentModel\TransitiveMembersGenerator.Execute.cs" />
42+
<Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\AsyncVoidReturningRelayCommandMethodAnalyzer.cs" />
4243
<Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\InvalidClassLevelNotifyDataErrorInfoAttributeAnalyzer.cs" />
4344
<Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\InvalidClassLevelNotifyPropertyChangedRecipientsAttributeAnalyzer.cs" />
4445
<Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\ClassUsingAttributeInsteadOfInheritanceAnalyzer.cs" />

‎src/CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/Analyzers/AsyncVoidReturningRelayCommandMethodAnalyzer.cs‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System.Collections.Immutable;
6+
using System.Linq;
7+
using CommunityToolkit.Mvvm.SourceGenerators.Extensions;
8+
using Microsoft.CodeAnalysis;
9+
using Microsoft.CodeAnalysis.Diagnostics;
10+
using static CommunityToolkit.Mvvm.SourceGenerators.Diagnostics.DiagnosticDescriptors;
11+
12+
namespace CommunityToolkit.Mvvm.SourceGenerators;
13+
14+
/// <summary>
15+
/// A diagnostic analyzer that generates a warning when using <c>[RelayCommand]</c> over an <see langword="async"/> <see cref="void"/> method.
16+
/// </summary>
17+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
18+
public sealed class AsyncVoidReturningRelayCommandMethodAnalyzer : DiagnosticAnalyzer
19+
{
20+
/// <inheritdoc/>
21+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(AsyncVoidReturningRelayCommandMethod);
22+
23+
/// <inheritdoc/>
24+
public override void Initialize(AnalysisContext context)
25+
{
26+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
27+
context.EnableConcurrentExecution();
28+
29+
context.RegisterCompilationStartAction(static context =>
30+
{
31+
// Get the symbol for [RelayCommand]
32+
if (context.Compilation.GetTypeByMetadataName("CommunityToolkit.Mvvm.Input.RelayCommandAttribute") is not INamedTypeSymbol relayCommandSymbol)
33+
{
34+
return;
35+
}
36+
37+
context.RegisterSymbolAction(context =>
38+
{
39+
// We're only looking for async void methods
40+
if (context.Symbol is not IMethodSymbol { IsAsync: true, ReturnsVoid: true } methodSymbol)
41+
{
42+
return;
43+
}
44+
45+
// We only care about methods annotated with [RelayCommand]
46+
if (!methodSymbol.HasAttributeWithType(relayCommandSymbol))
47+
{
48+
return;
49+
}
50+
51+
// Warn on async void methods using [RelayCommand] (they should return a Task instead)
52+
context.ReportDiagnostic(Diagnostic.Create(
53+
AsyncVoidReturningRelayCommandMethod,
54+
context.Symbol.Locations.FirstOrDefault(),
55+
context.Symbol));
56+
}, SymbolKind.Method);
57+
});
58+
}
59+
}

‎src/CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ internal static class DiagnosticDescriptors
2929
/// </summary>
3030
public const string FieldReferenceForObservablePropertyFieldId = "MVVMTK0034";
3131

32+
/// <summary>
33+
/// The diagnostic id for <see cref="AsyncVoidReturningRelayCommandMethod"/>.
34+
/// </summary>
35+
public const string AsyncVoidReturningRelayCommandMethodId = "MVVMTK0039";
36+
3237
/// <summary>
3338
/// Gets a <see cref="DiagnosticDescriptor"/> indicating when a duplicate declaration of <see cref="INotifyPropertyChanged"/> would happen.
3439
/// <para>
@@ -637,4 +642,20 @@ internal static class DiagnosticDescriptors
637642
isEnabledByDefault: true,
638643
description: "All attributes targeting the generated field or property for a method annotated with [RelayCommand] must be using valid expressions.",
639644
helpLinkUri: "https://aka.ms/mvvmtoolkit/errors/mvvmtk0038");
645+
646+
/// <summary>
647+
/// Gets a <see cref="DiagnosticDescriptor"/> indicating when a method with <c>[RelayCommand]</c> is async void.
648+
/// <para>
649+
/// Format: <c>"The method {0} annotated with [RelayCommand] is async void (make sure to return a Task type instead)"</c>.
650+
/// </para>
651+
/// </summary>
652+
public static readonly DiagnosticDescriptor AsyncVoidReturningRelayCommandMethod = new DiagnosticDescriptor(
653+
id: AsyncVoidReturningRelayCommandMethodId,
654+
title: "Async void returning method annotated with RelayCommand",
655+
messageFormat: "The method {0} annotated with [RelayCommand] is async void (make sure to return a Task type instead)",
656+
category: typeof(RelayCommandGenerator).FullName,
657+
defaultSeverity: DiagnosticSeverity.Warning,
658+
isEnabledByDefault: true,
659+
description: "All asynchronous methods annotated with [RelayCommand] should return a Task type, to benefit from the additional support provided by AsyncRelayCommand and AsyncRelayCommand<T>.",
660+
helpLinkUri: "https://aka.ms/mvvmtoolkit/errors/mvvmtk0039");
640661
}

‎tests/CommunityToolkit.Mvvm.SourceGenerators.Roslyn401.UnitTests/Test_AsyncVoidReturningRelayCommandMethodCodeFixer.cs‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System.Threading.Tasks;
6+
using CommunityToolkit.Mvvm.Input;
7+
using Microsoft.CodeAnalysis.Testing;
8+
using Microsoft.VisualStudio.TestTools.UnitTesting;
9+
using CSharpCodeFixTest = Microsoft.CodeAnalysis.CSharp.Testing.CSharpCodeFixTest<
10+
CommunityToolkit.Mvvm.SourceGenerators.AsyncVoidReturningRelayCommandMethodAnalyzer,
11+
CommunityToolkit.Mvvm.CodeFixers.AsyncVoidReturningRelayCommandMethodCodeFixer,
12+
Microsoft.CodeAnalysis.Testing.Verifiers.MSTestVerifier>;
13+
using CSharpCodeFixVerifier = Microsoft.CodeAnalysis.CSharp.Testing.CSharpCodeFixVerifier<
14+
CommunityToolkit.Mvvm.SourceGenerators.AsyncVoidReturningRelayCommandMethodAnalyzer,
15+
CommunityToolkit.Mvvm.CodeFixers.AsyncVoidReturningRelayCommandMethodCodeFixer,
16+
Microsoft.CodeAnalysis.Testing.Verifiers.MSTestVerifier>;
17+
18+
namespace CommunityToolkit.Mvvm.SourceGenerators.Roslyn401.UnitTests;
19+
20+
[TestClass]
21+
public class Test_AsyncVoidReturningRelayCommandMethodCodeFixer
22+
{
23+
[TestMethod]
24+
public async Task AsyncVoidMethod_FileContainsSystemThreadingTasksUsingDirective()
25+
{
26+
string original = """
27+
using System.Threading.Tasks;
28+
using CommunityToolkit.Mvvm.Input;
29+
30+
partial class C
31+
{
32+
[RelayCommand]
33+
private async void Foo()
34+
{
35+
}
36+
}
37+
""";
38+
39+
string @fixed = """
40+
using System.Threading.Tasks;
41+
using CommunityToolkit.Mvvm.Input;
42+
43+
partial class C
44+
{
45+
[RelayCommand]
46+
private async Task Foo()
47+
{
48+
}
49+
}
50+
""";
51+
52+
CSharpCodeFixTest test = new()
53+
{
54+
TestCode = original,
55+
FixedCode = @fixed,
56+
ReferenceAssemblies = ReferenceAssemblies.Net.Net60
57+
};
58+
59+
test.TestState.AdditionalReferences.Add(typeof(RelayCommand).Assembly);
60+
test.ExpectedDiagnostics.AddRange(new[]
61+
{
62+
// /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)
63+
CSharpCodeFixVerifier.Diagnostic().WithSpan(7, 24, 7, 27).WithArguments("C.Foo()")
64+
});
65+
66+
await test.RunAsync();
67+
}
68+
69+
[TestMethod]
70+
public async Task AsyncVoidMethod_FileDoesNotContainSystemThreadingTasksUsingDirective()
71+
{
72+
string original = """
73+
using CommunityToolkit.Mvvm.Input;
74+
75+
partial class C
76+
{
77+
[RelayCommand]
78+
private async void Foo()
79+
{
80+
}
81+
}
82+
""";
83+
84+
string @fixed = """
85+
using System.Threading.Tasks;
86+
using CommunityToolkit.Mvvm.Input;
87+
88+
partial class C
89+
{
90+
[RelayCommand]
91+
private async Task Foo()
92+
{
93+
}
94+
}
95+
""";
96+
97+
CSharpCodeFixTest test = new()
98+
{
99+
TestCode = original,
100+
FixedCode = @fixed,
101+
ReferenceAssemblies = ReferenceAssemblies.Net.Net60
102+
};
103+
104+
test.TestState.AdditionalReferences.Add(typeof(RelayCommand).Assembly);
105+
test.ExpectedDiagnostics.AddRange(new[]
106+
{
107+
// /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)
108+
CSharpCodeFixVerifier.Diagnostic().WithSpan(6, 24, 6, 27).WithArguments("C.Foo()")
109+
});
110+
111+
await test.RunAsync();
112+
}
113+
}

‎tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsDiagnostics.cs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1801,6 +1801,25 @@ public class TestAttribute : Attribute
18011801
VerifyGeneratedDiagnostics<RelayCommandGenerator>(source, "MVVMTK0038");
18021802
}
18031803

1804+
[TestMethod]
1805+
public async Task AsyncVoidReturningRelayCommandMethodAnalyzer()
1806+
{
1807+
string source = """
1808+
using System;
1809+
using CommunityToolkit.Mvvm.Input;
1810+
1811+
public partial class MyViewModel
1812+
{
1813+
[RelayCommand]
1814+
private async void {|MVVMTK0039:TestAsync|}()
1815+
{
1816+
}
1817+
}
1818+
""";
1819+
1820+
await VerifyAnalyzerDiagnosticsAndSuccessfulGeneration<AsyncVoidReturningRelayCommandMethodAnalyzer>(source, LanguageVersion.CSharp8);
1821+
}
1822+
18041823
/// <summary>
18051824
/// Verifies the diagnostic errors for a given analyzer, and that all available source generators can run successfully with the input source (including subsequent compilation).
18061825
/// </summary>

0 commit comments

Comments
 (0)