Skip to content

Commit

Permalink
Add OnChangedMethodAttribute (#464)
Browse files Browse the repository at this point in the history 
* Add OnChangedMethodAttribute

* Allow suppressing default method with [OnChangedMethod(null)]

* Add tests

* Ensure that explicitly configured methods are always called

* Apply the InjectOnPropertyNameChanged config only to default methods
  • Loading branch information
@ltrzesniewski @SimonCropp
ltrzesniewski authored and SimonCropp committed Dec 8, 2019
1 parent 96ef699 commit 65ea5fe
Show file tree
Hide file tree
Showing 20 changed files with 449 additions and 89 deletions.
3 changes: 2 additions & 1 deletion PropertyChanged.Fody/AttributeCleaner.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ public partial class ModuleWeaver
"PropertyChanged.AlsoNotifyForAttribute",
"PropertyChanged.DependsOnAttribute",
"PropertyChanged.AddINotifyPropertyChangedInterfaceAttribute",
"PropertyChanged.SuppressPropertyChangedWarningsAttribute"
"PropertyChanged.SuppressPropertyChangedWarningsAttribute",
"PropertyChanged.OnChangedMethodAttribute"
};

HashSet<string> assemblyLevelAttributeNames = new HashSet<string>
Expand Down
7 changes: 1 addition & 6 deletions PropertyChanged.Fody/ModuleWeaver.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,7 @@ public override void Execute()
ProcessDependsOnAttributes();
WalkPropertyData();
CheckForWarnings();

if (InjectOnPropertyNameChanged)
{
ProcessOnChangedMethods();
}

ProcessOnChangedMethods();
CheckForStackOverflow();
FindComparisonMethods();
InitEventArgsCache();
Expand Down
4 changes: 3 additions & 1 deletion PropertyChanged.Fody/OnChangedMethod.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

public enum OnChangedTypes
{
None,
NoArg,
BeforeAfter,
}
Expand All @@ -10,4 +11,5 @@ public class OnChangedMethod
{
public MethodReference MethodReference;
public OnChangedTypes OnChangedType;
}
public bool IsDefaultMethod;
}
164 changes: 119 additions & 45 deletions PropertyChanged.Fody/OnChangedWalker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,61 +14,132 @@ void ProcessOnChangedMethods(List<TypeNode> notifyNodes)
{
foreach (var notifyNode in notifyNodes)
{
notifyNode.OnChangedMethods = GetOnChangedMethods(notifyNode).ToList();
ProcessOnChangedMethods(notifyNode);
ProcessOnChangedMethods(notifyNode.Nodes);
}
}

