[generator] Replace XmlDocument with XLinq and report error location (#151)

XmlDocument never supported source locations, therefore it was impossible
to report exact error location in metadata fixup (which was quite annoying).

What were alternatives?

- XPathDocument: it is no-go because it is not editable. ApiFixup step
  requires modifications on the API description XML tree.
- XPathNavigator on XLinq: it seemed doable, because XLinq is for modifying
  XML tree, and it can create XPathNavigator from XNode. What actually
  happens, however, was that their XPathNavigator does not support
  CreateAttributes(), which was required for <attr /> operation.
  Another attempt to work around by set_OuterXml() failed because it also
  does not support ReplaceRange() which was internally used by OuterXml.

Therefore, I ended up to entirely replace XmlDocument with XLinq.
It was not limited to ApiFixup but also *everywhere* in generator,
because the API descriptions are also the XPath query targets.

Author: atsushieno

src/utils/EnumMappings.Xml.cs

@@ -2,7 +2,8 @@
 using System.IO;
 using System.Linq;
 using System.Xml;
-
+using System.Xml.Linq;
+using System.Xml.XPath;
 using Xamarin.Android.Tools;
 
 namespace MonoDroid.Generation {
@@ -23,19 +24,18 @@ internal static TextReader FieldXmlToCsv (string file)
 				return null;
 
 			var sw = new StringWriter ();
-			var doc = new XmlDocument ();
-			doc.Load (file);
+			var doc = XDocument.Load (file, LoadOptions.SetBaseUri | LoadOptions.SetLineInfo);
 
-			foreach (XmlElement e in doc.SelectNodes ("/enum-field-mappings/mapping")) {
+			foreach (var e in doc.XPathSelectElements ("/enum-field-mappings/mapping")) {
 				string enu = GetMandatoryAttribute (e, "clr-enum-type");
-				string jni_type = e.HasAttribute ("jni-class")
+				string jni_type = e.Attribute ("jni-class") != null
 					? e.XGetAttribute ("jni-class")
-					: e.HasAttribute ("jni-interface")
+					: e.Attribute ("jni-interface") != null
 						? "I:" + e.XGetAttribute ("jni-interface")
 						: GetMandatoryAttribute (e, "jni-class or jni-interface");
-				bool bitfield = e.HasAttribute ("bitfield") && e.XGetAttribute ("bitfield") == "true";
-				foreach (XmlElement m in e.SelectNodes ("field")) {
-					string verstr = m.HasAttribute ("api-level")
+				bool bitfield = e.Attribute ("bitfield") != null && e.XGetAttribute ("bitfield") == "true";
+				foreach (var m in e.XPathSelectElements ("field")) {
+					string verstr = m.Attribute ("api-level") != null
 						? m.XGetAttribute ("api-level")
 						: "0";
 					string member   = GetMandatoryAttribute (m, "clr-name");
@@ -48,10 +48,10 @@ internal static TextReader FieldXmlToCsv (string file)
 			return new StringReader (sw.ToString ());
 		}
 
-		static string GetMandatoryAttribute (XmlElement e, string name)
+		static string GetMandatoryAttribute (XElement e, string name)
 		{
-			if (!e.HasAttribute (name)) {
-				throw new InvalidOperationException (String.Format ("Mandatory attribute '{0}' is missing on a mapping element: {1}", name, e.OuterXml));
+			if (e.Attribute (name) == null) {
+				throw new InvalidOperationException (String.Format ("Mandatory attribute '{0}' is missing on a mapping element: {1}", name, e.ToString ()));
 			}
 			return e.XGetAttribute (name);
 		}

src/utils/XmlExtensions.cs

@@ -2,16 +2,17 @@
 using System.IO;
 using System.Linq;
 using System.Xml;
+using System.Xml.Linq;
 using System.Xml.XPath;
 
 namespace Xamarin.Android.Tools {
 
 	static class XmlExtensions {
 
-		public static string XGetAttribute (this XmlElement element, string name)
+		public static string XGetAttribute (this XElement element, string name)
 		{
-			var attr = element.GetAttribute (name);
-			return attr != null ? attr.Trim () : null;
+			var attr = element.Attribute (name);
+			return attr != null ? attr.Value.Trim () : null;
 		}
 
 		public static string XGetAttribute (this XPathNavigator nav, string name, string ns)

tools/generator/ApiFixup.cs

@@ -3,213 +3,192 @@
 using System.Linq;
 using System.Xml;
 using System.Xml.XPath;
+using System.Xml.Linq;
 
 using Xamarin.Android.Tools;
 
 namespace MonoDroid.Generation
 {
 	public class ApiFixup
 	{
-		XmlDocument api_doc;
+		XDocument api_doc;
 		string apiSource = "";
 
 		public string ApiSource { get { return apiSource; } }
 
-		public ApiFixup (XmlDocument apiDoc)
+		public ApiFixup (XDocument apiDoc)
 		{
 			api_doc = apiDoc;
-			var api = api_doc.DocumentElement;
+			var api = api_doc.Root;
 			if (api != null)
 				apiSource = api.XGetAttribute ("api-source");
 		}
 
-		public void Process (IEnumerable<XmlDocument> metaDocs, string apiLevel, int productVersion)
+		public void Process (IEnumerable<XDocument> metaDocs, string apiLevel, int productVersion)
 		{
 			foreach (var metaDoc in metaDocs)
 				Process (metaDoc, apiLevel, productVersion);
 		}
 
-		bool ShouldSkip (XPathNavigator node, int apiLevel, int productVersion)
+		bool ShouldSkip (XElement node, int apiLevel, int productVersion)
 		{
 			if (apiLevel > 0) {
-				string apiSince = node.XGetAttribute ("api-since", "");
-				string apiUntil = node.XGetAttribute ("api-until", "");
+				string apiSince = node.XGetAttribute ("api-since");
+				string apiUntil = node.XGetAttribute ("api-until");
 				if (!string.IsNullOrEmpty (apiSince) && int.Parse (apiSince) > apiLevel)
 					return true;
 				if (!string.IsNullOrEmpty (apiUntil) && int.Parse (apiUntil) < apiLevel)
 					return true;
 			}
 			if (productVersion > 0) {
-				var product_version = node.XGetAttribute ("product-version", "");
+				var product_version = node.XGetAttribute ("product-version");
 				if (!string.IsNullOrEmpty (product_version) && int.Parse (product_version) > productVersion)
 					return true;
 			}
 			return false;
 		}
 
-		bool ShouldApply (XPathNavigator node) 
+		bool ShouldApply (XElement node) 
 		{
 			if (!string.IsNullOrEmpty (apiSource)) {
-				var targetsource = node.XGetAttribute ("api-source", "");
+				var targetsource = node.XGetAttribute ("api-source");
 				if (string.IsNullOrEmpty (targetsource))
 						return true;
 				return targetsource == apiSource;
 			}
 			return true;
 		}
 
-		void Process (XmlDocument meta_doc, string apiLevelString, int productVersion)
+		void Process (XDocument meta_doc, string apiLevelString, int productVersion)
 		{
-			XPathNavigator api_nav = api_doc.CreateNavigator ();
-			XPathNavigator meta_nav = meta_doc.CreateNavigator ();
 			int apiLevel = 0;
 			int.TryParse (apiLevelString, out apiLevel);
 
-			XPathNodeIterator metadata = meta_nav.Select ("/metadata/*");
+			var metadataChildren = meta_doc.XPathSelectElements ("/metadata/*");
 			string prev_path = null;
-			XPathNavigator attr_last_cache = null;
+			XElement attr_last_cache = null;
 
-			while (metadata.MoveNext ()) {
-				var metanav = metadata.Current;
-				if (ShouldSkip (metanav, apiLevel, productVersion))
+			foreach (var metaitem in metadataChildren) {
+				if (ShouldSkip (metaitem, apiLevel, productVersion))
 					continue;
-				if (!ShouldApply (metanav))
+				if (!ShouldApply (metaitem))
 					continue;
-				string path = metanav.XGetAttribute ("path", "");
+				string path = metaitem.XGetAttribute ("path");
 				if (path != prev_path)
 					attr_last_cache = null;
 				prev_path = path;
 
-				switch (metanav.LocalName) {
+				switch (metaitem.Name.LocalName) {
 				case "remove-node":
 					try {
-						XPathNodeIterator api_iter = api_nav.Select (path);
-						List<XmlElement> matches = new List<XmlElement> ();
-						while (api_iter.MoveNext ())
-							matches.Add (((IHasXmlNode)api_iter.Current).GetNode () as XmlElement);
-						foreach (XmlElement api_node in matches)
-							api_node.ParentNode.RemoveChild (api_node);
-						if (matches.Count == 0)
+						var nodes = api_doc.XPathSelectElements (path).ToArray ();
+						if (nodes.Any ())
+							foreach (var node in nodes)
+								node.Remove ();
+						else
 							// BG8A00
-							Report.Warning (0, Report.WarningApiFixup + 0, "<remove-node path=\"{0}\"/> matched no nodes.", path);
+							Report.Warning (0, Report.WarningApiFixup + 0, null, metaitem, "<remove-node path=\"{0}\"/> matched no nodes.", path);
 					} catch (XPathException e) {
 						// BG4A01
-						Report.Error (Report.ErrorApiFixup + 1, e, "Invalid XPath specification: {0}", path);
+						Report.Error (Report.ErrorApiFixup + 1, e, metaitem, "Invalid XPath specification: {0}", path);
 					}
 					break;
 				case "add-node":
 					try {
-						XPathNodeIterator api_iter = api_nav.Select (path);
+						var nodes = api_doc.XPathSelectElements (path);
 						bool matched = false;
-						while (api_iter.MoveNext ()) {
-							XmlElement api_node = ((IHasXmlNode)api_iter.Current).GetNode () as XmlElement;
-							foreach (XmlNode child in ((IHasXmlNode)metanav).GetNode().ChildNodes)
-								api_node.AppendChild (api_doc.ImportNode (child, true));
+						if (!nodes.Any ())
+							// BG8A01
+							Report.Warning (0, Report.WarningApiFixup + 1, null, metaitem, "<add-node path=\"{0}\"/> matched no nodes.", path);
+						else {
+							foreach (var node in nodes)
+								node.Add (metaitem.Nodes ());
 							matched = true;
 						}
-						if (!matched)
-							// BG8A01
-							Report.Warning (0, Report.WarningApiFixup + 1, "<add-node path=\"{0}\"/> matched no nodes.", path);
 					} catch (XPathException e) {
 						// BG4A02
-						Report.Error (Report.ErrorApiFixup + 2, e, "Invalid XPath specification: {0}", path);
+						Report.Error (Report.ErrorApiFixup + 2, e, metaitem, "Invalid XPath specification: {0}", path);
 					}
 					break;
 				case "change-node":
 					try {
-						XPathNodeIterator api_iter = api_nav.Select (path);
+						var nodes = api_doc.XPathSelectElements (path);
 						bool matched = false;
-						while (api_iter.MoveNext ()) {
-							XmlElement node = ( (IHasXmlNode) api_iter.Current).GetNode () as XmlElement;
-							XmlElement parent = node.ParentNode as XmlElement;
-							XmlElement new_node = api_doc.CreateElement (metanav.Value);
-							
-							foreach (XmlNode child in node.ChildNodes)
-								new_node.AppendChild (child.Clone ());
-							foreach (XmlAttribute attribute in node.Attributes)
-								new_node.Attributes.Append ( (XmlAttribute) attribute.Clone ());
-							
-							parent.ReplaceChild (new_node, node);
+						foreach (var node in nodes) {
+							var newChild = new XElement (metaitem.Value);
+							newChild.Add (node.Attributes ());
+							newChild.Add (node.Nodes ());
+							node.ReplaceWith (newChild);
 							matched = true;
 						}
 
 						if (!matched)
 							// BG8A03
-							Report.Warning (0, Report.WarningApiFixup + 3, "<change-node-type path=\"{0}\"/> matched no nodes.", path);
+							Report.Warning (0, Report.WarningApiFixup + 3, null, metaitem, "<change-node-type path=\"{0}\"/> matched no nodes.", path);
 					} catch (XPathException e) {
 						// BG4A03
-						Report.Error (Report.ErrorApiFixup + 3, e, "Invalid XPath specification: {0}", path);
+						Report.Error (Report.ErrorApiFixup + 3, e, metaitem, "Invalid XPath specification: {0}", path);
 					}
 					break;
 				case "attr":
 					try {
-						string attr_name = metanav.XGetAttribute ("name", "");
+						string attr_name = metaitem.XGetAttribute ("name");
 						if (string.IsNullOrEmpty (attr_name))
 							// BG4A07
-							Report.Error (Report.ErrorApiFixup + 7, "Target attribute name is not specified for path: {0}", path);
-						var nodes = attr_last_cache != null ?
-					            (IEnumerable<XPathNavigator>) new XPathNavigator [] {attr_last_cache} :
-								api_nav.Select (path).OfType<XPathNavigator> ();
+							Report.Error (Report.ErrorApiFixup + 7, null, metaitem, "Target attribute name is not specified for path: {0}", path);
+						var nodes = attr_last_cache != null ? new XElement [] { attr_last_cache } : api_doc.XPathSelectElements (path);
 						int attr_matched = 0;
 						foreach (var n in nodes) {
-							XmlElement node = ((IHasXmlNode) n).GetNode () as XmlElement;
-							node.SetAttribute (attr_name, metanav.Value);
-							//attr_last_cache = n;
+							n.SetAttributeValue (attr_name, metaitem.Value);
 							attr_matched++;
 						}
 						if (attr_matched == 0)
 							// BG8A04
-							Report.Warning (0, Report.WarningApiFixup + 4, "<attr path=\"{0}\"/> matched no nodes.", path);
+							Report.Warning (0, Report.WarningApiFixup + 4, null, metaitem, "<attr path=\"{0}\"/> matched no nodes.", path);
 						if (attr_matched != 1)
 							attr_last_cache = null;
 					} catch (XPathException e) {
 						// BG4A04
-						Report.Error (Report.ErrorApiFixup + 4, e, "Invalid XPath specification: {0}", path);
+						Report.Error (Report.ErrorApiFixup + 4, e, metaitem, "Invalid XPath specification: {0}", path);
 					}
 					break;
 				case "move-node":
 					try {
-						XPathExpression expr = api_nav.Compile (path);
-						string parent = metanav.Value;
-						XPathNodeIterator parent_iter = api_nav.Select (parent);
+						string parent = metaitem.Value;
+						var parents = api_doc.XPathSelectElements (parent);
 						bool matched = false;
-						while (parent_iter.MoveNext ()) {
-							XmlNode parent_node = ((IHasXmlNode)parent_iter.Current).GetNode ();
-							XPathNodeIterator path_iter = parent_iter.Current.Clone ().Select (expr);
-							while (path_iter.MoveNext ()) {
-								XmlNode node = ((IHasXmlNode)path_iter.Current).GetNode ();
-								parent_node.AppendChild (node.Clone ());
-								node.ParentNode.RemoveChild (node);
-							}
+						foreach (var parent_node in parents) {
+							var nodes = parent_node.XPathSelectElements (path).ToArray ();
+							foreach (var node in nodes)
+								node.Remove ();
+							parent_node.Add (nodes);
 							matched = true;
 						}
 						if (!matched)
 							// BG8A05
-							Report.Warning (0, Report.WarningApiFixup + 5, "<move-node path=\"{0}\"/> matched no nodes.", path);
+							Report.Warning (0, Report.WarningApiFixup + 5, null, metaitem, "<move-node path=\"{0}\"/> matched no nodes.", path);
 					} catch (XPathException e) {
 						// BG4A05
-						Report.Error (Report.ErrorApiFixup + 5, e, "Invalid XPath specification: {0}", path);
+						Report.Error (Report.ErrorApiFixup + 5, e, metaitem, "Invalid XPath specification: {0}", path);
 					}
 					break;
 				case "remove-attr":
 					try {
-						string name = metanav.XGetAttribute ("name", "");
-						XPathNodeIterator api_iter = api_nav.Select (path);
+						string name = metaitem.XGetAttribute ("name");
+						var nodes = api_doc.XPathSelectElements (path);
 						bool matched = false;
-						
-						while (api_iter.MoveNext ()) {
-							XmlElement node = ( (IHasXmlNode) api_iter.Current).GetNode () as XmlElement;
-							
-							node.RemoveAttribute (name);
+
+						foreach (var node in nodes) {
+							node.RemoveAttributes ();
 							matched = true;
 						}
 
 						if (!matched)
 							// BG8A06
-							Report.Warning (0, Report.WarningApiFixup + 6, "<remove-attr path=\"{0}\"/> matched no nodes.", path);
+							Report.Warning (0, Report.WarningApiFixup + 6, null, metaitem, "<remove-attr path=\"{0}\"/> matched no nodes.", path);
 					} catch (XPathException e) {
 						// BG4A06
-						Report.Error (Report.ErrorApiFixup + 6, e, "Invalid XPath specification: {0}", path);
+						Report.Error (Report.ErrorApiFixup + 6, e, metaitem, "Invalid XPath specification: {0}", path);
 					}
 					break;
 				}

tools/generator/ClassGen.cs

@@ -12,6 +12,7 @@
 using Xamarin.Android.Tools;
 
 using MonoDroid.Utils;
+using System.Xml.Linq;
 
 namespace MonoDroid.Generation {
 #if USE_CECIL
@@ -75,26 +76,23 @@ public class XmlClassGen : ClassGen {
 		bool is_final;
 		string base_type;
 
-		public XmlClassGen (XmlElement pkg, XmlElement elem)
+		public XmlClassGen (XElement pkg, XElement elem)
 			: base (new XmlGenBaseSupport (pkg, elem))//FIXME: should not be xml specific
 		{
 			is_abstract = elem.XGetAttribute ("abstract") == "true";
 			is_final = elem.XGetAttribute ("final") == "true";
 			base_type = elem.XGetAttribute ("extends");
-			foreach (XmlNode node in elem.ChildNodes) {
-				XmlElement child = node as XmlElement;
-				if (child == null)
-					continue;
-				switch (node.Name) {
+			foreach (var child in elem.Elements ()) {
+				switch (child.Name.LocalName) {
 				case "implements":
 					string iname = child.XGetAttribute ("name-generic-aware");
 					iname = iname.Length > 0 ? iname : child.XGetAttribute ("name");
 					AddInterface (iname);
 					break;
 				case "method":
-					var synthetic = child.GetAttribute ("synthetic") == "true";
-					var finalizer = child.GetAttribute ("name") == "finalize" &&
-						child.GetAttribute ("jni-signature") == "()V";
+					var synthetic = child.XGetAttribute ("synthetic") == "true";
+					var finalizer = child.XGetAttribute ("name") == "finalize" &&
+						child.XGetAttribute ("jni-signature") == "()V";
 					if (!(synthetic || finalizer))
 						AddMethod (new XmlMethod (this, child));
 					break;
@@ -107,7 +105,7 @@ public XmlClassGen (XmlElement pkg, XmlElement elem)
 				case "typeParameters":
 					break; // handled at GenBaseSupport
 				default:
-					Report.Warning (0, Report.WarningClassGen + 1, "unexpected class child {0}.", node.Name);
+					Report.Warning (0, Report.WarningClassGen + 1, "unexpected class child {0}.", child.Name);
 					break;
 				}
 			}