diff --git a/documentation/wiki/ChangeWaves.md b/documentation/wiki/ChangeWaves.md index f90568b743e..ad791807d9c 100644 --- a/documentation/wiki/ChangeWaves.md +++ b/documentation/wiki/ChangeWaves.md @@ -25,7 +25,6 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t ### 17.14 - [.SLNX support - use the new parser for .sln and .slnx](https://github.com/dotnet/msbuild/pull/10836) -- [TreatWarningsAsErrors, WarningsAsMessages, WarningsAsErrors, WarningsNotAsErrors are now supported on the engine side of MSBuild](https://github.com/dotnet/msbuild/pull/10942) - [Support custom culture in RAR](https://github.com/dotnet/msbuild/pull/11000) ### 17.12 diff --git a/src/Build.UnitTests/WarningsAsMessagesAndErrors_Tests.cs b/src/Build.UnitTests/WarningsAsMessagesAndErrors_Tests.cs index 6041fbc45ac..db2d9eab3ad 100644 --- a/src/Build.UnitTests/WarningsAsMessagesAndErrors_Tests.cs +++ b/src/Build.UnitTests/WarningsAsMessagesAndErrors_Tests.cs @@ -35,19 +35,6 @@ 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 /// @@ -104,6 +91,22 @@ 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() { @@ -174,99 +177,22 @@ 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] - /// - /// 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() + public void TreatWarningsAsMessagesWhenSpecifiedThroughAdditiveProperty() { MockLogger logger = ObjectModelHelpers.BuildProjectExpectSuccess( GetTestProject( customProperties: new List> { new KeyValuePair("MSBuildWarningsAsMessages", "123"), - new KeyValuePair("WarningsAsMessages", $@"$(MSBuildWarningsAsMessages); + new KeyValuePair("MSBuildWarningsAsMessages", $@"$(MSBuildWarningsAsMessages); {ExpectedEventCode.ToLowerInvariant()}"), - new KeyValuePair("MSBuildWarningsAsMessages", "$(WarningsAsMessages);ABC") + new KeyValuePair("MSBuildWarningsAsMessages", "$(MSBuildWarningsAsMessages);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() { @@ -276,8 +202,7 @@ public void NotTreatWarningsAsMessagesWhenCodeNotSpecified() { new KeyValuePair("MSBuildWarningsAsMessages", "123"), new KeyValuePair("MSBuildWarningsAsMessages", "$(MSBuildWarningsAsMessages);ABC") - }), - _output); + })); VerifyBuildWarningEvent(logger); } @@ -348,33 +273,27 @@ 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 useMSPrefix = true) + bool allWarningsAreErrors = false) { using (TestEnvironment env = TestEnvironment.Create(_output)) { - var prefix = useMSPrefix ? "MSBuild" : ""; TransientTestProjectWithFiles proj = env.CreateTestProjectWithFiles($@" - <{prefix}TreatWarningsAsErrors>{allWarningsAreErrors} - <{prefix}WarningsAsMessages>{WarningsAsMessages} - <{prefix}WarningsAsErrors>{WarningsAsErrors} + {allWarningsAreErrors} + {WarningsAsMessages} + {WarningsAsErrors} @@ -391,83 +310,6 @@ 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. /// @@ -529,11 +371,8 @@ public void TaskLogsWarningAsError_BatchedBuild() [Theory] [InlineData("MSB1234", false, 1, 1)] [InlineData("MSB0000", true, 0, 2)] - [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) + public void TaskReturnsTrue_Tests(string warningsAsErrors, bool treatAllWarningsAsErrors, int warningCountShouldBe, int errorCountShouldBe) { - string prefix = useMSPrefix ? "MSBuild" : ""; using (TestEnvironment env = TestEnvironment.Create(_output)) { TransientTestProjectWithFiles proj = env.CreateTestProjectWithFiles($@" @@ -541,8 +380,8 @@ public void TaskReturnsTrue_Tests(string warningsAsErrors, bool treatAllWarnings - <{prefix}TreatWarningsAsErrors>{treatAllWarningsAsErrors} - <{prefix}WarningsAsErrors>{warningsAsErrors} + {treatAllWarningsAsErrors} + {warningsAsErrors} diff --git a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs index a5b8e6b9717..b1c05600fae 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs @@ -1390,17 +1390,14 @@ 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.MSBuildPrefix + MSBuildConstants.TreatWarningsAsErrors)?.Trim(), "true", StringComparison.OrdinalIgnoreCase) || - (String.Equals(project.GetEngineRequiredPropertyValue(MSBuildConstants.TreatWarningsAsErrors)?.Trim(), "true", StringComparison.OrdinalIgnoreCase) && - ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_14))) + if (String.Equals(project.GetEngineRequiredPropertyValue(MSBuildConstants.TreatWarningsAsErrors)?.Trim(), "true", StringComparison.OrdinalIgnoreCase)) { // If signals the IEventSourceSink to treat all warnings as errors loggingService.AddWarningsAsErrors(buildEventContext, new HashSet()); } else { - ISet warningsAsErrors = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.MSBuildPrefix + MSBuildConstants.WarningsAsErrors), - project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsAsErrors)); + ISet warningsAsErrors = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsAsErrors)); if (warningsAsErrors?.Count > 0) { @@ -1408,17 +1405,14 @@ private void ConfigureWarningsAsErrorsAndMessages() } } - ISet warningsNotAsErrors = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.MSBuildPrefix + MSBuildConstants.WarningsNotAsErrors), - project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsNotAsErrors)); - + ISet warningsNotAsErrors = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsNotAsErrors)); if (warningsNotAsErrors?.Count > 0) { loggingService.AddWarningsNotAsErrors(buildEventContext, warningsNotAsErrors); } - ISet warningsAsMessages = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.MSBuildPrefix + MSBuildConstants.WarningsAsMessages), - project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsAsMessages)); + ISet warningsAsMessages = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsAsMessages)); if (warningsAsMessages?.Count > 0) { @@ -1436,37 +1430,14 @@ private void ConfigureKnownImmutableFolders() } } - private static ISet ParseWarningCodes(string warnings, string warningsNoPrefix) + private static ISet ParseWarningCodes(string 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)) + if (String.IsNullOrWhiteSpace(warnings)) { - result2 = new HashSet(ExpressionShredder.SplitSemiColonSeparatedList(warningsNoPrefix), StringComparer.OrdinalIgnoreCase); - } - - if (result1 != null) - { - if (result2 != null) - { - result1.UnionWith(result2); - } - return result1; + return null; } - return result2; + return new HashSet(ExpressionShredder.SplitSemiColonSeparatedList(warnings), StringComparer.OrdinalIgnoreCase); } private sealed class DedicatedThreadsTaskScheduler : TaskScheduler diff --git a/src/Shared/Constants.cs b/src/Shared/Constants.cs index 7eb6195d897..caf4f6e9eb2 100644 --- a/src/Shared/Constants.cs +++ b/src/Shared/Constants.cs @@ -28,30 +28,25 @@ 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 = "TreatWarningsAsErrors"; + internal const string TreatWarningsAsErrors = "MSBuildTreatWarningsAsErrors"; /// /// Name of the property that indicates a list of warnings to treat as errors. /// - internal const string WarningsAsErrors = "WarningsAsErrors"; + internal const string WarningsAsErrors = "MSBuildWarningsAsErrors"; /// /// Name of the property that indicates a list of warnings to not treat as errors. /// - internal const string WarningsNotAsErrors = "WarningsNotAsErrors"; + internal const string WarningsNotAsErrors = "MSBuildWarningsNotAsErrors"; /// /// Name of the property that indicates the list of warnings to treat as messages. /// - internal const string WarningsAsMessages = "WarningsAsMessages"; + internal const string WarningsAsMessages = "MSBuildWarningsAsMessages"; /// /// 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 a3f2e0f320a..f70abcc7bfa 100644 --- a/src/UnitTests.Shared/ObjectModelHelpers.cs +++ b/src/UnitTests.Shared/ObjectModelHelpers.cs @@ -782,9 +782,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, ITestOutputHelper testOutputHelper = null) + public static MockLogger BuildProjectExpectFailure(string projectContents) { - MockLogger logger = new MockLogger(testOutputHelper); + MockLogger logger = new MockLogger(); BuildProjectExpectFailure(projectContents, logger); return logger; }