diff --git a/ref/net46/Microsoft.Build/Microsoft.Build.cs b/ref/net46/Microsoft.Build/Microsoft.Build.cs index cf8242bad1e..3f049e8b470 100644 --- a/ref/net46/Microsoft.Build/Microsoft.Build.cs +++ b/ref/net46/Microsoft.Build/Microsoft.Build.cs @@ -938,6 +938,7 @@ public BuildParameters(Microsoft.Build.Evaluation.ProjectCollection projectColle public System.Collections.Generic.ICollection Toolsets { get { throw null; } } public System.Globalization.CultureInfo UICulture { get { throw null; } set { } } public bool UseSynchronousLogging { get { throw null; } set { } } + public System.Collections.Generic.ISet WarningsAsErrors { get { throw null; } set { } } public Microsoft.Build.Execution.BuildParameters Clone() { throw null; } public Microsoft.Build.Evaluation.Toolset GetToolset(string toolsVersion) { throw null; } } diff --git a/src/XMakeBuildEngine/BackEnd/BuildManager/BuildManager.cs b/src/XMakeBuildEngine/BackEnd/BuildManager/BuildManager.cs index 9f3d793288e..b5725db6bcd 100644 --- a/src/XMakeBuildEngine/BackEnd/BuildManager/BuildManager.cs +++ b/src/XMakeBuildEngine/BackEnd/BuildManager/BuildManager.cs @@ -424,7 +424,7 @@ public void BeginBuild(BuildParameters parameters) } // Set up the logging service. - ILoggingService loggingService = CreateLoggingService(_buildParameters.Loggers, _buildParameters.ForwardingLoggers); + ILoggingService loggingService = CreateLoggingService(_buildParameters.Loggers, _buildParameters.ForwardingLoggers, _buildParameters.WarningsAsErrors); _nodeManager.RegisterPacketHandler(NodePacketType.LogMessage, LogMessagePacket.FactoryForDeserialization, loggingService as INodePacketHandler); try @@ -575,7 +575,17 @@ public BuildSubmission PendBuildRequest(BuildRequestData requestData) public BuildResult BuildRequest(BuildRequestData requestData) { BuildSubmission submission = PendBuildRequest(requestData); - return submission.Execute(); + BuildResult result = submission.Execute(); + + ILoggingService loggingService = ((IBuildComponentHost)this).LoggingService; + + // Check if the user had specified /warnaserror and then override the result if there were any errors. + // This ensures the old behavior stays in tact where builds could succeed even if a failure was logged. + if (loggingService.WarningsAsErrors != null && loggingService.HasBuildSubmissionLoggedErrors(submission.SubmissionId)) + { + result.SetOverallResult(overallResult: false); + } + return result; } /// @@ -621,6 +631,12 @@ public void EndBuild() if (loggingService != null) { + // Override the build success if the user specified /warnaserror and any errors were logged outside of a build submission. + if (_overallBuildSuccess && loggingService.WarningsAsErrors != null && loggingService.HasBuildSubmissionLoggedErrors(-1)) + { + _overallBuildSuccess = _overallBuildSuccess && !loggingService.HasBuildSubmissionLoggedErrors(0); + } + loggingService.LogBuildFinished(_overallBuildSuccess); } @@ -1578,6 +1594,18 @@ private void ReportResultsToSubmission(BuildResult result) if (_buildSubmissions.ContainsKey(result.SubmissionId)) { BuildSubmission submission = _buildSubmissions[result.SubmissionId]; + + if (result.OverallResult == BuildResultCode.Success) + { + ILoggingService loggingService = ((IBuildComponentHost)this).LoggingService; + // Check if the user had specified /warnaserror and then override the result if there were any errors. + // This ensures the old behavior stays in tact where builds could succeed even if a failure was logged. + if (loggingService.WarningsAsErrors != null && loggingService.HasBuildSubmissionLoggedErrors(submission.SubmissionId)) + { + result.SetOverallResult(overallResult: false); + } + } + submission.CompleteResults(result); // If the request failed because we caught an exception from the loggers, we can assume we will receive no more logging messages for @@ -1723,7 +1751,7 @@ private void OnProjectStarted(object sender, ProjectStartedEventArgs e) /// /// Creates a logging service around the specified set of loggers. /// - private ILoggingService CreateLoggingService(IEnumerable loggers, IEnumerable forwardingLoggers) + private ILoggingService CreateLoggingService(IEnumerable loggers, IEnumerable forwardingLoggers, ISet warningAsErrors) { int cpuCount = _buildParameters.MaxNodeCount; @@ -1750,6 +1778,7 @@ private ILoggingService CreateLoggingService(IEnumerable loggers, IEnum loggingService.OnLoggingThreadException += _loggingThreadExceptionEventHandler; loggingService.OnProjectStarted += _projectStartedEventHandler; loggingService.OnProjectFinished += _projectFinishedEventHandler; + loggingService.WarningsAsErrors = warningAsErrors; try { diff --git a/src/XMakeBuildEngine/BackEnd/BuildManager/BuildParameters.cs b/src/XMakeBuildEngine/BackEnd/BuildManager/BuildParameters.cs index 3249229a840..34459823cfc 100644 --- a/src/XMakeBuildEngine/BackEnd/BuildManager/BuildParameters.cs +++ b/src/XMakeBuildEngine/BackEnd/BuildManager/BuildParameters.cs @@ -200,6 +200,11 @@ public class BuildParameters : INodePacketTranslatable /// private bool _onlyLogCriticalEvents = false; + /// + /// A list of warnings to treat as errors. + /// + private ISet _warningsAsErrors = null; + /// /// The location of the toolset definitions. /// @@ -320,6 +325,7 @@ private BuildParameters(BuildParameters other) _disableInProcNode = other._disableInProcNode; _logTaskInputs = other._logTaskInputs; _logInitialPropertiesAndItems = other._logInitialPropertiesAndItems; + _warningsAsErrors = other._warningsAsErrors == null ? null : new HashSet(other._warningsAsErrors, StringComparer.OrdinalIgnoreCase); } #if FEATURE_THREAD_PRIORITY @@ -583,6 +589,15 @@ public bool OnlyLogCriticalEvents set { _onlyLogCriticalEvents = value; } } + /// + /// A list of warnings to treat as errors. To treat all warnings as errors, set this to an empty . + /// + public ISet WarningsAsErrors + { + get { return _warningsAsErrors; } + set { _warningsAsErrors = value; } + } + /// /// Locations to search for toolsets. /// diff --git a/src/XMakeBuildEngine/BackEnd/Components/Logging/BuildEventArgTransportSink.cs b/src/XMakeBuildEngine/BackEnd/Components/Logging/BuildEventArgTransportSink.cs index 2305a0e6c55..bce7890e6de 100644 --- a/src/XMakeBuildEngine/BackEnd/Components/Logging/BuildEventArgTransportSink.cs +++ b/src/XMakeBuildEngine/BackEnd/Components/Logging/BuildEventArgTransportSink.cs @@ -81,6 +81,19 @@ public bool HaveLoggedBuildFinishedEvent set; } + /// + /// This property is ignored by this event sink and relies on the receiver to treat warnings as errors. + /// + public ISet WarningsAsErrors + { + get; + set; + } + + /// + /// This property is ignored by this event sink and relies on the receiver to keep track of whether or not any errors have been logged. + /// + public ISet BuildSubmissionIdsThatHaveLoggedErrors { get; } = null; #endregion #region IBuildEventSink Methods diff --git a/src/XMakeBuildEngine/BackEnd/Components/Logging/EventSourceSink.cs b/src/XMakeBuildEngine/BackEnd/Components/Logging/EventSourceSink.cs index 7b11f16d6d1..5e7009663c9 100644 --- a/src/XMakeBuildEngine/BackEnd/Components/Logging/EventSourceSink.cs +++ b/src/XMakeBuildEngine/BackEnd/Components/Logging/EventSourceSink.cs @@ -5,6 +5,7 @@ //----------------------------------------------------------------------- using System; +using System.Collections.Generic; using Microsoft.Build.Framework; using Microsoft.Build.Shared; @@ -129,6 +130,24 @@ public bool HaveLoggedBuildFinishedEvent get; set; } + + /// + /// A list of warnings to treat as errors. If null, nothing is treated as an error. If an empty set, all warnings are treated as errors. + /// + public ISet WarningsAsErrors + { + get; + set; + } + + /// + /// A list of build submission IDs that have logged errors. If an error is logged outside of a submission, the submission ID is "0". + /// + public ISet BuildSubmissionIdsThatHaveLoggedErrors + { + get; + } = new HashSet(); + #endregion #region Methods @@ -203,7 +222,34 @@ public void Consume(BuildEventArgs buildEvent) } else if (buildEvent is BuildWarningEventArgs) { - this.RaiseWarningEvent(null, (BuildWarningEventArgs)buildEvent); + BuildWarningEventArgs warningEvent = (BuildWarningEventArgs) buildEvent; + + // Treat this warning as an error if an empty set of warnings was specified or this code was specified + if (WarningsAsErrors != null && (WarningsAsErrors.Count == 0 || WarningsAsErrors.Contains(warningEvent.Code))) + { + BuildErrorEventArgs errorEvent = new BuildErrorEventArgs( + warningEvent.Subcategory, + warningEvent.Code, + warningEvent.File, + warningEvent.LineNumber, + warningEvent.ColumnNumber, + warningEvent.EndLineNumber, + warningEvent.EndColumnNumber, + warningEvent.Message, + warningEvent.HelpKeyword, + warningEvent.SenderName, + warningEvent.Timestamp) + { + BuildEventContext = warningEvent.BuildEventContext, + ProjectFile = warningEvent.ProjectFile, + }; + + this.RaiseErrorEvent(null, errorEvent); + } + else + { + this.RaiseWarningEvent(null, warningEvent); + } } else if (buildEvent is BuildErrorEventArgs) { @@ -308,6 +354,9 @@ private void RaiseMessageEvent(object sender, BuildMessageEventArgs buildEvent) /// ExceptionHandling.IsCriticalException exceptions will not be wrapped private void RaiseErrorEvent(object sender, BuildErrorEventArgs buildEvent) { + // Keep track of build submissions that have logged errors. If there is no build context, add a submission ID of "-1" + BuildSubmissionIdsThatHaveLoggedErrors.Add(buildEvent != null && buildEvent.BuildEventContext != null ? buildEvent.BuildEventContext.SubmissionId : -1); + if (ErrorRaised != null) { try diff --git a/src/XMakeBuildEngine/BackEnd/Components/Logging/ILoggingService.cs b/src/XMakeBuildEngine/BackEnd/Components/Logging/ILoggingService.cs index 6aab65dfc10..9e4f0c1e068 100644 --- a/src/XMakeBuildEngine/BackEnd/Components/Logging/ILoggingService.cs +++ b/src/XMakeBuildEngine/BackEnd/Components/Logging/ILoggingService.cs @@ -153,8 +153,24 @@ bool RunningOnRemoteNode get; set; } + + /// + /// Set of warnings to treat as errors. An empty non-null set will treat all warnings as errors. + /// + ISet WarningsAsErrors + { + get; + set; + } #endregion + /// + /// Determines if the specified submission has logged an errors. + /// + /// The ID of the build submission. A value of "0" means that an error was logged outside of any build submission. + /// true if the build submission logged an errors, otherwise false. + bool HasBuildSubmissionLoggedErrors(int submissionId); + #region Register /// @@ -447,6 +463,20 @@ bool HaveLoggedBuildFinishedEvent set; } + /// + /// A list of warnings to treat as errors. If null, nothing is treated as an error. If an empty set, all warnings are treated as errors. + /// + ISet WarningsAsErrors + { + get; + set; + } + + /// + /// A list of build submissions that have logged errors. + /// + ISet BuildSubmissionIdsThatHaveLoggedErrors { get; } + #endregion /// /// Entry point for a sink to consume an event. @@ -461,7 +491,7 @@ bool HaveLoggedBuildFinishedEvent void Consume(BuildEventArgs buildEvent); /// - /// Shutsdown the sink and any resources it may be holding + /// Shuts down the sink and any resources it may be holding /// void ShutDown(); } diff --git a/src/XMakeBuildEngine/BackEnd/Components/Logging/LoggingService.cs b/src/XMakeBuildEngine/BackEnd/Components/Logging/LoggingService.cs index 55d343b4f2f..3f98aa03119 100644 --- a/src/XMakeBuildEngine/BackEnd/Components/Logging/LoggingService.cs +++ b/src/XMakeBuildEngine/BackEnd/Components/Logging/LoggingService.cs @@ -9,6 +9,7 @@ using System.Collections.Generic; using System.Globalization; using System.IO; +using System.Linq; using System.Reflection; using System.Text; using System.Threading; @@ -218,6 +219,11 @@ internal partial class LoggingService : ILoggingService, INodePacketHandler, IBu /// private LoggerMode _logMode = NativeMethodsShared.IsMono ? LoggerMode.Synchronous : LoggerMode.Asynchronous; + /// + /// A list of warnings to treat as errors. + /// + private ISet _warningAsErrors = null; + #endregion #endregion @@ -444,6 +450,27 @@ public LoggerMode LoggingMode } } + /// + /// Get of warnings to treat as errors. An empty non-null set will treat all warnings as errors. + /// + public ISet WarningsAsErrors + { + get { return _warningAsErrors; } + set { _warningAsErrors = value; } + } + + /// + /// Determines if the specified submission has logged an errors. + /// + /// The ID of the build submission. A value of "0" means that an error was logged outside of any build submission. + /// true if the build submission logged an errors, otherwise false. + public bool HasBuildSubmissionLoggedErrors(int submissionId) + { + // Determine if any of the event sinks have logged an error with this submission ID + return (_filterEventSource != null && _filterEventSource.BuildSubmissionIdsThatHaveLoggedErrors.Contains(submissionId)) + || (_eventSinkDictionary != null && _eventSinkDictionary.Values.Any(i => i.BuildSubmissionIdsThatHaveLoggedErrors.Contains(submissionId))); + } + /// /// Return whether or not the LoggingQueue has any events left in it /// @@ -784,7 +811,10 @@ public bool RegisterDistributedLogger(ILogger centralLogger, LoggerDescription f IForwardingLogger localForwardingLogger = null; // create an eventSourceSink which the central logger will register with to receive the events from the forwarding logger - EventSourceSink eventSourceSink = new EventSourceSink(); + EventSourceSink eventSourceSink = new EventSourceSink + { + WarningsAsErrors = WarningsAsErrors == null ? null : new HashSet(WarningsAsErrors, StringComparer.OrdinalIgnoreCase) + }; // If the logger is already in the list it should not be registered again. if (_iloggerList.Contains(centralLogger)) @@ -1096,8 +1126,11 @@ private void CreateFilterEventSource() { if (_filterEventSource == null) { - _filterEventSource = new EventSourceSink(); - _filterEventSource.Name = "Sink for Distributed/Filter loggers"; + _filterEventSource = new EventSourceSink + { + Name = "Sink for Distributed/Filter loggers", + WarningsAsErrors = WarningsAsErrors == null ? null : new HashSet(WarningsAsErrors, StringComparer.OrdinalIgnoreCase) + }; } } diff --git a/src/XMakeBuildEngine/BackEnd/Shared/BuildResult.cs b/src/XMakeBuildEngine/BackEnd/Shared/BuildResult.cs index a6fa40adb9f..318b4cd6011 100644 --- a/src/XMakeBuildEngine/BackEnd/Shared/BuildResult.cs +++ b/src/XMakeBuildEngine/BackEnd/Shared/BuildResult.cs @@ -639,6 +639,15 @@ internal BuildResult Clone() return result; } + /// + /// Sets the overall result. + /// + /// true if the result is success, otherwise false. + internal void SetOverallResult(bool overallResult) + { + _baseOverallResult = false; + } + /// /// Creates the target result dictionary. /// diff --git a/src/XMakeBuildEngine/UnitTests/BackEnd/BuildManager_Tests.cs b/src/XMakeBuildEngine/UnitTests/BackEnd/BuildManager_Tests.cs index dfdadd786c7..0d02eea0cfc 100644 --- a/src/XMakeBuildEngine/UnitTests/BackEnd/BuildManager_Tests.cs +++ b/src/XMakeBuildEngine/UnitTests/BackEnd/BuildManager_Tests.cs @@ -3594,6 +3594,101 @@ public void Regress265010() } } + /// + /// Verifies that all warnings are treated as errors and that the overall build result is a failure. + /// + [Fact] + public void WarningsAreTreatedAsErrorsAll() + { + string contents = ObjectModelHelpers.CleanupFileContents(@" + + + + + + +"); + _parameters.WarningsAsErrors = new HashSet(); + + Project project = CreateProject(contents, ObjectModelHelpers.MSBuildDefaultToolsVersion, _projectCollection, true); + ProjectInstance instance = _buildManager.GetProjectInstanceForBuild(project); + _buildManager.BeginBuild(_parameters); + BuildResult result1 = _buildManager.BuildRequest(new BuildRequestData(instance, new string[] { "target1" })); + _buildManager.EndBuild(); + + Assert.Equal(0, _logger.WarningCount); + Assert.Equal(2, _logger.ErrorCount); + + Assert.Equal(BuildResultCode.Failure, result1.OverallResult); + Assert.True(result1.HasResultsForTarget("target1")); + } + + /// + /// Verifies that only the specified warnings are treated as errors and that the overall build result is a failure. + /// + [Fact] + public void WarningsAreTreatedAsErrorsSpecific() + { + string contents = ObjectModelHelpers.CleanupFileContents(@" + + + + + + + +"); + _parameters.WarningsAsErrors = new HashSet { "ABC123" }; + + Project project = CreateProject(contents, ObjectModelHelpers.MSBuildDefaultToolsVersion, _projectCollection, true); + ProjectInstance instance = _buildManager.GetProjectInstanceForBuild(project); + _buildManager.BeginBuild(_parameters); + BuildResult result1 = _buildManager.BuildRequest(new BuildRequestData(instance, new string[] { "target1" })); + _buildManager.EndBuild(); + + Assert.Equal(2, _logger.WarningCount); + Assert.Equal(1, _logger.ErrorCount); + + Assert.Equal(BuildResultCode.Failure, result1.OverallResult); + Assert.True(result1.HasResultsForTarget("target1")); + } + + /// + /// Verifies that when building targets which emit warnings, they still show as succeeding but the overall build result is a failure. + /// + [Fact] + public void WarningsAreTreatedAsErrorsButTargetsStillSucceed() + { + string contents = ObjectModelHelpers.CleanupFileContents(@" + + + + + + + + +"); + _parameters.WarningsAsErrors = new HashSet { "ABC123" }; + + Project project = CreateProject(contents, ObjectModelHelpers.MSBuildDefaultToolsVersion, _projectCollection, true); + ProjectInstance instance = _buildManager.GetProjectInstanceForBuild(project); + _buildManager.BeginBuild(_parameters); + BuildResult buildResult = _buildManager.BuildRequest(new BuildRequestData(instance, new string[] { "target1", "target2" })); + _buildManager.EndBuild(); + + Assert.Equal(0, _logger.WarningCount); + Assert.Equal(1, _logger.ErrorCount); + + Assert.Equal(BuildResultCode.Failure, buildResult.OverallResult); + Assert.True(buildResult.HasResultsForTarget("target1")); + Assert.True(buildResult.HasResultsForTarget("target2")); + // The two targets should still show as success because they don't know their warning was changed to an error + // Logging a warning as an error does not change execution, only the final result of the build + Assert.Equal(TargetResultCode.Success, buildResult.ResultsByTarget["target1"].ResultCode); + Assert.Equal(TargetResultCode.Success, buildResult.ResultsByTarget["target2"].ResultCode); + } + /// /// Helper for cache tests. Builds a project and verifies the right cache files are created. /// diff --git a/src/XMakeBuildEngine/UnitTests/BackEnd/EventSourceSink_Tests.cs b/src/XMakeBuildEngine/UnitTests/BackEnd/EventSourceSink_Tests.cs index 553eecaa7af..e60d99a46de 100644 --- a/src/XMakeBuildEngine/UnitTests/BackEnd/EventSourceSink_Tests.cs +++ b/src/XMakeBuildEngine/UnitTests/BackEnd/EventSourceSink_Tests.cs @@ -83,6 +83,93 @@ public void ConsumeEventsGoodEventsNoHandlers() eventHelper.RaiseBuildEvent(RaiseEventHelper.GenericStatusEvent); } + /// + /// Verifies that a warning is logged as an error when it's warning code specified. + /// + [Fact] + public void TreatWarningAsErrorWhenSpecified() + { + BuildWarningEventArgs expectedBuildEvent = RaiseEventHelper.Warning; + + EventSourceSink eventSourceSink = new EventSourceSink() + { + WarningsAsErrors = new HashSet + { + "123", + expectedBuildEvent.Code, + "ABC", + }, + }; + + RaiseEventHelper raiseEventHelper = new RaiseEventHelper(eventSourceSink); + EventHandlerHelper eventHandlerHelper = new EventHandlerHelper(eventSourceSink, null); + + raiseEventHelper.RaiseBuildEvent(RaiseEventHelper.Warning); + + Assert.IsType(eventHandlerHelper.RaisedEvent); + + BuildErrorEventArgs actualBuildEvent = (BuildErrorEventArgs) eventHandlerHelper.RaisedEvent; + + Assert.Equal(expectedBuildEvent.Code, actualBuildEvent.Code); + Assert.Equal(expectedBuildEvent.File, actualBuildEvent.File); + Assert.Equal(expectedBuildEvent.ProjectFile, actualBuildEvent.ProjectFile); + Assert.Equal(expectedBuildEvent.Subcategory, actualBuildEvent.Subcategory); + Assert.Equal(expectedBuildEvent.HelpKeyword, actualBuildEvent.HelpKeyword); + Assert.Equal(expectedBuildEvent.Message, actualBuildEvent.Message); + Assert.Equal(expectedBuildEvent.SenderName, actualBuildEvent.SenderName); + Assert.Equal(expectedBuildEvent.ColumnNumber, actualBuildEvent.ColumnNumber); + Assert.Equal(expectedBuildEvent.EndColumnNumber, actualBuildEvent.EndColumnNumber); + Assert.Equal(expectedBuildEvent.EndLineNumber, actualBuildEvent.EndLineNumber); + Assert.Equal(expectedBuildEvent.LineNumber, actualBuildEvent.LineNumber); + Assert.Equal(expectedBuildEvent.BuildEventContext, actualBuildEvent.BuildEventContext); + Assert.Equal(expectedBuildEvent.ThreadId, actualBuildEvent.ThreadId); + Assert.Equal(expectedBuildEvent.Timestamp, actualBuildEvent.Timestamp); + } + + /// + /// Verifies that a warning is not treated as an error when other warning codes are specified. + /// + [Fact] + public void NotTreatWarningAsErrorWhenNotSpecified() + { + BuildWarningEventArgs expectedBuildEvent = RaiseEventHelper.Warning; + + EventSourceSink eventSourceSink = new EventSourceSink() + { + WarningsAsErrors = new HashSet + { + "123", + "ABC", + }, + }; + + RaiseEventHelper raiseEventHelper = new RaiseEventHelper(eventSourceSink); + EventHandlerHelper eventHandlerHelper = new EventHandlerHelper(eventSourceSink, null); + + raiseEventHelper.RaiseBuildEvent(RaiseEventHelper.Warning); + + Assert.Equal(expectedBuildEvent, eventHandlerHelper.RaisedEvent); + } + + /// + /// Verifies that a warning is not treated as an error when other warning codes are specified. + /// + [Fact] + public void TreatWarningAsErrorWhenAllSpecified() + { + EventSourceSink eventSourceSink = new EventSourceSink() + { + WarningsAsErrors = new HashSet(), + }; + + RaiseEventHelper raiseEventHelper = new RaiseEventHelper(eventSourceSink); + EventHandlerHelper eventHandlerHelper = new EventHandlerHelper(eventSourceSink, null); + + raiseEventHelper.RaiseBuildEvent(RaiseEventHelper.Warning); + + Assert.IsType(eventHandlerHelper.RaisedEvent); + } + #region TestsThrowingLoggingExceptions /// diff --git a/src/XMakeBuildEngine/UnitTests/BackEnd/MockLoggingService.cs b/src/XMakeBuildEngine/UnitTests/BackEnd/MockLoggingService.cs index 7ec91aa0fa6..c3559fac92b 100644 --- a/src/XMakeBuildEngine/UnitTests/BackEnd/MockLoggingService.cs +++ b/src/XMakeBuildEngine/UnitTests/BackEnd/MockLoggingService.cs @@ -147,6 +147,16 @@ public string[] PropertiesToSerialize set; } + /// + /// List of warnings to treat as errors. + /// + public ISet WarningsAsErrors + { + get; + set; + } + + /// /// Is the logging service on a remote node, this is used to determine if properties need to be serialized /// @@ -475,6 +485,11 @@ public void LogTelemetry(BuildEventContext buildEventContext, string eventName, { } + public bool HasBuildSubmissionLoggedErrors(int submissionId) + { + return false; + } + #endregion } } diff --git a/src/XMakeCommandLine/CommandLineSwitches.cs b/src/XMakeCommandLine/CommandLineSwitches.cs index 8dba43b5e11..bb528896521 100644 --- a/src/XMakeCommandLine/CommandLineSwitches.cs +++ b/src/XMakeCommandLine/CommandLineSwitches.cs @@ -102,6 +102,7 @@ internal enum ParameterizedSwitch ClientToServerPipeHandle, ServerToClientPipeHandle, #endif + WarningsAsErrors, NumberOfParameterizedSwitches } @@ -273,8 +274,9 @@ bool unquoteParameters new ParameterizedSwitchInfo( new string[] { "preprocess", "pp" }, ParameterizedSwitch.Preprocess, null, false, null, true ), #if !FEATURE_NAMED_PIPES_FULL_DUPLEX new ParameterizedSwitchInfo( new string[] { "clientToServerPipeHandle", "c2s" }, ParameterizedSwitch.ClientToServerPipeHandle, null, false, null, true ), - new ParameterizedSwitchInfo( new string[] { "serverToClientPipeHandle", "s2c" }, ParameterizedSwitch.ServerToClientPipeHandle, null, false, null, true ) + new ParameterizedSwitchInfo( new string[] { "serverToClientPipeHandle", "s2c" }, ParameterizedSwitch.ServerToClientPipeHandle, null, false, null, true ), #endif + new ParameterizedSwitchInfo( new string[] { "warnaserror", "err" }, ParameterizedSwitch.WarningsAsErrors, null, true, null, true ), }; /// diff --git a/src/XMakeCommandLine/Resources/Strings.resx b/src/XMakeCommandLine/Resources/Strings.resx index f17e740be32..bc16c2675d3 100644 --- a/src/XMakeCommandLine/Resources/Strings.resx +++ b/src/XMakeCommandLine/Resources/Strings.resx @@ -574,6 +574,25 @@ Copyright (C) Microsoft Corporation. All rights reserved. MSBuild XML and any tasks and loggers it uses. + + /warnaserror[:code[;code2]] + List of warning codes to treats as errors. Use a semicolon + or a comma to separate multiple warning codes. To treat all + warnings as errors use the switch with no values. + (Short form: /err[:c;[c2]]) + + Example: + /warnaserror:MSB4130 + + When a warning is treated as an error the target will + continue to execute as if it was a warning but the overall + build result will show as failed. + + + LOCALIZATION: "warnaserror" should not be localized. + LOCALIZATION: None of the lines should be longer than a standard width console window, eg 80 chars. + + MSBUILD : Configuration error MSB1043: The application could not start. {0} diff --git a/src/XMakeCommandLine/UnitTests/CommandLineSwitches_Tests.cs b/src/XMakeCommandLine/UnitTests/CommandLineSwitches_Tests.cs index a138a7d9531..c0c7d49fafb 100644 --- a/src/XMakeCommandLine/UnitTests/CommandLineSwitches_Tests.cs +++ b/src/XMakeCommandLine/UnitTests/CommandLineSwitches_Tests.cs @@ -4,8 +4,10 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Globalization; using System.IO; - +using System.Linq; +using System.Resources; using Microsoft.Build.CommandLine; using Microsoft.Build.Construction; using Microsoft.Build.Framework; @@ -1090,7 +1092,8 @@ public void InvalidToolsVersionErrors() true, new StringWriter(), false, - false); + false, + warningsAsErrors: null); } finally { @@ -1152,6 +1155,127 @@ public void ExtractAnyLoggerParameterPickLast() Assert.Equal("v=q", result); } + /// + /// Verifies that when the /warnaserror switch is not specified, the set of warnings is null. + /// + [Fact] + public void ProcessWarnAsErrorSwitchNotSpecified() + { + CommandLineSwitches commandLineSwitches = new CommandLineSwitches(); + + MSBuildApp.GatherCommandLineSwitches(new ArrayList(new[] { "" }), commandLineSwitches); + + Assert.Null(MSBuildApp.ProcessWarnAsErrorSwitch(commandLineSwitches)); + } + + /// + /// Verifies that the /warnaserror switch is parsed properly when codes are specified. + /// + [Fact] + public void ProcessWarnAsErrorSwitchWithCodes() + { + ISet expectedWarningsAsErors = new HashSet(StringComparer.OrdinalIgnoreCase) { "a", "B", "c", "D", "e" }; + + CommandLineSwitches commandLineSwitches = new CommandLineSwitches(); + + MSBuildApp.GatherCommandLineSwitches(new ArrayList(new[] + { + "\"/warnaserror: a,B , c \"", // Leading, trailing, leading and trailing whitespace + "/warnaserror:A,b,C", // Repeats of different case + "\"/warnaserror:, ,,\"", // Empty items + "/err:D,d,E,e", // A different source with new items and uses the short form + "/warnaserror:a", // A different source with a single duplicate + "/warnaserror:a,b", // A different source with multiple duplicates + "/warnaserror", // Nothing specified + }), commandLineSwitches); + + ISet actualWarningsAsErrors = MSBuildApp.ProcessWarnAsErrorSwitch(commandLineSwitches); + + Assert.NotNull(actualWarningsAsErrors); + + Assert.Equal(expectedWarningsAsErors, actualWarningsAsErrors, StringComparer.OrdinalIgnoreCase); + } + + /// + /// Verifies that the /warnaserror switch is parsed properly when no codes are specified. + /// + [Fact] + public void ProcessWarnAsErrorSwitchEmpty() + { + CommandLineSwitches commandLineSwitches = new CommandLineSwitches(); + + MSBuildApp.GatherCommandLineSwitches(new ArrayList(new [] { "/warnaserror" }), commandLineSwitches); + + ISet actualWarningsAsErrors = MSBuildApp.ProcessWarnAsErrorSwitch(commandLineSwitches); + + Assert.NotNull(actualWarningsAsErrors); + + Assert.Equal(0, actualWarningsAsErrors.Count); + } + + /// + /// Verifies that help messages are correctly formed with the right width and leading spaces. + /// + [Fact] + public void HelpMessagesAreValid() + { + ResourceManager resourceManager = new ResourceManager("MSBuild.Strings", typeof(AssemblyResources).Assembly); + + const string switchLeadingSpaces = " "; + const string otherLineLeadingSpaces = " "; + const string examplesLeadingSpaces = " "; + + foreach (KeyValuePair item in resourceManager.GetResourceSet(CultureInfo.CurrentUICulture, createIfNotExists: true, tryParents: true) + .Cast().Where(i => i.Key is string && ((string)i.Key).StartsWith("HelpMessage_")) + .Select(i => new KeyValuePair((string)i.Key, (string)i.Value))) + { + string[] helpMessageLines = item.Value.Split(new[] { Environment.NewLine }, StringSplitOptions.RemoveEmptyEntries); + + for (int i = 0; i < helpMessageLines.Length; i++) + { + // All lines should be 80 characters or less + Assert.True(helpMessageLines[i].Length <= 80, $"Line {i + 1} of '{item.Key}' should be no longer than 80 characters."); + + if (i == 0) + { + if (helpMessageLines[i].Trim().StartsWith("/") || helpMessageLines[i].Trim().StartsWith("@")) + { + // If the first line in a switch it needs a certain amount of leading spaces + Assert.True(helpMessageLines[i].StartsWith(switchLeadingSpaces), $"Line {i + 1} of '{item.Key}' should start with '{switchLeadingSpaces}'."); + } + else + { + // Otherwise it should have no leading spaces because it's a section + Assert.False(helpMessageLines[i].StartsWith(" "), $"Line {i + 1} of '{item.Key}' should not have any leading spaces."); + } + } + else + { + // Ignore empty lines + if (!String.IsNullOrWhiteSpace(helpMessageLines[i])) + { + + if (item.Key.Contains("Examples")) + { + // Examples require a certain number of leading spaces + Assert.True(helpMessageLines[i].StartsWith(examplesLeadingSpaces), $"Line {i + 1} of '{item.Key}' should start with '{examplesLeadingSpaces}'."); + } + else if (helpMessageLines[i].Trim().StartsWith("/") || helpMessageLines[i].Trim().StartsWith("@")) + { + // Switches require a certain number of leading spaces + Assert.True(helpMessageLines[i].StartsWith(switchLeadingSpaces), $"Line {i + 1} of '{item.Key}' should start with '{switchLeadingSpaces}'."); + } + else + { + // All other lines require a certain number of leading spaces + Assert.True(helpMessageLines[i].StartsWith(otherLineLeadingSpaces), $"Line {i + 1} of '{item.Key}' should start with '{otherLineLeadingSpaces}'."); + } + } + } + } + } + } + /// /// Verifies that a switch collection has an error registered for the given command line arg. /// diff --git a/src/XMakeCommandLine/XMake.cs b/src/XMakeCommandLine/XMake.cs index e039d48f342..b92cfa1be0e 100644 --- a/src/XMakeCommandLine/XMake.cs +++ b/src/XMakeCommandLine/XMake.cs @@ -11,6 +11,7 @@ using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.IO; +using System.Linq; using System.Reflection; using System.Security; using System.Text; @@ -545,6 +546,7 @@ string [] commandLine TextWriter preprocessWriter = null; bool debugger = false; bool detailedSummary = false; + ISet warningsAsErrors = null; CommandLineSwitches switchesFromAutoResponseFile; CommandLineSwitches switchesNotFromAutoResponseFile; @@ -571,6 +573,7 @@ string [] commandLine ref preprocessWriter, ref debugger, ref detailedSummary, + ref warningsAsErrors, recursing: false )) { @@ -599,7 +602,7 @@ string [] commandLine #if FEATURE_XML_SCHEMA_VALIDATION needToValidateProject, schemaFile, #endif - cpuCount, enableNodeReuse, preprocessWriter, debugger, detailedSummary)) + cpuCount, enableNodeReuse, preprocessWriter, debugger, detailedSummary, warningsAsErrors)) { exitType = ExitType.BuildError; } @@ -890,7 +893,8 @@ internal static bool BuildProject bool enableNodeReuse, TextWriter preprocessWriter, bool debugger, - bool detailedSummary + bool detailedSummary, + ISet warningsAsErrors ) { if (String.Equals(Path.GetExtension(projectFile), ".vcproj", StringComparison.OrdinalIgnoreCase) || @@ -1054,6 +1058,7 @@ bool detailedSummary parameters.ToolsetDefinitionLocations = Microsoft.Build.Evaluation.ToolsetDefinitionLocations.ConfigurationFile | Microsoft.Build.Evaluation.ToolsetDefinitionLocations.Registry; parameters.DetailedSummary = detailedSummary; parameters.LogTaskInputs = logTaskInputs; + parameters.WarningsAsErrors = warningsAsErrors; if (!String.IsNullOrEmpty(toolsVersion)) { @@ -1785,6 +1790,7 @@ private static bool ProcessCommandLineSwitches ref TextWriter preprocessWriter, ref bool debugger, ref bool detailedSummary, + ref ISet warningsAsErrors, bool recursing ) { @@ -1888,6 +1894,7 @@ bool recursing ref preprocessWriter, ref debugger, ref detailedSummary, + ref warningsAsErrors, recursing: true ); } @@ -1922,6 +1929,8 @@ bool recursing #endif detailedSummary = commandLineSwitches.IsParameterlessSwitchSet(CommandLineSwitches.ParameterlessSwitch.DetailedSummary); + warningsAsErrors = ProcessWarnAsErrorSwitch(commandLineSwitches); + // figure out which loggers are going to listen to build events string[][] groupedFileLoggerParameters = commandLineSwitches.GetFileLoggerParameters(); @@ -1966,7 +1975,7 @@ bool recursing } ErrorUtilities.VerifyThrow(!invokeBuild || !String.IsNullOrEmpty(projectFile), "We should have a project file if we're going to build."); - + return invokeBuild; } @@ -2029,6 +2038,27 @@ internal static TextWriter ProcessPreprocessSwitch(string[] parameters) return writer; } + internal static ISet ProcessWarnAsErrorSwitch(CommandLineSwitches commandLineSwitches) + { + // TODO: Parse an environment variable as well? + + if (!commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.WarningsAsErrors)) + { + return null; + } + + string[] parameters = commandLineSwitches[CommandLineSwitches.ParameterizedSwitch.WarningsAsErrors]; + + ISet warningsAsErrors = new HashSet(StringComparer.OrdinalIgnoreCase); + + foreach (string code in parameters.SelectMany(parameter => parameter.Split(new[] { ',', ';' }, StringSplitOptions.RemoveEmptyEntries).Where(i => !String.IsNullOrWhiteSpace(i)).Select(i => i.Trim()))) + { + warningsAsErrors.Add(code); + } + + return warningsAsErrors; + } + /// /// Uses the input from thinNodeMode switch to start a local node server /// @@ -3121,6 +3151,7 @@ private static void ShowHelpMessage() Console.WriteLine(AssemblyResources.GetString("HelpMessage_18_DistributedLoggerSwitch")); Console.WriteLine(AssemblyResources.GetString("HelpMessage_21_DistributedFileLoggerSwitch")); Console.WriteLine(AssemblyResources.GetString("HelpMessage_11_LoggerSwitch")); + Console.WriteLine(AssemblyResources.GetString("HelpMessage_28_WarnAsErrorSwitch")); #if FEATURE_XML_SCHEMA_VALIDATION Console.WriteLine(AssemblyResources.GetString("HelpMessage_15_ValidateSwitch")); #endif