From 5495737014b3836dc3228da8563f6549c238e3ff Mon Sep 17 00:00:00 2001 From: Julien Lebosquain Date: Tue, 28 Jul 2026 15:40:40 +0200 Subject: [PATCH] Allow single parameter with any type when binding to method (#21867) * Allow single parameter with any type when binding to method * Port method binding logic to ReflectionBinding * Don't depend on the order of methods * Handle overrides properly * Fix nullability warning --- .../Plugins/ReflectionMethodAccessorPlugin.cs | 124 +++++++-- .../XamlIlBindingPathHelper.cs | 97 ++++++- .../XamlIlTrampolineBuilder.cs | 6 +- .../Data/BindingTests_Method.cs | 225 +++++++++++++---- .../CompiledBindingExtensionTests.cs | 239 ++++++++++++++++-- 5 files changed, 574 insertions(+), 117 deletions(-) diff --git a/src/Avalonia.Base/Data/Core/Plugins/ReflectionMethodAccessorPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/ReflectionMethodAccessorPlugin.cs index d2e6f23e29..5213a2180f 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/ReflectionMethodAccessorPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/ReflectionMethodAccessorPlugin.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.Linq; using System.Linq.Expressions; using System.Reflection; @@ -10,10 +11,9 @@ namespace Avalonia.Data.Core.Plugins [RequiresDynamicCode(TrimmingMessages.ExpressionNodeRequiresDynamicCodeMessage)] internal class ReflectionMethodAccessorPlugin : IPropertyAccessorPlugin { - private readonly Dictionary<(Type, string), MethodInfo?> _methodLookup = - new Dictionary<(Type, string), MethodInfo?>(); + private readonly Dictionary<(Type, string), MethodLookupResult> _methodLookup = new(); - public bool Match(object obj, string methodName) => GetFirstMethodWithName(obj.GetType(), methodName) != null; + public bool Match(object obj, string methodName) => GetMethod(obj.GetType(), methodName).IsMatch; public IPropertyAccessor? Start(WeakReference reference, string methodName) { @@ -23,63 +23,133 @@ namespace Avalonia.Data.Core.Plugins if (!reference.TryGetTarget(out var instance) || instance is null) return null; - var method = GetFirstMethodWithName(instance.GetType(), methodName); + var result = GetMethod(instance.GetType(), methodName); - if (method is not null) + if (result.Method is { } method) { return new Accessor(reference, method); } else { - var message = $"Could not find CLR method '{methodName}' on '{instance}'"; - var exception = new MissingMemberException(message); + Exception exception = result.Error is { } error + ? new AmbiguousMatchException(error) + : new MissingMemberException($"Could not find CLR method '{methodName}' on '{instance}'"); return new PropertyError(new BindingNotification(exception, BindingErrorType.Error)); } } - private MethodInfo? GetFirstMethodWithName( + private MethodLookupResult GetMethod( [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods)] Type type, string methodName) { var key = (type, methodName); - if (!_methodLookup.TryGetValue(key, out var methodInfo)) + if (!_methodLookup.TryGetValue(key, out var result)) { - methodInfo = TryFindAndCacheMethod(type, methodName); + result = FindBestCommandMethod(type, methodName); + _methodLookup.Add(key, result); } - return methodInfo; + return result; } - private MethodInfo? TryFindAndCacheMethod( + /// + /// Finds the method named which can be bound to a command. + /// + /// + /// Priority: + /// 1. One parameter method + /// 1a. Object parameter (amongst several overloads) + /// 1b. Single method with one parameter + /// 2. Zero parameters method + /// + private static MethodLookupResult FindBestCommandMethod( [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods)] Type type, string methodName) { - MethodInfo? found = null; - const BindingFlags bindingFlags = BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Static | BindingFlags.Instance; - var methods = type.GetMethods(bindingFlags); + List? candidates = null; - foreach (var methodInfo in methods) + foreach (var methodInfo in type.GetMethods(bindingFlags)) { if (methodInfo.Name == methodName) + (candidates ??= []).Add(methodInfo); + } + + if (candidates is null) + return default; + + MethodInfo? zeroParamCandidate = null; + Dictionary? oneParamCandidates = null; + + foreach (var candidate in candidates) + { + var parameters = candidate.GetParameters(); + + switch (parameters.Length) { - var parameters = methodInfo.GetParameters(); - if (parameters.Length == 1 && parameters[0].ParameterType == typeof(object)) - { - found = methodInfo; + case 0: + zeroParamCandidate = GetMostDerived(zeroParamCandidate, candidate); + break; + + case 1: + // Reflection can return several methods with the same parameter type when one hides another: + // only keep the most derived one, so that overridden or hidden methods are handled properly. + var parameterType = parameters[0].ParameterType; + oneParamCandidates ??= new Dictionary(); + oneParamCandidates[parameterType] = GetMostDerived(oneParamCandidates.GetValueOrDefault(parameterType), candidate); break; - } - else if (parameters.Length == 0) - { - found = methodInfo; - } } } - _methodLookup.Add((type, methodName), found); + if (oneParamCandidates is not null) + { + // Object parameter always wins + if (oneParamCandidates.TryGetValue(typeof(object), out var objectParamCandidate)) + return new MethodLookupResult(objectParamCandidate, null); + + if (oneParamCandidates.Count == 1) + return new MethodLookupResult(oneParamCandidates.Values.First(), null); + + var parameterTypes = oneParamCandidates.Keys + .Select(t => $"'{t.FullName}'") + .OrderBy(s => s, StringComparer.Ordinal) + .ToArray(); + + return new MethodLookupResult( + null, + $"Unable to resolve method of name '{methodName}' on type '{type.FullName}'. " + + $"Found {parameterTypes.Length} overloads accepting one parameter: {string.Join(", ", parameterTypes)}. " + + "Expected either a single overload with one parameter, or an overload accepting System.Object."); + } + + if (zeroParamCandidate is { } found) + return new MethodLookupResult(found, null); + + return new MethodLookupResult( + null, + $"Unable to resolve method of name '{methodName}' on type '{type.FullName}'. " + + $"Found {candidates.Count} overloads accepting more than one parameter. " + + "Expected a method with zero or one parameter."); + } - return found; + private static MethodInfo GetMostDerived(MethodInfo? existing, MethodInfo candidate) + { + if (existing is null) + return candidate; + + return existing.DeclaringType is { } existingType && + candidate.DeclaringType is { } candidateType && + existingType != candidateType && existingType.IsAssignableFrom(candidateType) ? + candidate : + existing; + } + + private readonly struct MethodLookupResult(MethodInfo? method, string? error) + { + public MethodInfo? Method { get; } = method; + public string? Error { get; } = error; + public bool IsMatch => Method is not null || Error is not null; } [RequiresDynamicCode(TrimmingMessages.ExpressionNodeRequiresDynamicCodeMessage)] diff --git a/src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/XamlIlBindingPathHelper.cs b/src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/XamlIlBindingPathHelper.cs index 84af961431..1046658bc8 100644 --- a/src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/XamlIlBindingPathHelper.cs +++ b/src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/XamlIlBindingPathHelper.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Reflection.Emit; @@ -212,23 +213,10 @@ namespace Avalonia.Markup.Xaml.XamlIl.CompilerExtensions } else if (GetAllDefinedMethods(targetType) .Where(p => p.Name == propName.PropertyName) - .OrderByDescending(m => m.Parameters.Count) .ToArray() is { Length: > 0 } methodCandidates) { - var objType = context.Configuration.WellKnownTypes.Object; - var candidate = methodCandidates - .FirstOrDefault(m => m.Parameters.Count == 0 - || (m.Parameters.Count == 1 && - m.Parameters[0].Equals(objType))); - if (candidate is null) - { - throw new XamlX.XamlTransformException( - $"Unable to resolve method of name '{propName.PropertyName}' on type '{targetType}'." + - $"Expected method with no parameters or a single object overload.", - lineInfo); - } - + var candidate = GetBestCommandMethod(methodCandidates, propName.PropertyName, targetType); nodes.Add(new XamlIlClrMethodPathElementNode(candidate, context.Configuration.WellKnownTypes.Delegate, propName.AcceptsNull)); } else @@ -453,6 +441,67 @@ namespace Avalonia.Markup.Xaml.XamlIl.CompilerExtensions } } } + + // Priority: + // 1. One parameter method + // 1a. Object parameter (amongst several overloads) + // 1b. Single method with one parameter + // 2. Zero parameters method + IXamlMethod GetBestCommandMethod(IXamlMethod[] candidates, string name, IXamlType targetType) + { + Debug.Assert(candidates.Length > 0); + + IXamlMethod? zeroParamCandidate = null; + HashSet? oneParamCandidates = null; + + foreach (var candidate in candidates) + { + Debug.Assert(candidate.Name == name); + + switch (candidate.Parameters.Count) + { + case 0: + zeroParamCandidate ??= candidate; + break; + + case 1: + // Object parameter always wins + if (candidate.Parameters[0].Is("System", "Object")) + return candidate; + + // Our candidates are ordered with the most derived class first: + // By adding the first candidate for a given parameter type, we automatically handle overridden or hidden methods. + oneParamCandidates ??= new HashSet(SingleParameterTypeXamlTypeComparer.Instance); + oneParamCandidates.Add(candidate); + break; + } + } + + if (oneParamCandidates is not null) + { + Debug.Assert(oneParamCandidates.Count > 0); + + if (oneParamCandidates.Count == 1) + return oneParamCandidates.First(); + + var parameterTypes = oneParamCandidates + .Select(m => $"'{m.Parameters[0].FullName}'") + .OrderBy(s => s, StringComparer.Ordinal) + .ToArray(); + + throw new XamlTransformException( + $"Unable to resolve method of name '{name}' on type '{targetType}'. " + + $"Found {parameterTypes.Length} overloads accepting one parameter: {string.Join(", ", parameterTypes)}. " + + "Expected either a single overload with one parameter, or an overload accepting System.Object.", + lineInfo); + } + + return zeroParamCandidate ?? throw new XamlTransformException( + $"Unable to resolve method of name '{name}' on type '{targetType}'. " + + $"Found {candidates.Length} overloads accepting more than one parameter. " + + $"Expected a method with zero or one parameter.", + lineInfo); + } } private static void EmitPropertyCall(XamlIlEmitContext context, IXamlILEmitter codeGen, bool acceptsNull) @@ -1064,6 +1113,26 @@ namespace Avalonia.Markup.Xaml.XamlIl.CompilerExtensions } } } + + private sealed class SingleParameterTypeXamlTypeComparer : IEqualityComparer + { + public static SingleParameterTypeXamlTypeComparer Instance { get; } = new(); + + public bool Equals(IXamlMethod? x, IXamlMethod? y) + { + Debug.Assert(x is { Parameters.Count: 1 }); + Debug.Assert(y is { Parameters.Count: 1 }); + + return x!.Parameters[0].Equals(y!.Parameters[0]); + } + + public int GetHashCode(IXamlMethod obj) + { + Debug.Assert(obj.Parameters.Count == 1); + + return obj.Parameters[0].GetHashCode(); + } + } } interface IXamlIlBindingPathNode : IXamlAstValueNode diff --git a/src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/XamlIlTrampolineBuilder.cs b/src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/XamlIlTrampolineBuilder.cs index 5245fa7d3c..b0f4db068a 100644 --- a/src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/XamlIlTrampolineBuilder.cs +++ b/src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/XamlIlTrampolineBuilder.cs @@ -48,9 +48,11 @@ namespace Avalonia.Markup.Xaml.XamlIl.CompilerExtensions } if (executeMethod.Parameters.Count != 0) { - Debug.Assert(executeMethod.Parameters.Count == 1 - && executeMethod.Parameters[0] == context.Configuration.WellKnownTypes.Object); + Debug.Assert(executeMethod.Parameters.Count == 1); gen.Ldarg(1); + + if (!executeMethod.Parameters[0].Is("System", "Object")) + gen.Unbox_Any(executeMethod.Parameters[0]); } gen.EmitCall(executeMethod, swallowResult: true); gen.Ret(); diff --git a/tests/Avalonia.Markup.Xaml.UnitTests/Data/BindingTests_Method.cs b/tests/Avalonia.Markup.Xaml.UnitTests/Data/BindingTests_Method.cs index a1ab150366..ca5cb6eb04 100644 --- a/tests/Avalonia.Markup.Xaml.UnitTests/Data/BindingTests_Method.cs +++ b/tests/Avalonia.Markup.Xaml.UnitTests/Data/BindingTests_Method.cs @@ -1,7 +1,9 @@ using System; +using System.Collections.Generic; using System.ComponentModel; using Avalonia.Controls; using Avalonia.Input; +using Avalonia.Logging; using Avalonia.UnitTests; using Xunit; @@ -33,28 +35,139 @@ namespace Avalonia.Markup.Xaml.UnitTests.Data } } + [Theory] + [InlineData("ObjectMethod", "hello", "Called ObjectMethod with hello")] + [InlineData("StringMethod", "hello", "Called StringMethod with hello")] + [InlineData("Int32Method", "42", "Called Int32Method with 42")] + [InlineData("Int32Method", "42", "Called Int32Method with 42")] + [InlineData("VirtualObjectMethod", "hello", "Called VirtualObjectMethod with hello")] + [InlineData("VirtualStringMethod", "hello", "Called VirtualStringMethod with hello")] + [InlineData("VirtualStringMethod", "", "Called VirtualStringMethod with ")] + [InlineData("VirtualInt32Method", "42", "Called VirtualInt32Method with 42")] + [InlineData("MethodWithNewSlot", "42", "Called MethodWithNewSlot with 42")] + public void Binding_Method_With_Parameter_To_Command_Uses_Single_Parameter_Overload( + string methodName, + string xamlParameter, + string expected) + { + using var app = UnitTestApplication.Start(TestServices.StyledWindow); + + var window = (Window)AvaloniaRuntimeXamlLoader.Load( + $$""" + + + + """); + var button = window.GetControl + + """); + var button = window.GetControl + + """); + var button = window.GetControl