Skip to content

Commit

Permalink
[vs17.12] Consistently respect unprefixed Warning-as-error/message/wa…
Browse files Browse the repository at this point in the history 
…rning properties (#11007)

Backports #10942 to vs17.12

* add WarningsAsMessages, WarningsAsErrors, WarningsNotAsErrors and TreatWarningsAsErrors to the engine (e.g. variant without prefix). test those so that nothing breaks

* Backport changwave 17.14 to 17.12

Normally we wouldn't have this changewave in this release, but because
we want to have a consistent changewave for behavior changed after
release, we're backporting it.

* Bump version

---------

Co-authored-by: Rainer Sigwald <raines@microsoft.com>
  • Loading branch information
@SimaTian @rainersigwald
SimaTian and rainersigwald authored Nov 21, 2024
1 parent 1cce779 commit 56cc2a0
Show file tree
Hide file tree
Showing 7 changed files with 242 additions and 44 deletions.
3 changes: 3 additions & 0 deletions documentation/wiki/ChangeWaves.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions eng/Versions.props
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
<!-- Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the MIT license. See License.txt in the project root for full license information. -->
<Project>
<PropertyGroup>
<VersionPrefix>17.12.12</VersionPrefix><DotNetFinalVersionKind>release</DotNetFinalVersionKind>
<DotNetFinalVersionKind>release</DotNetFinalVersionKind>
<VersionPrefix>17.12.13</VersionPrefix><DotNetFinalVersionKind>release</DotNetFinalVersionKind>
<PackageValidationBaselineVersion>17.11.4</PackageValidationBaselineVersion>
<AssemblyVersion>15.1.0.0</AssemblyVersion>
<PreReleaseVersionLabel>preview</PreReleaseVersionLabel>
Expand Down
215 changes: 188 additions & 27 deletions src/Build.UnitTests/WarningsAsMessagesAndErrors_Tests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,19 @@ public void TreatAllWarningsAsErrors()
ObjectModelHelpers.BuildProjectExpectSuccess(GetTestProject(treatAllWarningsAsErrors: false));
}

[Fact]
public void TreatAllWarningsAsErrorsNoPrefix()
{
MockLogger logger = ObjectModelHelpers.BuildProjectExpectFailure(GetTestProject(customProperties: new Dictionary<string, string>
{
{"TreatWarningsAsErrors", "true"},
}));

VerifyBuildErrorEvent(logger);

ObjectModelHelpers.BuildProjectExpectSuccess(GetTestProject(treatAllWarningsAsErrors: false));
}

/// <summary>
/// https://github.com/dotnet/msbuild/issues/2667
/// </summary>
Expand Down Expand Up @@ -91,22 +104,6 @@ public void TreatWarningsAsErrorsWhenSpecifiedIndirectly()
VerifyBuildErrorEvent(logger);
}

