Skip to content

Commit

Permalink
Merge pull request #248 from CommunityToolkit/dev/block-conflicting-g…
Browse files Browse the repository at this point in the history
…enerated-properties

Block [ObservableProperty] on properties causing conflicts
  • Loading branch information
Sergio0694 authored May 13, 2022
2 parents 0fb704d + 914f31d commit b8db4b0
Show file tree
Hide file tree
Showing 5 changed files with 156 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,4 @@ MVVMTK0020 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator
MVVMTK0021 | CommunityToolkit.Mvvm.SourceGenerators.ObservableRecipientGenerator | Error | See https://aka.ms/mvvmtoolkit/error
MVVMTK0022 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Error | See https://aka.ms/mvvmtoolkit/error
MVVMTK0023 | CommunityToolkit.Mvvm.SourceGenerators.ICommandGenerator | Error | See https://aka.ms/mvvmtoolkit/error
MVVMTK0024 | CommunityToolkit.Mvvm.SourceGenerators.ICommandGenerator | Error | See https://aka.ms/mvvmtoolkit/error
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,20 @@ internal static class Execute
return null;
}

// Check for special cases that are explicitly not allowed
if (IsGeneratedPropertyInvalid(propertyName, fieldSymbol.Type))
{
builder.Add(
InvalidObservablePropertyError,
fieldSymbol,
fieldSymbol.ContainingType,
fieldSymbol.Name);

diagnostics = builder.ToImmutable();

return null;
}

ImmutableArray<string>.Builder propertyChangedNames = ImmutableArray.CreateBuilder<string>();
ImmutableArray<string>.Builder propertyChangingNames = ImmutableArray.CreateBuilder<string>();
ImmutableArray<string>.Builder notifiedCommandNames = ImmutableArray.CreateBuilder<string>();
Expand Down Expand Up @@ -178,6 +192,29 @@ private static bool IsTargetTypeValid(
return isObservableObject || hasObservableObjectAttribute || hasINotifyPropertyChangedAttribute;
}

/// <summary>
/// Checks whether the generated property would be a special case that is marked as invalid.
/// </summary>
/// <param name="propertyName">The property name.</param>
/// <param name="propertyType">The property type.</param>
/// <returns>Whether the generated property is invalid.</returns>
private static bool IsGeneratedPropertyInvalid(string propertyName, ITypeSymbol propertyType)
{
// If the generated property name is called "Property" and the type is either object or it is PropertyChangedEventArgs or
// PropertyChangingEventArgs (or a type derived from either of those two types), consider it invalid. This is needed because
// if such a property was generated, the partial On<PROPERTY_NAME>Changing and OnPropertyChanging(PropertyChangingEventArgs)
// methods, as well as the partial On<PROPERTY_NAME>Changed and OnPropertyChanged(PropertyChangedEventArgs) methods.
if (propertyName == "Property")
{
return
propertyType.SpecialType == SpecialType.System_Object ||
propertyType.HasOrInheritsFromFullyQualifiedName("global::System.ComponentModel.PropertyChangedEventArgs") ||
propertyType.HasOrInheritsFromFullyQualifiedName("global::System.ComponentModel.PropertyChangingEventArgs");
}

return false;
}

/// <summary>
/// Tries to gather dependent properties from the given attribute.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -379,4 +379,20 @@ internal static class DiagnosticDescriptors
isEnabledByDefault: true,
description: "Methods with multiple overloads cannot be annotated with [ICommand], as command methods must be unique within their containing type.",
helpLinkUri: "https://aka.ms/mvvmtoolkit");

