From ea0d55dd0d6e344f004542f80093de01609702ce Mon Sep 17 00:00:00 2001 From: Stephane Delcroix Date: Wed, 26 Jun 2019 14:35:54 +0200 Subject: [PATCH] [X] resolve event handlers in base generic types (#6194) * [XamlC] fix Debug harness generation code * [X] resolve event handlers in base generic types - fixes #6176 --- .../CreateObjectVisitor.cs | 16 ++++---- Xamarin.Forms.Build.Tasks/DebugXamlCTask.cs | 5 ++- .../MethodReferenceExtensions.cs | 2 +- Xamarin.Forms.Build.Tasks/NodeILExtensions.cs | 2 +- .../SetPropertiesVisitor.cs | 19 +++++---- .../TypeDefinitionExtensions.cs | 11 +++--- .../Issues/Gh6176.xaml | 11 ++++++ .../Issues/Gh6176.xaml.cs | 39 +++++++++++++++++++ 8 files changed, 80 insertions(+), 25 deletions(-) create mode 100644 Xamarin.Forms.Xaml.UnitTests/Issues/Gh6176.xaml create mode 100644 Xamarin.Forms.Xaml.UnitTests/Issues/Gh6176.xaml.cs diff --git a/Xamarin.Forms.Build.Tasks/CreateObjectVisitor.cs b/Xamarin.Forms.Build.Tasks/CreateObjectVisitor.cs index 7e4aae0a0..c6ecf5e03 100644 --- a/Xamarin.Forms.Build.Tasks/CreateObjectVisitor.cs +++ b/Xamarin.Forms.Build.Tasks/CreateObjectVisitor.cs @@ -96,10 +96,10 @@ namespace Xamarin.Forms.Build.Tasks MethodDefinition ctorInfo = null; if (node.Properties.ContainsKey(XmlName.xArguments) && !node.Properties.ContainsKey(XmlName.xFactoryMethod)) { - factoryCtorInfo = typedef.AllMethods().FirstOrDefault(md => md.IsConstructor && - !md.IsStatic && - md.HasParameters && - md.MatchXArguments(node, typeref, Module, Context)); + factoryCtorInfo = typedef.AllMethods().FirstOrDefault(md => md.methodDef.IsConstructor && + !md.methodDef.IsStatic && + md.methodDef.HasParameters && + md.methodDef.MatchXArguments(node, typeref, Module, Context)).methodDef; if (factoryCtorInfo == null) { throw new XamlParseException( string.Format("No constructors found for {0} with matching x:Arguments", typedef.FullName), node); @@ -109,10 +109,10 @@ namespace Xamarin.Forms.Build.Tasks Context.IL.Append(PushCtorXArguments(factoryCtorInfo.ResolveGenericParameters(typeref, Module), node)); } else if (node.Properties.ContainsKey(XmlName.xFactoryMethod)) { var factoryMethod = (string)(node.Properties [XmlName.xFactoryMethod] as ValueNode).Value; - factoryMethodInfo = typedef.AllMethods().FirstOrDefault(md => !md.IsConstructor && - md.Name == factoryMethod && - md.IsStatic && - md.MatchXArguments(node, typeref, Module, Context)); + factoryMethodInfo = typedef.AllMethods().FirstOrDefault(md => !md.methodDef.IsConstructor && + md.methodDef.Name == factoryMethod && + md.methodDef.IsStatic && + md.methodDef.MatchXArguments(node, typeref, Module, Context)).methodDef; if (factoryMethodInfo == null) { throw new XamlParseException( String.Format("No static method found for {0}::{1} ({2})", typedef.FullName, factoryMethod, null), node); diff --git a/Xamarin.Forms.Build.Tasks/DebugXamlCTask.cs b/Xamarin.Forms.Build.Tasks/DebugXamlCTask.cs index 2fc0abd03..af934c52e 100644 --- a/Xamarin.Forms.Build.Tasks/DebugXamlCTask.cs +++ b/Xamarin.Forms.Build.Tasks/DebugXamlCTask.cs @@ -113,8 +113,9 @@ namespace Xamarin.Forms.Build.Tasks var br2 = Instruction.Create(OpCodes.Ldarg_0); var ret = Instruction.Create(OpCodes.Ret); il.Emit(OpCodes.Ldarg_0); - il.Emit(OpCodes.Callvirt, - module.ImportReference(typeDef.BaseType.Resolve().GetConstructors().First(c => c.HasParameters == false))); + var baseCtor = module.ImportReference(typeDef.BaseType.Resolve().GetConstructors().First(c => c.HasParameters == false)); + baseCtor = module.ImportReference(baseCtor.ResolveGenericParameters(typeDef.BaseType, module)); + il.Emit(OpCodes.Callvirt, baseCtor); il.Emit(OpCodes.Nop); il.Emit(OpCodes.Ldarg_1); diff --git a/Xamarin.Forms.Build.Tasks/MethodReferenceExtensions.cs b/Xamarin.Forms.Build.Tasks/MethodReferenceExtensions.cs index 7e3f8e5da..97d0a2e25 100644 --- a/Xamarin.Forms.Build.Tasks/MethodReferenceExtensions.cs +++ b/Xamarin.Forms.Build.Tasks/MethodReferenceExtensions.cs @@ -15,7 +15,7 @@ namespace Xamarin.Forms.Build.Tasks var reference = new MethodReference(self.Name, ImportUnresolvedType(self.ReturnType, module)) { - DeclaringType = module.ImportReference(declaringTypeRef), + DeclaringType = declaringTypeRef, HasThis = self.HasThis, ExplicitThis = self.ExplicitThis, CallingConvention = self.CallingConvention diff --git a/Xamarin.Forms.Build.Tasks/NodeILExtensions.cs b/Xamarin.Forms.Build.Tasks/NodeILExtensions.cs index 01e0a00b4..a17411e27 100644 --- a/Xamarin.Forms.Build.Tasks/NodeILExtensions.cs +++ b/Xamarin.Forms.Build.Tasks/NodeILExtensions.cs @@ -169,7 +169,7 @@ namespace Xamarin.Forms.Build.Tasks .Methods.FirstOrDefault(md => md.Name == "ConvertFromInvariantString" && md.Parameters.Count == 2) : typeConverter.ResolveCached() .AllMethods() - .FirstOrDefault(md => md.Name == "ConvertFromInvariantString" && md.Parameters.Count == 1); + .FirstOrDefault(md => md.methodDef.Name == "ConvertFromInvariantString" && md.methodDef.Parameters.Count == 1).methodDef; var convertFromInvariantStringReference = module.ImportReference(convertFromInvariantStringDefinition); yield return Instruction.Create(OpCodes.Newobj, typeConverterCtorRef); diff --git a/Xamarin.Forms.Build.Tasks/SetPropertiesVisitor.cs b/Xamarin.Forms.Build.Tasks/SetPropertiesVisitor.cs index 6809796a3..628610297 100644 --- a/Xamarin.Forms.Build.Tasks/SetPropertiesVisitor.cs +++ b/Xamarin.Forms.Build.Tasks/SetPropertiesVisitor.cs @@ -941,31 +941,34 @@ namespace Xamarin.Forms.Build.Tasks while (declaringType.IsNested) declaringType = declaringType.DeclaringType; var handler = declaringType.AllMethods().FirstOrDefault(md => { - if (md.Name != value as string) + if (md.methodDef.Name != value as string) return false; //check if the handler signature matches the Invoke signature; var invoke = module.ImportReference(eventinfo.EventType.ResolveCached().GetMethods().First(eventmd => eventmd.Name == "Invoke")); invoke = invoke.ResolveGenericParameters(eventinfo.EventType, module); - if (!md.ReturnType.InheritsFromOrImplements(invoke.ReturnType) || invoke.Parameters.Count != md.Parameters.Count) + if (!md.methodDef.ReturnType.InheritsFromOrImplements(invoke.ReturnType) || invoke.Parameters.Count != md.methodDef.Parameters.Count) return false; if (!invoke.ContainsGenericParameter) for (var i = 0; i < invoke.Parameters.Count;i++) - if (!invoke.Parameters[i].ParameterType.InheritsFromOrImplements(md.Parameters[i].ParameterType)) + if (!invoke.Parameters[i].ParameterType.InheritsFromOrImplements(md.methodDef.Parameters[i].ParameterType)) return false; //TODO check generic parameters if any return true; }); - if (handler == null) + MethodReference handlerRef = null; + if (handler.methodDef != null) + handlerRef = handler.methodDef.ResolveGenericParameters(handler.declTypeRef, module); + if (handler.methodDef == null) throw new XamlParseException($"EventHandler \"{value}\" with correct signature not found in type \"{declaringType}\"", iXmlLineInfo); //FIXME: eventually get the right ctor instead fo the First() one, just in case another one could exists (not even sure it's possible). var ctor = module.ImportReference(eventinfo.EventType.ResolveCached().GetConstructors().First()); ctor = ctor.ResolveGenericParameters(eventinfo.EventType, module); - if (handler.IsStatic) { + if (handler.methodDef.IsStatic) { yield return Create(Ldnull); } else { if (context.Root is VariableDefinition) @@ -978,11 +981,11 @@ namespace Xamarin.Forms.Build.Tasks throw new InvalidProgramException(); } - if (handler.IsVirtual) { + if (handler.methodDef.IsVirtual) { yield return Create(Ldarg_0); - yield return Create(Ldvirtftn, handler); + yield return Create(Ldvirtftn, handlerRef); } else - yield return Create(Ldftn, handler); + yield return Create(Ldftn, handlerRef); yield return Create(Newobj, module.ImportReference(ctor)); //Check if the handler has the same signature as the ctor (it should) diff --git a/Xamarin.Forms.Build.Tasks/TypeDefinitionExtensions.cs b/Xamarin.Forms.Build.Tasks/TypeDefinitionExtensions.cs index 9b3b377e0..37532c9b6 100644 --- a/Xamarin.Forms.Build.Tasks/TypeDefinitionExtensions.cs +++ b/Xamarin.Forms.Build.Tasks/TypeDefinitionExtensions.cs @@ -46,13 +46,14 @@ namespace Xamarin.Forms.Build.Tasks return ctor; } - public static IEnumerable AllMethods(this TypeDefinition self) + public static IEnumerable<(MethodDefinition methodDef, TypeReference declTypeRef)> AllMethods(this TypeDefinition self) { - while (self != null) - { + TypeReference selfTypeRef = self; + while (self != null) { foreach (var md in self.Methods) - yield return md; - self = self.BaseType == null ? null : self.BaseType.ResolveCached(); + yield return (md, selfTypeRef); + selfTypeRef = self.BaseType; + self = self.BaseType?.ResolveCached(); } } } diff --git a/Xamarin.Forms.Xaml.UnitTests/Issues/Gh6176.xaml b/Xamarin.Forms.Xaml.UnitTests/Issues/Gh6176.xaml new file mode 100644 index 000000000..b48768ba6 --- /dev/null +++ b/Xamarin.Forms.Xaml.UnitTests/Issues/Gh6176.xaml @@ -0,0 +1,11 @@ + + + + + + diff --git a/Xamarin.Forms.Xaml.UnitTests/Issues/Gh6176.xaml.cs b/Xamarin.Forms.Xaml.UnitTests/Issues/Gh6176.xaml.cs new file mode 100644 index 000000000..ea5c63dd4 --- /dev/null +++ b/Xamarin.Forms.Xaml.UnitTests/Issues/Gh6176.xaml.cs @@ -0,0 +1,39 @@ +using System; +using System.Collections.Generic; +using NUnit.Framework; +using Xamarin.Forms; +using Xamarin.Forms.Core.UnitTests; + +namespace Xamarin.Forms.Xaml.UnitTests +{ + public class Gh6176VM + { + } + + public class Gh6176Base : ContentPage where TVM: class + { + public TVM ViewModel => BindingContext as TVM; + protected void ShowMenu(object sender, EventArgs e) { } + } + + public partial class Gh6176 + { + public Gh6176() => InitializeComponent(); + public Gh6176(bool useCompiledXaml) + { + //this stub will be replaced at compile time + } + + [TestFixture] class Tests + { + [SetUp] public void Setup() => Device.PlatformServices = new MockPlatformServices(); + [TearDown] public void TearDown() => Device.PlatformServices = null; + + [Test] + public void XamlCDoesntFail([Values(false, true)]bool useCompiledXaml) + { + var layout = new Gh6176(useCompiledXaml); + } + } + } +}