2005-07-18 Iain McCoy <iain@mccoy.id.au>

        * Mono.Windows.Serialization/XamlParser.cs: some refactoring and
        improvements in error reporting
        * Test/XamlParser.cs: added a few tests and improved the readability
        of the baked-in XAML documents somewhat. Additionally added some
        comments explaining the approach taken by the tests and test
        scaffolding.


svn path=/trunk/mcs/; revision=47366

3 files changed, 202 insertions, 47 deletions

diff --git a/mcs/class/PresentationFramework/ChangeLog b/mcs/class/PresentationFramework/ChangeLog
index 3061d75b2ec..a30c26d2d2a 100644
--- a/mcs/class/PresentationFramework/ChangeLog
+++ b/mcs/class/PresentationFramework/ChangeLog
@@ -1,3 +1,12 @@
+2005-07-18  Iain McCoy  <iain@mccoy.id.au>
+
+	* Mono.Windows.Serialization/XamlParser.cs: some refactoring and
+	improvements in error reporting
+	* Test/XamlParser.cs: added a few tests and improved the readability
+	of the baked-in	XAML documents somewhat. Additionally added some
+	comments explaining the approach taken by the tests and test
+	scaffolding.
+
 2005-07-17  Iain McCoy  <iain@mccoy.id.au>
 
 	* Test/XamlParser.cs: add a bunch of tests
