[Bug]ObservableValidator.ValidateAllProperties() throws exception when no Validation rules exists.

[svestin]

Possible bug in Microsoft.Toolkit.Mvvm version 7.0.2 (running on .Net Framework 4.7.2):
(note: same behaviour in 7.1.0)

Windows 10 Enterprise 1909, 18363.1734
Visual Studio 2019, 16.11.3

Maybe I have misunderstood the usage of ObservableValidator.ValidateAllProperties(), but
when I run the example below, I end up with the unexpected exception:
“System.ArgumentException: 'Non-empty collection required Parameter name: expressions' ”

public class MyBase : ObservableValidator
{
    private int? _myDummyInt = 0;

    //[Required]  //When rule included; it works
    public int? MyDummyInt
    {
        get { return _myDummyInt; }
        set { _myDummyInt = value; }
    }

    public void ValidateAll()
    {
        ValidateAllProperties();
    }
}

and then use it like

MyBase x = new MyBase();
x.ValidateAll();

However, if I add the RequiredAttribute on MyDummyInt it works fine.

So why do I need to Validate a class with not validation rules?

The MyBase class normally has no properties that needs validation.
I then have some classes that derives from MyBase
MyDerived1: HAS some properties with validation rules.
MyDerived2: Has NO properties with validation rules.

Then I have a List storing instances of MyDerived1 and MyDerived2.

Now I would like to validate the instances in my list.
Sadly I get the unexpected exception when I try to validate an instance of MyDerived2 (that has no validation rules).

At the moment I have to add the MyDummyInt (with the RequiredAttribute) in the base class to avoid the unexpected exception.

Maybe it is possible to check if a class is missing validation attributes? Then I could avoid calling ValidateAllProperties() if there are no validation attributes used.

Any help would be very much appreciated.
Best Regards

[ghost]

Hello svestin, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

[michael-hawker]

@svestin have you tried the newly release 7.1 package as well, does it exhibit the same issue?

FYI @Sergio0694

[svestin]

@michael-hawker : Yes I see the same behavior in 7.1.0.

[Sergio0694]

Hi @svestin, I cannot seem to be able to repro this issue on my end, are you sure your repro steps are complete?
I tried both with and without source generators (which would be what the MVVM Toolkit did prior to 7.1).

I defined these two classes:

public class MyBase : ObservableValidator
{
    public int? MyDummyInt { get; set; } = 0;

    public void ValidateAll()
    {
        ValidateAllProperties();
    }
}

public class MyDerived2 : MyBase
{
    public string Name { get; set; }

    public int SomeRandomproperty { get; set; }
}

Then tested them like so:

MyBase viewmodel = new();

viewmodel.ValidateAll();

MyDerived2 viewmodel2 = new();

viewmodel2.ValidateAll();

Which worked just fine, it just didn't do anything.

When using source generators, it makes sense that this works correctly, as you can also inspect the generated code.
For MyBase, for instance, the MVVM Toolkit simply generates this method:

namespace Microsoft.Toolkit.Mvvm.ComponentModel.__Internals
{
    internal static partial class __ObservableValidatorExtensions
    {
        [global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
        [global::System.Obsolete("This method is not intended to be called directly by user code")]
        public static global::System.Action<object> CreateAllPropertiesValidator(global::UnitTests.Mvvm.Test_ObservableRecipientAttribute.MyBase _)
        {
            static void ValidateAllProperties(object obj)
            {
                var instance = (global::UnitTests.Mvvm.Test_ObservableRecipientAttribute.MyBase)obj;
            }

            return ValidateAllProperties;
        }
    }
}

Which is just a no-op. When disabling this path and using the fallback code path with compiled LINQ expressions, you can still see how the implementation will just skip all properties with no validation attributes, and just do nothing:

WindowsCommunityToolkit/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs

Lines 516 to 518 in fef65a5

