Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add diagnostics for invalid expressions for forwarded attributes #684

Merged
merged 2 commits into from
May 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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();

Expand All @@ -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(
Expand All @@ -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();

Expand All @@ -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.
Expand All @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
{
Expand All @@ -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);
}
Expand Down Expand Up @@ -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),
Expand All @@ -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)
Expand All @@ -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();
Expand All @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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:
Expand All @@ -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));
}
}

Expand Down Expand Up @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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))
Expand Down
Loading