[Fact]
public void TreatWarningsAsErrorsWhenSpecifiedThroughAdditiveProperty()
{
MockLogger logger = ObjectModelHelpers.BuildProjectExpectFailure(
GetTestProject(
customProperties: new List<KeyValuePair<string, string>>
{
new KeyValuePair<string, string>("MSBuildWarningsAsErrors", "123"),
new KeyValuePair<string, string>("MSBuildWarningsAsErrors", $@"$(MSBuildWarningsAsErrors);
{ExpectedEventCode.ToLowerInvariant()}"),
new KeyValuePair<string, string>("MSBuildWarningsAsErrors", "$(MSBuildWarningsAsErrors);ABC")
}));

VerifyBuildErrorEvent(logger);
}

[Fact]
public void NotTreatWarningsAsErrorsWhenCodeNotSpecified()
{
Expand Down Expand Up @@ -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<KeyValuePair<string, string>>
{
new KeyValuePair<string, string>($"{prefix}WarningsAsMessages", "123"),
new KeyValuePair<string, string>($"{prefix}WarningsAsMessages", $@"$({prefix}WarningsAsMessages);
{ExpectedEventCode.ToLowerInvariant()}"),
new KeyValuePair<string, string>($"{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<KeyValuePair<string, string>>
{
new KeyValuePair<string, string>("MSBuildWarningsAsMessages", "123"),
new KeyValuePair<string, string>("MSBuildWarningsAsMessages", $@"$(MSBuildWarningsAsMessages);
new KeyValuePair<string, string>("WarningsAsMessages", $@"$(MSBuildWarningsAsMessages);
{ExpectedEventCode.ToLowerInvariant()}"),
new KeyValuePair<string, string>("MSBuildWarningsAsMessages", "$(MSBuildWarningsAsMessages);ABC")
new KeyValuePair<string, string>("MSBuildWarningsAsMessages", "$(WarningsAsMessages);ABC")
}));

VerifyBuildMessageEvent(logger);
}

[Fact]
public void TreatWarningsNotAsErrorsWhenSpecifiedThroughAdditivePropertyCombination()
{
MockLogger logger = ObjectModelHelpers.BuildProjectExpectSuccess(
GetTestProject(
customProperties: new List<KeyValuePair<string, string>>
{
new KeyValuePair<string, string>("MSBuildWarningsNotAsErrors", "123"),
new KeyValuePair<string, string>("WarningsNotAsErrors", $@"$(MSBuildWarningsNotAsErrors);
{ExpectedEventCode.ToLowerInvariant()}"),
new KeyValuePair<string, string>("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<KeyValuePair<string, string>>
{
new KeyValuePair<string, string>($@"{prefix}WarningsAsErrors", "123"),
new KeyValuePair<string, string>($@"{prefix}WarningsAsErrors", $@"$({prefix}WarningsAsErrors);
{ExpectedEventCode.ToLowerInvariant()}"),
new KeyValuePair<string, string>($@"{prefix}WarningsAsErrors", $@"$({prefix}WarningsAsErrors);ABC")
}),
_output);

VerifyBuildErrorEvent(logger);
}

[Fact]
public void TreatWarningsAsErrorsWhenSpecifiedThroughAdditivePropertyCombination()
{
MockLogger logger = ObjectModelHelpers.BuildProjectExpectFailure(
GetTestProject(
customProperties: new List<KeyValuePair<string, string>>
{
new KeyValuePair<string, string>("WarningsAsErrors", "123"),
new KeyValuePair<string, string>("MSBuildWarningsAsErrors", $@"$(WarningsAsErrors);
{ExpectedEventCode.ToLowerInvariant()}"),
new KeyValuePair<string, string>("WarningsAsErrors", "$(MSBuildWarningsAsErrors);ABC")
}),
_output);

VerifyBuildErrorEvent(logger);
}

[Fact]
public void NotTreatWarningsAsMessagesWhenCodeNotSpecified()
{
Expand All @@ -202,7 +276,8 @@ public void NotTreatWarningsAsMessagesWhenCodeNotSpecified()
{
new KeyValuePair<string, string>("MSBuildWarningsAsMessages", "123"),
new KeyValuePair<string, string>("MSBuildWarningsAsMessages", "$(MSBuildWarningsAsMessages);ABC")
}));
}),
_output);

VerifyBuildWarningEvent(logger);
}
Expand Down Expand Up @@ -273,27 +348,33 @@ private string GetTestProject(bool? treatAllWarningsAsErrors = null, string warn
</Project>";
}


[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($@"
<Project>
<UsingTask TaskName = ""ReturnFailureWithoutLoggingErrorTask"" AssemblyName=""Microsoft.Build.Engine.UnitTests""/>
<UsingTask TaskName = ""CustomLogAndReturnTask"" AssemblyName=""Microsoft.Build.Engine.UnitTests""/>
<PropertyGroup>
<MSBuildTreatWarningsAsErrors>{allWarningsAreErrors}</MSBuildTreatWarningsAsErrors>
<MSBuildWarningsAsMessages>{WarningsAsMessages}</MSBuildWarningsAsMessages>
<MSBuildWarningsAsErrors>{WarningsAsErrors}</MSBuildWarningsAsErrors>
<{prefix}TreatWarningsAsErrors>{allWarningsAreErrors}</{prefix}TreatWarningsAsErrors>
<{prefix}WarningsAsMessages>{WarningsAsMessages}</{prefix}WarningsAsMessages>
<{prefix}WarningsAsErrors>{WarningsAsErrors}</{prefix}WarningsAsErrors>
</PropertyGroup>
<Target Name='Build'>
<CustomLogAndReturnTask Return=""true"" ReturnHasLoggedErrors=""true"" WarningCode=""{WarningToLog}""/>
Expand All @@ -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($@"
<Project>
<PropertyGroup>
<{prefix}TreatWarningsAsErrors>true</{prefix}TreatWarningsAsErrors>
<{prefix}WarningsNotAsErrors>{Warning}</{prefix}WarningsNotAsErrors>
</PropertyGroup>
<Target Name='Build'>
<Warning Text=""some random text"" Code='{Warning}' />
</Target>
</Project>");

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 ? "<MSBuildTreatWarningsAsErrors>true</MSBuildTreatWarningsAsErrors>" : "";
env.SetEnvironmentVariable("MSBUILDDISABLEFEATURESFROMVERSION", ChangeWaves.Wave17_14.ToString());
TransientTestProjectWithFiles proj = env.CreateTestProjectWithFiles($@"
<Project>
<PropertyGroup>
{treatWarningsAsErrorsCodeProperty}
<{property}>{propertyData}</{property}>
</PropertyGroup>
<Target Name='Build'>
<Warning Text=""some random text"" Code='{warningCode}' />
</Target>
</Project>");
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);
}
}
}

/// <summary>
/// Item1 and Item2 log warnings and continue, item 3 logs a warn-> error and prevents item 4 from running in the batched build.
/// </summary>
Expand Down Expand Up @@ -371,17 +529,20 @@ 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($@"
<Project>
<UsingTask TaskName = ""ReturnFailureWithoutLoggingErrorTask"" AssemblyName=""Microsoft.Build.Engine.UnitTests""/>
<UsingTask TaskName = ""CustomLogAndReturnTask"" AssemblyName=""Microsoft.Build.Engine.UnitTests""/>
<PropertyGroup>
<MSBuildTreatWarningsAsErrors>{treatAllWarningsAsErrors}</MSBuildTreatWarningsAsErrors>
<MSBuildWarningsAsErrors>{warningsAsErrors}</MSBuildWarningsAsErrors>
<{prefix}TreatWarningsAsErrors>{treatAllWarningsAsErrors}</{prefix}TreatWarningsAsErrors>
<{prefix}WarningsAsErrors>{warningsAsErrors}</{prefix}WarningsAsErrors>
</PropertyGroup>
<Target Name='Build'>
<CustomLogAndReturnTask Return=""true"" WarningCode=""MSB1234""/>
Expand Down
Loading

0 comments on commit 56cc2a0

Please sign in to comment.