IEnumerable<OnChangedMethod> GetOnChangedMethods(TypeNode notifyNode)
void ProcessOnChangedMethods(TypeNode notifyNode)
{
var methods = notifyNode.TypeDefinition.Methods;
var methods = new Dictionary<string, OnChangedMethod>();

var onChangedMethods = methods.Where(x => x.Name.StartsWith("On") &&
x.Name.EndsWith("Changed") &&
x.Name != "OnChanged");

foreach (var methodDefinition in onChangedMethods)
if (InjectOnPropertyNameChanged)
{
if (methodDefinition.IsStatic)
{
var message = $"The type {notifyNode.TypeDefinition.FullName} has a On_PropertyName_Changed method ({methodDefinition.Name}) which is static.";
throw new WeavingException(message);
}

if (methodDefinition.ReturnType.FullName != "System.Void")
foreach (var methodDefinition in notifyNode.TypeDefinition.Methods)
{
var message = $"The type {notifyNode.TypeDefinition.FullName} has a On_PropertyName_Changed method ({methodDefinition.Name}) that has a non void return value. Ensure the return type void.";
throw new WeavingException(message);
}

var typeDefinitions = new Stack<TypeDefinition>();
typeDefinitions.Push(notifyNode.TypeDefinition);
var methodName = methodDefinition.Name;

if (IsNoArgOnChangedMethod(methodDefinition))
{
ValidateOnChangedMethod(notifyNode, methodDefinition);

yield return new OnChangedMethod
{
OnChangedType = OnChangedTypes.NoArg,
MethodReference = GetMethodReference(typeDefinitions, methodDefinition)
};
if (!methodName.StartsWith("On") || !methodName.EndsWith("Changed") || methodName == "OnChanged")
continue;

var onChangedMethod = CreateOnChangedMethod(notifyNode, methodDefinition, true);
if (onChangedMethod == null)
continue;

if (methods.ContainsKey(methodName))
throw new WeavingException($"The type {notifyNode.TypeDefinition.FullName} has a On_PropertyName_Changed method ({methodDefinition.Name}) which has multiple valid overloads.");

methods.Add(methodName, onChangedMethod);
}
else if (IsBeforeAfterOnChangedMethod(methodDefinition))
}

foreach (var propertyData in notifyNode.PropertyDatas)
{
var hasCustomMethods = false;

foreach (var attribute in propertyData.PropertyDefinition.CustomAttributes.GetAttributes("PropertyChanged.OnChangedMethodAttribute"))
{
ValidateOnChangedMethod(notifyNode, methodDefinition);

yield return new OnChangedMethod
hasCustomMethods = true;
var methodName = (string)attribute.ConstructorArguments[0].Value;

if (string.IsNullOrEmpty(methodName))
continue;

if (!methods.TryGetValue(methodName, out var onChangedMethod))
{
OnChangedType = OnChangedTypes.BeforeAfter,
MethodReference = GetMethodReference(typeDefinitions, methodDefinition)
};
onChangedMethod = FindOnChangedMethod(notifyNode, methodName);
methods.Add(methodName, onChangedMethod);
}

propertyData.OnChangedMethods.Add(onChangedMethod);
}
else if (!EventInvokerNames.Contains(methodDefinition.Name))

if (InjectOnPropertyNameChanged && !hasCustomMethods && methods.TryGetValue("On" + propertyData.PropertyDefinition.Name + "Changed", out var defaultMethod))
propertyData.OnChangedMethods.Add(defaultMethod);
}
}

OnChangedMethod FindOnChangedMethod(TypeNode notifyNode, string methodName)
{
OnChangedMethod foundMethod = null;

foreach (var methodDefinition in notifyNode.TypeDefinition.Methods)
{
if (methodDefinition.Name != methodName)
continue;

var method = CreateOnChangedMethod(notifyNode, methodDefinition, false);
if (method == null)
continue;

if (foundMethod != null)
throw new WeavingException($"The type {notifyNode.TypeDefinition.FullName} has multiple valid overloads of a On_PropertyName_Changed method named {methodName}).");

foundMethod = method;
}

if (foundMethod == null)
throw new WeavingException($"The type {notifyNode.TypeDefinition.FullName} does not have a valid On_PropertyName_Changed method named {methodName}).");

return foundMethod;
}

OnChangedMethod CreateOnChangedMethod(TypeNode notifyNode, MethodDefinition methodDefinition, bool isDefaultMethod)
{
if (methodDefinition.IsStatic)
{
var message = $"The type {notifyNode.TypeDefinition.FullName} has a On_PropertyName_Changed method ({methodDefinition.Name}) which is static.";
throw new WeavingException(message);
}

if (methodDefinition.ReturnType.FullName != "System.Void")
{
var message = $"The type {notifyNode.TypeDefinition.FullName} has a On_PropertyName_Changed method ({methodDefinition.Name}) that has a non void return value. Ensure the return type void.";
throw new WeavingException(message);
}

var typeDefinitions = new Stack<TypeDefinition>();
typeDefinitions.Push(notifyNode.TypeDefinition);

var onChangedType = OnChangedTypes.None;

if (IsNoArgOnChangedMethod(methodDefinition))
{
onChangedType = OnChangedTypes.NoArg;
}
else if (IsBeforeAfterOnChangedMethod(methodDefinition))
{
onChangedType = OnChangedTypes.BeforeAfter;
}

if (onChangedType != OnChangedTypes.None)
{
ValidateOnChangedMethod(notifyNode, methodDefinition, isDefaultMethod);

return new OnChangedMethod
{
EmitConditionalWarning(methodDefinition, $"Unsupported signature for a On_PropertyName_Changed method: {methodDefinition.Name} in {methodDefinition.DeclaringType.FullName}");
}
OnChangedType = onChangedType,
MethodReference = GetMethodReference(typeDefinitions, methodDefinition),
IsDefaultMethod = isDefaultMethod
};
}

