[Xaml] fix Namescope being different than tree (#4089)
authorStephane Delcroix <stephane@delcroix.org>
Tue, 16 Oct 2018 15:10:04 +0000 (17:10 +0200)
committerGitHub <noreply@github.com>
Tue, 16 Oct 2018 15:10:04 +0000 (17:10 +0200)
As part of #2556, the namescoping tree was assumed to be equiv to the
object tree, which is obviously wrong. This fixes it, while keeping the
old behavior as a fallback.

We might go further, but I can't come up with a reasonable scenario
failing.

- fixes #3821

Xamarin.Forms.Build.Tasks/NodeILExtensions.cs
Xamarin.Forms.Xaml.UnitTests/Issues/Gh3821.xaml [new file with mode: 0644]
Xamarin.Forms.Xaml.UnitTests/Issues/Gh3821.xaml.cs [new file with mode: 0644]
Xamarin.Forms.Xaml.UnitTests/Issues/Gh3821View.xaml [new file with mode: 0644]
Xamarin.Forms.Xaml.UnitTests/Issues/Gh3821View.xaml.cs [new file with mode: 0644]
Xamarin.Forms.Xaml.UnitTests/Xamarin.Forms.Xaml.UnitTests.csproj
Xamarin.Forms.Xaml/MarkupExtensions/ReferenceExtension.cs
Xamarin.Forms.Xaml/XamlServiceProvider.cs

index 076041c..85c3ca3 100644 (file)
@@ -495,7 +495,12 @@ namespace Xamarin.Forms.Build.Tasks
                                foreach (var instruction in PushTargetProperty(bpRef, propertyRef, declaringTypeReference, module))
                                        yield return instruction;
 
-                               yield return Create(Newobj, module.ImportCtorReference(("Xamarin.Forms.Xaml", "Xamarin.Forms.Xaml.Internals", "SimpleValueTargetProvider"), paramCount: 2));
+                               if (context.Scopes.TryGetValue(node, out var scope))
+                                       yield return Create(Ldloc, scope.Item1);
+                               else
+                                       yield return Create(Ldnull);
+
+                               yield return Create(Newobj, module.ImportCtorReference(("Xamarin.Forms.Xaml", "Xamarin.Forms.Xaml.Internals", "SimpleValueTargetProvider"), paramCount: 3));
                                //store the provider so we can register it again with a different key
                                yield return Create(Dup);
                                var refProvider = new VariableDefinition(module.ImportReference(("mscorlib", "System", "Object")));
diff --git a/Xamarin.Forms.Xaml.UnitTests/Issues/Gh3821.xaml b/Xamarin.Forms.Xaml.UnitTests/Issues/Gh3821.xaml
new file mode 100644 (file)
index 0000000..23514f0
--- /dev/null
@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ContentPage xmlns="http://xamarin.com/schemas/2014/forms"
+                        xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
+                        xmlns:local="using:Xamarin.Forms.Xaml.UnitTests"
+                        x:Class="Xamarin.Forms.Xaml.UnitTests.Gh3821"
+                        x:Name="root">
+       <StackLayout>
+               <local:Gh3821View Text="{Binding Text, Source={x:Reference root}}" />
+       </StackLayout>
+</ContentPage>
diff --git a/Xamarin.Forms.Xaml.UnitTests/Issues/Gh3821.xaml.cs b/Xamarin.Forms.Xaml.UnitTests/Issues/Gh3821.xaml.cs
new file mode 100644 (file)
index 0000000..b82e773
--- /dev/null
@@ -0,0 +1,56 @@
+using System;
+using System.Collections.Generic;
+using NUnit.Framework;
+using Xamarin.Forms;
+using Xamarin.Forms.Core.UnitTests;
+
+namespace Xamarin.Forms.Xaml.UnitTests
+{
+       //[XamlCompilation(XamlCompilationOptions.Skip)]
+       public partial class Gh3821 : ContentPage
+       {
+               public Gh3821()
+               {
+                       InitializeComponent();
+               }
+
+               public Gh3821(bool useCompiledXaml)
+               {
+                       //this stub will be replaced at compile time
+               }
+
+               string _text;
+               public string Text {
+                       get => _text;
+                       set {
+                               _text = value;
+                               OnPropertyChanged();
+                       }
+               }
+
+               [TestFixture]
+               class Tests
+               {
+                       [SetUp]
+                       public void Setup()
+                       {
+                               Device.PlatformServices = new MockPlatformServices();
+                       }
+
+                       [TearDown]
+                       public void TearDown()
+                       {
+                               Device.PlatformServices = null;
+                       }
+
+                       [TestCase(true), TestCase(false)]
+                       public void NoConflictsInNamescopes(bool useCompiledXaml)
+                       {
+                               var layout = new Gh3821(useCompiledXaml) { Text = "root" };
+                               var view = ((Gh3821View)((StackLayout)layout.Content).Children[0]);
+                               var label0 = ((Label)((Gh3821View)((StackLayout)layout.Content).Children[0]).Content);
+                               Assert.That(label0.Text, Is.EqualTo("root"));
+                       }
+               }
+       }
+}
\ No newline at end of file
diff --git a/Xamarin.Forms.Xaml.UnitTests/Issues/Gh3821View.xaml b/Xamarin.Forms.Xaml.UnitTests/Issues/Gh3821View.xaml
new file mode 100644 (file)
index 0000000..5aa61a3
--- /dev/null
@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ContentView xmlns="http://xamarin.com/schemas/2014/forms"
+                        xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
+                        x:Class="Xamarin.Forms.Xaml.UnitTests.Gh3821View"
+                        x:Name="root">
+       <Label Text="{Binding Text, Source={x:Reference root}}" />
+</ContentView>
\ No newline at end of file
diff --git a/Xamarin.Forms.Xaml.UnitTests/Issues/Gh3821View.xaml.cs b/Xamarin.Forms.Xaml.UnitTests/Issues/Gh3821View.xaml.cs
new file mode 100644 (file)
index 0000000..fe57a39
--- /dev/null
@@ -0,0 +1,27 @@
+using System;
+using System.Collections.Generic;
+using Xamarin.Forms;
+
+namespace Xamarin.Forms.Xaml.UnitTests
+{
+       public partial class Gh3821View : ContentView
+       {
+               public Gh3821View()
+               {
+                       InitializeComponent();
+               }
+
+               public Gh3821View(bool useCompiledXaml)
+               {
+                       //this stub will be replaced at compile time
+               }
+
+               public static readonly BindableProperty TextProperty =
+                       BindableProperty.Create("Text", typeof(string), typeof(Gh3821View), default(string));
+
+               public string Text {
+                       get { return (string)GetValue(TextProperty); }
+                       set { SetValue(TextProperty, value); }
+               }
+       }
+}
index 0c0ec1b..06c451d 100644 (file)
     <Compile Include="Issues\Gh3862.xaml.cs">
       <DependentUpon>Gh3862.xaml</DependentUpon>
     </Compile>
+    <Compile Include="Issues\Gh3821.xaml.cs">
+      <DependentUpon>Gh3821.xaml</DependentUpon>
+    </Compile>
+    <Compile Include="Issues\Gh3821View.xaml.cs">
+      <DependentUpon>Gh3821View.xaml</DependentUpon>
+    </Compile>
   </ItemGroup>
   <Import Project="$(MSBuildBinPath)\Microsoft.CSharp.targets" />
   <PropertyGroup>
       <Generator>MSBuild:UpdateDesignTimeXaml</Generator>
       <SubType>Designer</SubType>
     </EmbeddedResource>
+    <EmbeddedResource Include="Issues\Gh3821.xaml">
+      <Generator>MSBuild:UpdateDesignTimeXaml</Generator>
+      <SubType>Designer</SubType>
+    </EmbeddedResource>
+    <EmbeddedResource Include="Issues\Gh3821View.xaml">
+      <SubType>Designer</SubType>
+      <Generator>MSBuild:UpdateDesignTimeXaml</Generator>
+    </EmbeddedResource>
   </ItemGroup>
   <ItemGroup>
     <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
index cdd4259..893b414 100644 (file)
@@ -14,36 +14,33 @@ namespace Xamarin.Forms.Xaml
                        if (serviceProvider == null)
                                throw new ArgumentNullException(nameof(serviceProvider));
                        var referenceProvider = serviceProvider.GetService<IReferenceProvider>();
-                       if (referenceProvider != null)
-                               return referenceProvider.FindByName(Name)
-                                          ?? throw new XamlParseException($"Can not find the object referenced by `{Name}`", serviceProvider?.GetService<IXmlLineInfoProvider>()?.XmlLineInfo ?? new XmlLineInfo());
+                       var value = referenceProvider?.FindByName(Name);
+                       if (value != null)
+                               return value;
 
-#pragma warning disable CS0612 // Type or member is obsolete
                        //legacy path. could be hit by code processed by previous versions of XamlC
-                       var valueProvider = serviceProvider.GetService<IProvideValueTarget>() as IProvideParentValues
-                                                                                          ?? throw new ArgumentException("serviceProvider does not provide an IProvideValueTarget");
-                       var namescopeprovider = serviceProvider.GetService<INameScopeProvider>();
-                       if (namescopeprovider != null && namescopeprovider.NameScope != null) {
-                               var value = namescopeprovider.NameScope.FindByName(Name);
-                               if (value != null)
-                                       return value;
-                       }
+#pragma warning disable CS0612 // Type or member is obsolete
+                       value = serviceProvider.GetService<INameScopeProvider>()?.NameScope?.FindByName(Name);
+                       if (value != null)
+                               return value;
+
+#pragma warning restore CS0612 // Type or member is obsolete
 
+                       //fallback
+                       var valueProvider = serviceProvider.GetService<IProvideValueTarget>() as IProvideParentValues
+                                                                  ?? throw new ArgumentException("serviceProvider does not provide an IProvideValueTarget");
                        foreach (var target in valueProvider.ParentObjects) {
-                               var bo = target as BindableObject;
-                               if (bo == null)
+                               if (!(target is BindableObject bo))
                                        continue;
-                               var ns = NameScope.GetNameScope(bo) as INameScope;
-                               if (ns == null)
+                               if (!(NameScope.GetNameScope(bo) is INameScope ns))
                                        continue;
-                               var value = ns.FindByName(Name);
+                               value = ns.FindByName(Name);
                                if (value != null)
                                        return value;
                        }
 
                        var lineInfo = serviceProvider?.GetService<IXmlLineInfoProvider>()?.XmlLineInfo ?? new XmlLineInfo();
                        throw new XamlParseException($"Can not find the object referenced by `{Name}`", lineInfo);
-#pragma warning restore CS0612 // Type or member is obsolete
                }
        }
 }
