[MSBuild] Fix the ${level} and ${number} capture.

Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=14767

	Some people, when confronted with a problem, think
	“I know, I'll use regular expressions.”   Now they have two problems.
	- http://regex.info/blog/2006-09-15/247

ToolTask uses a regular expression to extract various bits of
information from the warning/error message. Unfortunately it was
inadequate, and didn't correctly match on some errors.

The error message in this case is:

	class1.cs(16,4): error CS0152: The label `case 1:' already occurs in this switch statement.

The problem was that while the previous regular expression would
succeed in matching the above string, it was horribly inaccurate in
what it captured:

	${file}   = "error CS0152"
	${line}   = "16"
	${column} = "4"
	${level}  = "The"
	${number} = "label `case 1"
	${message}= "' already occurs in this switch statement."

Did I mention hilariously wrong?

There were two problems with the previous pattern:

 1. It allowed filename+(line,column) to be _repeated_, which is
    clearly insane. Filename/etc. shouldn't be repeated.

 2. ${number} captured ".*\d", i.e. anything ending with a number, so
    when the message had a number embedded within it, it was captured.

For good measure, use RegexOptions.IgnorePatternWhitespace so that
we don't have a horrendously long regex pattern to compare for diffs
the next time this needs fixing, and add NUnit tests.

5 files changed, 159 insertions, 3 deletions

diff --git a/mcs/class/Microsoft.Build.Utilities/Makefile b/mcs/class/Microsoft.Build.Utilities/Makefile
index 32f34e3f401..585e5bb0a4f 100644
--- a/mcs/class/Microsoft.Build.Utilities/Makefile
+++ b/mcs/class/Microsoft.Build.Utilities/Makefile
@@ -19,7 +19,7 @@ LIB_MCS_FLAGS = \
 	/r:System.dll				\
 	/r:$(BUILD_FRAMEWORK)
 
-TEST_MCS_FLAGS = /r:$(BUILD_FRAMEWORK) -r:System.dll
+TEST_MCS_FLAGS = /r:$(BUILD_FRAMEWORK) -r:System.dll -r:System.Core.dll
 
 export TESTING_MONO=a
 XBUILD_DIR=../../tools/xbuild
diff --git a/mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities.Test.csproj b/mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities.Test.csproj
index e8eeb392c04..718e3c2e01f 100644
--- a/mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities.Test.csproj
+++ b/mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities.Test.csproj
@@ -51,5 +51,6 @@
     <Compile Include="Test\Microsoft.Build.Utilities\LoggerTest.cs" />
     <Compile Include="Test\Microsoft.Build.Utilities\TaskItemTest.cs" />
     <Compile Include="Test\Microsoft.Build.Utilities\TaskLoggingHelperTest.cs" />
+    <Compile Include="Test\Microsoft.Build.Utilities\ToolTaskTest.cs" />
   </ItemGroup>
-</Project>
\ No newline at end of file
+</Project>
diff --git a/mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities/ToolTask.cs b/mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities/ToolTask.cs
index 4094c122e3b..3192344d278 100644
--- a/mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities/ToolTask.cs
+++ b/mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities/ToolTask.cs
@@ -466,11 +466,24 @@ namespace Microsoft.Build.Utilities
 
 		// Snatched from our codedom code, with some changes to make it compatible with csc
 		// (the line+column group is optional is csc)