if (!EventInvokerNames.Contains(methodDefinition.Name))
{
EmitConditionalWarning(methodDefinition, $"Unsupported signature for a On_PropertyName_Changed method: {methodDefinition.Name} in {methodDefinition.DeclaringType.FullName}");
}

return null;
}

public static bool IsNoArgOnChangedMethod(MethodDefinition method)
Expand All @@ -85,16 +156,19 @@ public static bool IsBeforeAfterOnChangedMethod(MethodDefinition method)
&& parameters[1].ParameterType.FullName == "System.Object";
}

void ValidateOnChangedMethod(TypeNode notifyNode, MethodDefinition method)
void ValidateOnChangedMethod(TypeNode notifyNode, MethodDefinition method, bool isDefaultMethod)
{
if (method.IsVirtual && !method.IsNewSlot)
return;

var propertyName = method.Name.Substring("On".Length, method.Name.Length - "On".Length - "Changed".Length);

if (notifyNode.PropertyDatas.All(i => i.PropertyDefinition.Name != propertyName))
if (isDefaultMethod)
{
EmitConditionalWarning(method, $"Type {method.DeclaringType.FullName} does not contain a {propertyName} property with an injected change notification, and therefore the {method.Name} method will not be called.");
var propertyName = method.Name.Substring("On".Length, method.Name.Length - "On".Length - "Changed".Length);

if (notifyNode.PropertyDatas.All(i => i.PropertyDefinition.Name != propertyName))
{
EmitConditionalWarning(method, $"Type {method.DeclaringType.FullName} does not contain a {propertyName} property with an injected change notification, and therefore the {method.Name} method will not be called.");
}
}
}
}
4 changes: 3 additions & 1 deletion PropertyChanged.Fody/PropertyData.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@

public class PropertyData
{
public TypeNode ParentType;
public FieldReference BackingFieldReference;
public List<PropertyDefinition> AlsoNotifyFor = new List<PropertyDefinition>();
public PropertyDefinition PropertyDefinition;
public MethodReference EqualsMethod;
public List<string> AlreadyNotifies = new List<string>();
}
public List<OnChangedMethod> OnChangedMethods = new List<OnChangedMethod>();
}
4 changes: 3 additions & 1 deletion PropertyChanged.Fody/PropertyDataWalker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ void GetPropertyData(PropertyDefinition propertyDefinition, TypeNode node)
}
node.PropertyDatas.Add(new PropertyData
{
ParentType = node,
BackingFieldReference = backingFieldReference,
PropertyDefinition = propertyDefinition,
// Compute full dependencies for the current property
Expand All @@ -65,6 +66,7 @@ void GetPropertyData(PropertyDefinition propertyDefinition, TypeNode node)
}
node.PropertyDatas.Add(new PropertyData
{
ParentType = node,
BackingFieldReference = backingFieldReference,
PropertyDefinition = propertyDefinition,
// Compute full dependencies for the current property
Expand Down Expand Up @@ -115,4 +117,4 @@ public void WalkPropertyData()
{
WalkPropertyData(NotifyNodes);
}
}
}
61 changes: 32 additions & 29 deletions PropertyChanged.Fody/PropertyWeaver.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,14 @@ List<int> GetIndexes()
void InjectAtIndex(int index)
{
index = AddIsChangedSetterCall(index);
var propertyDefinitions = propertyData.AlsoNotifyFor.Distinct();

index = propertyDefinitions.Aggregate(index, AddEventInvokeCall);
AddEventInvokeCall(index, propertyData.PropertyDefinition);
var alsoNotifyProperties = propertyData.AlsoNotifyFor
.Distinct()
.Select(n => propertyData.ParentType.PropertyDatas.FirstOrDefault(p => p.PropertyDefinition == n))
.Where(p => p != null);

index = alsoNotifyProperties.Aggregate(index, AddEventInvokeCall);
AddEventInvokeCall(index, propertyData);
}

IEnumerable<int> FindSetFieldInstructions()
Expand Down Expand Up @@ -117,9 +121,11 @@ int AddIsChangedSetterCall(int index)
return index;
}

