[Xamarin.Android.Build.Tasks] Emit an Error if a custom view cannot b…

…e fixed up.

Context dotnet#1711

When using a custom view within a layout file we replace the
`namespace.classname` with an `md5hash.classname`. We do this
by using the `acwmap.txt` file to make known types onto the
`md5` hashed ones. We do this in a case sensitive manner. We
also only support Camel case and lower case type names.

	ClassLibrary1.CustomView=md5d6f7135293df7527c983d45d07471c5e.CustomTextView
	classlibrary1.CustomView=md5d6f7135293df7527c983d45d07471c5e.CustomTextView

Given that a user is able to type these manually it is highly
probable that typo's will occur. If for example a user types

	Classlibrary1.CustomView

this will NOT be fixed up. Instead the user will recieve the
following error at runtime.

	FATAL UNHANDLED EXCEPTION: Android.Views.InflateException: Binary XML file line #1: Binary XML file line #1: Error inflating class Classlibrary1.CustomTextView ---> Android.Views.InflateException: Binary XML file line #1: Error inflating class Classlibrary1.CustomTextView ---> Java.Lang.ClassNotFoundException: Didn't find class "Classlibrary1.CustomTextView"

if you are using the control in a number of places this runtime error
does nothing to help track down the problem.

Instead what we should be doing is detecting these issues and emitting
a build error. This will provide the user not only the problem but
also a link to the file causing the probem.

TODO
----

[ ] Fix up the name so it points to the file in `Resources` not `res`
[ ] Add a Unit test.
[ ] Add Error code and document.

Documentation/guides/messages/README.md

@@ -43,6 +43,8 @@
 ### XA1xxx Project Related
 
 + [XA1000](xa1000.md): There was an problem parsing {file}. This is likely due to incomplete or invalid xml.
++ [XA1001](xa1001.md): AndroidResgen: Warning while updating Resource XML '{filename}': {Message}
++ [XA1002](xa1002.md): We found a matching key '{Key}' for '{Item}'. But the casing was incorrect. Please correct the casing
 
 ### XA2xxx Linker
 

Documentation/guides/messages/xa1001.md

@@ -0,0 +1,7 @@
+# Compiler Warning XA1001
+
+AndroidResgen: Warning while updating Resource XML '{filename}': {Message}
+
+This warning is raised if we encounter an unknown issue when processing
+the layout and resources. It is a generic warning that does not describe 
+any specific problem. The details will be in the `{Message}`.

Documentation/guides/messages/xa1002.md

@@ -0,0 +1,13 @@
+# Compiler Error XA1000
+
+This error will be emitted when we are unable to find a matching custom new in the 
+ResourceCaseMap string. 
+
+As part of the build process `Namespace.CustomViewFoo` items in layout files are
+replaced with `{MD5Hash}.CustomViewFoo`. We attempt to replace a couple of variants
+of the `Namespace.CustomViewFoo`. One is the original casing found in the C# source
+files. The other is all lowercase. If we cannot find a match we will do a case 
+insensitive lookup to see if there are items which do match. If we find one this 
+error will be raised.
+
+We found a matching key 'Namespace.CustomViewFoo' for 'NameSpace.CustomViewFoo'. But the casing was incorrect. Please correct the casing

src/Xamarin.Android.Build.Tasks/Tasks/Aapt.cs

@@ -92,7 +92,7 @@ public class Aapt : AsyncTask
 
 		public string ResourceSymbolsTextFileDirectory { get; set; }
 
-		Dictionary<string,string> resource_name_case_map = new Dictionary<string,string> ();
+		Dictionary<string,string> resource_name_case_map;
 		AssemblyIdentityMap assemblyMap = new AssemblyIdentityMap ();
 		string resourceDirectory;
 
