Merge pull request #684 from CommunityToolkit/dev/fix-attribute-expre…

…ssions

Add diagnostics for invalid expressions for forwarded attributes

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

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;
 /// <summary>
 /// A model representing an attribute declaration.
 /// </summary>
+/// <param name="TypeName">The type name of the attribute.</param>
+/// <param name="ConstructorArgumentInfo">The <see cref="TypedConstantInfo"/> values for all constructor arguments for the attribute.</param>
+/// <param name="NamedArgumentInfo">The <see cref="TypedConstantInfo"/> values for all named arguments for the attribute.</param>
 internal sealed record AttributeInfo(
  string TypeName,
  EquatableArray<TypedConstantInfo> ConstructorArgumentInfo,
@@ -26,7 +30,7 @@ internal sealed record AttributeInfo(
  /// </summary>
  /// <param name="attributeData">The input <see cref="AttributeData"/> value.</param>
  /// <returns>A <see cref="AttributeInfo"/> instance representing <paramref name="attributeData"/>.</returns>
- 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<string, TypedConstant> 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)
  /// <param name="semanticModel">The <see cref="SemanticModel"/> instance for the current run.</param>
  /// <param name="arguments">The sequence of <see cref="AttributeArgumentSyntax"/> instances to process.</param>
  /// <param name="token">The cancellation token for the current operation.</param>
- /// <returns>A <see cref="AttributeInfo"/> instance representing the input attribute data.</returns>
- public static AttributeInfo From(INamedTypeSymbol typeSymbol, SemanticModel semanticModel, IEnumerable<AttributeArgumentSyntax> arguments, CancellationToken token)
+ /// <param name="info">The resulting <see cref="AttributeInfo"/> instance, if available</param>
+ /// <returns>Whether a resulting <see cref="AttributeInfo"/> instance could be created.</returns>
+ public static bool TryCreate(
+ INamedTypeSymbol typeSymbol,
+ SemanticModel semanticModel,
+ IEnumerable<AttributeArgumentSyntax> 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;
  }
 
  /// <summary>

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
  /// <param name="arg">The input <see cref="TypedConstant"/> value.</param>
  /// <returns>A <see cref="TypedConstantInfo"/> instance representing <paramref name="arg"/>.</returns>
  /// <exception cref="ArgumentException">Thrown if the input argument is not valid.</exception>
- 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<TypedConstantInfo> items = arg.Values.Select(From).ToImmutableArray();
+ ImmutableArray<TypedConstantInfo> items = arg.Values.Select(Create).ToImmutableArray();
 
  return new Array(elementTypeName, items);
  }
@@ -69,24 +70,28 @@ public static TypedConstantInfo From(TypedConstant arg)
  /// <param name="semanticModel">The <see cref="SemanticModel"/> that was used to retrieve <paramref name="operation"/>.</param>
  /// <param name="expression">The <see cref="ExpressionSyntax"/> that <paramref name="operation"/> was retrieved from.</param>
  /// <param name="token">The cancellation token for the current operation.</param>
- /// <returns>A <see cref="TypedConstantInfo"/> instance representing <paramref name="operation"/>.</returns>
+ /// <param name="info">The resulting <see cref="TypedConstantInfo"/> instance, if available</param>
+ /// <returns>Whether a resulting <see cref="TypedConstantInfo"/> instance could be created.</returns>
  /// <exception cref="ArgumentException">Thrown if the input argument is not valid.</exception>
- 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<ushort>(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<TypedConstantInfo>.Empty);
+ info = new Array(elementTypeName, ImmutableArray<TypedConstantInfo>.Empty);
+
+ return true;
  }
 
  using ImmutableArrayBuilder<TypedConstantInfo> items = ImmutableArrayBuilder<TypedConstantInfo>.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;
  }
 }

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<AttributeArgumentSyntax>(), token));
+ IEnumerable<AttributeArgumentSyntax> attributeArguments = attribute.ArgumentList?.Arguments ?? Enumerable.Empty<AttributeArgumentSyntax>();
+
+ // 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);
  }
  }
 

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

@@ -598,11 +598,43 @@ internal static class DiagnosticDescriptors
  /// </summary>
  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");
+
+ /// <summary>
+ /// Gets a <see cref="DiagnosticDescriptor"/> indicating when a field with <c>[ObservableProperty]</c> is using an invalid attribute expression targeting the property.
+ /// <para>
+ /// Format: <c>"The field {0} annotated with [ObservableProperty] is using attribute "{1}" with an invalid expression (are you passing any incorrect parameters to the attribute constructor?)"</c>.
+ /// </para>
+ /// </summary>
+ 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");
+
+ /// <summary>
+ /// Gets a <see cref="DiagnosticDescriptor"/> indicating when a method with <c>[RelayCommand]</c> is using an invalid attribute targeting the field or property.
+ /// <para>
+ /// Format: <c>"The method {0} annotated with [RelayCommand] is using attribute "{1}" with an invalid expression (are you passing any incorrect parameters to the attribute constructor?)"</c>.
+ /// </para>
+ /// </summary>
+ 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");
 }

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<AttributeArgumentSyntax>(), token);
+ IEnumerable<AttributeArgumentSyntax> attributeArguments = attribute.ArgumentList?.Arguments ?? Enumerable.Empty<AttributeArgumentSyntax>();
+
+ // 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))