diff --git a/documentation/wiki/ChangeWaves.md b/documentation/wiki/ChangeWaves.md index 09e7ca1394c..c29f35c53b3 100644 --- a/documentation/wiki/ChangeWaves.md +++ b/documentation/wiki/ChangeWaves.md @@ -23,6 +23,9 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t ## Current Rotation of Change Waves +### 17.14 +- [TreatWarningsAsErrors, WarningsAsMessages, WarningsAsErrors, WarningsNotAsErrors are now supported on the engine side of MSBuild](https://github.com/dotnet/msbuild/pull/10942) + ### 17.12 - [Log TaskParameterEvent for scalar parameters](https://github.com/dotnet/msbuild/pull/9908) - [Convert.ToString during a property evaluation uses the InvariantCulture for all types](https://github.com/dotnet/msbuild/pull/9874) diff --git a/eng/Versions.props b/eng/Versions.props index c6560cfe588..2fa36298c64 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -2,8 +2,7 @@ - 17.12.12release - release + 17.12.13release 17.11.4 15.1.0.0 preview diff --git a/src/Build.UnitTests/WarningsAsMessagesAndErrors_Tests.cs b/src/Build.UnitTests/WarningsAsMessagesAndErrors_Tests.cs index db2d9eab3ad..6041fbc45ac 100644 --- a/src/Build.UnitTests/WarningsAsMessagesAndErrors_Tests.cs +++ b/src/Build.UnitTests/WarningsAsMessagesAndErrors_Tests.cs @@ -35,6 +35,19 @@ public void TreatAllWarningsAsErrors() ObjectModelHelpers.BuildProjectExpectSuccess(GetTestProject(treatAllWarningsAsErrors: false)); } + [Fact] + public void TreatAllWarningsAsErrorsNoPrefix() + { + MockLogger logger = ObjectModelHelpers.BuildProjectExpectFailure(GetTestProject(customProperties: new Dictionary + { + {"TreatWarningsAsErrors", "true"}, + })); + + VerifyBuildErrorEvent(logger); + + ObjectModelHelpers.BuildProjectExpectSuccess(GetTestProject(treatAllWarningsAsErrors: false)); + } + /// /// https://github.com/dotnet/msbuild/issues/2667 /// @@ -91,22 +104,6 @@ public void TreatWarningsAsErrorsWhenSpecifiedIndirectly() VerifyBuildErrorEvent(logger); } - [Fact] - public void TreatWarningsAsErrorsWhenSpecifiedThroughAdditiveProperty() - { - MockLogger logger = ObjectModelHelpers.BuildProjectExpectFailure( - GetTestProject( - customProperties: new List> - { - new KeyValuePair("MSBuildWarningsAsErrors", "123"), - new KeyValuePair("MSBuildWarningsAsErrors", $@"$(MSBuildWarningsAsErrors); - {ExpectedEventCode.ToLowerInvariant()}"), - new KeyValuePair("MSBuildWarningsAsErrors", "$(MSBuildWarningsAsErrors);ABC") - })); - - VerifyBuildErrorEvent(logger); - } - [Fact] public void NotTreatWarningsAsErrorsWhenCodeNotSpecified() { @@ -177,22 +174,99 @@ public void TreatWarningsAsMessagesWhenSpecifiedIndirectly() VerifyBuildMessageEvent(logger); } + [Theory] + [InlineData(true)] + [InlineData(false)] + public void TreatWarningsAsMessagesWhenSpecifiedThroughAdditiveProperty(bool usePrefix) + { + string prefix = usePrefix ? "MSBuild" : ""; + MockLogger logger = ObjectModelHelpers.BuildProjectExpectSuccess( + GetTestProject( + customProperties: new List> + { + new KeyValuePair($"{prefix}WarningsAsMessages", "123"), + new KeyValuePair($"{prefix}WarningsAsMessages", $@"$({prefix}WarningsAsMessages); + {ExpectedEventCode.ToLowerInvariant()}"), + new KeyValuePair($"{prefix}WarningsAsMessages", $"$({prefix}WarningsAsMessages);ABC") + })); + + VerifyBuildMessageEvent(logger); + } + [Fact] - public void TreatWarningsAsMessagesWhenSpecifiedThroughAdditiveProperty() + /// + /// This is for chaining the properties together via addition. + /// Furthermore it is intended to check if the prefix and no prefix variant interacts properly with each other. + /// + public void TreatWarningsAsMessagesWhenSpecifiedThroughAdditivePropertyCombination() { MockLogger logger = ObjectModelHelpers.BuildProjectExpectSuccess( GetTestProject( customProperties: new List> { new KeyValuePair("MSBuildWarningsAsMessages", "123"), - new KeyValuePair("MSBuildWarningsAsMessages", $@"$(MSBuildWarningsAsMessages); + new KeyValuePair("WarningsAsMessages", $@"$(MSBuildWarningsAsMessages); {ExpectedEventCode.ToLowerInvariant()}"), - new KeyValuePair("MSBuildWarningsAsMessages", "$(MSBuildWarningsAsMessages);ABC") + new KeyValuePair("MSBuildWarningsAsMessages", "$(WarningsAsMessages);ABC") })); VerifyBuildMessageEvent(logger); } + [Fact] + public void TreatWarningsNotAsErrorsWhenSpecifiedThroughAdditivePropertyCombination() + { + MockLogger logger = ObjectModelHelpers.BuildProjectExpectSuccess( + GetTestProject( + customProperties: new List> + { + new KeyValuePair("MSBuildWarningsNotAsErrors", "123"), + new KeyValuePair("WarningsNotAsErrors", $@"$(MSBuildWarningsNotAsErrors); + {ExpectedEventCode.ToLowerInvariant()}"), + new KeyValuePair("MSBuildWarningsNotAsErrors", "$(WarningsNotAsErrors);ABC") + }), + _output); + + VerifyBuildWarningEvent(logger); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void TreatWarningsAsErrorsWhenSpecifiedThroughAdditiveProperty(bool MSBuildPrefix) + { + string prefix = MSBuildPrefix ? "MSBuild" : ""; + MockLogger logger = ObjectModelHelpers.BuildProjectExpectFailure( + GetTestProject( + customProperties: new List> + { + new KeyValuePair($@"{prefix}WarningsAsErrors", "123"), + new KeyValuePair($@"{prefix}WarningsAsErrors", $@"$({prefix}WarningsAsErrors); + {ExpectedEventCode.ToLowerInvariant()}"), + new KeyValuePair($@"{prefix}WarningsAsErrors", $@"$({prefix}WarningsAsErrors);ABC") + }), + _output); + + VerifyBuildErrorEvent(logger); + } + + [Fact] + public void TreatWarningsAsErrorsWhenSpecifiedThroughAdditivePropertyCombination() + { + MockLogger logger = ObjectModelHelpers.BuildProjectExpectFailure( + GetTestProject( + customProperties: new List> + { + new KeyValuePair("WarningsAsErrors", "123"), + new KeyValuePair("MSBuildWarningsAsErrors", $@"$(WarningsAsErrors); + {ExpectedEventCode.ToLowerInvariant()}"), + new KeyValuePair("WarningsAsErrors", "$(MSBuildWarningsAsErrors);ABC") + }), + _output); + + VerifyBuildErrorEvent(logger); + } + [Fact] public void NotTreatWarningsAsMessagesWhenCodeNotSpecified() { @@ -202,7 +276,8 @@ public void NotTreatWarningsAsMessagesWhenCodeNotSpecified() { new KeyValuePair("MSBuildWarningsAsMessages", "123"), new KeyValuePair("MSBuildWarningsAsMessages", "$(MSBuildWarningsAsMessages);ABC") - })); + }), + _output); VerifyBuildWarningEvent(logger); } @@ -273,27 +348,33 @@ private string GetTestProject(bool? treatAllWarningsAsErrors = null, string warn "; } + [Theory] [InlineData("MSB1235", "MSB1234", "MSB1234", "MSB1234", false)] // Log MSB1234, treat as error via MSBuildWarningsAsErrors [InlineData("MSB1235", "", "MSB1234", "MSB1234", true)] // Log MSB1234, expect MSB1234 as error via MSBuildTreatWarningsAsErrors [InlineData("MSB1234", "MSB1234", "MSB1234", "MSB4181", true)]// Log MSB1234, MSBuildWarningsAsMessages takes priority + [InlineData("MSB1235", "MSB1234", "MSB1234", "MSB1234", false, false)] // Log MSB1234, treat as error via BuildWarningsAsErrors + [InlineData("MSB1235", "", "MSB1234", "MSB1234", true, false)] // Log MSB1234, expect MSB1234 as error via BuildTreatWarningsAsErrors + [InlineData("MSB1234", "MSB1234", "MSB1234", "MSB4181", true, false)]// Log MSB1234, BuildWarningsAsMessages takes priority public void WarningsAsErrorsAndMessages_Tests(string WarningsAsMessages, string WarningsAsErrors, string WarningToLog, string LogShouldContain, - bool allWarningsAreErrors = false) + bool allWarningsAreErrors = false, + bool useMSPrefix = true) { using (TestEnvironment env = TestEnvironment.Create(_output)) { + var prefix = useMSPrefix ? "MSBuild" : ""; TransientTestProjectWithFiles proj = env.CreateTestProjectWithFiles($@" - {allWarningsAreErrors} - {WarningsAsMessages} - {WarningsAsErrors} + <{prefix}TreatWarningsAsErrors>{allWarningsAreErrors} + <{prefix}WarningsAsMessages>{WarningsAsMessages} + <{prefix}WarningsAsErrors>{WarningsAsErrors} @@ -310,6 +391,83 @@ public void WarningsAsErrorsAndMessages_Tests(string WarningsAsMessages, } } + [Theory] + + [InlineData(true)]// Log MSB1234, BuildWarningsNotAsErrors takes priority + [InlineData(false)] + public void WarningsNotAsErrorsAndMessages_Tests(bool useMSPrefix) + { + string Warning = "MSB1235"; + using (TestEnvironment env = TestEnvironment.Create(_output)) + { + string prefix = useMSPrefix ? "MSBuild" : ""; + TransientTestProjectWithFiles proj = env.CreateTestProjectWithFiles($@" + + + <{prefix}TreatWarningsAsErrors>true + <{prefix}WarningsNotAsErrors>{Warning} + + + + + "); + + MockLogger logger = proj.BuildProjectExpectSuccess(); + + logger.WarningCount.ShouldBe(1); + logger.ErrorCount.ShouldBe(0); + + logger.AssertLogContains(Warning); + } + } + + + + [Theory] + [InlineData("TreatWarningsAsErrors", "true", false)] // All warnings are treated as errors + [InlineData("WarningsAsErrors", "MSB1007", false)] + [InlineData("WarningsAsMessages", "MSB1007", false)] + [InlineData("WarningsNotAsErrors", "MSB1007", true)] + [InlineData("WarningsNotAsErrors", "MSB1007", false)] + public void WarningsChangeWaveTest(string property, string propertyData, bool treatWarningsAsErrors) + { + using (TestEnvironment env = TestEnvironment.Create(_output)) + { + string warningCode = "MSB1007"; + string treatWarningsAsErrorsCodeProperty = treatWarningsAsErrors ? "true" : ""; + env.SetEnvironmentVariable("MSBUILDDISABLEFEATURESFROMVERSION", ChangeWaves.Wave17_14.ToString()); + TransientTestProjectWithFiles proj = env.CreateTestProjectWithFiles($@" + + + {treatWarningsAsErrorsCodeProperty} + <{property}>{propertyData} + + + + + "); + if (treatWarningsAsErrors) + { + // Since the "no prefix" variations can't do anything with the change wave disabled, this should always fail. + MockLogger logger = proj.BuildProjectExpectFailure(); + logger.ErrorCount.ShouldBe(1); + logger.AssertLogContains($"error {warningCode}"); + + logger.AssertLogContains(warningCode); + } + else + { + MockLogger logger = proj.BuildProjectExpectSuccess(); + + logger.WarningCount.ShouldBe(1); + logger.AssertLogContains($"warning {warningCode}"); + logger.ErrorCount.ShouldBe(0); + + logger.AssertLogContains(warningCode); + } + } + } + /// /// Item1 and Item2 log warnings and continue, item 3 logs a warn-> error and prevents item 4 from running in the batched build. /// @@ -371,8 +529,11 @@ public void TaskLogsWarningAsError_BatchedBuild() [Theory] [InlineData("MSB1234", false, 1, 1)] [InlineData("MSB0000", true, 0, 2)] - public void TaskReturnsTrue_Tests(string warningsAsErrors, bool treatAllWarningsAsErrors, int warningCountShouldBe, int errorCountShouldBe) + [InlineData("MSB1234", false, 1, 1, false)] + [InlineData("MSB0000", true, 0, 2, false)] + public void TaskReturnsTrue_Tests(string warningsAsErrors, bool treatAllWarningsAsErrors, int warningCountShouldBe, int errorCountShouldBe, bool useMSPrefix = true) { + string prefix = useMSPrefix ? "MSBuild" : ""; using (TestEnvironment env = TestEnvironment.Create(_output)) { TransientTestProjectWithFiles proj = env.CreateTestProjectWithFiles($@" @@ -380,8 +541,8 @@ public void TaskReturnsTrue_Tests(string warningsAsErrors, bool treatAllWarnings - {treatAllWarningsAsErrors} - {warningsAsErrors} + <{prefix}TreatWarningsAsErrors>{treatAllWarningsAsErrors} + <{prefix}WarningsAsErrors>{warningsAsErrors} diff --git a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs index 99abd0ef00f..77f3399d11f 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs @@ -1390,14 +1390,17 @@ private void ConfigureWarningsAsErrorsAndMessages() // Ensure everything that is required is available at this time if (project != null && buildEventContext != null && loggingService != null && buildEventContext.ProjectInstanceId != BuildEventContext.InvalidProjectInstanceId) { - if (String.Equals(project.GetEngineRequiredPropertyValue(MSBuildConstants.TreatWarningsAsErrors)?.Trim(), "true", StringComparison.OrdinalIgnoreCase)) + if (String.Equals(project.GetEngineRequiredPropertyValue(MSBuildConstants.MSBuildPrefix + MSBuildConstants.TreatWarningsAsErrors)?.Trim(), "true", StringComparison.OrdinalIgnoreCase) || + (String.Equals(project.GetEngineRequiredPropertyValue(MSBuildConstants.TreatWarningsAsErrors)?.Trim(), "true", StringComparison.OrdinalIgnoreCase) && + ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_14))) { // If signals the IEventSourceSink to treat all warnings as errors loggingService.AddWarningsAsErrors(buildEventContext, new HashSet()); } else { - ISet warningsAsErrors = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsAsErrors)); + ISet warningsAsErrors = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.MSBuildPrefix + MSBuildConstants.WarningsAsErrors), + project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsAsErrors)); if (warningsAsErrors?.Count > 0) { @@ -1405,14 +1408,17 @@ private void ConfigureWarningsAsErrorsAndMessages() } } - ISet warningsNotAsErrors = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsNotAsErrors)); + ISet warningsNotAsErrors = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.MSBuildPrefix + MSBuildConstants.WarningsNotAsErrors), + project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsNotAsErrors)); + if (warningsNotAsErrors?.Count > 0) { loggingService.AddWarningsNotAsErrors(buildEventContext, warningsNotAsErrors); } - ISet warningsAsMessages = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsAsMessages)); + ISet warningsAsMessages = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.MSBuildPrefix + MSBuildConstants.WarningsAsMessages), + project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsAsMessages)); if (warningsAsMessages?.Count > 0) { @@ -1430,14 +1436,37 @@ private void ConfigureKnownImmutableFolders() } } - private static ISet ParseWarningCodes(string warnings) + private static ISet ParseWarningCodes(string warnings, string warningsNoPrefix) { - if (String.IsNullOrWhiteSpace(warnings)) + // When this changewave is rotated out and this gets deleted, please consider removing + // the $(NoWarn) + // and the two following lines from the msbuild/src/Tasks/Microsoft.Common.CurrentVersion.targets + if (!ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_14)) + { + warningsNoPrefix = null; + } + + HashSet result1 = null; + if (!String.IsNullOrWhiteSpace(warnings)) + { + result1 = new HashSet(ExpressionShredder.SplitSemiColonSeparatedList(warnings), StringComparer.OrdinalIgnoreCase); + } + HashSet result2 = null; + if (!String.IsNullOrWhiteSpace(warningsNoPrefix)) { - return null; + result2 = new HashSet(ExpressionShredder.SplitSemiColonSeparatedList(warningsNoPrefix), StringComparer.OrdinalIgnoreCase); + } + + if (result1 != null) + { + if (result2 != null) + { + result1.UnionWith(result2); + } + return result1; } - return new HashSet(ExpressionShredder.SplitSemiColonSeparatedList(warnings), StringComparer.OrdinalIgnoreCase); + return result2; } private sealed class DedicatedThreadsTaskScheduler : TaskScheduler diff --git a/src/Framework/ChangeWaves.cs b/src/Framework/ChangeWaves.cs index 1d682c4fc75..8e58f93835c 100644 --- a/src/Framework/ChangeWaves.cs +++ b/src/Framework/ChangeWaves.cs @@ -27,7 +27,8 @@ internal static class ChangeWaves { internal static readonly Version Wave17_10 = new Version(17, 10); internal static readonly Version Wave17_12 = new Version(17, 12); - internal static readonly Version[] AllWaves = { Wave17_10, Wave17_12 }; + internal static readonly Version Wave17_14 = new Version(17, 14); + internal static readonly Version[] AllWaves = { Wave17_10, Wave17_12, Wave17_14 }; /// /// Special value indicating that all features behind all Change Waves should be enabled. diff --git a/src/Shared/Constants.cs b/src/Shared/Constants.cs index e0ac6bae417..81d03ed9a0c 100644 --- a/src/Shared/Constants.cs +++ b/src/Shared/Constants.cs @@ -28,25 +28,30 @@ internal static class MSBuildConstants /// internal const string SdksPath = "MSBuildSDKsPath"; + /// + /// The prefix that was originally used. Now extracted out for the purpose of allowing even the non-prefixed variant. + /// + internal const string MSBuildPrefix = "MSBuild"; + /// /// Name of the property that indicates that all warnings should be treated as errors. /// - internal const string TreatWarningsAsErrors = "MSBuildTreatWarningsAsErrors"; + internal const string TreatWarningsAsErrors = "TreatWarningsAsErrors"; /// /// Name of the property that indicates a list of warnings to treat as errors. /// - internal const string WarningsAsErrors = "MSBuildWarningsAsErrors"; + internal const string WarningsAsErrors = "WarningsAsErrors"; /// /// Name of the property that indicates a list of warnings to not treat as errors. /// - internal const string WarningsNotAsErrors = "MSBuildWarningsNotAsErrors"; + internal const string WarningsNotAsErrors = "WarningsNotAsErrors"; /// /// Name of the property that indicates the list of warnings to treat as messages. /// - internal const string WarningsAsMessages = "MSBuildWarningsAsMessages"; + internal const string WarningsAsMessages = "WarningsAsMessages"; /// /// The name of the environment variable that users can specify to override where NuGet assemblies are loaded from in the NuGetSdkResolver. diff --git a/src/UnitTests.Shared/ObjectModelHelpers.cs b/src/UnitTests.Shared/ObjectModelHelpers.cs index 1d560f54315..73268e06989 100644 --- a/src/UnitTests.Shared/ObjectModelHelpers.cs +++ b/src/UnitTests.Shared/ObjectModelHelpers.cs @@ -780,9 +780,9 @@ public static void BuildProjectExpectSuccess( /// /// The project file content in string format. /// The that was used during evaluation and build. - public static MockLogger BuildProjectExpectFailure(string projectContents) + public static MockLogger BuildProjectExpectFailure(string projectContents, ITestOutputHelper testOutputHelper = null) { - MockLogger logger = new MockLogger(); + MockLogger logger = new MockLogger(testOutputHelper); BuildProjectExpectFailure(projectContents, logger); return logger; }