/// <summary>
/// Gets a <see cref="DiagnosticDescriptor"/> indicating when a generated property created with <c>[ObservableProperty]</c> would cause conflicts with other generated members.
/// <para>
/// Format: <c>"The field {0}.{1} cannot be used to generate an observable property, as its name or type would cause conflicts with other generated members"</c>.
/// </para>
/// </summary>
public static readonly DiagnosticDescriptor InvalidObservablePropertyError = new DiagnosticDescriptor(
id: "MVVMTK0024",
title: "Invalid generated property declaration",
messageFormat: "The field {0}.{1} cannot be used to generate an observable property, as its name or type would cause conflicts with other generated members",
category: typeof(ObservablePropertyGenerator).FullName,
defaultSeverity: DiagnosticSeverity.Error,
isEnabledByDefault: true,
description: "The fields annotated with [ObservableProperty] cannot result in a property name or have a type that would cause conflicts with other generated members.",
helpLinkUri: "https://aka.ms/mvvmtoolkit");
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,25 @@ namespace CommunityToolkit.Mvvm.SourceGenerators.Extensions;
/// </summary>
internal static class ITypeSymbolExtensions
{
/// <summary>
/// Checks whether or not a given <see cref="ITypeSymbol"/> has or inherits from a specified type.
/// </summary>
/// <param name="typeSymbol">The target <see cref="ITypeSymbol"/> instance to check.</param>
/// <param name="name">The full name of the type to check for inheritance.</param>
/// <returns>Whether or not <paramref name="typeSymbol"/> is or inherits from <paramref name="name"/>.</returns>
public static bool HasOrInheritsFromFullyQualifiedName(this ITypeSymbol typeSymbol, string name)
{
for (ITypeSymbol? currentType = typeSymbol; currentType is not null; currentType = currentType.BaseType)
{
if (currentType.HasFullyQualifiedName(name))
{
return true;
}
}

return false;
}

/// <summary>
/// Checks whether or not a given <see cref="ITypeSymbol"/> inherits from a specified type.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1105,6 +1105,89 @@ private void GreetUser(object value)
VerifyGeneratedDiagnostics<ICommandGenerator>(source, "MVVMTK0023");
}

[TestMethod]
public void InvalidObservablePropertyError_Object()
{
string source = @"
using CommunityToolkit.Mvvm.ComponentModel;
namespace MyApp
{
public partial class MyViewModel : ObservableObject
{
[ObservableProperty]
public object property;
}
}";

VerifyGeneratedDiagnostics<ObservablePropertyGenerator>(source, "MVVMTK0024");
}

[TestMethod]
public void InvalidObservablePropertyError_PropertyChangingEventArgs()
{
string source = @"
using System.ComponentModel;
using CommunityToolkit.Mvvm.ComponentModel;
namespace MyApp
{
public partial class MyViewModel : ObservableObject
{
[ObservableProperty]
public PropertyChangingEventArgs property;
}
}";

VerifyGeneratedDiagnostics<ObservablePropertyGenerator>(source, "MVVMTK0024");
}

[TestMethod]
public void InvalidObservablePropertyError_PropertyChangedEventArgs()
{
string source = @"
using System.ComponentModel;
using CommunityToolkit.Mvvm.ComponentModel;
namespace MyApp
{
public partial class MyViewModel : ObservableObject
{
[ObservableProperty]
public PropertyChangedEventArgs property;
}
}";

VerifyGeneratedDiagnostics<ObservablePropertyGenerator>(source, "MVVMTK0024");
}

[TestMethod]
public void InvalidObservablePropertyError_CustomTypeDerivedFromPropertyChangedEventArgs()
{
string source = @"
using System.ComponentModel;
using CommunityToolkit.Mvvm.ComponentModel;
namespace MyApp
{
public class MyPropertyChangedEventArgs : PropertyChangedEventArgs
{
public MyPropertyChangedEventArgs(string propertyName)
: base(propertyName)
{
}
}
public partial class MyViewModel : ObservableObject
{
[ObservableProperty]
public MyPropertyChangedEventArgs property;
}
}";

VerifyGeneratedDiagnostics<ObservablePropertyGenerator>(source, "MVVMTK0024");
}

/// <summary>
/// Verifies the output of a source generator.
/// </summary>
Expand Down

0 comments on commit b8db4b0

Please sign in to comment.