	from property in type.GetProperties(BindingFlags.Instance | BindingFlags.Public)
	where property.GetIndexParameters().Length == 0 &&
	property.GetCustomAttributes<ValidationAttribute>(true).Any()

If no results are yielded here, the generated method will just not perform any validation (the body will be empty).

I'm going to remove the bug label here, feel free to add more info!
Also, could you double check this on .NET 5 too? Thanks! 😄

[svestin]

More info:
This time on another computer with Win 10 Pro 21H1, 19043.1237
Visual Studio 2019, 16.11.3
MvvmToolkit 7.1.0

If I create a WPF-app with .Net5 (and as a result a SDK-style project file), everything work as expected.

Repeated the same small example on WPF .Net Framework 4.8 (old stye project file) and now I end up with

Exception thrown: 'System.ArgumentException' in System.Core.dll
An unhandled exception of type 'System.ArgumentException' occurred in System.Core.dll
Non-empty collection required

Unhandled Exception: System.ArgumentException: Non-empty collection required
Parameter name: expressions
   at System.Dynamic.Utils.ContractUtils.RequiresNotEmpty[T](ICollection`1 collection, String paramName)
   at System.Linq.Expressions.Expression.Block(IEnumerable`1 variables, IEnumerable`1 expressions)
   at System.Linq.Expressions.Expression.Block(IEnumerable`1 expressions)
   at Microsoft.Toolkit.Mvvm.ComponentModel.ObservableValidator.<ValidateAllProperties>g__GetValidationActionFallback|30_1(Type type) in /_/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs:line 515
   at Microsoft.Toolkit.Mvvm.ComponentModel.ObservableValidator.<ValidateAllProperties>g__GetValidationAction|30_0(Type type) in /_/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs:line 485
   at Microsoft.Toolkit.Mvvm.ComponentModel.ObservableValidator.<>c.<ValidateAllProperties>b__30_2(Type t) in /_/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs:line 532
   at System.Runtime.CompilerServices.ConditionalWeakTable`2.GetValue(TKey key, CreateValueCallback createValueCallback)
   at Microsoft.Toolkit.Mvvm.ComponentModel.ObservableValidator.ValidateAllProperties() in /_/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs:line 532
   at MvvmToolkit_net48.MyBase.ValidateAll() in C:\source\MvvmToolkitBugTest\MvvmToolkit_net48\MyBase.cs:line 23
   at MvvmToolkit_net48.MainWindow.Button_Click(Object sender, RoutedEventArgs e) in C:\source\MvvmToolkitBugTest\MvvmToolkit_net48\MainWindow.xaml.cs:line 31
   at System.Windows.RoutedEventHandlerInfo.InvokeHandler(Object target, RoutedEventArgs routedEventArgs)
   at System.Windows.EventRoute.InvokeHandlersImpl(Object source, RoutedEventArgs args, Boolean reRaised)
   at System.Windows.UIElement.RaiseEventImpl(DependencyObject sender, RoutedEventArgs args)
   at System.Windows.UIElement.RaiseEvent(RoutedEventArgs e)
   at System.Windows.Controls.Primitives.ButtonBase.OnClick()
   at System.Windows.Controls.Button.OnClick()
   at System.Windows.Controls.Primitives.ButtonBase.OnMouseLeftButtonUp(MouseButtonEventArgs e)
   at System.Windows.UIElement.OnMouseLeftButtonUpThunk(Object sender, MouseButtonEventArgs e)
   at System.Windows.Input.MouseButtonEventArgs.InvokeEventHandler(Delegate genericHandler, Object genericTarget)
   at System.Windows.RoutedEventArgs.InvokeHandler(Delegate handler, Object target)
   at System.Windows.RoutedEventHandlerInfo.InvokeHandler(Object target, RoutedEventArgs routedEventArgs)
   at System.Windows.EventRoute.InvokeHandlersImpl(Object source, RoutedEventArgs args, Boolean reRaised)
   at System.Windows.UIElement.ReRaiseEventAs(DependencyObject sender, RoutedEventArgs args, RoutedEvent newEvent)
   at System.Windows.UIElement.OnMouseUpThunk(Object sender, MouseButtonEventArgs e)
   at System.Windows.Input.MouseButtonEventArgs.InvokeEventHandler(Delegate genericHandler, Object genericTarget)
   at System.Windows.RoutedEventArgs.InvokeHandler(Delegate handler, Object target)
   at System.Windows.RoutedEventHandlerInfo.InvokeHandler(Object target, RoutedEventArgs routedEventArgs)
   at System.Windows.EventRoute.InvokeHandlersImpl(Object source, RoutedEventArgs args, Boolean reRaised)
   at System.Windows.UIElement.RaiseEventImpl(DependencyObject sender, RoutedEventArgs args)
   at System.Windows.UIElement.RaiseTrustedEvent(RoutedEventArgs args)
   at System.Windows.UIElement.RaiseEvent(RoutedEventArgs args, Boolean trusted)
   at System.Windows.Input.InputManager.ProcessStagingArea()
   at System.Windows.Input.InputManager.ProcessInput(InputEventArgs input)
   at System.Windows.Input.InputProviderSite.ReportInput(InputReport inputReport)
   at System.Windows.Interop.HwndMouseInputProvider.ReportInput(IntPtr hwnd, InputMode mode, Int32 timestamp, RawMouseActions actions, Int32 x, Int32 y, Int32 wheel)
   at System.Windows.Interop.HwndMouseInputProvider.FilterMessage(IntPtr hwnd, WindowMessage msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at System.Windows.Interop.HwndSource.InputFilterMessage(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at MS.Win32.HwndWrapper.WndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at MS.Win32.HwndSubclass.DispatcherCallbackOperation(Object o)
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
   at System.Windows.Threading.Dispatcher.LegacyInvokeImpl(DispatcherPriority priority, TimeSpan timeout, Delegate method, Object args, Int32 numArgs)
   at MS.Win32.HwndSubclass.SubclassWndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam)
   at MS.Win32.UnsafeNativeMethods.DispatchMessage(MSG& msg)
   at System.Windows.Threading.Dispatcher.PushFrameImpl(DispatcherFrame frame)
   at System.Windows.Threading.Dispatcher.PushFrame(DispatcherFrame frame)
   at System.Windows.Application.RunDispatcher(Object ignore)
   at System.Windows.Application.RunInternal(Window window)
   at System.Windows.Application.Run(Window window)
   at System.Windows.Application.Run()
   at MvvmToolkit_net48.App.Main()
The program '[15100] MvvmToolkit_net48.exe' has exited with code 0 (0x0).

Originally I reported the problem in .Net Framework 4.7.2

Looking in Nuget manager I noticed that there is a difference in presentation for 4.7.2, 4.8 compared to .Net5. If that somehow helps.

Thanks for looking into my problem!

e
MvvmToolkitBugTest.zip

[Sergio0694]

Oooh, that helps, thanks for the additional info! 😄
I think this bug is an interesting combination of (1) the source generators somehow not being picked up in your project and (2) .NET Framework seemingly having a bug (or at least a different behavior) that is causing the compiled LINQ expression to crash if there are no input expressions in its constructor. Specifically, I believe this is the part that's causing the crash:

WindowsCommunityToolkit/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs

Lines 515 to 525 in fef65a5

	BlockExpression body = Expression.Block(
	from property in type.GetProperties(BindingFlags.Instance | BindingFlags.Public)
	where property.GetIndexParameters().Length == 0 &&
	property.GetCustomAttributes<ValidationAttribute>(true).Any()
	let getter = property.GetMethod
	where getter is not null
	select Expression.Call(inst0, validateMethod, new Expression[]
	{
	Expression.Convert(Expression.Call(inst0, getter), typeof(object)),
	Expression.Constant(property.Name)
	}));

That Expression.Block is evidently not liking receiving an empty collection on .NET Framework.

@michael-hawker If we plan on pushing out a hotfix for 7.1, I'd be happy to also add a workaround for this scenario. The fact we're still officially supporting .NET Standard 2.0 and that this workaround would also be a small optimization for this specific scenario makes me feel like it'd be worth adding even though the issue doesn't happen on .NET 5. Thoughts? 🙂

EDIT: I'll just fix this and open a PR, and then we can just merge it and push the fix whenever we release the next version 👍