From: Jonathan Pryor Date: Tue, 24 Sep 2013 20:16:32 +0000 (-0400) Subject: [MSBuild] Fix the ${level} and ${number} capture. X-Git-Url: http://wien.tomnetworks.com/gitweb/?a=commitdiff_plain;h=7a2d2c247c481c291fed5f7f1213189d9426108a;p=mono.git [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. --- 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 @@ + - \ No newline at end of file + 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*(?[^\(]+) # filename (optional) + (\((?\d*)(,(?\d*[\+]*))?\))? # line+column (optional) + :\s+)? + (?\w+) # error|warning + \s+ + (?[^:]*\d) # CS1234 + : + \s* + (?.*)$"; + static Regex errorRegex; static Regex CscErrorRegex { get { if (errorRegex == null) - errorRegex = new Regex (@"^(\s*(?[^\(]+)(\((?\d*)(,(?\d*[\+]*))?\))?:\s+)*(?\w+)\s+(?.*\d):\s*(?.*)", 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 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 Codes = new List (); + + 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); + } + } +} +