@@ -251,9 +251,7 @@ public override bool Execute ()
 
 		void DoExecute ()
 		{
-			if (ResourceNameCaseMap != null)
-				foreach (var arr in ResourceNameCaseMap.Split (';').Select (l => l.Split ('|')).Where (a => a.Length == 2))
-					resource_name_case_map [arr [1]] = arr [0]; // lowercase -> original
+			resource_name_case_map = MonoAndroidHelper.LoadResourceCaseMap (ResourceNameCaseMap);
 
 			assemblyMap.Load (Path.Combine (WorkingDirectory, AssemblyIdentityMapFile));
 

src/Xamarin.Android.Build.Tasks/Tasks/Aapt2.cs

@@ -18,7 +18,7 @@ namespace Xamarin.Android.Tasks {
 
 	public class Aapt2 : AsyncTask {
 
-		protected Dictionary<string, string> resource_name_case_map = new Dictionary<string, string> ();
+		protected Dictionary<string, string> resource_name_case_map;
 
 		public ITaskItem [] ResourceDirectories { get; set; }
 
@@ -168,9 +168,7 @@ protected void LogEventsFromTextOutput (string singleLine, MessageImportance mes
 
 		protected void LoadResourceCaseMap ()
 		{
-			if (ResourceNameCaseMap != null)
-				foreach (var arr in ResourceNameCaseMap.Split (';').Select (l => l.Split ('|')).Where (a => a.Length == 2))
-					resource_name_case_map [arr [1]] = arr [0]; // lowercase -> original
+			resource_name_case_map = MonoAndroidHelper.LoadResourceCaseMap (ResourceNameCaseMap);
 		}
 	}
 }

src/Xamarin.Android.Build.Tasks/Tasks/ConvertResourcesCases.cs

@@ -1,6 +1,7 @@
 // Copyright (C) 2011 Xamarin, Inc. All rights reserved.
 
 using System;
+using System.Diagnostics;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
@@ -20,13 +21,19 @@ public class ConvertResourcesCases : Task
 
 		public string AndroidConversionFlagFile { get; set; }
 
+		public string ResourceNameCaseMap { get; set; }
+
+		Dictionary<string,string> resource_name_case_map;
+
 		public override bool Execute ()
 		{
 			Log.LogDebugMessage ("ConvertResourcesCases Task");
 			Log.LogDebugMessage ("  ResourceDirectories: {0}", ResourceDirectories);
 			Log.LogDebugMessage ("  AcwMapFile: {0}", AcwMapFile);
 			Log.LogDebugMessage ("  AndroidConversionFlagFile: {0}", AndroidConversionFlagFile);
+			Log.LogDebugMessage ("  ResourceNameCaseMap: {0}", ResourceNameCaseMap);
 
+			resource_name_case_map = MonoAndroidHelper.LoadResourceCaseMap (ResourceNameCaseMap);
 			var acw_map = MonoAndroidHelper.LoadAcwMapFile (AcwMapFile);
 
 			// Look in the resource xml's for capitalized stuff and fix them
@@ -71,7 +78,26 @@ void FixupResources (ITaskItem item, Dictionary<string, string> acwMap)
 				MonoAndroidHelper.SetWriteable (tmpdest);
 				try {
 					AndroidResource.UpdateXmlResource (resdir, tmpdest, acwMap,
-						ResourceDirectories.Where (s => s != item).Select(s => s.ItemSpec));
+						ResourceDirectories.Where (s => s != item).Select(s => s.ItemSpec), (t, m) => {
+							string targetfile = file;
+							if (targetfile.StartsWith (resdir, StringComparison.InvariantCultureIgnoreCase)) {
+								targetfile = file.Substring (resdir.Length).TrimStart (Path.DirectorySeparatorChar);
+								if (resource_name_case_map.TryGetValue (targetfile, out string temp))
+									targetfile = temp;
+								targetfile = Path.Combine ("Resources", targetfile);
+							}
+							switch (t) {
+								case TraceLevel.Error:
+									Log.LogCodedError ("XA1002", file: targetfile, lineNumber: 0, message: m);
+									break;
+								case TraceLevel.Warning:
+									Log.LogCodedWarning ("XA1001", file: targetfile, lineNumber: 0, message: m);
+									break;
+								default:
+									Log.LogDebugMessage (m);
+									break;
+							}
+						});
 
 					// We strip away an eventual UTF-8 BOM from the XML file.
 					// This is a requirement for the Android designer because the desktop Java renderer

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/ConvertResourcesCasesTests.cs

@@ -0,0 +1,87 @@
+using System;
+using System.Collections.Generic;
+using NUnit.Framework;
+using Xamarin.ProjectTools;
+using System.IO;
+using System.Linq;
+using Microsoft.Build.Framework;
+using System.Text;
+using Xamarin.Android.Tasks;
+using Microsoft.Build.Utilities;
+
+namespace Xamarin.Android.Build.Tests {
+
+	[TestFixture]
+	[Parallelizable (ParallelScope.Self)]
+	public class ConvertResourcesCasesTests  : BaseTest {
+		[Test]
+		public void CheckClassIsReplacedWithMd5 ()
+		{
+			var path = Path.Combine (Root, "temp", "CheckClassIsReplacedWithMd5");
+			Directory.CreateDirectory (path);
+			var resPath = Path.Combine (path, "res");
+			Directory.CreateDirectory (Path.Combine (resPath, "layout"));
+			File.WriteAllText (Path.Combine (resPath, "layout", "main.xml"), @"<?xml version='1.0' ?>
+<LinearLayout xmlns:android='http://schemas.android.com/apk/res/android'>
+<ClassLibrary1.CustomView xmlns:android='http://schemas.android.com/apk/res/android' />
+<classlibrary1.CustomView xmlns:android='http://schemas.android.com/apk/res/android' />
+</LinearLayout>
+");
+			var errors = new List<BuildErrorEventArgs> ();
+			IBuildEngine engine = new MockBuildEngine (TestContext.Out, errors);
+			var task = new ConvertResourcesCases {
+				BuildEngine = engine
+			};
+			task.ResourceDirectories = new ITaskItem [] {
+				new TaskItem (resPath),
+			};
+			task.AcwMapFile = Path.Combine (path, "acwmap.txt");
+			File.WriteAllLines (task.AcwMapFile, new string [] {
+				"ClassLibrary1.CustomView;md5d6f7135293df7527c983d45d07471c5e.CustomTextView",
+				"classlibrary1.CustomView;md5d6f7135293df7527c983d45d07471c5e.CustomTextView",
+			});
+			Assert.IsTrue (task.Execute (), "Task should have executed successfully");
+			var output = File.ReadAllText (Path.Combine (resPath, "layout", "main.xml"));
+			StringAssert.Contains ("md5d6f7135293df7527c983d45d07471c5e.CustomTextView", output, "md5d6f7135293df7527c983d45d07471c5e.CustomTextView should exist in the main.xml");
+			StringAssert.DoesNotContain ("ClassLibrary1.CustomView", output, "ClassLibrary1.CustomView should have been replaced.");
+			StringAssert.DoesNotContain ("classlibrary1.CustomView", output, "classlibrary1.CustomView should have been replaced.");
+			Directory.Delete (path, recursive: true);
+		}
+
+		[Test]
+		public void CheckClassIsNotReplacedWithMd5 ()
+		{
+			var path = Path.Combine (Root, "temp", "CheckClassIsNotReplacedWithMd5");
+			Directory.CreateDirectory (path);
+			var resPath = Path.Combine (path, "res");
+			Directory.CreateDirectory (Path.Combine (resPath, "layout"));
+			File.WriteAllText (Path.Combine (resPath, "layout", "main.xml"), @"<?xml version='1.0' ?>
+<LinearLayout xmlns:android='http://schemas.android.com/apk/res/android'>
+<ClassLibrary1.CustomView xmlns:android='http://schemas.android.com/apk/res/android' />
+<classLibrary1.CustomView xmlns:android='http://schemas.android.com/apk/res/android' />
+</LinearLayout>
+");
+			var errors = new List<BuildErrorEventArgs> ();
+			IBuildEngine engine = new MockBuildEngine (TestContext.Out, errors);
+			var task = new ConvertResourcesCases {
+				BuildEngine = engine
+			};
+			task.ResourceDirectories = new ITaskItem [] {
+				new TaskItem (resPath),
+			};
+			task.AcwMapFile = Path.Combine (path, "acwmap.txt");
+			File.WriteAllLines (task.AcwMapFile, new string [] {
+				"ClassLibrary1.CustomView;md5d6f7135293df7527c983d45d07471c5e.CustomTextView",
+				"classlibrary1.CustomView;md5d6f7135293df7527c983d45d07471c5e.CustomTextView",
+			});
+			Assert.IsTrue (task.Execute (), "Task should have executed successfully");
+			var output = File.ReadAllText (Path.Combine (resPath, "layout", "main.xml"));
+			StringAssert.Contains ("md5d6f7135293df7527c983d45d07471c5e.CustomTextView", output, "md5d6f7135293df7527c983d45d07471c5e.CustomTextView should exist in the main.xml");
+			StringAssert.DoesNotContain ("ClassLibrary1.CustomView", output, "ClassLibrary1.CustomView should have been replaced.");
+			StringAssert.Contains ("classLibrary1.CustomView", output, "classLibrary1.CustomView should have been replaced.");
+			Assert.AreEqual (1, errors.Count, "One Error should have been raised.");
+			Assert.AreEqual ("XA1000", errors [0].Code, "XA1000 should have been raised.");
+			Directory.Delete (path, recursive: true);
+		}
+	}
+}

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Xamarin.Android.Build.Tests.csproj

@@ -84,6 +84,7 @@
     <Compile Include="Aapt2Tests.cs" />
     <Compile Include="Tasks\ValidateJavaVersionTests.cs" />
     <Compile Include="ZipArchiveExTests.cs" />
+    <Compile Include="ConvertResourcesCasesTests.cs" />
   </ItemGroup>
   <ItemGroup>
     <Content Include="Expected\GenerateDesignerFileExpected.cs">

src/Xamarin.Android.Build.Tasks/Utilities/AndroidResource.cs

@@ -1,22 +1,24 @@
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.IO;
 using System.Linq;
+using System.Text;
 using System.Text.RegularExpressions;
 using System.Xml.Linq;
 using System.Xml.XPath;
 
 namespace Monodroid {
 	static class AndroidResource {
 
-		public static void UpdateXmlResource (string res, string filename, Dictionary<string, string> acwMap, IEnumerable<string> additionalDirectories = null)
+		public static void UpdateXmlResource (string res, string filename, Dictionary<string, string> acwMap, IEnumerable<string> additionalDirectories = null, Action<TraceLevel, string> logMessage = null)
 		{
 			// use a temporary file so we only update the real file if things actually changed
 			string tmpfile = filename + ".bk";
 			try {
 				XDocument doc = XDocument.Load (filename, LoadOptions.SetLineInfo);
 
-				UpdateXmlResource (res, doc.Root, acwMap, additionalDirectories);
+				UpdateXmlResource (res, doc.Root, acwMap, additionalDirectories, logMessage);
 				using (var stream = File.OpenWrite (tmpfile))
 					using (var xw = new LinePreservedXmlWriter (new StreamWriter (stream)))
 						xw.WriteNode (doc.CreateNavigator (), false);
@@ -27,7 +29,8 @@ public static void UpdateXmlResource (string res, string filename, Dictionary<st
 				if (File.Exists (tmpfile)) {
 					File.Delete (tmpfile);
 				}
-				Console.Error.WriteLine ("AndroidResgen: Warning while updating Resource XML '{0}': {1}", filename, e.Message);
+				if (logMessage != null)
+					logMessage (TraceLevel.Warning, $"AndroidResgen: Warning while updating Resource XML '{filename}': {e.Message}");
 				return;
 			}
 		}
@@ -59,10 +62,10 @@ static IEnumerable<T> Prepend<T> (this IEnumerable<T> l, T another) where T : XN
 				yield return e;
 		}
 
-		static void UpdateXmlResource (string resourcesBasePath, XElement e, Dictionary<string, string> acwMap, IEnumerable<string> additionalDirectories = null)
+		static void UpdateXmlResource (string resourcesBasePath, XElement e, Dictionary<string, string> acwMap, IEnumerable<string> additionalDirectories = null, Action<TraceLevel, string> logMessage = null)
 		{
 			foreach (var elem in GetElements (e).Prepend (e)) {
-				TryFixCustomView (elem, acwMap);
+				TryFixCustomView (elem, acwMap, logMessage);
 			}
 
 			foreach (var path in fixResourcesAliasPaths) {
@@ -184,17 +187,17 @@ private static bool TryFixFragment (XAttribute attr, Dictionary<string, string>
 				return false;
 
 			if (attr.Name == "class" || attr.Name == android + "name") {
-				if (acwMap.ContainsKey (attr.Value)) {
-					attr.Value = acwMap[attr.Value];
+				if (acwMap.TryGetValue (attr.Value, out string mappedValue)) {
+					attr.Value = mappedValue;
 
 					return true;
 				}
 				else if (attr.Value?.Contains (',') ?? false) {
 					// attr.Value could be an assembly-qualified name that isn't in acw-map.txt;
 					// see e5b1c92c, https://github.com/xamarin/xamarin-android/issues/1296#issuecomment-365091948
 					var n = attr.Value.Substring (0, attr.Value.IndexOf (','));
-					if (acwMap.ContainsKey (n)) {
-						attr.Value = acwMap [n];
+					if (acwMap.TryGetValue (n, out mappedValue)) {
+						attr.Value = mappedValue;
 						return true;
 					}
 				}
@@ -217,15 +220,23 @@ private static bool TryFixResAuto (XAttribute attr, Dictionary<string, string> a
 			return false;
 		}
 
-		private static bool TryFixCustomView (XElement elem, Dictionary<string, string> acwMap)
+		private static bool TryFixCustomView (XElement elem, Dictionary<string, string> acwMap, Action<TraceLevel, string> logMessage = null)
 		{
 			// Looks for any <My.DotNet.Class ...
 			// and tries to change it to the ACW name
-			if (acwMap.ContainsKey (elem.Name.ToString ())) {
-				elem.Name = acwMap[elem.Name.ToString ()];
+			string name = elem.Name.ToString ();
+			if (acwMap.TryGetValue (name, out string mappedValue)) {
+				elem.Name = mappedValue;
 				return true;
 			}
-
+			if (logMessage == null)
+				return false;
+			var matchingKey = acwMap.FirstOrDefault(x => String.Equals(x.Key, name, StringComparison.OrdinalIgnoreCase));
+			if (matchingKey.Key != null) {
+				// we have elements with slightly different casing.
+				// lets issue a error.
+				logMessage (TraceLevel.Error, $"We found a matching key '{matchingKey.Key}' for '{name}'. But the casing was incorrect. Please correct the casing");
+			}
 			return false;
 		}
 
@@ -238,8 +249,7 @@ private static bool TryFixCustomClassAttribute (XAttribute attr, Dictionary<stri
 			    && (attr.Parent.Name != "transition" || attr.Name.LocalName != "class")) // For custom transitions
 				return false;
 
-			string mappedValue;
-			if (!acwMap.TryGetValue (attr.Value, out mappedValue))
+			if (!acwMap.TryGetValue (attr.Value, out string mappedValue))
 				return false;
 
 			attr.Value = mappedValue;

src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs

@@ -553,5 +553,15 @@ public static string TryGetAndroidJarPath (TaskLoggingHelper log, string platfor
 			}
 			return Path.Combine (platformPath, "android.jar");
 		}
+
+		public static Dictionary<string, string> LoadResourceCaseMap (string resourceCaseMap)
+		{
+			var result = new Dictionary<string, string> ();
+			if (resourceCaseMap != null) {
+				foreach (var arr in resourceCaseMap.Split (';').Select (l => l.Split ('|')).Where (a => a.Length == 2))
+					result [arr [1]] = arr [0]; // lowercase -> original
+			}
+			return result;
+		}
 	}
 }

src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets

@@ -2214,6 +2214,7 @@ because xbuild doesn't support framework reference assemblies.
   </GenerateJavaStubs>
   <ConvertResourcesCases 
 	ResourceDirectories="$(MonoAndroidResDirIntermediate);@(LibraryResourceDirectories)"
+	ResourceNameCaseMap="$(_AndroidResourceNameCaseMap)"
 	AcwMapFile="$(_AcwMapFile)" />
 </Target>