From ba466a78ede711e262fa27c62600c5a1a954d771 Mon Sep 17 00:00:00 2001 From: Juraj Ahel Date: Fri, 7 Feb 2025 16:26:22 +0100 Subject: [PATCH 1/4] add use case of class with a setter with a backing field that has custom code side effects (before or after value assignment) --- PropertyChanged.sln | 15 ++++++ .../AssemblyWithSetterSideEffects.csproj | 7 +++ .../AssemblyWithSetterSideEffects/Classes.cs | 52 ++++++++++++++++++ Tests/AssemblyWithSetterSideEffectsTests.cs | 53 +++++++++++++++++++ 4 files changed, 127 insertions(+) create mode 100644 TestAssemblies/AssemblyWithSetterSideEffects/AssemblyWithSetterSideEffects.csproj create mode 100644 TestAssemblies/AssemblyWithSetterSideEffects/Classes.cs create mode 100644 Tests/AssemblyWithSetterSideEffectsTests.cs diff --git a/PropertyChanged.sln b/PropertyChanged.sln index eb57fc71..06aee3c9 100644 --- a/PropertyChanged.sln +++ b/PropertyChanged.sln @@ -68,6 +68,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "PropertyChanged.Fody.Analyz EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "PropertyChanged.Fody.Analyzer.Tests", "PropertyChanged.Fody.Analyzer.Tests\PropertyChanged.Fody.Analyzer.Tests.csproj", "{35A8A625-B010-4F9D-A901-9B07D9E13A16}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "AssemblyWithSetterSideEffects", "TestAssemblies\AssemblyWithSetterSideEffects\AssemblyWithSetterSideEffects.csproj", "{E4E54CE2-DD54-41B9-BDC0-DF45FA575535}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -338,6 +340,18 @@ Global {35A8A625-B010-4F9D-A901-9B07D9E13A16}.Release|ARM.Build.0 = Release|Any CPU {35A8A625-B010-4F9D-A901-9B07D9E13A16}.Release|x86.ActiveCfg = Release|Any CPU {35A8A625-B010-4F9D-A901-9B07D9E13A16}.Release|x86.Build.0 = Release|Any CPU + {E4E54CE2-DD54-41B9-BDC0-DF45FA575535}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {E4E54CE2-DD54-41B9-BDC0-DF45FA575535}.Debug|Any CPU.Build.0 = Debug|Any CPU + {E4E54CE2-DD54-41B9-BDC0-DF45FA575535}.Debug|ARM.ActiveCfg = Debug|Any CPU + {E4E54CE2-DD54-41B9-BDC0-DF45FA575535}.Debug|ARM.Build.0 = Debug|Any CPU + {E4E54CE2-DD54-41B9-BDC0-DF45FA575535}.Debug|x86.ActiveCfg = Debug|Any CPU + {E4E54CE2-DD54-41B9-BDC0-DF45FA575535}.Debug|x86.Build.0 = Debug|Any CPU + {E4E54CE2-DD54-41B9-BDC0-DF45FA575535}.Release|Any CPU.ActiveCfg = Release|Any CPU + {E4E54CE2-DD54-41B9-BDC0-DF45FA575535}.Release|Any CPU.Build.0 = Release|Any CPU + {E4E54CE2-DD54-41B9-BDC0-DF45FA575535}.Release|ARM.ActiveCfg = Release|Any CPU + {E4E54CE2-DD54-41B9-BDC0-DF45FA575535}.Release|ARM.Build.0 = Release|Any CPU + {E4E54CE2-DD54-41B9-BDC0-DF45FA575535}.Release|x86.ActiveCfg = Release|Any CPU + {E4E54CE2-DD54-41B9-BDC0-DF45FA575535}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -363,6 +377,7 @@ Global {89CC72A8-43A9-4593-9FA5-61091C989B40} = {546A6338-E25E-4796-AF08-A4742E3BDF7B} {9D3B5C35-6523-45D9-B11D-0FBB67C0866A} = {546A6338-E25E-4796-AF08-A4742E3BDF7B} {10BDE166-0BFC-41D9-9326-805717A054B3} = {546A6338-E25E-4796-AF08-A4742E3BDF7B} + {E4E54CE2-DD54-41B9-BDC0-DF45FA575535} = {546A6338-E25E-4796-AF08-A4742E3BDF7B} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {ABD5CA78-3690-4D62-8642-3463D9FAE144} diff --git a/TestAssemblies/AssemblyWithSetterSideEffects/AssemblyWithSetterSideEffects.csproj b/TestAssemblies/AssemblyWithSetterSideEffects/AssemblyWithSetterSideEffects.csproj new file mode 100644 index 00000000..9cf53932 --- /dev/null +++ b/TestAssemblies/AssemblyWithSetterSideEffects/AssemblyWithSetterSideEffects.csproj @@ -0,0 +1,7 @@ + + + + net48;net6.0 + true + + \ No newline at end of file diff --git a/TestAssemblies/AssemblyWithSetterSideEffects/Classes.cs b/TestAssemblies/AssemblyWithSetterSideEffects/Classes.cs new file mode 100644 index 00000000..cd4e736f --- /dev/null +++ b/TestAssemblies/AssemblyWithSetterSideEffects/Classes.cs @@ -0,0 +1,52 @@ +using System.ComponentModel; + +public class WithSideEffectBeforeValueAssignment : + INotifyPropertyChanged +{ + public event PropertyChangedEventHandler PropertyChanged; + public void OnPropertyChanged(string propertyName) + { + PropertyChanged?.Invoke(this, new(propertyName)); + } + + string _property1; + public string Property1 + { + get => _property1; + set + { + CallSideEffectBefore(); + _property1 = value; + } + } + + + public int SideEffectBeforeCallCount { get; set; } + void CallSideEffectBefore() => SideEffectBeforeCallCount++; +} + +public class WithSideEffectAfterValueAssignment : + INotifyPropertyChanged +{ + public event PropertyChangedEventHandler PropertyChanged; + public void OnPropertyChanged(string propertyName) + { + PropertyChanged?.Invoke(this, new(propertyName)); + } + + + + string _property1; + public string Property1 + { + get => _property1; + set + { + _property1 = value; + CallSideEffectAfter(); + } + } + + public int SideEffectAfterCallCount { get; set; } + void CallSideEffectAfter() => SideEffectAfterCallCount++; +} \ No newline at end of file diff --git a/Tests/AssemblyWithSetterSideEffectsTests.cs b/Tests/AssemblyWithSetterSideEffectsTests.cs new file mode 100644 index 00000000..66415833 --- /dev/null +++ b/Tests/AssemblyWithSetterSideEffectsTests.cs @@ -0,0 +1,53 @@ +public class AssemblyWithSetterSideEffectsTests +{ + static readonly string[] peVerifyIgnoreCodes = + { +#if NETCOREAPP + "0x80131869" +#endif + }; + + const string assemblyName = "AssemblyWithSetterSideEffects.dll"; + + [Theory] + [InlineData([true, "same", "same", 2])] + [InlineData([true, "different1", "different2", 2])] + [InlineData([false, "same", "same", 2])] + [InlineData([false, "different1", "different2", 2])] + public void CallsPreAssignmentSideEffect(bool checkForEquality, string firstAssignment, string secondAssignment, int expectedCallCount) + { + const string className = "WithSideEffectBeforeValueAssignment"; + + var weaver = new ModuleWeaver(){CheckForEquality = checkForEquality}; + var testResult = weaver.ExecuteTestRun(assemblyName, ignoreCodes: peVerifyIgnoreCodes); + + var instance = testResult.GetInstance(className); + + instance.Property1 = firstAssignment; + instance.Property1 = secondAssignment; + + var callCount = (int)instance.SideEffectBeforeCallCount; + Assert.Equal(expectedCallCount, callCount); + } + + [Theory] + [InlineData([true, "same", "same", 2])] + [InlineData([true, "different1", "different2", 2])] + [InlineData([false, "same", "same", 2])] + [InlineData([false, "different1", "different2", 2])] + public void CallsPostAssignmentSideEffect(bool checkForEquality, string firstAssignment, string secondAssignment, int expectedCallCount) + { + const string className = "WithSideEffectAfterValueAssignment"; + + var weaver = new ModuleWeaver() { CheckForEquality = checkForEquality }; + var testResult = weaver.ExecuteTestRun(assemblyName, ignoreCodes: peVerifyIgnoreCodes); + + var instance = testResult.GetInstance(className); + + instance.Property1 = firstAssignment; + instance.Property1 = secondAssignment; + + var callCount = (int)instance.SideEffectAfterCallCount; + Assert.Equal(expectedCallCount, callCount); + } +} \ No newline at end of file From 3997a11bd4b947c44afe1ea2686426dedaeadf33 Mon Sep 17 00:00:00 2001 From: Juraj Ahel Date: Fri, 7 Feb 2025 17:20:33 +0100 Subject: [PATCH 2/4] test: add missing project reference to Test project --- Tests/Tests.csproj | 1 + 1 file changed, 1 insertion(+) diff --git a/Tests/Tests.csproj b/Tests/Tests.csproj index 4f3ebde1..e0b0db7f 100644 --- a/Tests/Tests.csproj +++ b/Tests/Tests.csproj @@ -36,5 +36,6 @@ + From 226dad1a648efbb362dbc9ac2841769f3492ad45 Mon Sep 17 00:00:00 2001 From: Juraj Ahel Date: Fri, 7 Feb 2025 18:27:54 +0100 Subject: [PATCH 3/4] add configurability of behavior to ensure not interferring with existing setter logic with tests --- .../CheckForSetterNonInterferenceConfig.cs | 19 ++++++++ ...heckForSetterNonInterferenceConfigTests.cs | 48 +++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 PropertyChanged.Fody/Config/CheckForSetterNonInterferenceConfig.cs create mode 100644 Tests/CheckForSetterNonInterferenceConfigTests.cs diff --git a/PropertyChanged.Fody/Config/CheckForSetterNonInterferenceConfig.cs b/PropertyChanged.Fody/Config/CheckForSetterNonInterferenceConfig.cs new file mode 100644 index 00000000..a3625398 --- /dev/null +++ b/PropertyChanged.Fody/Config/CheckForSetterNonInterferenceConfig.cs @@ -0,0 +1,19 @@ +using System.Linq; +using System.Xml; + +public partial class ModuleWeaver +{ + // default: false to preserve behavior from PropertyChanged.Fody 2.x + public bool EnsureNonInterferenceWithCustomSetterBehaviors = false; + + public void ResolveCheckForSetterNonInterferenceConfig() + { + var value = Config?.Attributes("EnsureNonInterferenceWithCustomSetterBehaviors") + .Select(a => a.Value) + .SingleOrDefault(); + if (value != null) + { + EnsureNonInterferenceWithCustomSetterBehaviors = XmlConvert.ToBoolean(value.ToLowerInvariant()); + } + } +} diff --git a/Tests/CheckForSetterNonInterferenceConfigTests.cs b/Tests/CheckForSetterNonInterferenceConfigTests.cs new file mode 100644 index 00000000..2f491621 --- /dev/null +++ b/Tests/CheckForSetterNonInterferenceConfigTests.cs @@ -0,0 +1,48 @@ +using System.Xml.Linq; + +public class CheckForSetterNonInterferenceConfigTests +{ + [Fact] + public void False() + { + var xElement = XElement.Parse(""); + var weaver = new ModuleWeaver { Config = xElement }; + weaver.ResolveCheckForSetterNonInterferenceConfig(); + Assert.False(weaver.EnsureNonInterferenceWithCustomSetterBehaviors); + } + + [Fact] + public void False0() + { + var xElement = XElement.Parse(""); + var weaver = new ModuleWeaver { Config = xElement }; + weaver.ResolveCheckForSetterNonInterferenceConfig(); + Assert.False(weaver.EnsureNonInterferenceWithCustomSetterBehaviors); + } + + [Fact] + public void True() + { + var xElement = XElement.Parse(""); + var weaver = new ModuleWeaver { Config = xElement }; + weaver.ResolveCheckForSetterNonInterferenceConfig(); + Assert.True(weaver.EnsureNonInterferenceWithCustomSetterBehaviors); + } + + [Fact] + public void True1() + { + var xElement = XElement.Parse(""); + var weaver = new ModuleWeaver { Config = xElement }; + weaver.ResolveCheckForSetterNonInterferenceConfig(); + Assert.True(weaver.EnsureNonInterferenceWithCustomSetterBehaviors); + } + + [Fact] + public void Default() + { + var weaver = new ModuleWeaver(); + weaver.ResolveCheckForSetterNonInterferenceConfig(); + Assert.False(weaver.EnsureNonInterferenceWithCustomSetterBehaviors); + } +} \ No newline at end of file From aae0ac23fa923a3d4a05fb75a7812ae11b68303f Mon Sep 17 00:00:00 2001 From: Juraj Ahel Date: Fri, 7 Feb 2025 18:35:00 +0100 Subject: [PATCH 4/4] make non setter interference behavior test depend on config. Conditionally disable failing test case for the missing implementation based on compilation flag, turned off by default. --- Directory.Build.props | 4 ++ Tests/AssemblyWithSetterSideEffectsTests.cs | 52 +++++++++++++++++++-- 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/Directory.Build.props b/Directory.Build.props index 64361db1..0f4dddef 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -9,4 +9,8 @@ all low + + + + diff --git a/Tests/AssemblyWithSetterSideEffectsTests.cs b/Tests/AssemblyWithSetterSideEffectsTests.cs index 66415833..c0b9e1ea 100644 --- a/Tests/AssemblyWithSetterSideEffectsTests.cs +++ b/Tests/AssemblyWithSetterSideEffectsTests.cs @@ -6,11 +6,13 @@ "0x80131869" #endif }; - + const string assemblyName = "AssemblyWithSetterSideEffects.dll"; [Theory] +#if TEMP_REQUIRE_IMPLEMENTATION_SETTER_NON_INTERFERENCE [InlineData([true, "same", "same", 2])] +#endif [InlineData([true, "different1", "different2", 2])] [InlineData([false, "same", "same", 2])] [InlineData([false, "different1", "different2", 2])] @@ -18,7 +20,7 @@ public void CallsPreAssignmentSideEffect(bool checkForEquality, string firstAssi { const string className = "WithSideEffectBeforeValueAssignment"; - var weaver = new ModuleWeaver(){CheckForEquality = checkForEquality}; + var weaver = new ModuleWeaver(){CheckForEquality = checkForEquality, EnsureNonInterferenceWithCustomSetterBehaviors = true }; var testResult = weaver.ExecuteTestRun(assemblyName, ignoreCodes: peVerifyIgnoreCodes); var instance = testResult.GetInstance(className); @@ -31,7 +33,9 @@ public void CallsPreAssignmentSideEffect(bool checkForEquality, string firstAssi } [Theory] +#if TEMP_REQUIRE_IMPLEMENTATION_SETTER_NON_INTERFERENCE [InlineData([true, "same", "same", 2])] +#endif [InlineData([true, "different1", "different2", 2])] [InlineData([false, "same", "same", 2])] [InlineData([false, "different1", "different2", 2])] @@ -39,7 +43,7 @@ public void CallsPostAssignmentSideEffect(bool checkForEquality, string firstAss { const string className = "WithSideEffectAfterValueAssignment"; - var weaver = new ModuleWeaver() { CheckForEquality = checkForEquality }; + var weaver = new ModuleWeaver() { CheckForEquality = checkForEquality, EnsureNonInterferenceWithCustomSetterBehaviors = true }; var testResult = weaver.ExecuteTestRun(assemblyName, ignoreCodes: peVerifyIgnoreCodes); var instance = testResult.GetInstance(className); @@ -50,4 +54,46 @@ public void CallsPostAssignmentSideEffect(bool checkForEquality, string firstAss var callCount = (int)instance.SideEffectAfterCallCount; Assert.Equal(expectedCallCount, callCount); } + + [Theory] + [InlineData([true, "same", "same", 1])] + [InlineData([true, "different1", "different2", 2])] + [InlineData([false, "same", "same", 2])] + [InlineData([false, "different1", "different2", 2])] + public void CallsPreAssignmentSideEffectLegacy(bool checkForEquality, string firstAssignment, string secondAssignment, int expectedCallCount) + { + const string className = "WithSideEffectBeforeValueAssignment"; + + var weaver = new ModuleWeaver() { CheckForEquality = checkForEquality, EnsureNonInterferenceWithCustomSetterBehaviors = false }; + var testResult = weaver.ExecuteTestRun(assemblyName, ignoreCodes: peVerifyIgnoreCodes); + + var instance = testResult.GetInstance(className); + + instance.Property1 = firstAssignment; + instance.Property1 = secondAssignment; + + var callCount = (int)instance.SideEffectBeforeCallCount; + Assert.Equal(expectedCallCount, callCount); + } + + [Theory] + [InlineData([true, "same", "same", 1])] + [InlineData([true, "different1", "different2", 2])] + [InlineData([false, "same", "same", 2])] + [InlineData([false, "different1", "different2", 2])] + public void CallsPostAssignmentSideEffectLegacy(bool checkForEquality, string firstAssignment, string secondAssignment, int expectedCallCount) + { + const string className = "WithSideEffectAfterValueAssignment"; + + var weaver = new ModuleWeaver() { CheckForEquality = checkForEquality, EnsureNonInterferenceWithCustomSetterBehaviors = false }; + var testResult = weaver.ExecuteTestRun(assemblyName, ignoreCodes: peVerifyIgnoreCodes); + + var instance = testResult.GetInstance(className); + + instance.Property1 = firstAssignment; + instance.Property1 = secondAssignment; + + var callCount = (int)instance.SideEffectAfterCallCount; + Assert.Equal(expectedCallCount, callCount); + } } \ No newline at end of file