diff --git a/src/Directory.Packages.props b/src/Directory.Packages.props index d11721b8..92f07a5a 100644 --- a/src/Directory.Packages.props +++ b/src/Directory.Packages.props @@ -10,7 +10,7 @@ - 5.1.2 + 5.1.3 9.0.0 diff --git a/src/ReactiveUI.Binding.Shared/Fallback/RuntimePropertyRaiser.cs b/src/ReactiveUI.Binding.Shared/Fallback/RuntimePropertyRaiser.cs index 8cfcb2c8..d52fcdb7 100644 --- a/src/ReactiveUI.Binding.Shared/Fallback/RuntimePropertyRaiser.cs +++ b/src/ReactiveUI.Binding.Shared/Fallback/RuntimePropertyRaiser.cs @@ -150,7 +150,12 @@ private static Members FindMembers(Type type) => private static RaiseMember? FindReactiveObjectExtension(Type type, bool changing) { var reactiveObject = type.GetInterface("ReactiveUI.IReactiveObject"); - var extensions = reactiveObject?.Assembly.GetType("ReactiveUI.IReactiveObjectExtensions"); + if (reactiveObject is null) + { + return null; + } + + var extensions = FindReactiveObjectExtensions(type, reactiveObject); if (extensions is null) { return null; @@ -167,12 +172,39 @@ private static Members FindMembers(Type type) => } // A static method closed over its first argument binds to Action like an instance method. - return ByName(method.MakeGenericMethod(reactiveObject!)); + return ByName(method.MakeGenericMethod(reactiveObject)); } return null; } + /// Finds the extension class that owns the object's ReactiveUI notification state. + /// The object's type. + /// The ReactiveUI interface it implements. + /// The extension class, or null when the object's assemblies declare neither flavour. + [RequiresUnreferencedCode(RequiresUnreferencedCodeMessage)] + private static Type? FindReactiveObjectExtensions(Type type, Type reactiveObject) + { + // The interface can live in ReactiveUI.Core; the base class identifies the flavour holding the state. + for (var current = type; current is not null; current = current.BaseType) + { + if (FindReactiveObjectExtensions(current.Assembly) is { } extensions) + { + return extensions; + } + } + + return FindReactiveObjectExtensions(reactiveObject.Assembly); + } + + /// Finds either ReactiveUI flavour's raise extensions in an assembly. + /// The assembly to inspect. + /// The extension class, or null when the assembly declares neither flavour. + [RequiresUnreferencedCode(RequiresUnreferencedCodeMessage)] + private static Type? FindReactiveObjectExtensions(Assembly assembly) => + assembly.GetType("ReactiveUI.IReactiveObjectExtensions") + ?? assembly.GetType("ReactiveUI.Reactive.IReactiveObjectExtensions"); + /// Finds a raise method, preferring an overload that takes event args over one that takes a name. /// The type. /// Whether to find a before-change raise method. diff --git a/src/examples/Platforms/ToPropertyVerification/Common/ReactiveObjectViewModel.cs b/src/examples/Platforms/ToPropertyVerification/Common/ReactiveObjectViewModel.cs index 295c5a1d..081fc595 100644 --- a/src/examples/Platforms/ToPropertyVerification/Common/ReactiveObjectViewModel.cs +++ b/src/examples/Platforms/ToPropertyVerification/Common/ReactiveObjectViewModel.cs @@ -23,6 +23,9 @@ public sealed class ReactiveObjectViewModel : global::ReactiveUI.ReactiveObject /// Backs . private readonly ObservableAsPropertyHelper _labelHelper; + /// Backs the reflection-based property. + private readonly ObservableAsPropertyHelper _unsafeCountHelper; + /// Initializes a new instance of the class. /// The item the view model follows. public ReactiveObjectViewModel(SourceItem item) @@ -30,6 +33,8 @@ public ReactiveObjectViewModel(SourceItem item) _countHelper = item.WhenChanged(static x => x.Count).ToProperty(this, static x => x.Count); _ = item.WhenChanged(static x => x.Label).ToProperty(this, static x => x.Label, out _labelHelper, InitialLabel); + + _unsafeCountHelper = item.WhenChanged(static x => x.Count).ToPropertyUnsafe(this, static x => x.UnsafeCount); } /// Gets the followed item's count. @@ -37,4 +42,7 @@ public ReactiveObjectViewModel(SourceItem item) /// Gets the followed item's label. public string Label => _labelHelper.Value; + + /// Gets the followed count through the reflection-based overload. + public int UnsafeCount => _unsafeCountHelper.Value; } diff --git a/src/examples/Platforms/ToPropertyVerification/Program.cs b/src/examples/Platforms/ToPropertyVerification/Program.cs index 19915f2a..c5b490d9 100644 --- a/src/examples/Platforms/ToPropertyVerification/Program.cs +++ b/src/examples/Platforms/ToPropertyVerification/Program.cs @@ -19,7 +19,7 @@ failures += Run("Deferred+scheduler combo, and out combined with initial value / initial-value factory", CombinedOverloadsScenario); -failures += Run("ReactiveObject (IReactiveObject raise mechanism), selector + out overloads", ReactiveObjectScenario); +failures += Run("ReactiveObject (IReactiveObject raise mechanism), generated + unsafe overloads", ReactiveObjectScenario); failures += Run("[ObservableAsProperty] partial properties", AttributeScenario); @@ -194,7 +194,7 @@ static void ReactiveObjectScenario() const string InitialLabel = "x"; const int UpdatedCount = 2; const string UpdatedLabel = "y"; - const int ExpectedNotificationCount = 2; + const int ExpectedNotificationCount = 3; SourceItem item = new() { Count = InitialCount, Label = InitialLabel }; ReactiveObjectViewModel vm = new(item); @@ -202,12 +202,14 @@ static void ReactiveObjectScenario() var changes = TrackChanges(vm); AssertEqual(InitialCount, vm.Count, "initial Count"); + AssertEqual(InitialCount, vm.UnsafeCount, "initial UnsafeCount"); // WhenChanged delivers the item's current label as soon as the helper subscribes, replacing the initial value. AssertEqual(InitialLabel, vm.Label, "initial Label from the item's current value"); item.Count = UpdatedCount; AssertEqual(UpdatedCount, vm.Count, "updated Count"); + AssertEqual(UpdatedCount, vm.UnsafeCount, "updated UnsafeCount"); item.Label = UpdatedLabel; AssertEqual(UpdatedLabel, vm.Label, "updated Label"); diff --git a/src/tests/ReactiveUI.Binding.Tests/Bindings/RuntimePropertyRaiserTests.cs b/src/tests/ReactiveUI.Binding.Tests/Bindings/RuntimePropertyRaiserTests.cs new file mode 100644 index 00000000..108f12d6 --- /dev/null +++ b/src/tests/ReactiveUI.Binding.Tests/Bindings/RuntimePropertyRaiserTests.cs @@ -0,0 +1,150 @@ +// Copyright (c) 2019-2026 ReactiveUI and Contributors. All rights reserved. +// ReactiveUI and Contributors licenses this file to you under the MIT license. +// See the LICENSE file in the project root for full license information. + +using System.ComponentModel; +using System.Runtime.CompilerServices; +using ReactiveUI.Binding.Fallback; + +namespace ReactiveUI.Binding.Tests.Bindings; + +/// Verifies runtime notification methods and optional before-change dispatch. +public sealed class RuntimePropertyRaiserTests +{ + /// Event-argument methods that return a value still deliver both cached notification arguments. + /// A task representing the asynchronous test. + [Test] + public async Task ReturningEventArgsMethods_RaiseBothNotifications() + { + var model = new ReturningArgsModel(); + var raiser = RuntimePropertyRaiser.Create(model, "Name"); + raiser.RaiseChanging(); + raiser.RaiseChanged(); + raiser.RaiseChanging(); + raiser.RaiseChanged(); + + await Assert.That(raiser.RaisesChanging).IsTrue(); + await Assert.That(string.Join(",", model.Raised)).IsEqualTo("changing:Name,changed:Name,changing:Name,changed:Name"); + await Assert.That(ReferenceEquals(model.ChangedArgs[0], model.ChangedArgs[1])).IsTrue(); + await Assert.That(ReferenceEquals(model.ChangingArgs[0], model.ChangingArgs[1])).IsTrue(); + } + + /// Name-based methods with return values remain usable as notification methods. + /// A task representing the asynchronous test. + [Test] + public async Task ReturningNameMethods_RaiseBothNotifications() + { + var model = new ReturningNameModel(); + var raiser = RuntimePropertyRaiser.Create(model, "Name"); + raiser.RaiseChanging(); + raiser.RaiseChanged(); + + await Assert.That(string.Join(",", model.Raised)).IsEqualTo("changing:Name,changed:Name"); + } + + /// A model supporting only after-change safely ignores before-change dispatch. + /// A task representing the asynchronous test. + [Test] + public async Task NoChangingMethod_IgnoresBeforeChange() + { + var model = new ToPropertyUnsafeTests.PublicArgsModel(); + var raiser = RuntimePropertyRaiser.Create(model, "Name"); + raiser.RaiseChanging(); + raiser.RaiseChanged(); + + await Assert.That(raiser.RaisesChanging).IsFalse(); + await Assert.That(model.Raised).IsEquivalentTo(["Name"]); + } + + /// Void event-argument methods bind both notification phases directly to the owner. + /// A task representing the asynchronous test. + [Test] + public async Task VoidEventArgsMethods_RaiseBothNotifications() + { + var model = new VoidArgsModel(); + var raiser = RuntimePropertyRaiser.Create(model, "Name"); + raiser.RaiseChanging(); + raiser.RaiseChanged(); + raiser.RaiseChanging(); + raiser.RaiseChanged(); + + await Assert.That(string.Join(",", model.Raised)).IsEqualTo("changing:Name,changed:Name,changing:Name,changed:Name"); + } + + /// Records argument-based notifications and returns the count for ordinary callers. + public sealed class ReturningArgsModel + { + /// Gets the notification order. + public List Raised { get; } = []; + + /// Gets after-change arguments. + public List ChangedArgs { get; } = []; + + /// Gets before-change arguments. + public List ChangingArgs { get; } = []; + + /// Records an after-change notification. + /// The notification arguments. + /// The number of notifications. + public int RaisePropertyChanged(PropertyChangedEventArgs args) + { + ChangedArgs.Add(args); + Raised.Add($"changed:{args.PropertyName}"); + return Raised.Count; + } + + /// Records a before-change notification. + /// The notification arguments. + /// The number of notifications. + public int RaisePropertyChanging(PropertyChangingEventArgs args) + { + ChangingArgs.Add(args); + Raised.Add($"changing:{args.PropertyName}"); + return Raised.Count; + } + } + + /// Records name-based notifications with a return value. + public sealed class ReturningNameModel + { + /// Gets the notification order. + public List Raised { get; } = []; + + /// Records an after-change notification. + /// The property name. + /// The name supplied to the notification method. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public string RaisePropertyChanged(string propertyName) + { + Raised.Add($"changed:{propertyName}"); + return propertyName; + } + + /// Records a before-change notification. + /// The property name. + /// The name supplied to the notification method. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public string RaisePropertyChanging(string propertyName) + { + Raised.Add($"changing:{propertyName}"); + return propertyName; + } + } + + /// Records notifications through void methods taking event arguments. + public sealed class VoidArgsModel + { + /// Gets the notification order. + public List Raised { get; } = []; + + /// Records an after-change notification. + /// The notification arguments. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void RaisePropertyChanged(PropertyChangedEventArgs args) => Raised.Add($"changed:{args.PropertyName}"); + + /// Records a before-change notification. + /// The notification arguments. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void RaisePropertyChanging(PropertyChangingEventArgs args) => Raised.Add($"changing:{args.PropertyName}"); + } +} diff --git a/src/tests/ReactiveUI.Binding.Tests/Bindings/ToPropertyUnsafeSplitAssemblyTests.cs b/src/tests/ReactiveUI.Binding.Tests/Bindings/ToPropertyUnsafeSplitAssemblyTests.cs new file mode 100644 index 00000000..6a3d0a6e --- /dev/null +++ b/src/tests/ReactiveUI.Binding.Tests/Bindings/ToPropertyUnsafeSplitAssemblyTests.cs @@ -0,0 +1,286 @@ +// Copyright (c) 2019-2026 ReactiveUI and Contributors. All rights reserved. +// ReactiveUI and Contributors licenses this file to you under the MIT license. +// See the LICENSE file in the project root for full license information. + +using System.Reactive.Subjects; +using System.Reflection; +using System.Reflection.Emit; +using System.Runtime.CompilerServices; + +namespace ReactiveUI.Binding.Tests.Bindings; + +/// Verifies notification dispatch when ReactiveUI's interface and extensions ship separately. +public sealed class ToPropertyUnsafeSplitAssemblyTests +{ + /// The observable helper's initial value. + private const string Initial = "initial"; + + /// The first value delivered by the source. + private const string First = "first"; + + /// The two notifications raised for one value, in order. + private const string NotificationPair = "changing:Name,changed:Name"; + + /// The original framework's extension class. + private const string ExtensionsName = "ReactiveUI.IReactiveObjectExtensions"; + + /// The after-change raise method name. + private const string ChangedMethod = "RaisePropertyChanged"; + + /// The sender's generic parameter name. + private const string SenderParameter = "TSender"; + + /// Records notifications emitted by the separately loaded extensions. + /// The notification owner. + /// The property name. + /// Whether the notification precedes the change. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static void Record(object owner, string name, bool changing) => + ReadNotifications(owner).Add($"{(changing ? "changing" : "changed")}:{name}"); + + /// Both notification phases use the extension assembly belonging to the model's base type. + /// The flavour's extension class name. + /// Whether the consumer subclasses the ReactiveUI model in a third assembly. + /// A task representing the asynchronous test. + [Test] + [Arguments("ReactiveUI.IReactiveObjectExtensions", false)] + [Arguments("ReactiveUI.IReactiveObjectExtensions", true)] + [Arguments("ReactiveUI.Reactive.IReactiveObjectExtensions", false)] + [Arguments("ReactiveUI.Reactive.IReactiveObjectExtensions", true)] + public async Task SplitAssembly_RaisesChangingThenChanged(string extensionsName, bool derived) + { + var model = (SplitModel)CreateModel(extensionsName, derived); + using var source = new Subject(); + using var helper = source.ToPropertyUnsafe(model, nameof(SplitModel.Name), Initial); + model.Helper = helper; + model.Raised.Clear(); + + source.OnNext(First); + source.OnNext("second"); + + await Assert.That(model.Name).IsEqualTo("second"); + await Assert.That(string.Join(",", model.Raised)).IsEqualTo("changing:Name,changed:Name,changing:Name,changed:Name"); + await Assert.That(model.GetType().GetInterface("ReactiveUI.IReactiveObject")!.Assembly == model.GetType().Assembly).IsFalse(); + } + + /// An implementation without framework extensions falls back to its own notification methods. + /// A task representing the asynchronous test. + [Test] + public async Task MissingExtensions_RaisesThroughMethods() + { + var model = CreateModel(null, true); + using var source = new Subject(); + using var helper = source.ToPropertyUnsafe(model, nameof(SplitModel.Name), Initial); + var raised = ReadNotifications(model); + raised.Clear(); + source.OnNext(First); + + await Assert.That(helper.Value).IsEqualTo(First); + await Assert.That(string.Join(",", raised)).IsEqualTo(NotificationPair); + } + + /// The original layout still resolves extensions from the interface assembly. + /// A task representing the asynchronous test. + [Test] + public async Task InterfaceAssemblyExtensions_RaiseBothNotifications() + { + var model = CreateModel(ExtensionsName, true, true); + using var source = new Subject(); + using var helper = source.ToPropertyUnsafe(model, nameof(SplitModel.Name), Initial); + var raised = ReadNotifications(model); + raised.Clear(); + source.OnNext(First); + + await Assert.That(helper.Value).IsEqualTo(First); + await Assert.That(string.Join(",", raised)).IsEqualTo(NotificationPair); + } + + /// The System.Reactive runtime resolves the same split assembly notification layouts. + /// The framework flavour's extension class. + /// A task representing the asynchronous test. + [Test] + [Arguments("ReactiveUI.IReactiveObjectExtensions")] + [Arguments("ReactiveUI.Reactive.IReactiveObjectExtensions")] + public async Task ReactiveRuntime_SplitAssembly_RaisesBothNotifications(string extensionsName) + { + var model = (SplitModel)CreateModel(extensionsName, true); + using var source = new Subject(); + using var helper = ReactiveUI.Binding.Reactive.ReactiveUIBindingExtensions.ToPropertyUnsafe(source, model, nameof(SplitModel.Name), Initial); + model.Raised.Clear(); + source.OnNext(First); + + await Assert.That(helper.Value).IsEqualTo(First); + await Assert.That(string.Join(",", model.Raised)).IsEqualTo(NotificationPair); + } + + /// Extensions returning a value remain callable by the reflection fallback. + /// A task representing the asynchronous test. + [Test] + public async Task ReturningExtensions_RaiseBothNotifications() + { + var model = (SplitModel)CreateModel(ExtensionsName, true, returnsValue: true); + using var source = new Subject(); + using var helper = source.ToPropertyUnsafe(model, nameof(SplitModel.Name), Initial); + model.Raised.Clear(); + source.OnNext(First); + + await Assert.That(helper.Value).IsEqualTo(First); + await Assert.That(string.Join(",", model.Raised)).IsEqualTo(NotificationPair); + } + + /// A framework without a before-change extension still raises its after-change notification. + /// A task representing the asynchronous test. + [Test] + public async Task ChangedOnlyExtensions_RaiseAfterChange() + { + var model = (SplitModel)CreateModel(ExtensionsName, true, changedOnly: true); + using var source = new Subject(); + using var helper = source.ToPropertyUnsafe(model, nameof(SplitModel.Name), Initial); + model.Raised.Clear(); + source.OnNext(First); + + await Assert.That(helper.Value).IsEqualTo(First); + await Assert.That(string.Join(",", model.Raised)).IsEqualTo("changed:Name"); + } + + /// Creates an interface assembly, a framework assembly, and optionally a consumer assembly. + /// The extensions' full name. + /// Whether to add a consumer subclass. + /// Whether extensions use the legacy interface assembly layout. + /// Whether extensions return the property name. + /// Whether the framework supplies only after-change extensions. + /// The independently compiled model. + private static object CreateModel(string? extensionsName, bool derived, bool interfaceExtensions = false, bool returnsValue = false, bool changedOnly = false) + { + var core = CreateModule(); + var interfaceBuilder = core.DefineType("ReactiveUI.IReactiveObject", TypeAttributes.Public | TypeAttributes.Interface | TypeAttributes.Abstract); + var reactiveObject = interfaceBuilder.CreateType()!; + var framework = CreateModule(); + if (extensionsName is not null) + { + DefineExtensions(interfaceExtensions ? core : framework, extensionsName, reactiveObject, returnsValue, changedOnly); + } + + var isolated = extensionsName is null || interfaceExtensions; + var modelBuilder = framework.DefineType("ReactiveUI.ReactiveObject", TypeAttributes.Public, isolated ? typeof(object) : typeof(SplitModel), [reactiveObject]); + if (isolated) + { + _ = modelBuilder.DefineField(nameof(SplitModel.Raised), typeof(List), FieldAttributes.Public); + if (extensionsName is null) + { + DefineRaiseMethods(modelBuilder); + } + } + + _ = modelBuilder.DefineDefaultConstructor(MethodAttributes.Public); + var modelType = modelBuilder.CreateType()!; + if (derived) + { + var consumer = CreateModule().DefineType("Consumer.ViewModel", TypeAttributes.Public | TypeAttributes.Sealed, modelType); + _ = consumer.DefineDefaultConstructor(MethodAttributes.Public); + modelType = consumer.CreateType()!; + } + + var model = Activator.CreateInstance(modelType)!; + if (isolated) + { + modelType.GetField(nameof(SplitModel.Raised))!.SetValue(model, new List()); + } + + return model; + } + + /// Declares the framework's generic raise extensions against the separate interface. + /// The framework or core module. + /// The extension class name. + /// The interface constraint. + /// Whether the methods return the name after raising. + /// Whether to omit the before-change method. + private static void DefineExtensions(ModuleBuilder module, string name, Type reactiveObject, bool returnsValue, bool changedOnly) + { + var extensions = module.DefineType(name, TypeAttributes.Public | TypeAttributes.Abstract | TypeAttributes.Sealed); + DefineUnrelatedOverloads(extensions); + foreach (var changing in changedOnly ? new[] { false } : new[] { false, true }) + { + var method = extensions.DefineMethod(changing ? "RaisePropertyChanging" : ChangedMethod, MethodAttributes.Public | MethodAttributes.Static); + var sender = method.DefineGenericParameters(SenderParameter)[0]; + sender.SetInterfaceConstraints(reactiveObject); + method.SetReturnType(returnsValue ? typeof(string) : typeof(void)); + method.SetParameters(sender, typeof(string)); + var body = method.GetILGenerator(); + body.Emit(OpCodes.Ldarg_0); + body.Emit(OpCodes.Box, sender); + body.Emit(OpCodes.Ldarg_1); + body.Emit(changing ? OpCodes.Ldc_I4_1 : OpCodes.Ldc_I4_0); + body.Emit(OpCodes.Call, typeof(ToPropertyUnsafeSplitAssemblyTests).GetMethod(nameof(Record))!); + if (returnsValue) + { + body.Emit(OpCodes.Ldarg_1); + } + + body.Emit(OpCodes.Ret); + } + + _ = extensions.CreateType(); + } + + /// Adds overloads that do not match the generic sender and string name API. + /// The extension class being built. + private static void DefineUnrelatedOverloads(TypeBuilder extensions) + { + var ordinary = extensions.DefineMethod(ChangedMethod, MethodAttributes.Public | MethodAttributes.Static, typeof(void), [typeof(object), typeof(string)]); + ordinary.GetILGenerator().Emit(OpCodes.Ret); + var missingName = extensions.DefineMethod(ChangedMethod, MethodAttributes.Public | MethodAttributes.Static); + var missingSender = missingName.DefineGenericParameters(SenderParameter)[0]; + missingName.SetReturnType(typeof(void)); + missingName.SetParameters(missingSender); + missingName.GetILGenerator().Emit(OpCodes.Ret); + var numericName = extensions.DefineMethod(ChangedMethod, MethodAttributes.Public | MethodAttributes.Static); + var numericSender = numericName.DefineGenericParameters(SenderParameter)[0]; + numericName.SetReturnType(typeof(void)); + numericName.SetParameters(numericSender, typeof(int)); + numericName.GetILGenerator().Emit(OpCodes.Ret); + } + + /// Declares ordinary model notification methods for the extension-free layout. + /// The model being built. + private static void DefineRaiseMethods(TypeBuilder model) + { + foreach (var changing in new[] { false, true }) + { + var method = model.DefineMethod(changing ? "RaisePropertyChanging" : ChangedMethod, MethodAttributes.Public, typeof(void), [typeof(string)]); + var body = method.GetILGenerator(); + body.Emit(OpCodes.Ldarg_0); + body.Emit(OpCodes.Ldarg_1); + body.Emit(changing ? OpCodes.Ldc_I4_1 : OpCodes.Ldc_I4_0); + body.Emit(OpCodes.Call, typeof(ToPropertyUnsafeSplitAssemblyTests).GetMethod(nameof(Record))!); + body.Emit(OpCodes.Ret); + } + } + + /// Creates a unique assembly so fixtures do not share interface identities or cached raisers. + /// The assembly's module. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static ModuleBuilder CreateModule() => + AssemblyBuilder.DefineDynamicAssembly(new($"SplitReactiveUI_{Guid.NewGuid():N}"), AssemblyBuilderAccess.Run).DefineDynamicModule("Main"); + + /// Reads each independent model's notification record. + /// The notification owner. + /// The notifications recorded for that model. + private static List ReadNotifications(object model) => model is SplitModel shared + ? shared.Raised + : (List)model.GetType().GetField(nameof(SplitModel.Raised))!.GetValue(model)!; + + /// The property and observable helper shared by the independent framework models. + public class SplitModel + { + /// Gets or sets the observable helper. + public ObservableAsPropertyHelper? Helper { get; set; } + + /// Gets the observable value. + public string? Name => Helper?.Value; + + /// Gets the notifications, in dispatch order. + public List Raised { get; } = []; + } +}