Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Commit

Permalink
[X] throw on mismatching param type for Add() (#11086)
Browse files Browse the repository at this point in the history
There was a `TODO: check parameter type` in this code, like if past me
knew that, at some point in time, what was only a potential edge case
bug, would explode in my face and ruin my faith in humanity.

Those FIXMEs... they are bottled 'I TOLD YOU SO' little time capsules

- fixes #11061
  • Loading branch information
StephaneDelcroix committed Jul 27, 2020
1 parent 92ed051 commit 5754d1a
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 15 deletions.
45 changes: 30 additions & 15 deletions Xamarin.Forms.Build.Tasks/SetPropertiesVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ public void Visit(ElementNode node, INode parentNode)
if (CanAddToResourceDictionary(parentVar, parentVar.VariableType, node, node, Context))
{
Context.IL.Append(parentVar.LoadAs(Module.GetTypeDefinition(("Xamarin.Forms.Core", "Xamarin.Forms", "ResourceDictionary")), Module));
Context.IL.Append(AddToResourceDictionary(node, node, Context));
Context.IL.Append(AddToResourceDictionary(parentVar, node, node, Context));
}
// Collection element, implicit content, or implicit collection element.
else if (parentVar.VariableType.ImplementsInterface(Module.ImportReference(("mscorlib", "System.Collections", "IEnumerable")))
Expand Down Expand Up @@ -192,7 +192,7 @@ public void Visit(ElementNode node, INode parentNode)
Context.IL.Append(GetPropertyValue(parent, parentList.XmlName, Context, node, out propertyType));

if (CanAddToResourceDictionary(parent, propertyType, node, node, Context)) {
Context.IL.Append(AddToResourceDictionary(node, node, Context));
Context.IL.Append(AddToResourceDictionary(parent, node, node, Context));
return;
}
var adderTuple = propertyType.GetMethods(md => md.Name == "Add" && md.Parameters.Count == 1, Module).FirstOrDefault() ??
Expand Down Expand Up @@ -1326,24 +1326,37 @@ static IEnumerable<Instruction> Get(VariableDefinition parent, string localName,
};
}

static bool CanAdd(VariableDefinition parent, XmlName propertyName, INode node, IXmlLineInfo lineInfo, ILContext context)
static bool CanAdd(VariableDefinition parent, XmlName propertyName, INode valueNode, IXmlLineInfo lineInfo, ILContext context)
{
var module = context.Body.Method.Module;
var localName = propertyName.LocalName;
bool attached;
var bpRef = GetBindablePropertyReference(parent, propertyName.NamespaceURI, ref localName, out attached, context, lineInfo);
TypeReference propertyType;
var bpRef = GetBindablePropertyReference(parent, propertyName.NamespaceURI, ref localName, out var attached, context, lineInfo);

if (!(valueNode is IElementNode elementNode))
return false;

if ( !CanGetValue(parent, bpRef, attached, null, context, out propertyType)
if ( !CanGetValue(parent, bpRef, attached, null, context, out TypeReference propertyType)
&& !CanGet(parent, localName, context, out propertyType))
return false;

//TODO check md.Parameters[0] type
var adderTuple = propertyType.GetMethods(md => md.Name == "Add" && md.Parameters.Count == 1, module).FirstOrDefault();
if (!context.Variables.TryGetValue(elementNode, out VariableDefinition varValue))
return false;

var adderTuple = propertyType.GetMethods(md => md.Name == "Add"
&& md.Parameters.Count == 1, module).FirstOrDefault();
if (adderTuple == null)
return false;

return true;
var adderRef = module.ImportReference(adderTuple.Item1);
adderRef = module.ImportReference(adderRef.ResolveGenericParameters(adderTuple.Item2, module));
var paramType = adderRef.Parameters[0].ParameterType.ResolveGenericParameters(adderRef);
if (varValue.VariableType.InheritsFromOrImplements(paramType))
return true;

if (varValue.VariableType.GetImplicitOperatorTo(paramType, module) != null)
return true;

return CanAddToResourceDictionary(parent, propertyType, elementNode, lineInfo, context);
}

static Dictionary<VariableDefinition, IList<string>> resourceNamesInUse = new Dictionary<VariableDefinition, IList<string>>();
Expand All @@ -1353,14 +1366,12 @@ static bool CanAddToResourceDictionary(VariableDefinition parent, TypeReference
&& collectionType.ResolveCached().BaseType?.FullName != "Xamarin.Forms.ResourceDictionary")
return false;


if (node.Properties.ContainsKey(XmlName.xKey)) {
var key = (node.Properties[XmlName.xKey] as ValueNode).Value as string;
if (!resourceNamesInUse.TryGetValue(parent, out var names))
resourceNamesInUse[parent] = (names = new List<string>());
if (names.Contains(key))
throw new BuildException(ResourceDictDuplicateKey, lineInfo, null, key);
names.Add(key);
return true;
}

Expand All @@ -1385,7 +1396,7 @@ static IEnumerable<Instruction> Add(VariableDefinition parent, XmlName propertyN
yield return instruction;

if (CanAddToResourceDictionary(parent, propertyType, elementNode, iXmlLineInfo, context)) {
foreach (var instruction in AddToResourceDictionary(elementNode, iXmlLineInfo, context))
foreach (var instruction in AddToResourceDictionary(parent, elementNode, iXmlLineInfo, context))
yield return instruction;
yield break;
}
Expand All @@ -1401,15 +1412,19 @@ static IEnumerable<Instruction> Add(VariableDefinition parent, XmlName propertyN
yield return Instruction.Create(OpCodes.Pop);
}

static IEnumerable<Instruction> AddToResourceDictionary(IElementNode node, IXmlLineInfo lineInfo, ILContext context)
static IEnumerable<Instruction> AddToResourceDictionary(VariableDefinition parent, IElementNode node, IXmlLineInfo lineInfo, ILContext context)
{
var module = context.Body.Method.Module;

if (node.Properties.ContainsKey(XmlName.xKey)) {
var names = resourceNamesInUse[parent];
var key = (node.Properties[XmlName.xKey] as ValueNode).Value as string;
names.Add(key);

// IL_0014: ldstr "key"
// IL_0019: ldstr "foo"
// IL_001e: callvirt instance void class [Xamarin.Forms.Core]Xamarin.Forms.ResourceDictionary::Add(string, object)
yield return Create(Ldstr, (node.Properties[XmlName.xKey] as ValueNode).Value as string);
yield return Create(Ldstr, (key));
foreach (var instruction in context.Variables[node].LoadAs(module.TypeSystem.Object, module))
yield return instruction;
yield return Create(Callvirt, module.ImportMethodReference(("Xamarin.Forms.Core", "Xamarin.Forms", "ResourceDictionary"),
Expand Down
7 changes: 7 additions & 0 deletions Xamarin.Forms.Xaml.UnitTests/Issues/Gh11061.xaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>
<ContentPage
xmlns="http://xamarin.com/schemas/2014/forms"
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
x:Class="Xamarin.Forms.Xaml.UnitTests.Gh11061"
MyDateTime="{Binding Date}">
</ContentPage>
38 changes: 38 additions & 0 deletions Xamarin.Forms.Xaml.UnitTests/Issues/Gh11061.xaml.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
using System;
using System.Collections.Generic;
using NUnit.Framework;
using Xamarin.Forms;
using Xamarin.Forms.Build.Tasks;
using Xamarin.Forms.Core.UnitTests;

namespace Xamarin.Forms.Xaml.UnitTests
{
[XamlCompilation(XamlCompilationOptions.Skip)]
public partial class Gh11061 : ContentPage
{
public DateTime MyDateTime { get; set; }

public Gh11061() => InitializeComponent();
public Gh11061(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 XamlCBindingOnNonBP([Values(false, true)] bool useCompiledXaml)
{
if (useCompiledXaml)
Assert.Throws<BuildException>(() => MockCompiler.Compile(typeof(Gh11061)));
else
Assert.Throws<XamlParseException>(()=> new Gh11061(useCompiledXaml) { BindingContext = new { Date = DateTime.Today} });
}
}
}
}

0 comments on commit 5754d1a

Please sign in to comment.