diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/AnalyzerReleases.Shipped.md b/src/CommunityToolkit.Mvvm.SourceGenerators/AnalyzerReleases.Shipped.md index 75abaf30d..0f9819023 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/AnalyzerReleases.Shipped.md +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/AnalyzerReleases.Shipped.md @@ -57,3 +57,12 @@ MVVMTK0035 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator Rule ID | Category | Severity | Notes --------|----------|----------|------- MVVMTK0036 | CommunityToolkit.Mvvm.SourceGenerators.RelayCommandGenerator | Error | See https://aka.ms/mvvmtoolkit/errors/mvvmtk0036 + +## Release 8.2.1 + +### New Rules + +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 diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/AttributeInfo.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/AttributeInfo.cs index 505988244..1009cd4c1 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/AttributeInfo.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/AttributeInfo.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Threading; using CommunityToolkit.Mvvm.SourceGenerators.Extensions; @@ -16,6 +17,9 @@ namespace CommunityToolkit.Mvvm.SourceGenerators.ComponentModel.Models; /// /// A model representing an attribute declaration. /// +/// The type name of the attribute. +/// The values for all constructor arguments for the attribute. +/// The values for all named arguments for the attribute. internal sealed record AttributeInfo( string TypeName, EquatableArray ConstructorArgumentInfo, @@ -26,7 +30,7 @@ internal sealed record AttributeInfo( /// /// The input value. /// A instance representing . - public static AttributeInfo From(AttributeData attributeData) + public static AttributeInfo Create(AttributeData attributeData) { string typeName = attributeData.AttributeClass!.GetFullyQualifiedName(); @@ -36,13 +40,13 @@ public static AttributeInfo From(AttributeData attributeData) // Get the constructor arguments foreach (TypedConstant typedConstant in attributeData.ConstructorArguments) { - constructorArguments.Add(TypedConstantInfo.From(typedConstant)); + constructorArguments.Add(TypedConstantInfo.Create(typedConstant)); } // Get the named arguments foreach (KeyValuePair namedConstant in attributeData.NamedArguments) { - namedArguments.Add((namedConstant.Key, TypedConstantInfo.From(namedConstant.Value))); + namedArguments.Add((namedConstant.Key, TypedConstantInfo.Create(namedConstant.Value))); } return new( @@ -58,8 +62,14 @@ public static AttributeInfo From(AttributeData attributeData) /// The instance for the current run. /// The sequence of instances to process. /// The cancellation token for the current operation. - /// A instance representing the input attribute data. - public static AttributeInfo From(INamedTypeSymbol typeSymbol, SemanticModel semanticModel, IEnumerable arguments, CancellationToken token) + /// The resulting instance, if available + /// Whether a resulting instance could be created. + public static bool TryCreate( + INamedTypeSymbol typeSymbol, + SemanticModel semanticModel, + IEnumerable arguments, + CancellationToken token, + [NotNullWhen(true)] out AttributeInfo? info) { string typeName = typeSymbol.GetFullyQualifiedName(); @@ -74,7 +84,13 @@ public static AttributeInfo From(INamedTypeSymbol typeSymbol, SemanticModel sema continue; } - TypedConstantInfo argumentInfo = TypedConstantInfo.From(operation, semanticModel, argument.Expression, token); + // Try to get the info for the current argument + if (!TypedConstantInfo.TryCreate(operation, semanticModel, argument.Expression, token, out TypedConstantInfo? argumentInfo)) + { + info = null; + + return false; + } // Try to get the identifier name if the current expression is a named argument expression. If it // isn't, then the expression is a normal attribute constructor argument, so no extra work is needed. @@ -88,10 +104,12 @@ public static AttributeInfo From(INamedTypeSymbol typeSymbol, SemanticModel sema } } - return new( + info = new AttributeInfo( typeName, constructorArguments.ToImmutable(), namedArguments.ToImmutable()); + + return true; } /// diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/TypedConstantInfo.Factory.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/TypedConstantInfo.Factory.cs index 825b075ec..486cb3211 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/TypedConstantInfo.Factory.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/TypedConstantInfo.Factory.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Immutable; +using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Threading; using CommunityToolkit.Mvvm.SourceGenerators.Helpers; @@ -22,7 +23,7 @@ partial record TypedConstantInfo /// The input value. /// A instance representing . /// Thrown if the input argument is not valid. - public static TypedConstantInfo From(TypedConstant arg) + public static TypedConstantInfo Create(TypedConstant arg) { if (arg.IsNull) { @@ -32,7 +33,7 @@ public static TypedConstantInfo From(TypedConstant arg) if (arg.Kind == TypedConstantKind.Array) { string elementTypeName = ((IArrayTypeSymbol)arg.Type!).ElementType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat); - ImmutableArray items = arg.Values.Select(From).ToImmutableArray(); + ImmutableArray items = arg.Values.Select(Create).ToImmutableArray(); return new Array(elementTypeName, items); } @@ -69,24 +70,28 @@ public static TypedConstantInfo From(TypedConstant arg) /// The that was used to retrieve . /// The that was retrieved from. /// The cancellation token for the current operation. - /// A instance representing . + /// The resulting instance, if available + /// Whether a resulting instance could be created. /// Thrown if the input argument is not valid. - public static TypedConstantInfo From( + public static bool TryCreate( IOperation operation, SemanticModel semanticModel, ExpressionSyntax expression, - CancellationToken token) + CancellationToken token, + [NotNullWhen(true)] out TypedConstantInfo? info) { if (operation.ConstantValue.HasValue) { // Enum values are constant but need to be checked explicitly in this case if (operation.Type?.TypeKind is TypeKind.Enum) { - return new Enum(operation.Type!.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat), operation.ConstantValue.Value!); + info = new Enum(operation.Type!.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat), operation.ConstantValue.Value!); + + return true; } // Handle all other constant literals normally - return operation.ConstantValue.Value switch + info = operation.ConstantValue.Value switch { null => new Null(), string text => new Primitive.String(text), @@ -104,11 +109,15 @@ public static TypedConstantInfo From( ushort ush => new Primitive.Of(ush), _ => throw new ArgumentException("Invalid primitive type") }; + + return true; } if (operation is ITypeOfOperation typeOfOperation) { - return new Type(typeOfOperation.TypeOperand.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)); + info = new Type(typeOfOperation.TypeOperand.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)); + + return true; } if (operation is IArrayCreationOperation) @@ -125,7 +134,9 @@ public static TypedConstantInfo From( // No initializer found, just return an empty array if (initializerExpression is null) { - return new Array(elementTypeName, ImmutableArray.Empty); + info = new Array(elementTypeName, ImmutableArray.Empty); + + return true; } using ImmutableArrayBuilder items = ImmutableArrayBuilder.Rent(); @@ -135,15 +146,25 @@ public static TypedConstantInfo From( { if (semanticModel.GetOperation(initializationExpression, token) is not IOperation initializationOperation) { - throw new ArgumentException("Failed to retrieve an operation for the current array element"); + goto Failure; } - items.Add(From(initializationOperation, semanticModel, initializationExpression, token)); + if (!TryCreate(initializationOperation, semanticModel, initializationExpression, token, out TypedConstantInfo? elementInfo)) + { + goto Failure; + } + + items.Add(elementInfo); } - return new Array(elementTypeName, items.ToImmutable()); + info = new Array(elementTypeName, items.ToImmutable()); + + return true; } - throw new ArgumentException("Invalid attribute argument value"); + Failure: + info = null; + + return false; } } diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs index caf2f213c..6f590b80c 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs @@ -2,6 +2,7 @@ // 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.Generic; using System.Collections.Immutable; using System.ComponentModel; using System.Diagnostics.CodeAnalysis; @@ -174,7 +175,7 @@ public static bool TryGetInfo( { hasAnyValidationAttributes = true; - forwardedAttributes.Add(AttributeInfo.From(attributeData)); + forwardedAttributes.Add(AttributeInfo.Create(attributeData)); } // Also track the current attribute for forwarding if it is of any of the following types: @@ -189,7 +190,7 @@ public static bool TryGetInfo( attributeData.AttributeClass?.HasFullyQualifiedMetadataName("System.ComponentModel.DataAnnotations.EditableAttribute") == true || attributeData.AttributeClass?.HasFullyQualifiedMetadataName("System.ComponentModel.DataAnnotations.KeyAttribute") == true) { - forwardedAttributes.Add(AttributeInfo.From(attributeData)); + forwardedAttributes.Add(AttributeInfo.Create(attributeData)); } } @@ -231,7 +232,21 @@ public static bool TryGetInfo( continue; } - forwardedAttributes.Add(AttributeInfo.From(attributeTypeSymbol, semanticModel, attribute.ArgumentList?.Arguments ?? Enumerable.Empty(), token)); + IEnumerable attributeArguments = attribute.ArgumentList?.Arguments ?? Enumerable.Empty(); + + // Try to extract the forwarded attribute + if (!AttributeInfo.TryCreate(attributeTypeSymbol, semanticModel, attributeArguments, token, out AttributeInfo? attributeInfo)) + { + builder.Add( + InvalidPropertyTargetedAttributeExpressionOnObservablePropertyField, + attribute, + fieldSymbol, + attribute.Name); + + continue; + } + + forwardedAttributes.Add(attributeInfo); } } diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs index fc28be102..1305b2db6 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs @@ -598,11 +598,43 @@ internal static class DiagnosticDescriptors /// public static readonly DiagnosticDescriptor InvalidFieldOrPropertyTargetedAttributeOnRelayCommandMethod = new DiagnosticDescriptor( id: "MVVMTK0036", - title: "Invalid field targeted attribute type", + title: "Invalid field or property targeted attribute type", messageFormat: "The method {0} annotated with [RelayCommand] is using attribute \"{1}\" which was not recognized as a valid type (are you missing a using directive?)", category: typeof(RelayCommandGenerator).FullName, defaultSeverity: DiagnosticSeverity.Error, isEnabledByDefault: true, description: "All attributes targeting the generated field or property for a method annotated with [RelayCommand] must correctly be resolved to valid types.", helpLinkUri: "https://aka.ms/mvvmtoolkit/errors/mvvmtk0036"); + + /// + /// Gets a indicating when a field with [ObservableProperty] is using an invalid attribute expression targeting the property. + /// + /// Format: "The field {0} annotated with [ObservableProperty] is using attribute "{1}" with an invalid expression (are you passing any incorrect parameters to the attribute constructor?)". + /// + /// + public static readonly DiagnosticDescriptor InvalidPropertyTargetedAttributeExpressionOnObservablePropertyField = new DiagnosticDescriptor( + id: "MVVMTK0037", + title: "Invalid property targeted attribute expression", + messageFormat: "The field {0} annotated with [ObservableProperty] is using attribute \"{1}\" with an invalid expression (are you passing any incorrect parameters to the attribute constructor?)", + category: typeof(ObservablePropertyGenerator).FullName, + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: "All attributes targeting the generated property for a field annotated with [ObservableProperty] must be using valid expressions.", + helpLinkUri: "https://aka.ms/mvvmtoolkit/errors/mvvmtk0037"); + + /// + /// Gets a indicating when a method with [RelayCommand] is using an invalid attribute targeting the field or property. + /// + /// Format: "The method {0} annotated with [RelayCommand] is using attribute "{1}" with an invalid expression (are you passing any incorrect parameters to the attribute constructor?)". + /// + /// + public static readonly DiagnosticDescriptor InvalidFieldOrPropertyTargetedAttributeExpressionOnRelayCommandMethod = new DiagnosticDescriptor( + id: "MVVMTK0038", + title: "Invalid field or property targeted attribute expression", + messageFormat: "The method {0} annotated with [RelayCommand] is using attribute \"{1}\" with an invalid expression (are you passing any incorrect parameters to the attribute constructor?)", + category: typeof(RelayCommandGenerator).FullName, + defaultSeverity: DiagnosticSeverity.Error, + 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"); } diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/Input/RelayCommandGenerator.Execute.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/Input/RelayCommandGenerator.Execute.cs index c3dd8cc64..c2bb7cca1 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/Input/RelayCommandGenerator.Execute.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/Input/RelayCommandGenerator.Execute.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System; +using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics.CodeAnalysis; using System.Globalization; @@ -1004,7 +1005,19 @@ static void GatherForwardedAttributes( continue; } - AttributeInfo attributeInfo = AttributeInfo.From(attributeTypeSymbol, semanticModel, attribute.ArgumentList?.Arguments ?? Enumerable.Empty(), token); + IEnumerable attributeArguments = attribute.ArgumentList?.Arguments ?? Enumerable.Empty(); + + // Try to extract the forwarded attribute + if (!AttributeInfo.TryCreate(attributeTypeSymbol, semanticModel, attributeArguments, token, out AttributeInfo? attributeInfo)) + { + diagnostics.Add( + InvalidFieldOrPropertyTargetedAttributeExpressionOnRelayCommandMethod, + attribute, + methodSymbol, + attribute.Name); + + continue; + } // Add the new attribute info to the right builder if (attributeList.Target?.Identifier is SyntaxToken(SyntaxKind.FieldKeyword)) diff --git a/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsDiagnostics.cs b/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsDiagnostics.cs index 414aa6566..d4e6f04ee 100644 --- a/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsDiagnostics.cs +++ b/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsDiagnostics.cs @@ -1672,6 +1672,30 @@ public partial class MyViewModel : ObservableObject VerifyGeneratedDiagnostics(source, "MVVMTK0035"); } + // See https://github.com/CommunityToolkit/dotnet/issues/683 + [TestMethod] + public void InvalidPropertyTargetedAttributeOnObservablePropertyField_InvalidExpression() + { + string source = """ + using System; + using CommunityToolkit.Mvvm.ComponentModel; + + public partial class MyViewModel : ObservableObject + { + [ObservableProperty] + [property: Test(TestAttribute.M)] + private int number; + } + + public class TestAttribute : Attribute + { + public static string M => ""; + } + """; + + VerifyGeneratedDiagnostics(source, "MVVMTK0037"); + } + [TestMethod] public void InvalidPropertyTargetedAttributeOnRelayCommandMethod_MissingUsingDirective() { @@ -1725,6 +1749,58 @@ private void Test() VerifyGeneratedDiagnostics(source, "MVVMTK0036"); } + // See https://github.com/CommunityToolkit/dotnet/issues/683 + [TestMethod] + public void InvalidPropertyTargetedAttributeOnRelayCommandMethod_InvalidExpressionOnFieldAttribute() + { + string source = """ + using System; + using CommunityToolkit.Mvvm.Input; + + public partial class MyViewModel + { + [RelayCommand] + [field: Test(TestAttribute.M)] + private void Test() + { + } + } + + public class TestAttribute : Attribute + { + public static string M => ""; + } + """; + + VerifyGeneratedDiagnostics(source, "MVVMTK0038"); + } + + // See https://github.com/CommunityToolkit/dotnet/issues/683 + [TestMethod] + public void InvalidPropertyTargetedAttributeOnRelayCommandMethod_InvalidExpressionOnPropertyAttribute() + { + string source = """ + using System; + using CommunityToolkit.Mvvm.Input; + + public partial class MyViewModel + { + [RelayCommand] + [property: Test(TestAttribute.M)] + private void Test() + { + } + } + + public class TestAttribute : Attribute + { + public static string M => ""; + } + """; + + VerifyGeneratedDiagnostics(source, "MVVMTK0038"); + } + /// /// Verifies the diagnostic errors for a given analyzer, and that all available source generators can run successfully with the input source (including subsequent compilation). ///