From 30c4ad472c2aa454d02c7639622b132fc68cf6a0 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 9 Mar 2023 13:53:42 +0100 Subject: [PATCH 1/5] Support partial [RelayCommand] methods --- .../Input/RelayCommandGenerator.Execute.cs | 9 +++ .../Test_RelayCommandAttribute.cs | 76 +++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/Input/RelayCommandGenerator.Execute.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/Input/RelayCommandGenerator.Execute.cs index 33b2a752e..4160f5e72 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/Input/RelayCommandGenerator.Execute.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/Input/RelayCommandGenerator.Execute.cs @@ -430,6 +430,15 @@ private static bool IsCommandDefinitionUnique(IMethodSymbol methodSymbol, in Imm return true; } + // If the two method symbols are partial and either is the implementation of the other one, this is allowed + if ((methodSymbol is { IsPartialDefinition: true, PartialImplementationPart: { } partialImplementation } && + SymbolEqualityComparer.Default.Equals(otherSymbol, partialImplementation)) || + (otherSymbol is { IsPartialDefinition: true, PartialImplementationPart: { } otherPartialImplementation } && + SymbolEqualityComparer.Default.Equals(methodSymbol, otherPartialImplementation))) + { + continue; + } + diagnostics.Add( MultipleRelayCommandMethodOverloadsError, methodSymbol, diff --git a/tests/CommunityToolkit.Mvvm.UnitTests/Test_RelayCommandAttribute.cs b/tests/CommunityToolkit.Mvvm.UnitTests/Test_RelayCommandAttribute.cs index 3114cf1e5..ecf4ff05d 100644 --- a/tests/CommunityToolkit.Mvvm.UnitTests/Test_RelayCommandAttribute.cs +++ b/tests/CommunityToolkit.Mvvm.UnitTests/Test_RelayCommandAttribute.cs @@ -659,6 +659,42 @@ static void ValidateTestAttribute(TestValidationAttribute testAttribute) Assert.AreEqual(testAttribute2.Animal, (Test_ObservablePropertyAttribute.Animal)67); } + // See https://github.com/CommunityToolkit/dotnet/issues/632 + [TestMethod] + public void Test_RelayCommandAttribute_WithPartialCommandMethodDefinitions() + { + ModelWithPartialCommandMethods model = new(); + + Assert.IsInstanceOfType(model.FooCommand); + Assert.IsInstanceOfType>(model.BarCommand); + Assert.IsInstanceOfType(model.BazCommand); + Assert.IsInstanceOfType(model.FooBarCommand); + + FieldInfo bazField = typeof(ModelWithPartialCommandMethods).GetField("bazCommand", BindingFlags.Instance | BindingFlags.NonPublic)!; + + Assert.IsNotNull(bazField.GetCustomAttribute()); + Assert.IsNotNull(bazField.GetCustomAttribute()); + Assert.AreEqual(bazField.GetCustomAttribute()!.Length, 1); + + PropertyInfo bazProperty = typeof(ModelWithPartialCommandMethods).GetProperty("BazCommand")!; + + Assert.IsNotNull(bazProperty.GetCustomAttribute()); + Assert.AreEqual(bazProperty.GetCustomAttribute()!.Length, 2); + Assert.IsNotNull(bazProperty.GetCustomAttribute()); + + FieldInfo fooBarField = typeof(ModelWithPartialCommandMethods).GetField("fooBarCommand", BindingFlags.Instance | BindingFlags.NonPublic)!; + + Assert.IsNotNull(fooBarField.GetCustomAttribute()); + Assert.IsNotNull(fooBarField.GetCustomAttribute()); + Assert.AreEqual(fooBarField.GetCustomAttribute()!.Length, 1); + + PropertyInfo fooBarProperty = typeof(ModelWithPartialCommandMethods).GetProperty("FooBarCommand")!; + + Assert.IsNotNull(fooBarProperty.GetCustomAttribute()); + Assert.AreEqual(fooBarProperty.GetCustomAttribute()!.Length, 2); + Assert.IsNotNull(fooBarProperty.GetCustomAttribute()); + } + #region Region public class Region { @@ -1202,4 +1238,44 @@ public TestValidationAttribute(object? o, Type t, bool flag, double d, string[] public Test_ObservablePropertyAttribute.Animal Animal { get; set; } } + + public partial class ModelWithPartialCommandMethods + { + [RelayCommand] + private partial void Foo(); + + private partial void Foo() + { + } + + private partial void Bar(string name); + + [RelayCommand] + private partial void Bar(string name) + { + } + + [RelayCommand] + [field: Required] + [property: MinLength(2)] + partial void Baz(); + + [field: MinLength(1)] + [property: XmlIgnore] + partial void Baz() + { + } + + [field: Required] + [property: MinLength(2)] + private partial Task FooBarAsync(); + + [RelayCommand] + [field: MinLength(1)] + [property: XmlIgnore] + private partial Task FooBarAsync() + { + return Task.CompletedTask; + } + } } From a845ad4b16789690ff31a6e99a1b59845f13ea8c Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 9 Mar 2023 14:13:34 +0100 Subject: [PATCH 2/5] Fix attributes gathering from partial command methods --- .../Input/RelayCommandGenerator.Execute.cs | 81 ++++++++++------ .../Test_SourceGeneratorsCodegen.cs | 93 +++++++++++++++++++ 2 files changed, 145 insertions(+), 29 deletions(-) diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/Input/RelayCommandGenerator.Execute.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/Input/RelayCommandGenerator.Execute.cs index 4160f5e72..95eeef1c6 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/Input/RelayCommandGenerator.Execute.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/Input/RelayCommandGenerator.Execute.cs @@ -961,52 +961,75 @@ private static void GatherForwardedAttributes( using ImmutableArrayBuilder fieldAttributesInfo = ImmutableArrayBuilder.Rent(); using ImmutableArrayBuilder propertyAttributesInfo = ImmutableArrayBuilder.Rent(); - foreach (SyntaxReference syntaxReference in methodSymbol.DeclaringSyntaxReferences) + static void GatherForwardedAttributes( + IMethodSymbol methodSymbol, + SemanticModel semanticModel, + CancellationToken token, + in ImmutableArrayBuilder diagnostics, + in ImmutableArrayBuilder fieldAttributesInfo, + in ImmutableArrayBuilder propertyAttributesInfo) { - // Try to get the target method declaration syntax node - if (syntaxReference.GetSyntax(token) is not MethodDeclarationSyntax methodDeclaration) + foreach (SyntaxReference syntaxReference in methodSymbol.DeclaringSyntaxReferences) { - continue; - } - - // Gather explicit forwarded attributes info - foreach (AttributeListSyntax attributeList in methodDeclaration.AttributeLists) - { - // Same as in the [ObservableProperty] generator, except we're also looking for fields here - if (attributeList.Target?.Identifier is not SyntaxToken(SyntaxKind.PropertyKeyword or SyntaxKind.FieldKeyword)) + // Try to get the target method declaration syntax node + if (syntaxReference.GetSyntax(token) is not MethodDeclarationSyntax methodDeclaration) { continue; } - foreach (AttributeSyntax attribute in attributeList.Attributes) + // Gather explicit forwarded attributes info + foreach (AttributeListSyntax attributeList in methodDeclaration.AttributeLists) { - // Get the symbol info for the attribute (once again just like in the [ObservableProperty] generator) - if (!semanticModel.GetSymbolInfo(attribute, token).TryGetAttributeTypeSymbol(out INamedTypeSymbol? attributeTypeSymbol)) + // Same as in the [ObservableProperty] generator, except we're also looking for fields here + if (attributeList.Target?.Identifier is not SyntaxToken(SyntaxKind.PropertyKeyword or SyntaxKind.FieldKeyword)) { - diagnostics.Add( - InvalidFieldOrPropertyTargetedAttributeOnRelayCommandMethod, - attribute, - methodSymbol, - attribute.Name); - continue; } - AttributeInfo attributeInfo = AttributeInfo.From(attributeTypeSymbol, semanticModel, attribute.ArgumentList?.Arguments ?? Enumerable.Empty(), token); - - // Add the new attribute info to the right builder - if (attributeList.Target?.Identifier is SyntaxToken(SyntaxKind.FieldKeyword)) - { - fieldAttributesInfo.Add(attributeInfo); - } - else + foreach (AttributeSyntax attribute in attributeList.Attributes) { - propertyAttributesInfo.Add(attributeInfo); + // Get the symbol info for the attribute (once again just like in the [ObservableProperty] generator) + if (!semanticModel.GetSymbolInfo(attribute, token).TryGetAttributeTypeSymbol(out INamedTypeSymbol? attributeTypeSymbol)) + { + diagnostics.Add( + InvalidFieldOrPropertyTargetedAttributeOnRelayCommandMethod, + attribute, + methodSymbol, + attribute.Name); + + continue; + } + + AttributeInfo attributeInfo = AttributeInfo.From(attributeTypeSymbol, semanticModel, attribute.ArgumentList?.Arguments ?? Enumerable.Empty(), token); + + // Add the new attribute info to the right builder + if (attributeList.Target?.Identifier is SyntaxToken(SyntaxKind.FieldKeyword)) + { + fieldAttributesInfo.Add(attributeInfo); + } + else + { + propertyAttributesInfo.Add(attributeInfo); + } } } } } + // Gather attributes from the method declaration + GatherForwardedAttributes(methodSymbol, semanticModel, token, in diagnostics, in fieldAttributesInfo, in propertyAttributesInfo); + + // If the method is a partial definition, also gather attributes from the implementation part + if (methodSymbol is { IsPartialDefinition: true, PartialImplementationPart: { } partialImplementation }) + { + GatherForwardedAttributes(partialImplementation, semanticModel, token, in diagnostics, in fieldAttributesInfo, in propertyAttributesInfo); + } + else if (methodSymbol is { IsPartialDefinition: false, PartialDefinitionPart: { } partialDefinition }) + { + // If the method is a partial implementation, also gather attributes from the definition part + GatherForwardedAttributes(partialDefinition, semanticModel, token, in diagnostics, in fieldAttributesInfo, in propertyAttributesInfo); + } + fieldAttributes = fieldAttributesInfo.ToImmutable(); propertyAttributes = propertyAttributesInfo.ToImmutable(); } diff --git a/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsCodegen.cs b/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsCodegen.cs index fa76a7126..a4ff70bdc 100644 --- a/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsCodegen.cs +++ b/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsCodegen.cs @@ -268,6 +268,99 @@ partial class MyViewModel VerifyGenerateSources(source, new[] { new RelayCommandGenerator() }, ("MyApp.MyViewModel.Test.g.cs", result)); } + // See https://github.com/CommunityToolkit/dotnet/issues/632 + [TestMethod] + public void RelayCommandMethodWithForwardedAttributesOverPartialDeclarations_MergesAttributes() + { + string source = """ + using CommunityToolkit.Mvvm.Input; + + #nullable enable + + namespace MyApp; + + partial class MyViewModel + { + [RelayCommand] + [field: Value(0)] + [property: Value(1)] + private partial void Test1() + { + } + + [field: Value(2)] + [property: Value(3)] + private partial void Test1(); + + [field: Value(0)] + [property: Value(1)] + private partial void Test2() + { + } + + [RelayCommand] + [field: Value(2)] + [property: Value(3)] + private partial void Test2(); + } + + public class ValueAttribute : Attribute + { + public ValueAttribute(object value) + { + } + } + """; + + string result1 = """ + // + #pragma warning disable + #nullable enable + namespace MyApp + { + partial class MyViewModel + { + /// The backing field for . + [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.RelayCommandGenerator", "8.1.0.0")] + [global::MyApp.ValueAttribute(0)] + [global::MyApp.ValueAttribute(2)] + private global::CommunityToolkit.Mvvm.Input.RelayCommand? test1Command; + /// Gets an instance wrapping . + [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.RelayCommandGenerator", "8.1.0.0")] + [global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage] + [global::MyApp.ValueAttribute(1)] + [global::MyApp.ValueAttribute(3)] + public global::CommunityToolkit.Mvvm.Input.IRelayCommand Test1Command => test1Command ??= new global::CommunityToolkit.Mvvm.Input.RelayCommand(new global::System.Action(Test1)); + } + } + """; + + string result2 = """ + // + #pragma warning disable + #nullable enable + namespace MyApp + { + partial class MyViewModel + { + /// The backing field for . + [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.RelayCommandGenerator", "8.1.0.0")] + [global::MyApp.ValueAttribute(2)] + [global::MyApp.ValueAttribute(0)] + private global::CommunityToolkit.Mvvm.Input.RelayCommand? test2Command; + /// Gets an instance wrapping . + [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.RelayCommandGenerator", "8.1.0.0")] + [global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage] + [global::MyApp.ValueAttribute(3)] + [global::MyApp.ValueAttribute(1)] + public global::CommunityToolkit.Mvvm.Input.IRelayCommand Test2Command => test2Command ??= new global::CommunityToolkit.Mvvm.Input.RelayCommand(new global::System.Action(Test2)); + } + } + """; + + VerifyGenerateSources(source, new[] { new RelayCommandGenerator() }, ("MyApp.MyViewModel.Test1.g.cs", result1), ("MyApp.MyViewModel.Test2.g.cs", result2)); + } + [TestMethod] public void ObservablePropertyWithinGenericAndNestedTypes() { From 9297335238ca653c2eb9b3ef270f5012880ad8af Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 9 Mar 2023 14:59:18 +0100 Subject: [PATCH 3/5] Fix partial methods support with Roslyn 4.0 Also ensure a consistent order for forwarded attributes --- .../Input/RelayCommandGenerator.Execute.cs | 16 +++++++++------- .../Polyfills/SyntaxValueProviderExtensions.cs | 9 +++++++++ .../Test_SourceGeneratorsCodegen.cs | 4 ++-- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/Input/RelayCommandGenerator.Execute.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/Input/RelayCommandGenerator.Execute.cs index 95eeef1c6..07f2a956b 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/Input/RelayCommandGenerator.Execute.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/Input/RelayCommandGenerator.Execute.cs @@ -1016,18 +1016,20 @@ static void GatherForwardedAttributes( } } - // Gather attributes from the method declaration - GatherForwardedAttributes(methodSymbol, semanticModel, token, in diagnostics, in fieldAttributesInfo, in propertyAttributesInfo); - // If the method is a partial definition, also gather attributes from the implementation part - if (methodSymbol is { IsPartialDefinition: true, PartialImplementationPart: { } partialImplementation }) + if (methodSymbol is { IsPartialDefinition: true } or { PartialDefinitionPart: not null }) { + IMethodSymbol partialDefinition = methodSymbol.PartialDefinitionPart ?? methodSymbol; + IMethodSymbol partialImplementation = methodSymbol.PartialImplementationPart ?? methodSymbol; + + // We always give priority to the partial definition, to ensure a predictable and testable ordering + GatherForwardedAttributes(partialDefinition, semanticModel, token, in diagnostics, in fieldAttributesInfo, in propertyAttributesInfo); GatherForwardedAttributes(partialImplementation, semanticModel, token, in diagnostics, in fieldAttributesInfo, in propertyAttributesInfo); } - else if (methodSymbol is { IsPartialDefinition: false, PartialDefinitionPart: { } partialDefinition }) + else { - // If the method is a partial implementation, also gather attributes from the definition part - GatherForwardedAttributes(partialDefinition, semanticModel, token, in diagnostics, in fieldAttributesInfo, in propertyAttributesInfo); + // If the method is not a partial definition/implementation, just gather attributes from the method with no modifications + GatherForwardedAttributes(methodSymbol, semanticModel, token, in diagnostics, in fieldAttributesInfo, in propertyAttributesInfo); } fieldAttributes = fieldAttributesInfo.ToImmutable(); diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/Polyfills/SyntaxValueProviderExtensions.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/Polyfills/SyntaxValueProviderExtensions.cs index 44a949823..402a421d6 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/Polyfills/SyntaxValueProviderExtensions.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/Polyfills/SyntaxValueProviderExtensions.cs @@ -59,6 +59,15 @@ public static IncrementalValuesProvider ForAttributeWithMetadataName( return null; } + // Edge case: if the symbol is a partial method, skip the implementation part and only process the partial method + // definition. This is needed because attributes will be reported as available on both the definition and the + // implementation part. To avoid generating duplicate files, we only give priority to the definition part. + // On Roslyn 4.3+, ForAttributeWithMetadataName will already only return the symbol the attribute was located on. + if (symbol is IMethodSymbol { IsPartialDefinition: false, PartialDefinitionPart: not null }) + { + return null; + } + // Create the GeneratorAttributeSyntaxContext value to pass to the input transform. The attributes array // will only ever have a single value, but that's fine with the attributes the various generators look for. GeneratorAttributeSyntaxContext syntaxContext = new( diff --git a/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsCodegen.cs b/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsCodegen.cs index a4ff70bdc..3a0ccd8f9 100644 --- a/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsCodegen.cs +++ b/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsCodegen.cs @@ -322,14 +322,14 @@ partial class MyViewModel { /// The backing field for . [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.RelayCommandGenerator", "8.1.0.0")] - [global::MyApp.ValueAttribute(0)] [global::MyApp.ValueAttribute(2)] + [global::MyApp.ValueAttribute(0)] private global::CommunityToolkit.Mvvm.Input.RelayCommand? test1Command; /// Gets an instance wrapping . [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.RelayCommandGenerator", "8.1.0.0")] [global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage] - [global::MyApp.ValueAttribute(1)] [global::MyApp.ValueAttribute(3)] + [global::MyApp.ValueAttribute(1)] public global::CommunityToolkit.Mvvm.Input.IRelayCommand Test1Command => test1Command ??= new global::CommunityToolkit.Mvvm.Input.RelayCommand(new global::System.Action(Test1)); } } From a925b19300502c8064f45833f5caf4580d816fa2 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 9 Mar 2023 15:04:05 +0100 Subject: [PATCH 4/5] Remove unnecessary loop, use list pattern instead --- .../Input/RelayCommandGenerator.Execute.cs | 65 ++++++++++--------- 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/Input/RelayCommandGenerator.Execute.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/Input/RelayCommandGenerator.Execute.cs index 07f2a956b..c3dd8cc64 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/Input/RelayCommandGenerator.Execute.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/Input/RelayCommandGenerator.Execute.cs @@ -969,48 +969,51 @@ static void GatherForwardedAttributes( in ImmutableArrayBuilder fieldAttributesInfo, in ImmutableArrayBuilder propertyAttributesInfo) { - foreach (SyntaxReference syntaxReference in methodSymbol.DeclaringSyntaxReferences) + // Get the single syntax reference for the input method symbol (there should be only one) + if (methodSymbol.DeclaringSyntaxReferences is not [SyntaxReference syntaxReference]) { - // Try to get the target method declaration syntax node - if (syntaxReference.GetSyntax(token) is not MethodDeclarationSyntax methodDeclaration) + return; + } + + // Try to get the target method declaration syntax node + if (syntaxReference.GetSyntax(token) is not MethodDeclarationSyntax methodDeclaration) + { + return; + } + + // Gather explicit forwarded attributes info + foreach (AttributeListSyntax attributeList in methodDeclaration.AttributeLists) + { + // Same as in the [ObservableProperty] generator, except we're also looking for fields here + if (attributeList.Target?.Identifier is not SyntaxToken(SyntaxKind.PropertyKeyword or SyntaxKind.FieldKeyword)) { continue; } - // Gather explicit forwarded attributes info - foreach (AttributeListSyntax attributeList in methodDeclaration.AttributeLists) + foreach (AttributeSyntax attribute in attributeList.Attributes) { - // Same as in the [ObservableProperty] generator, except we're also looking for fields here - if (attributeList.Target?.Identifier is not SyntaxToken(SyntaxKind.PropertyKeyword or SyntaxKind.FieldKeyword)) + // Get the symbol info for the attribute (once again just like in the [ObservableProperty] generator) + if (!semanticModel.GetSymbolInfo(attribute, token).TryGetAttributeTypeSymbol(out INamedTypeSymbol? attributeTypeSymbol)) { + diagnostics.Add( + InvalidFieldOrPropertyTargetedAttributeOnRelayCommandMethod, + attribute, + methodSymbol, + attribute.Name); + continue; } - foreach (AttributeSyntax attribute in attributeList.Attributes) + AttributeInfo attributeInfo = AttributeInfo.From(attributeTypeSymbol, semanticModel, attribute.ArgumentList?.Arguments ?? Enumerable.Empty(), token); + + // Add the new attribute info to the right builder + if (attributeList.Target?.Identifier is SyntaxToken(SyntaxKind.FieldKeyword)) + { + fieldAttributesInfo.Add(attributeInfo); + } + else { - // Get the symbol info for the attribute (once again just like in the [ObservableProperty] generator) - if (!semanticModel.GetSymbolInfo(attribute, token).TryGetAttributeTypeSymbol(out INamedTypeSymbol? attributeTypeSymbol)) - { - diagnostics.Add( - InvalidFieldOrPropertyTargetedAttributeOnRelayCommandMethod, - attribute, - methodSymbol, - attribute.Name); - - continue; - } - - AttributeInfo attributeInfo = AttributeInfo.From(attributeTypeSymbol, semanticModel, attribute.ArgumentList?.Arguments ?? Enumerable.Empty(), token); - - // Add the new attribute info to the right builder - if (attributeList.Target?.Identifier is SyntaxToken(SyntaxKind.FieldKeyword)) - { - fieldAttributesInfo.Add(attributeInfo); - } - else - { - propertyAttributesInfo.Add(attributeInfo); - } + propertyAttributesInfo.Add(attributeInfo); } } } From 66f582bd94a8bd922661ae9ea960fd144fb232ca Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 9 Mar 2023 15:35:01 +0100 Subject: [PATCH 5/5] Fix [RelayCommand] attribute over methods with no attributes --- ...ityToolkit.Mvvm.SourceGenerators.projitems | 1 + .../MethodDeclarationSyntaxExtensions.cs | 33 +++++++++ .../Extensions/SyntaxNodeExtensions.cs | 3 +- .../Input/RelayCommandGenerator.cs | 2 +- .../Test_SourceGeneratorsCodegen.cs | 70 +++++++++++++++++++ 5 files changed, 106 insertions(+), 3 deletions(-) create mode 100644 src/CommunityToolkit.Mvvm.SourceGenerators/Extensions/MethodDeclarationSyntaxExtensions.cs diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/CommunityToolkit.Mvvm.SourceGenerators.projitems b/src/CommunityToolkit.Mvvm.SourceGenerators/CommunityToolkit.Mvvm.SourceGenerators.projitems index b019f6d49..879399168 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/CommunityToolkit.Mvvm.SourceGenerators.projitems +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/CommunityToolkit.Mvvm.SourceGenerators.projitems @@ -55,6 +55,7 @@ + diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/Extensions/MethodDeclarationSyntaxExtensions.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/Extensions/MethodDeclarationSyntaxExtensions.cs new file mode 100644 index 000000000..9ce12e785 --- /dev/null +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/Extensions/MethodDeclarationSyntaxExtensions.cs @@ -0,0 +1,33 @@ +// 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 Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace CommunityToolkit.Mvvm.SourceGenerators.Extensions; + +/// +/// Extension methods for the type. +/// +internal static class MethodDeclarationSyntaxExtensions +{ + /// + /// Checks whether a given has or could potentially have any attribute lists. + /// + /// The input to check. + /// Whether has or potentially has any attribute lists. + public static bool HasOrPotentiallyHasAttributeLists(this MethodDeclarationSyntax methodDeclaration) + { + // If the declaration has any attribute lists, there's nothing left to do + if (methodDeclaration.AttributeLists.Count > 0) + { + return true; + } + + // If there are no attributes, check whether the method declaration has the partial keyword. If it + // does, there could potentially be attribute lists on the other partial definition/implementation. + return methodDeclaration.Modifiers.Any(SyntaxKind.PartialKeyword); + } +} diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/Extensions/SyntaxNodeExtensions.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/Extensions/SyntaxNodeExtensions.cs index 06fb95cbd..52b7ccbfc 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/Extensions/SyntaxNodeExtensions.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/Extensions/SyntaxNodeExtensions.cs @@ -26,8 +26,7 @@ internal static class SyntaxNodeExtensions public static bool IsFirstSyntaxDeclarationForSymbol(this SyntaxNode syntaxNode, ISymbol symbol) { return - symbol.DeclaringSyntaxReferences.Length > 0 && - symbol.DeclaringSyntaxReferences[0] is SyntaxReference syntaxReference && + symbol.DeclaringSyntaxReferences is [SyntaxReference syntaxReference, ..] && syntaxReference.SyntaxTree == syntaxNode.SyntaxTree && syntaxReference.Span == syntaxNode.Span; } diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/Input/RelayCommandGenerator.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/Input/RelayCommandGenerator.cs index e18f83b9c..ddf24bf05 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/Input/RelayCommandGenerator.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/Input/RelayCommandGenerator.cs @@ -27,7 +27,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) context.SyntaxProvider .ForAttributeWithMetadataName( "CommunityToolkit.Mvvm.Input.RelayCommandAttribute", - static (node, _) => node is MethodDeclarationSyntax { Parent: ClassDeclarationSyntax, AttributeLists.Count: > 0 }, + static (node, _) => node is MethodDeclarationSyntax { Parent: ClassDeclarationSyntax } methodDeclaration && methodDeclaration.HasOrPotentiallyHasAttributeLists(), static (context, token) => { if (!context.SemanticModel.Compilation.HasLanguageVersionAtLeastEqualTo(LanguageVersion.CSharp8)) diff --git a/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsCodegen.cs b/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsCodegen.cs index 3a0ccd8f9..158eefbba 100644 --- a/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsCodegen.cs +++ b/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsCodegen.cs @@ -268,6 +268,76 @@ partial class MyViewModel VerifyGenerateSources(source, new[] { new RelayCommandGenerator() }, ("MyApp.MyViewModel.Test.g.cs", result)); } + // See https://github.com/CommunityToolkit/dotnet/issues/632 + [TestMethod] + public void RelayCommandMethodWithPartialDeclarations_TriggersCorrectly() + { + string source = """ + using CommunityToolkit.Mvvm.Input; + + #nullable enable + + namespace MyApp; + + partial class MyViewModel + { + [RelayCommand] + private partial void Test1() + { + } + + private partial void Test1(); + + private partial void Test2() + { + } + + [RelayCommand] + private partial void Test2(); + } + """; + + string result1 = """ + // + #pragma warning disable + #nullable enable + namespace MyApp + { + partial class MyViewModel + { + /// The backing field for . + [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.RelayCommandGenerator", "8.1.0.0")] + private global::CommunityToolkit.Mvvm.Input.RelayCommand? test1Command; + /// Gets an instance wrapping . + [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.RelayCommandGenerator", "8.1.0.0")] + [global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage] + public global::CommunityToolkit.Mvvm.Input.IRelayCommand Test1Command => test1Command ??= new global::CommunityToolkit.Mvvm.Input.RelayCommand(new global::System.Action(Test1)); + } + } + """; + + string result2 = """ + // + #pragma warning disable + #nullable enable + namespace MyApp + { + partial class MyViewModel + { + /// The backing field for . + [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.RelayCommandGenerator", "8.1.0.0")] + private global::CommunityToolkit.Mvvm.Input.RelayCommand? test2Command; + /// Gets an instance wrapping . + [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.RelayCommandGenerator", "8.1.0.0")] + [global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage] + public global::CommunityToolkit.Mvvm.Input.IRelayCommand Test2Command => test2Command ??= new global::CommunityToolkit.Mvvm.Input.RelayCommand(new global::System.Action(Test2)); + } + } + """; + + VerifyGenerateSources(source, new[] { new RelayCommandGenerator() }, ("MyApp.MyViewModel.Test1.g.cs", result1), ("MyApp.MyViewModel.Test2.g.cs", result2)); + } + // See https://github.com/CommunityToolkit/dotnet/issues/632 [TestMethod] public void RelayCommandMethodWithForwardedAttributesOverPartialDeclarations_MergesAttributes()