diff --git a/mcs/class/PresentationFramework/Mono.Windows.Serialization/XamlParser.cs b/mcs/class/PresentationFramework/Mono.Windows.Serialization/XamlParser.cs
index b79a7777bf9..ad726c84519 100644
--- a/mcs/class/PresentationFramework/Mono.Windows.Serialization/XamlParser.cs
+++ b/mcs/class/PresentationFramework/Mono.Windows.Serialization/XamlParser.cs
@@ -79,17 +79,11 @@ namespace Mono.Windows.Serialization {
 		{
 			while (reader.Read()) {
 				Debug.WriteLine("NOW PARSING: " + reader.NodeType + "; " + reader.Name + "; " + reader.Value);
-				if (begun && currentState == null && reader.NodeType != XmlNodeType.Whitespace && reader.NodeType != XmlNodeType.Comment)
+				if (goneTooFar())
 					throw new Exception("Too far: " + reader.NodeType + ", " + reader.Name);
 				if (currentState != null && currentState.type == CurrentType.Code)
 				{
-					if (reader.NodeType == XmlNodeType.EndElement &&
-							reader.LocalName == "Code" && 
-							reader.NamespaceURI == XAML_NAMESPACE) {
-						parseEndElement();
-					} else {
-						currentState.obj = (string)currentState.obj + reader.Value;
-					}
+					processElementInCodeState();
 					continue;
 				}
 				switch (reader.NodeType) {
@@ -115,10 +109,32 @@ namespace Mono.Windows.Serialization {
 				}
 			}
 		}
+		void processElementInCodeState()
+		{
+			if (reader.NodeType == XmlNodeType.EndElement &&
+					reader.LocalName == "Code" && 
+					reader.NamespaceURI == XAML_NAMESPACE) {
+				parseEndElement();
+			} else {
+				currentState.obj = (string)currentState.obj + reader.Value;
+			}
+		}
+		bool goneTooFar()
+		{
+
+			if (begun && 
+					currentState == null && 
+					reader.NodeType != XmlNodeType.Whitespace && 
+					reader.NodeType != XmlNodeType.Comment)
+				return true;
+			else
+				return false;
+		}
+
 		void parsePI()
 		{
 			if (reader.Name != "Mapping")
-				Console.WriteLine("Unknown processing instruction");
+				throw new Exception("Unknown processing instruction");
 			mapper.AddMappingProcessingInstruction(reader.Value);
 		}
 
@@ -135,7 +151,9 @@ namespace Mono.Windows.Serialization {
 			//  - It's a direct child of an IAddChild element
 			//    and does not have a dot in its name
 			//    
-			//  parseObjectElement will verify the second case
+			//  We just check that it doesn't have a dot in it here
+			//  since parseObjectElement will confirm that it is
+			//  a direct child of an IAddChild.
 			//
 			//  If it's a dotted name, then it is a property.
 			//  What it is a property of depends on the bit of the
@@ -169,6 +187,9 @@ namespace Mono.Windows.Serialization {
 			return false;
 		}
 
+		// handle an x:Code element. Most of the handling for this is
+		// at the start of the main parsing loop, in the 
+		// processElementInCodeState() function
 		void parseCodeElement()
 		{
 			push(CurrentType.Code, "");
@@ -176,16 +197,33 @@ namespace Mono.Windows.Serialization {
 
 		void parseText()
 		{
-			if (currentState.type == CurrentType.Object || currentState.type == CurrentType.PropertyObject) {
+			switch (currentState.type) {
+			case CurrentType.Object:
+			case CurrentType.PropertyObject:
+				abortIfNotAddChild("text");
 				writer.CreateElementText(reader.Value);
-			} else if (currentState.type == CurrentType.DependencyProperty) {
+				break;
+			case CurrentType.DependencyProperty:
 				DependencyProperty dp = (DependencyProperty)currentState.obj;
 				writer.CreateDependencyPropertyText(reader.Value, dp.PropertyType);
-			} else {
+				break;
+			case CurrentType.Property:
 				PropertyInfo prop = (PropertyInfo)currentState.obj;
 				writer.CreatePropertyText(reader.Value, prop.PropertyType);
+				break;
+			default:
+				throw new NotImplementedException();
 			}
 		}
+
+		void abortIfNotAddChild(string thing)
+		{
+			if (!isAddChild((Type)currentState.obj))
+				throw new Exception("Cannot add " + thing +
+						" to instance of '" + 
+						((Type)currentState.obj) + 
+						"'.");
+		}
 		
 		void parseNormalPropertyElement(string propertyName)
 		{
@@ -202,9 +240,7 @@ namespace Mono.Windows.Serialization {
 			writer.CreateProperty(prop);
 
 			if (reader.HasAttributes) {
-				Console.WriteLine("Property node should not have attributes");
-				return;
-				// TODO: exception
+				throw new Exception("Property node should not have attributes.");
 			}
 		}
 
@@ -220,7 +256,10 @@ namespace Mono.Windows.Serialization {
 
 			writer.CreateDependencyProperty(typeAttachedTo, propertyName, dp.PropertyType);
 		}
-
+		bool isAddChild(Type t)
+		{
+			return (t.GetInterface("System.Windows.Serialization.IAddChild") != null);
+		}
 		void parseObjectElement()
 		{
 			Type parent;
@@ -229,27 +268,46 @@ namespace Mono.Windows.Serialization {
 			parent = mapper.GetType(reader.NamespaceURI, reader.Name);
 			if (parent == null)
 				throw new Exception("Class '" + reader.Name + "' not found.");
-			if (parent.GetInterface("System.Windows.Serialization.IAddChild") == null)
-				{} //TODO: throw exception
+		
+			// whichever of these functions runs will push something
 			if (currentState == null) {
-				if (reader.GetAttribute("Name", XAML_NAMESPACE) != null)
-					throw new Exception("The XAML Name attribute can not be applied to top level elements\n"+
-							"Do you mean the Class attribute?");
-				begun = true;
-				createTopLevel(parent.AssemblyQualifiedName, reader.GetAttribute("Class", XAML_NAMESPACE));
+				parseTopLevelObjectElement(parent);
 			} else {
-				string name = reader.GetAttribute("Name", XAML_NAMESPACE);
-				if (name == null)
-					name = reader.GetAttribute("Name", reader.NamespaceURI);
-
-				if (currentState.type == CurrentType.Object)
-					addChild(parent, name);
-				else if (currentState.type == CurrentType.Property)
-					addPropertyChild(parent, name);
-				else
-					throw new NotImplementedException();
+				parseChildObjectElement(parent);
 			}
 			
+			processObjectAttributes();
+
+			if (isEmpty) {
+				closeEmptyObjectElement();
+			}
+		}
+		void parseTopLevelObjectElement(Type parent)
+		{
+			if (reader.GetAttribute("Name", XAML_NAMESPACE) != null)
+				throw new Exception("The XAML Name attribute can not be applied to top level elements\n"+
+						"Do you mean the Class attribute?");
+			begun = true;
+			createTopLevel(parent.AssemblyQualifiedName, reader.GetAttribute("Class", XAML_NAMESPACE));
+		}
+
+		void parseChildObjectElement(Type parent)
+		{
+			string name = reader.GetAttribute("Name", XAML_NAMESPACE);
+			if (name == null)
+				name = reader.GetAttribute("Name", reader.NamespaceURI);
+
+			if (currentState.type == CurrentType.Object) {
+				abortIfNotAddChild("object");
+				addChild(parent, name);
+			} else if (currentState.type == CurrentType.Property) {
+				addPropertyChild(parent, name);
+			} else {
+				throw new NotImplementedException();
+			}
+		}
+		void processObjectAttributes()
+		{
 			if (reader.MoveToFirstAttribute()) {
 				do {
 					if (reader.Name.StartsWith("xmlns"))
@@ -262,17 +320,17 @@ namespace Mono.Windows.Serialization {
 						parseDependencyPropertyAttribute();
 				} while (reader.MoveToNextAttribute());
 			}
-			
+		}
 
-			if (isEmpty) {
-				if (currentState.type == CurrentType.Object) {
-					writer.EndObject();
-				} else if (currentState.type == CurrentType.PropertyObject) {
-					ParserState state = (ParserState)oldStates[oldStates.Count - 1];
-					writer.EndPropertyObject(((PropertyInfo)state.obj).PropertyType);
-				}
-				pop();
+		void closeEmptyObjectElement()
+		{
+			if (currentState.type == CurrentType.Object) {
+				writer.EndObject();
+			} else if (currentState.type == CurrentType.PropertyObject) {
+				ParserState state = (ParserState)oldStates[oldStates.Count - 1];
+				writer.EndPropertyObject(((PropertyInfo)state.obj).PropertyType);
 			}
+			pop();
 		}
 
 		void createTopLevel(string parentName, string className)
diff --git a/mcs/class/PresentationFramework/Test/XamlParser.cs b/mcs/class/PresentationFramework/Test/XamlParser.cs
index 7c2d44e0ed5..161beef041f 100644
--- a/mcs/class/PresentationFramework/Test/XamlParser.cs
+++ b/mcs/class/PresentationFramework/Test/XamlParser.cs
@@ -4,6 +4,24 @@
 //
 // (C) iain@mccoy.id.au
 // 
+//
+// As you may be aware, testing a parser is something of a beastly job. The
+// approach taken by these tests is to feed a file to XamlParser and see if it
+// tells the code generator to do what you'd expect. This tests both the parsing
+// and type-checking bits of XamlParser.
+//
+// The various Happening classes each represent methods on the XamlWriter
+// interface that XamlParser could call. The constructor of a Happening takes 
+// the same arguments as the XamlWriter method it represents, and merely stashes
+// those values in the suitable public fields.
+//
+// The ParserTester class takes a Xaml document and a list of Happenings, and 
+// handles comparison of the calls to ParserTester's methods to the expected 
+// sequence of Happenings.
+//
+// I think this strikes a tolerable balance between the need to keep the tests 
+// simple and the need to avoid writing 10 zillion lines of hard-to-follow test
+// code.
 
 using NUnit.Framework;
 using System;
@@ -193,7 +211,30 @@ public class XamlParserTest : Assertion {
 	public void TestTextPropertyAsElement()
 	{
 		string s = "<ConsoleApp xmlns=\"console\" xmlns:x=\"http://schemas.microsoft.com/winfx/xaml/2005\">\n"+
-			"<ConsoleWriter><ConsoleWriter.Text>Hello</ConsoleWriter.Text></ConsoleWriter>\n" +
+			"<ConsoleWriter>\n" + 
+			"<ConsoleWriter.Text>Hello</ConsoleWriter.Text>\n" +
+			"</ConsoleWriter>\n" +
+			"</ConsoleApp>";
+		ParserTester pt = new ParserTester(MAPPING + s, 
+				new CreateTopLevelHappening(typeof(ConsoleApp), null),
+				new CreateObjectHappening(typeof(ConsoleWriter), null),
+				new CreatePropertyHappening(typeof(ConsoleWriter).GetProperty("Text")),
+				new CreatePropertyTextHappening("Hello", typeof(ConsoleValue)),
+				new EndPropertyHappening(),
+				new EndObjectHappening(), //ConsoleWriter
+				new EndObjectHappening(), //ConsoleApp
+				new FinishHappening());
+		pt.Test();
+	}
+
+	[Test]
+	[ExpectedException(typeof(Exception), "Property node should not have attributes.")]
+	public void TestTextPropertyAsElementWithAttribute()
+	{
+		string s = "<ConsoleApp xmlns=\"console\" xmlns:x=\"http://schemas.microsoft.com/winfx/xaml/2005\">\n"+
+			"<ConsoleWriter>\n" + 
+			"<ConsoleWriter.Text z=\"y\">Hello</ConsoleWriter.Text>\n"+
+			"</ConsoleWriter>\n" +
 			"</ConsoleApp>";
 		ParserTester pt = new ParserTester(MAPPING + s, 
 				new CreateTopLevelHappening(typeof(ConsoleApp), null),
@@ -212,7 +253,9 @@ public class XamlParserTest : Assertion {
 	public void TestTextPropertyAsElementWithIncorrectName()
 	{
 		string s = "<ConsoleApp xmlns=\"console\" xmlns:x=\"http://schemas.microsoft.com/winfx/xaml/2005\">\n"+
-			"<ConsoleWriter><ConsoleWriter.Texxxt>Hello</ConsoleWriter.Text></ConsoleWriter>\n" +
+			"<ConsoleWriter>\n" + 
+			"<ConsoleWriter.Texxxt>Hello</ConsoleWriter.Text>\n" + 
+			"</ConsoleWriter>\n" +
 			"</ConsoleApp>";
 		ParserTester pt = new ParserTester(MAPPING + s, 
 				new CreateTopLevelHappening(typeof(ConsoleApp), null),
@@ -268,7 +311,9 @@ public class XamlParserTest : Assertion {
 	public void TestDependencyPropertyAsChildElement()
 	{
 		string s = "<ConsoleApp xmlns=\"console\" xmlns:x=\"http://schemas.microsoft.com/winfx/xaml/2005\">\n"+
-			"<ConsoleWriter><ConsoleApp.Repetitions>3</ConsoleApp.Repetitions></ConsoleWriter>" +
+			"<ConsoleWriter>\n" + 
+			"<ConsoleApp.Repetitions>3</ConsoleApp.Repetitions>\n" +
+			"</ConsoleWriter>" +
 			"</ConsoleApp>";
 		ParserTester pt = new ParserTester(MAPPING + s,
 				new CreateTopLevelHappening(typeof(ConsoleApp), null),
@@ -287,7 +332,9 @@ public class XamlParserTest : Assertion {
 	public void TestDependencyPropertyAsChildElementWithIncorrectName()
 	{
 		string s = "<ConsoleApp xmlns=\"console\" xmlns:x=\"http://schemas.microsoft.com/winfx/xaml/2005\">\n"+
-			"<ConsoleWriter><ConsoleApp.Reps>3</ConsoleApp.Reps></ConsoleWriter>" +
+			"<ConsoleWriter>\n"+
+			"<ConsoleApp.Reps>3</ConsoleApp.Reps>\n" + 
+			"</ConsoleWriter>" +
 			"</ConsoleApp>";
 		ParserTester pt = new ParserTester(MAPPING + s,
 				new CreateTopLevelHappening(typeof(ConsoleApp), null),
@@ -322,6 +369,47 @@ public class XamlParserTest : Assertion {
 				new FinishHappening());
 		pt.Test();
 	}
+
+	[Test]
+	[ExpectedException(typeof(Exception), "Cannot add object to instance of 'Xaml.TestVocab.Console.ConsoleValueString'.")]
+	public void TestRestrictionOfAddingObjectsToIAddChilds()
+	{
+		string s = "<ConsoleApp xmlns=\"console\" xmlns:x=\"http://schemas.microsoft.com/winfx/xaml/2005\">\n"+
+			"<ConsoleValueString>\n" +
+			"<ConsoleWriter />" +
+			"</ConsoleValueString>\n" +
+			"</ConsoleApp>";
+		ParserTester pt = new ParserTester(MAPPING + s,
+				new CreateTopLevelHappening(typeof(ConsoleApp), null),
+				new CreateObjectHappening(typeof(ConsoleValueString), null),
+				new CreateObjectHappening(typeof(ConsoleWriter), null),
+				new EndObjectHappening(),
+				new EndObjectHappening(),
+				new EndObjectHappening(),
+				new FinishHappening());
+		pt.Test();
+	}
+
+	[Test]
+	[ExpectedException(typeof(Exception), "Cannot add text to instance of 'Xaml.TestVocab.Console.ConsoleValueString'.")]
+	public void TestRestrictionOfAddingTextToIAddChilds()
+	{
+		string s = "<ConsoleApp xmlns=\"console\" xmlns:x=\"http://schemas.microsoft.com/winfx/xaml/2005\">\n"+
+			"<ConsoleValueString>\n" +
+			"xyz" +
+			"</ConsoleValueString>\n" +
+			"</ConsoleApp>";
+		ParserTester pt = new ParserTester(MAPPING + s,
+				new CreateTopLevelHappening(typeof(ConsoleApp), null),
+				new CreateObjectHappening(typeof(ConsoleValueString), null),
+				new CreateElementTextHappening("xyz"),
+				new EndObjectHappening(),
+				new EndObjectHappening(),
+				new EndObjectHappening(),
+				new FinishHappening());
+		pt.Test();
+	}
+
 }