Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use invariant culture for converting propertie's values to string #9874

Merged
1 change: 1 addition & 0 deletions documentation/wiki/ChangeWaves.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t
- [Update Traits when environment has been changed](https://github.com/dotnet/msbuild/pull/9655)
- [Exec task does not trim leading whitespaces for ConsoleOutput](https://github.com/dotnet/msbuild/pull/9722)
- [Keep the encoding of standard output & error consistent with the console code page for ToolTask](https://github.com/dotnet/msbuild/pull/9539)
- [Convert.ToString during a property evaluation uses the InvariantCulture for numeric types (double, long, int)](https://github.com/dotnet/msbuild/pull/9874)


### 17.8
Expand Down
96 changes: 96 additions & 0 deletions src/Build.UnitTests/Evaluation/Expander_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Runtime.InteropServices;
using System.Runtime.Versioning;
using System.Text;
using System.Threading;
using System.Xml;
using Microsoft.Build.BackEnd;
using Microsoft.Build.Collections;
Expand Down Expand Up @@ -4859,5 +4860,100 @@ public void ExpandItemVectorFunctions_Exists_Directories()
squiggleItems.Select(i => i.EvaluatedInclude).ShouldBe(new[] { alphaBetaPath, alphaDeltaPath }, Case.Insensitive);
}
}

[Fact]
public void ExpandItem_ConvertToStringUsingInvariantCultureForNumberData()
{
var currentThread = Thread.CurrentThread;
var originalCulture = currentThread.CurrentCulture;
var originalUICulture = currentThread.CurrentUICulture;

try
{
var svSECultureInfo = new CultureInfo("sv-SE");
using (var env = TestEnvironment.Create())
{
currentThread.CurrentCulture = svSECultureInfo;
currentThread.CurrentUICulture = svSECultureInfo;
var root = env.CreateFolder();

var projectFile = env.CreateFile(root, ".proj",
@"<Project>

<PropertyGroup>
<_value>$([MSBuild]::Subtract(0, 1))</_value>
<_otherValue Condition=""'$(_value)' &gt;= -1"">test-value</_otherValue>
</PropertyGroup>
<Target Name=""Build"" />
</Project>");
ProjectInstance projectInstance = new ProjectInstance(projectFile.Path);
projectInstance.GetPropertyValue("_value").ShouldBe("-1");
projectInstance.GetPropertyValue("_otherValue").ShouldBe("test-value");
}
}
finally
{
currentThread.CurrentCulture = originalCulture;
currentThread.CurrentUICulture = originalUICulture;
}
f-alizada marked this conversation as resolved.
Show resolved Hide resolved
}

[Fact]
public void ExpandItem_ConvertToStringUsingInvariantCultureForNumberData_RespectingChangeWave()
{
// Note: Skipping the test since it is not a valid scenario when ICU mode is not used.
if (!ICUModeAvailable())
{
return;
}

var currentThread = Thread.CurrentThread;
var originalCulture = currentThread.CurrentCulture;
var originalUICulture = currentThread.CurrentUICulture;

try
{
var svSECultureInfo = new CultureInfo("sv-SE");
using (var env = TestEnvironment.Create())
{
env.SetEnvironmentVariable("MSBUILDDISABLEFEATURESFROMVERSION", ChangeWaves.Wave17_10.ToString());
currentThread.CurrentCulture = svSECultureInfo;
currentThread.CurrentUICulture = svSECultureInfo;
var root = env.CreateFolder();

var projectFile = env.CreateFile(root, ".proj",
@"<Project>

<PropertyGroup>
<_value>$([MSBuild]::Subtract(0, 1))</_value>
<_otherValue Condition=""'$(_value)' &gt;= -1"">test-value</_otherValue>
</PropertyGroup>
<Target Name=""Build"" />
</Project>");
var exception = Should.Throw<InvalidProjectFileException>(() =>
{
new ProjectInstance(projectFile.Path);
});
exception.BaseMessage.ShouldContain("A numeric comparison was attempted on \"$(_value)\"");
}
}
finally
{
currentThread.CurrentCulture = originalCulture;
currentThread.CurrentUICulture = originalUICulture;
}
}

/// <summary>
/// Determines if ICU mode is enabled.
/// Copied from: https://learn.microsoft.com/en-us/dotnet/core/extensions/globalization-icu#determine-if-your-app-is-using-icu
/// </summary>
private static bool ICUModeAvailable()
{
SortVersion sortVersion = CultureInfo.InvariantCulture.CompareInfo.Version;
byte[] bytes = sortVersion.SortId.ToByteArray();
int version = bytes[3] << 24 | bytes[2] << 16 | bytes[1] << 8 | bytes[0];
return version != 0 && version == sortVersion.FullVersion;
}
}
}
15 changes: 14 additions & 1 deletion src/Build/Evaluation/Expander.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1478,12 +1478,25 @@ internal static string ConvertToString(object valueToConvert)
else
{
// The fall back is always to just convert to a string directly.
convertedString = valueToConvert.ToString();
// Issue: https://github.com/dotnet/msbuild/issues/9757
if (IsNumberType(valueToConvert) && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_10))
{
convertedString = Convert.ToString(valueToConvert, CultureInfo.InvariantCulture.NumberFormat);
JanKrivanek marked this conversation as resolved.
Show resolved Hide resolved
}
else
{
convertedString = valueToConvert.ToString();
}
}

return convertedString;
}

private static bool IsNumberType(object obj)
{
return obj is double || obj is long || obj is int;
}

/// <summary>
/// Look up a simple property reference by the name of the property, e.g. "Foo" when expanding $(Foo).
/// </summary>
Expand Down