[MSBuild] Fix the ${level} and ${number} capture.
authorJonathan Pryor <jonpryor@vt.edu>
Tue, 24 Sep 2013 20:16:32 +0000 (16:16 -0400)
committerJonathan Pryor <jonpryor@vt.edu>
Tue, 24 Sep 2013 20:29:31 +0000 (16:29 -0400)
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.

mcs/class/Microsoft.Build.Utilities/Makefile
mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities.Test.csproj
mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities/ToolTask.cs
mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities_test.dll.sources
mcs/class/Microsoft.Build.Utilities/Test/Microsoft.Build.Utilities/ToolTaskTest.cs [new file with mode: 0644]

index 32f34e3f401d90b638cb82e3ee384890da74329f..585e5bb0a4f3e0eff7b65d00b3d29fdfec61a96c 100644 (file)
@@ -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
index e8eeb392c04a7e2d7b20e6b3553ec90ea2692739..718e3c2e01f8feba2f1824fdee277fb81408c5db 100644 (file)
@@ -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>
index 4094c122e3b9e9afc86574ece4e1ea98afbd9cb9..3192344d278790158cbdafc000d361972ada350b 100644 (file)
@@ -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;
                        }
                }
index 8f797a06ec86c881980f33b19b2629628d48ba7f..b1a3758e98012e9862b68bcf0d790ab2040661d5 100644 (file)
@@ -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 (file)
index 0000000..8b3ae6e
--- /dev/null
@@ -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);
+               }
+       }
+}
+