summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStephane Delcroix <stephane@delcroix.org>2016-12-30 16:58:48 (GMT)
committerE.Z. Hart <hartez@users.noreply.github.com>2016-12-30 16:58:48 (GMT)
commit32dab1d3c7641688e9435be22297e092bcdb5ee6 (patch)
tree46679ada9e2610ea05aa68810a6a85a375e1955b
parentefc1e93f8156df8e84605b118a2f455b8dcf36ab (diff)
downloadxamarin-forms-32dab1d3c7641688e9435be22297e092bcdb5ee6.zip
xamarin-forms-32dab1d3c7641688e9435be22297e092bcdb5ee6.tar.gz
xamarin-forms-32dab1d3c7641688e9435be22297e092bcdb5ee6.tar.bz2
[XamlC] detect duplicate x:Name at compile time (#655)
* [XamlC] detect duplicate x:Name at compile time * invoking methods with the right arguments produces better results
-rw-r--r--Xamarin.Forms.Build.Tasks/ILContext.cs7
-rw-r--r--Xamarin.Forms.Build.Tasks/NodeILExtensions.cs2
-rw-r--r--Xamarin.Forms.Build.Tasks/SetNamescopesAndRegisterNamesVisitor.cs48
-rw-r--r--Xamarin.Forms.Core/Element.cs1
-rw-r--r--Xamarin.Forms.Core/Internals/INameScope.cs3
-rw-r--r--Xamarin.Forms.Core/Internals/NameScope.cs1
-rw-r--r--Xamarin.Forms.Xaml.UnitTests/Issues/Issue2125.xaml7
-rw-r--r--Xamarin.Forms.Xaml.UnitTests/Issues/Issue2125.xaml.cs33
-rw-r--r--Xamarin.Forms.Xaml.UnitTests/Issues/Issue2450.xaml.cs9
-rw-r--r--Xamarin.Forms.Xaml.UnitTests/Xamarin.Forms.Xaml.UnitTests.csproj6
-rw-r--r--Xamarin.Forms.Xaml/RegisterXNamesVisitor.cs3
11 files changed, 46 insertions, 74 deletions
diff --git a/Xamarin.Forms.Build.Tasks/ILContext.cs b/Xamarin.Forms.Build.Tasks/ILContext.cs
index 8f36227..bcd483b 100644
--- a/Xamarin.Forms.Build.Tasks/ILContext.cs
+++ b/Xamarin.Forms.Build.Tasks/ILContext.cs
@@ -1,6 +1,9 @@
+using System;
using System.Collections.Generic;
+
using Mono.Cecil;
using Mono.Cecil.Cil;
+
using Xamarin.Forms.Xaml;
namespace Xamarin.Forms.Build.Tasks
@@ -13,7 +16,7 @@ namespace Xamarin.Forms.Build.Tasks
Body = body;
Values = new Dictionary<IValueNode, object>();
Variables = new Dictionary<IElementNode, VariableDefinition>();
- Scopes = new Dictionary<INode, VariableDefinition>();
+ Scopes = new Dictionary<INode, Tuple<VariableDefinition, IList<string>>>();
TypeExtensions = new Dictionary<INode, TypeReference>();
ParentContextValues = parentContextValues;
Module = module;
@@ -23,7 +26,7 @@ namespace Xamarin.Forms.Build.Tasks
public Dictionary<IElementNode, VariableDefinition> Variables { get; private set; }
- public Dictionary<INode, VariableDefinition> Scopes { get; private set; }
+ public Dictionary<INode, Tuple<VariableDefinition, IList<string>>> Scopes { get; private set; }
public Dictionary<INode, TypeReference> TypeExtensions { get; }
diff --git a/Xamarin.Forms.Build.Tasks/NodeILExtensions.cs b/Xamarin.Forms.Build.Tasks/NodeILExtensions.cs
index ec06879..cfd22f3 100644
--- a/Xamarin.Forms.Build.Tasks/NodeILExtensions.cs
+++ b/Xamarin.Forms.Build.Tasks/NodeILExtensions.cs
@@ -488,7 +488,7 @@ namespace Xamarin.Forms.Build.Tasks
yield return Instruction.Create(OpCodes.Dup); //Duplicate the namescopeProvider
var setNamescope = module.Import(typeof (NameScopeProvider).GetProperty("NameScope").GetSetMethod());
- yield return Instruction.Create(OpCodes.Ldloc, context.Scopes[node]);
+ yield return Instruction.Create(OpCodes.Ldloc, context.Scopes[node].Item1);
yield return Instruction.Create(OpCodes.Callvirt, setNamescope);
yield return Instruction.Create(OpCodes.Callvirt, addService);
}
diff --git a/Xamarin.Forms.Build.Tasks/SetNamescopesAndRegisterNamesVisitor.cs b/Xamarin.Forms.Build.Tasks/SetNamescopesAndRegisterNamesVisitor.cs
index 8e03b92..860ff44 100644
--- a/Xamarin.Forms.Build.Tasks/SetNamescopesAndRegisterNamesVisitor.cs
+++ b/Xamarin.Forms.Build.Tasks/SetNamescopesAndRegisterNamesVisitor.cs
@@ -1,4 +1,6 @@
+using System.Collections.Generic;
using System.Linq;
+using System.Xml;
using Mono.Cecil.Cil;
using Xamarin.Forms.Internals;
using Xamarin.Forms.Xaml;
@@ -33,7 +35,7 @@ namespace Xamarin.Forms.Build.Tasks
{
Context.Scopes[node] = Context.Scopes[parentNode];
if (IsXNameProperty(node, parentNode))
- RegisterName((string)node.Value, Context.Scopes[node], Context.Variables[(IElementNode)parentNode], node);
+ RegisterName((string)node.Value, Context.Scopes[node].Item1, Context.Scopes[node].Item2, Context.Variables[(IElementNode)parentNode], node);
}
public void Visit(MarkupNode node, INode parentNode)
@@ -43,26 +45,27 @@ namespace Xamarin.Forms.Build.Tasks
public void Visit(ElementNode node, INode parentNode)
{
- VariableDefinition ns;
- if (parentNode == null || IsDataTemplate(node, parentNode) || IsStyle(node, parentNode))
- ns = CreateNamescope();
- else
- ns = Context.Scopes[parentNode];
- if (
- Context.Variables[node].VariableType.InheritsFromOrImplements(
- Context.Body.Method.Module.Import(typeof (BindableObject))))
- SetNameScope(node, ns);
- Context.Scopes[node] = ns;
+ VariableDefinition namescopeVarDef;
+ IList<string> namesInNamescope;
+ if (parentNode == null || IsDataTemplate(node, parentNode) || IsStyle(node, parentNode)) {
+ namescopeVarDef = CreateNamescope();
+ namesInNamescope = new List<string>();
+ } else {
+ namescopeVarDef = Context.Scopes[parentNode].Item1;
+ namesInNamescope = Context.Scopes[parentNode].Item2;
+ }
+ if (Context.Variables[node].VariableType.InheritsFromOrImplements(Context.Body.Method.Module.Import(typeof (BindableObject))))
+ SetNameScope(node, namescopeVarDef);
+ Context.Scopes[node] = new System.Tuple<VariableDefinition, IList<string>>(namescopeVarDef, namesInNamescope);
}
public void Visit(RootNode node, INode parentNode)
{
- var ns = CreateNamescope();
- if (
- Context.Variables[node].VariableType.InheritsFromOrImplements(
- Context.Body.Method.Module.Import(typeof (BindableObject))))
- SetNameScope(node, ns);
- Context.Scopes[node] = ns;
+ var namescopeVarDef = CreateNamescope();
+ IList<string> namesInNamescope = new List<string>();
+ if (Context.Variables[node].VariableType.InheritsFromOrImplements(Context.Body.Method.Module.Import(typeof (BindableObject))))
+ SetNameScope(node, namescopeVarDef);
+ Context.Scopes[node] = new System.Tuple<VariableDefinition, IList<string>>(namescopeVarDef, namesInNamescope);
}
public void Visit(ListNode node, INode parentNode)
@@ -121,18 +124,21 @@ namespace Xamarin.Forms.Build.Tasks
Context.IL.Emit(OpCodes.Call, setNS);
}
- void RegisterName(string str, VariableDefinition scope, VariableDefinition element, INode node)
+ void RegisterName(string str, VariableDefinition namescopeVarDef, IList<string> namesInNamescope, VariableDefinition element, INode node)
{
+ if (namesInNamescope.Contains(str))
+ throw new XamlParseException($"An element with the name \"{str}\" already exists in this NameScope", node as IXmlLineInfo);
+ namesInNamescope.Add(str);
+
var module = Context.Body.Method.Module;
var nsRef = module.Import(typeof (INameScope));
var nsDef = nsRef.Resolve();
- var registerInfo = nsDef.Methods.First(md => md.Name == "RegisterName" && md.Parameters.Count == 3);
+ var registerInfo = nsDef.Methods.First(md => md.Name == "RegisterName" && md.Parameters.Count == 2);
var register = module.Import(registerInfo);
- Context.IL.Emit(OpCodes.Ldloc, scope);
+ Context.IL.Emit(OpCodes.Ldloc, namescopeVarDef);
Context.IL.Emit(OpCodes.Ldstr, str);
Context.IL.Emit(OpCodes.Ldloc, element);
- Context.IL.Append(node.PushXmlLineInfo(Context));
Context.IL.Emit(OpCodes.Callvirt, register);
}
}
diff --git a/Xamarin.Forms.Core/Element.cs b/Xamarin.Forms.Core/Element.cs
index 19d7d85..d0f713e 100644
--- a/Xamarin.Forms.Core/Element.cs
+++ b/Xamarin.Forms.Core/Element.cs
@@ -280,6 +280,7 @@ namespace Xamarin.Forms
namescope.RegisterName(name, scopedElement);
}
+ [Obsolete]
void INameScope.RegisterName(string name, object scopedElement, IXmlLineInfo xmlLineInfo)
{
INameScope namescope = GetNameScope();
diff --git a/Xamarin.Forms.Core/Internals/INameScope.cs b/Xamarin.Forms.Core/Internals/INameScope.cs
index ca52060..6485527 100644
--- a/Xamarin.Forms.Core/Internals/INameScope.cs
+++ b/Xamarin.Forms.Core/Internals/INameScope.cs
@@ -1,3 +1,4 @@
+using System;
using System.Xml;
namespace Xamarin.Forms.Internals
@@ -6,7 +7,7 @@ namespace Xamarin.Forms.Internals
{
object FindByName(string name);
void RegisterName(string name, object scopedElement);
- void RegisterName(string name, object scopedElement, IXmlLineInfo xmlLineInfo);
void UnregisterName(string name);
+ [Obsolete]void RegisterName(string name, object scopedElement, IXmlLineInfo xmlLineInfo);
}
} \ No newline at end of file
diff --git a/Xamarin.Forms.Core/Internals/NameScope.cs b/Xamarin.Forms.Core/Internals/NameScope.cs
index b29534b..7bb5ff5 100644
--- a/Xamarin.Forms.Core/Internals/NameScope.cs
+++ b/Xamarin.Forms.Core/Internals/NameScope.cs
@@ -26,6 +26,7 @@ namespace Xamarin.Forms.Internals
_names[name] = scopedElement;
}
+ [Obsolete]
void INameScope.RegisterName(string name, object scopedElement, IXmlLineInfo xmlLineInfo)
{
try
diff --git a/Xamarin.Forms.Xaml.UnitTests/Issues/Issue2125.xaml b/Xamarin.Forms.Xaml.UnitTests/Issues/Issue2125.xaml
deleted file mode 100644
index c2bc239..0000000
--- a/Xamarin.Forms.Xaml.UnitTests/Issues/Issue2125.xaml
+++ /dev/null
@@ -1,7 +0,0 @@
-´╗┐<?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.Issue2125">
- <StackLayout>
- <Label x:Name="label"/>
- <Label x:Name="label"/>
- </StackLayout>
-</ContentPage> \ No newline at end of file
diff --git a/Xamarin.Forms.Xaml.UnitTests/Issues/Issue2125.xaml.cs b/Xamarin.Forms.Xaml.UnitTests/Issues/Issue2125.xaml.cs
deleted file mode 100644
index 8c4dcf8..0000000
--- a/Xamarin.Forms.Xaml.UnitTests/Issues/Issue2125.xaml.cs
+++ /dev/null
@@ -1,33 +0,0 @@
-´╗┐using System;
-using System.Collections.Generic;
-
-using Xamarin.Forms;
-
-using NUnit.Framework;
-
-namespace Xamarin.Forms.Xaml.UnitTests
-{
- public partial class Issue2125 : ContentPage
- {
- public Issue2125 ()
- {
- InitializeComponent ();
- }
-
- public Issue2125 (bool useCompiledXaml)
- {
- //this stub will be replaced at compile time
- }
-
- [TestFixture]
- public class Tests
- {
- [TestCase (false)]
- [TestCase (true)]
- public void DuplicatexName (bool useCompiledXaml)
- {
- Assert.Throws (new XamlParseExceptionConstraint (5, 10), () => new Issue2125 (useCompiledXaml));
- }
- }
- }
-} \ No newline at end of file
diff --git a/Xamarin.Forms.Xaml.UnitTests/Issues/Issue2450.xaml.cs b/Xamarin.Forms.Xaml.UnitTests/Issues/Issue2450.xaml.cs
index 0cf6f56..2d33093 100644
--- a/Xamarin.Forms.Xaml.UnitTests/Issues/Issue2450.xaml.cs
+++ b/Xamarin.Forms.Xaml.UnitTests/Issues/Issue2450.xaml.cs
@@ -5,6 +5,8 @@ using Xamarin.Forms.Core.UnitTests;
namespace Xamarin.Forms.Xaml.UnitTests
{
+ //this covers Issue2125 as well
+ [XamlCompilation(XamlCompilationOptions.Skip)]
public partial class Issue2450 : ContentPage
{
public Issue2450 ()
@@ -30,7 +32,12 @@ namespace Xamarin.Forms.Xaml.UnitTests
[TestCase (true)]
public void ThrowMeaningfulExceptionOnDuplicateXName (bool useCompiledXaml)
{
- Assert.Throws (new XamlParseExceptionConstraint (8, 10, m => m == "An element with the name \"label0\" already exists in this NameScope"), () => new Issue2450 (useCompiledXaml));
+ if (useCompiledXaml)
+ Assert.Throws(new XamlParseExceptionConstraint(8, 10, m => m == "An element with the name \"label0\" already exists in this NameScope"),
+ () => MockCompiler.Compile(typeof(Issue2450)));
+ else
+ Assert.Throws(new XamlParseExceptionConstraint(8, 10, m => m == "An element with the name \"label0\" already exists in this NameScope"),
+ () => new Issue2450(useCompiledXaml));
}
}
}
diff --git a/Xamarin.Forms.Xaml.UnitTests/Xamarin.Forms.Xaml.UnitTests.csproj b/Xamarin.Forms.Xaml.UnitTests/Xamarin.Forms.Xaml.UnitTests.csproj
index 8864a38..b66a082 100644
--- a/Xamarin.Forms.Xaml.UnitTests/Xamarin.Forms.Xaml.UnitTests.csproj
+++ b/Xamarin.Forms.Xaml.UnitTests/Xamarin.Forms.Xaml.UnitTests.csproj
@@ -131,9 +131,6 @@
<Compile Include="Issues\Issue2114.xaml.cs">
<DependentUpon>Issue2114.xaml</DependentUpon>
</Compile>
- <Compile Include="Issues\Issue2125.xaml.cs">
- <DependentUpon>Issue2125.xaml</DependentUpon>
- </Compile>
<Compile Include="Validation\StaticExtensionException.xaml.cs">
<DependentUpon>StaticExtensionException.xaml</DependentUpon>
</Compile>
@@ -469,9 +466,6 @@
<EmbeddedResource Include="Issues\Issue2114.xaml">
<Generator>MSBuild:UpdateDesignTimeXaml</Generator>
</EmbeddedResource>
- <EmbeddedResource Include="Issues\Issue2125.xaml">
- <Generator>MSBuild:UpdateDesignTimeXaml</Generator>
- </EmbeddedResource>
<EmbeddedResource Include="Validation\StaticExtensionException.xaml">
<Generator>MSBuild:UpdateDesignTimeXaml</Generator>
</EmbeddedResource>
diff --git a/Xamarin.Forms.Xaml/RegisterXNamesVisitor.cs b/Xamarin.Forms.Xaml/RegisterXNamesVisitor.cs
index 7985cff..a7da3b9 100644
--- a/Xamarin.Forms.Xaml/RegisterXNamesVisitor.cs
+++ b/Xamarin.Forms.Xaml/RegisterXNamesVisitor.cs
@@ -39,8 +39,7 @@ namespace Xamarin.Forms.Xaml
{
if (ae.ParamName != "name")
throw ae;
- throw new XamlParseException(
- string.Format("An element with the name \"{0}\" already exists in this NameScope", (string)node.Value), node);
+ throw new XamlParseException($"An element with the name \"{(string)node.Value}\" already exists in this NameScope", node);
}
}