int AddEventInvokeCall(int index, PropertyDefinition property)
int AddEventInvokeCall(int index, PropertyData targetProperty)
{
index = AddOnChangedMethodCall(index, property);
var property = targetProperty.PropertyDefinition;

index = AddOnChangedMethodCalls(index, targetProperty);
if (propertyData.AlreadyNotifies.Contains(property.Name))
{
moduleWeaver.LogDebug($"\t\t\t{property.Name} skipped since call already exists");
Expand All @@ -146,34 +152,31 @@ int AddEventInvokeCall(int index, PropertyDefinition property)
return AddSimpleInvokerCall(index, property);
}

int AddOnChangedMethodCall(int index, PropertyDefinition property)
int AddOnChangedMethodCalls(int index, PropertyData targetProperty)
{
if (!moduleWeaver.InjectOnPropertyNameChanged)
foreach (var onChangedMethod in targetProperty.OnChangedMethods)
{
return index;
}
var onChangedMethodName = $"On{property.Name}Changed";
if (ContainsCallToMethod(onChangedMethodName))
{
return index;
}
var onChangedMethod = typeNode
.OnChangedMethods
.FirstOrDefault(x => x.MethodReference.Name == onChangedMethodName);
if (onChangedMethod == null)
{
return index;
}
if (onChangedMethod.IsDefaultMethod)
{
if (!moduleWeaver.InjectOnPropertyNameChanged)
continue;

if (onChangedMethod.OnChangedType == OnChangedTypes.NoArg)
{
return AddSimpleOnChangedCall(index, onChangedMethod.MethodReference);
if (ContainsCallToMethod(onChangedMethod.MethodReference.Name))
continue;
}

switch (onChangedMethod.OnChangedType)
{
case OnChangedTypes.NoArg:
index = AddSimpleOnChangedCall(index, onChangedMethod.MethodReference);
break;

case OnChangedTypes.BeforeAfter:
index = AddBeforeAfterOnChangedCall(index, targetProperty.PropertyDefinition, onChangedMethod.MethodReference);
break;
}
}

if (onChangedMethod.OnChangedType == OnChangedTypes.BeforeAfter)
{
return AddBeforeAfterOnChangedCall(index, property, onChangedMethod.MethodReference);
}
return index;
}

Expand Down Expand Up @@ -333,4 +336,4 @@ public Instruction CreateCall(MethodReference methodReference)
{
return Instruction.Create(OpCodes.Callvirt, methodReference);
}
}
}
3 changes: 1 addition & 2 deletions PropertyChanged.Fody/TypeNode.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,5 @@ public TypeNode()
public EventInvokerMethod EventInvoker;
public MethodReference IsChangedInvoker;
public List<PropertyData> PropertyDatas;
public List<OnChangedMethod> OnChangedMethods;
public List<PropertyDefinition> AllProperties;
}
}
Loading

0 comments on commit 65ea5fe

Please sign in to comment.