[Xamarin.Android.Build.Tasks] APT0000 errors are for *errors* (#1047)

Fixes: https://devdiv.visualstudio.com/DevDiv/_workitems?id=528572

The current Regex was capturing output which was not an error or a
warning. It turns out that `aapt` is not very consistent with its
error messaging: sometimes it uses `error`, other times it uses
`Error` or `ERROR`.

Or in the worse case, they don't flag it as anything.

This commit adds a few more tests to `AndroidRegExTests` as well as
alters the logic in the `<Aapt/>` task a bit as well. This logic will
only log an error if:

 1. We have an error `level` group, or
 2. we have a `file` name group and `line` number group.

This seems to be fairly consistent when looking at the `aapt` source
code.

We also update the tests to add a few extra `Known` errors which the
regex might not pick. This is because they do not contain a `level`
or a `file` and `line` number.

Author: dellis1972

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

@@ -117,7 +117,7 @@ bool RunAapt (string commandLine)
 			using (var proc = new Process ()) {
 				proc.OutputDataReceived += (sender, e) => {
 					if (e.Data != null)
-						LogEventsFromTextOutput (e.Data, MessageImportance.Normal);
+						LogMessage (e.Data, MessageImportance.Normal);
 					else
 						stdout_completed.Set ();
 				};
@@ -369,7 +369,7 @@ protected void LogEventsFromTextOutput (string singleLine, MessageImportance mes
 				int line = 0;
 				if (!string.IsNullOrEmpty (match.Groups["line"]?.Value))
 					line = int.Parse (match.Groups["line"].Value) + 1;
-				var level = match.Groups["level"].Value;
+				var level = match.Groups["level"].Value.ToLowerInvariant ();
 				var message = match.Groups ["message"].Value;
 				if (message.Contains ("fakeLogOpen") || level.Contains ("warning")) {
 					LogWarning (singleLine);
@@ -388,17 +388,13 @@ protected void LogEventsFromTextOutput (string singleLine, MessageImportance mes
 				if (message.StartsWith ("error: ", StringComparison.InvariantCultureIgnoreCase))
 					message = message.Substring ("error: ".Length);
 
-				LogError ("APT0000", message, file, line);
-				return;
-			}
-
-			// Handle additional error that doesn't match the regex
-			if (singleLine.Trim ().StartsWith ("invalid resource directory name:")) {
-				LogError ("APT0000", string.Format ("Invalid resource directory name: \"{0}\".", singleLine.Substring (singleLine.LastIndexOfAny (new char[] { '\\', '/' }) + 1)), ToolName);
-				return;
+				if (level.Contains ("error") || (line != 0 && !string.IsNullOrEmpty (file))) {
+					LogError ("APT0000", message, file, line);
+					return;
+				}
 			}
 
-			LogMessage (singleLine, messageImportance);
+			LogError ("APT0000", string.Format("{0} \"{1}\".", singleLine.Trim(), singleLine.Substring(singleLine.LastIndexOfAny(new char[] { '\\', '/' }) + 1)), ToolName);
 		}
 	}
 }

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

@@ -105,7 +105,7 @@ public static Regex AndroidErrorRegex {
 \s*
 (?<message>.*)
 $
-", RegexOptions.Compiled | RegexOptions.ExplicitCapture | RegexOptions.IgnorePatternWhitespace);
+", RegexOptions.Compiled | RegexOptions.ExplicitCapture | RegexOptions.IgnorePatternWhitespace | RegexOptions.IgnoreCase);
 				return androidErrorRegex;
 			}
 		}

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

@@ -58,7 +58,70 @@ public IEnumerator GetEnumerator ()
 					/*expectedLevel*/	"",
 					/*expectedMessage*/	"Invalid file name: must contain only [a-z0-9_.]"
 				};
-
+				yield return new object [] {
+					/*message*/		"max res 10, skipping values-sw600dp",
+					/*expectedToMatch*/	true,
+					/*expectedFile*/	"",
+					/*expectedLine*/	"",
+					/*expectedLevel*/	"",
+					/*expectedMessage*/	"max res 10, skipping values-sw600dp"
+				};
+				yield return new object [] {
+					/*message*/		"max res 10, skipping values-sw600dp-land",
+					/*expectedToMatch*/	true,
+					/*expectedFile*/	"",
+					/*expectedLine*/	"",
+					/*expectedLevel*/	"",
+					/*expectedMessage*/	"max res 10, skipping values-sw600dp-land"
+				};
+				yield return new object [] {
+					/*message*/		"Error: unable to generate entry for resource data",
+					/*expectedToMatch*/	true,
+					/*expectedFile*/	"",
+					/*expectedLine*/	"",
+					/*expectedLevel*/	"Error",
+					/*expectedMessage*/	"unable to generate entry for resource data"
+				};
+				yield return new object [] {
+					/*message*/		"Error: malformed resource filename",
+					/*expectedToMatch*/	true,
+					/*expectedFile*/	"",
+					/*expectedLine*/	"",
+					/*expectedLevel*/	"Error",
+					/*expectedMessage*/	"malformed resource filename"
+				};
+				yield return new object [] {
+					/*message*/		"warning: Multiple AndroidManifest.xml files found, using foo\\AndroidManifest.xml",
+					/*expectedToMatch*/	true,
+					/*expectedFile*/	"",
+					/*expectedLine*/	"",
+					/*expectedLevel*/	"warning",
+					/*expectedMessage*/	"Multiple AndroidManifest.xml files found, using foo\\AndroidManifest.xml"
+				};
+				yield return new object [] {
+					/*message*/		"Resources/values/theme.xml:55: No start tag found",
+					/*expectedToMatch*/	true,
+					/*expectedFile*/	"Resources/values/theme.xml",
+					/*expectedLine*/	"55",
+					/*expectedLevel*/	"",
+					/*expectedMessage*/	"No start tag found"
+				};
+				yield return new object [] {
+					/*message*/		"package name is required with --rename-manifest-package.",
+					/*expectedToMatch*/	true,
+					/*expectedFile*/	"",
+					/*expectedLine*/	"",
+					/*expectedLevel*/	"",
+					/*expectedMessage*/	"package name is required with --rename-manifest-package."
+				};
+				yield return new object [] {
+					/*message*/		"invalid resource directory name: bar-55",
+					/*expectedToMatch*/	true,
+					/*expectedFile*/	"",
+					/*expectedLine*/	"",
+					/*expectedLevel*/	"",
+					/*expectedMessage*/	"invalid resource directory name: bar-55"
+				};
 			}
 		}