\ No newline at end of file
index 6c60091..a04793a 100644 (file)
@@ -133,13 +133,19 @@ namespace Xamarin.Forms.Xaml.Internals
        {
                readonly object[] objectAndParents;
                readonly object targetProperty;
+               readonly INameScope scope;
 
                [Obsolete("SimpleValueTargetProvider(object[] objectAndParents) is obsolete as of version 2.3.4. Please use SimpleValueTargetProvider(object[] objectAndParents, object targetProperty) instead.")]
                public SimpleValueTargetProvider(object[] objectAndParents) : this (objectAndParents, null)
                {
                }
 
-               public SimpleValueTargetProvider(object[] objectAndParents, object targetProperty)
+               [Obsolete("SimpleValueTargetProvider(object[] objectAndParents) is obsolete as of version 3.3.0. Please use SimpleValueTargetProvider(object[] objectAndParents, object targetProperty, NameScope scope) instead.")]
+               public SimpleValueTargetProvider(object[] objectAndParents, object targetProperty) : this (objectAndParents, targetProperty, null)
+               {
+               }
+
+               public SimpleValueTargetProvider(object[] objectAndParents, object targetProperty, INameScope scope)
                {
                        if (objectAndParents == null)
                                throw new ArgumentNullException(nameof(objectAndParents));
@@ -148,6 +154,7 @@ namespace Xamarin.Forms.Xaml.Internals
 
                        this.objectAndParents = objectAndParents;
                        this.targetProperty = targetProperty;
+                       this.scope = scope;
                }
 
                IEnumerable<object> IProvideParentValues.ParentObjects
@@ -161,6 +168,9 @@ namespace Xamarin.Forms.Xaml.Internals
 
                public object FindByName(string name)
                {
+                       if (scope != null)
+                               return scope.FindByName(name);
+
                        for (var i = 0; i < objectAndParents.Length; i++) {
                                var bo = objectAndParents[i] as BindableObject;
                                if (bo == null) continue;