+		const string ErrorRegexPattern = @"
+			^
+			(\s*(?<file>[^\(]+)                         # filename (optional)
+			 (\((?<line>\d*)(,(?<column>\d*[\+]*))?\))? # line+column (optional)
+			 :\s+)?
+			(?<level>\w+)                               # error|warning
+			\s+
+			(?<number>[^:]*\d)                          # CS1234
+			:
+			\s*
+			(?<message>.*)$";
+
 		static Regex errorRegex;
 		static Regex CscErrorRegex {
 			get {
 				if (errorRegex == null)
-					errorRegex = new Regex (@"^(\s*(?<file>[^\(]+)(\((?<line>\d*)(,(?<column>\d*[\+]*))?\))?:\s+)*(?<level>\w+)\s+(?<number>.*\d):\s*(?<message>.*)", RegexOptions.Compiled | RegexOptions.ExplicitCapture);
+					errorRegex = new Regex (ErrorRegexPattern,
+							RegexOptions.Compiled | RegexOptions.ExplicitCapture | RegexOptions.IgnorePatternWhitespace);
 				return errorRegex;
 			}
 		}
diff --git a/mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities_test.dll.sources b/mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities_test.dll.sources
index 8f797a06ec8..b1a3758e980 100644
--- a/mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities_test.dll.sources
+++ b/mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities_test.dll.sources
@@ -2,3 +2,4 @@ Microsoft.Build.Utilities/CommandLineBuilderTest.cs
 Microsoft.Build.Utilities/LoggerTest.cs
 Microsoft.Build.Utilities/TaskItemTest.cs
 Microsoft.Build.Utilities/TaskLoggingHelperTest.cs
+Microsoft.Build.Utilities/ToolTaskTest.cs
diff --git a/mcs/class/Microsoft.Build.Utilities/Test/Microsoft.Build.Utilities/ToolTaskTest.cs b/mcs/class/Microsoft.Build.Utilities/Test/Microsoft.Build.Utilities/ToolTaskTest.cs
new file mode 100644
index 00000000000..8b3ae6e69c4
--- /dev/null
+++ b/mcs/class/Microsoft.Build.Utilities/Test/Microsoft.Build.Utilities/ToolTaskTest.cs
@@ -0,0 +1,141 @@
+//
+// ToolTaskTest.cs:
+//
+// Author:
+//   Jonathan Pryor (jonp@xamarin.com)
+//
+// (C) 2013 Xamarin Inc.
+//
+// Permission is hereby granted, free of charge, to any person obtaining
+// a copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to
+// permit persons to whom the Software is furnished to do so, subject to
+// the following conditions:
+//
+// The above copyright notice and this permission notice shall be
+// included in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
+// LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
+// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+// WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+using System;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.IO;
+using System.Linq;
+
+using Microsoft.Build.Framework;
+using Microsoft.Build.Utilities;
+
+using NUnit.Framework;
+
+namespace MonoTests.Microsoft.Build.Utilities {
+
+	[TestFixture]
+	public class ToolTaskTest {
+
+		[Test]
+		public void LogEventsFromTextOutput ()
+		{
+			var messages = new[] {
+				new {Code = "CS0152", Line = "class1.cs(16,4): error CS0152: The label `case 1:' already occurs in this switch statement" },
+			};
+
+			var task = new LogEventsFromTextOutputToolTask ();
+			foreach (var m in messages) {
+				task.LogEventsFromTextOutput (m.Line);
+				Assert.IsTrue (task.Codes.Count > 0, "No error logged for line: {0}", m.Line);
+				Assert.AreEqual (m.Code, task.Codes [0]);
+				task.Codes.Clear ();
+			}
+		}
+	}
+
+	class LogEventsFromTextOutputToolTask : ToolTask {
+
+		public List<string> Codes {
+			get {return engine.Codes;}
+		}
+
+		CodeLoggingBuildEngine engine = new CodeLoggingBuildEngine ();
+
+		public LogEventsFromTextOutputToolTask ()
+		{
+			BuildEngine = engine;
+		}
+
+		protected override string GenerateFullPathToTool ()
+		{
+			throw new NotImplementedException ();
+		}
+
+		protected override string ToolName {
+			get {throw new NotImplementedException ();}
+		}
+
+		public void LogEventsFromTextOutput (string line)
+		{
+			base.LogEventsFromTextOutput (line, MessageImportance.Normal);
+		}
+	}
+
+	class CodeLoggingBuildEngine : IBuildEngine {
+
+		public List<string> Codes = new List<string> ();
+
+		public int ColumnNumberOfTaskNode {
+			get {
+				throw new NotImplementedException ();
+			}
+		}
+
+		public bool ContinueOnError {
+			get {
+				throw new NotImplementedException ();
+			}
+		}
+
+		public int LineNumberOfTaskNode {
+			get {
+				throw new NotImplementedException ();
+			}
+		}
+
+		public string ProjectFileOfTaskNode {
+			get {
+				throw new NotImplementedException ();
+			}
+		}
+
+		public bool BuildProjectFile (string projectFileName, string[] targetNames, System.Collections.IDictionary globalProperties, System.Collections.IDictionary targetOutputs)
+		{
+			throw new NotImplementedException ();
+		}
+
+		public void LogCustomEvent (CustomBuildEventArgs e)
+		{
+		}
+
+		public void LogErrorEvent (BuildErrorEventArgs e)
+		{
+			Codes.Add (e.Code);
+		}
+
+		public void LogMessageEvent (BuildMessageEventArgs e)
+		{
+		}
+
+		public void LogWarningEvent (BuildWarningEventArgs e)
+		{
+			Codes.Add (e.Code);
+		}
+	}
+}
+