Skip to content

Fix invalid "double.ToString" behavior with two section separators #85565

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

Fix invalid "double.ToString" behavior with two section separators #85565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6c443da
refactor(70460): Replacement FactAttribute by TheoryAttribute.
May 3, 2023
59a97ea
fix(70460): Removing excess negative sign insert.
May 3, 2023
f5d64f9
fix(70460): Fix invalid sign applying.
May 4, 2023
1b365be
feat(70460): Adding tests for Half and Single.
May 4, 2023
f1e4584
feat(70460): Adding "ignoreCase" argument name.
May 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 0 additions & 14 deletions src/libraries/System.Private.CoreLib/src/System/Number.Formatting.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3165,15 +3165,6 @@ internal static unsafe void NumberToStringFormat<TChar>(ref ValueListBuilder<TCh
number.Scale += scaleAdjust;
int pos = scientific ? digitCount : number.Scale + digitCount - decimalPos;
RoundNumber(ref number, pos, isCorrectlyRounded: false);
if (dig[0] == 0)
{
src = FindSection(format, 2);
if (src != section)
{
section = src;
continue;
}
}
}
else
{
Expand Down Expand Up @@ -3446,11 +3437,6 @@ internal static unsafe void NumberToStringFormat<TChar>(ref ValueListBuilder<TCh
}
}
}

if (number.IsNegative && (section == 0) && (number.Scale == 0) && (vlb.Length > 0))
{
vlb.Insert(0, info.NegativeSignTChar<TChar>());
}
}

private static void FormatCurrency<TChar>(ref ValueListBuilder<TChar> vlb, ref NumberBuffer number, int nMaxDigits, NumberFormatInfo info) where TChar : unmanaged, IUtfChar<TChar>
Expand Down
46 changes: 21 additions & 25 deletions src/libraries/System.Runtime/tests/System/DoubleTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -781,6 +781,10 @@ public static IEnumerable<object[]> ToString_TestData()
{
yield return new object[] { -4567.0, "G", null, "-4567" };
yield return new object[] { -4567.89101, "G", null, "-4567.89101" };
yield return new object[] { -0.001, "+0.00;-0.00", null, "-0.00" };
yield return new object[] { -0.0, string.Empty, null, "-0" };
yield return new object[] { -0.0, "#", null, "" };
yield return new object[] { 0.0, "+0.00;-0.00", null, "+0.00" };
yield return new object[] { 0.0, "G", null, "0" };
yield return new object[] { 4567.0, "G", null, "4567" };
yield return new object[] { 4567.89101, "G", null, "4567.89101" };
Expand Down Expand Up @@ -823,7 +827,6 @@ public static IEnumerable<object[]> ToString_TestData_NotNetFramework()
yield return testData;
}


yield return new object[] { double.MinValue, "G", null, "-1.7976931348623157E+308" };
yield return new object[] { double.MaxValue, "G", null, "1.7976931348623157E+308" };

Expand All @@ -838,39 +841,32 @@ public static IEnumerable<object[]> ToString_TestData_NotNetFramework()
yield return new object[] { 32.5, "N100", invariantFormat, "32.5000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" };
}

[Fact]
public static void Test_ToString_NotNetFramework()
[Theory]
[MemberData(nameof(ToString_TestData_NotNetFramework))]
public static void Test_ToString_NotNetFramework(double d, string format, IFormatProvider provider, string expected)
Copy link
Member

@tannergooding tannergooding May 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corresponding tests need to be added to Single and potentially Half

Copy link
Contributor Author

@Maximys Maximys May 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added by b4c6067a234b1b98b06367dfe9346b396f07bab4 commit

{
using (new ThreadCultureChange(CultureInfo.InvariantCulture))
{
foreach (object[] testdata in ToString_TestData_NotNetFramework())
bool isDefaultProvider = (provider == null || provider == NumberFormatInfo.CurrentInfo);
if (string.IsNullOrEmpty(format) || format.ToUpperInvariant() == "G")
{
ToString((double)testdata[0], (string)testdata[1], (IFormatProvider)testdata[2], (string)testdata[3]);
if (isDefaultProvider)
{
Assert.Equal(expected, d.ToString());
Assert.Equal(expected, d.ToString((IFormatProvider)null));
}
Assert.Equal(expected, d.ToString(provider));
}
}
}

private static void ToString(double d, string format, IFormatProvider provider, string expected)
{
bool isDefaultProvider = (provider == null || provider == NumberFormatInfo.CurrentInfo);
if (string.IsNullOrEmpty(format) || format.ToUpperInvariant() == "G")
{
if (isDefaultProvider)
{
Assert.Equal(expected, d.ToString());
Assert.Equal(expected, d.ToString((IFormatProvider)null));
Assert.Equal(expected, d.ToString(format.ToUpperInvariant()), ignoreCase: true);
Assert.Equal(expected, d.ToString(format.ToLowerInvariant()), ignoreCase: true);
Assert.Equal(expected, d.ToString(format.ToUpperInvariant(), null), ignoreCase: true);
Assert.Equal(expected, d.ToString(format.ToLowerInvariant(), null), ignoreCase: true);
}
Assert.Equal(expected, d.ToString(provider));
}
if (isDefaultProvider)
{
Assert.Equal(expected.Replace('e', 'E'), d.ToString(format.ToUpperInvariant())); // If format is upper case, then exponents are printed in upper case
Assert.Equal(expected.Replace('E', 'e'), d.ToString(format.ToLowerInvariant())); // If format is lower case, then exponents are printed in upper case
Assert.Equal(expected.Replace('e', 'E'), d.ToString(format.ToUpperInvariant(), null));
Assert.Equal(expected.Replace('E', 'e'), d.ToString(format.ToLowerInvariant(), null));
Assert.Equal(expected, d.ToString(format.ToUpperInvariant(), provider), ignoreCase: true);
Assert.Equal(expected, d.ToString(format.ToLowerInvariant(), provider), ignoreCase: true);
}
Assert.Equal(expected.Replace('e', 'E'), d.ToString(format.ToUpperInvariant(), provider));
Assert.Equal(expected.Replace('E', 'e'), d.ToString(format.ToLowerInvariant(), provider));
}

[Fact]
Expand Down
70 changes: 33 additions & 37 deletions src/libraries/System.Runtime/tests/System/HalfTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -930,39 +930,42 @@ public static void Parse_Utf8Span_Invalid(string value, NumberStyles style, IFor

public static IEnumerable<object[]> ToString_TestData()
{
yield return new object[] { -4570.0f, "G", null, "-4570" };
yield return new object[] { 0.0f, "G", null, "0" };
yield return new object[] { 4570.0f, "G", null, "4570" };
yield return new object[] { (Half)(-4570.0f), "G", null, "-4570" };
yield return new object[] { (Half)(-0.001f), "+0.00;-0.00", null, "-0.00" };
yield return new object[] { (Half)(-0.0f), string.Empty, null, "-0" };
yield return new object[] { (Half)(- 0.0f), "#", null, "" };
yield return new object[] { Half.Zero, "G", null, "0" };
yield return new object[] { (Half)4570.0f, "G", null, "4570" };

yield return new object[] { float.NaN, "G", null, "NaN" };
yield return new object[] { Half.NaN, "G", null, "NaN" };

yield return new object[] { 2468.0f, "N", null, "2,468.00" };
yield return new object[] { (Half)2468.0f, "N", null, "2,468.00" };

// Changing the negative pattern doesn't do anything without also passing in a format string
var customNegativePattern = new NumberFormatInfo() { NumberNegativePattern = 0 };
yield return new object[] { -6310.0f, "G", customNegativePattern, "-6310" };
yield return new object[] { (Half)(-6310.0f), "G", customNegativePattern, "-6310" };

var customNegativeSignDecimalGroupSeparator = new NumberFormatInfo()
{
NegativeSign = "#",
NumberDecimalSeparator = "~",
NumberGroupSeparator = "*"
};
yield return new object[] { -2468.0f, "N", customNegativeSignDecimalGroupSeparator, "#2*468~00" };
yield return new object[] { 2468.0f, "N", customNegativeSignDecimalGroupSeparator, "2*468~00" };
yield return new object[] { (Half)(-2468.0f), "N", customNegativeSignDecimalGroupSeparator, "#2*468~00" };
yield return new object[] { (Half)(2468.0f), "N", customNegativeSignDecimalGroupSeparator, "2*468~00" };

var customNegativeSignGroupSeparatorNegativePattern = new NumberFormatInfo()
{
NegativeSign = "xx", // Set to trash to make sure it doesn't show up
NumberGroupSeparator = "*",
NumberNegativePattern = 0
};
yield return new object[] { -2468.0f, "N", customNegativeSignGroupSeparatorNegativePattern, "(2*468.00)" };
yield return new object[] { (Half)(-2468.0f), "N", customNegativeSignGroupSeparatorNegativePattern, "(2*468.00)" };

NumberFormatInfo invariantFormat = NumberFormatInfo.InvariantInfo;
yield return new object[] { float.NaN, "G", invariantFormat, "NaN" };
yield return new object[] { float.PositiveInfinity, "G", invariantFormat, "Infinity" };
yield return new object[] { float.NegativeInfinity, "G", invariantFormat, "-Infinity" };
yield return new object[] { Half.NaN, "G", invariantFormat, "NaN" };
yield return new object[] { Half.PositiveInfinity, "G", invariantFormat, "Infinity" };
yield return new object[] { Half.NegativeInfinity, "G", invariantFormat, "-Infinity" };
}

public static IEnumerable<object[]> ToString_TestData_NotNetFramework()
Expand All @@ -987,39 +990,32 @@ public static IEnumerable<object[]> ToString_TestData_NotNetFramework()
yield return new object[] { 32.5f, "N100", invariantFormat, "32.5000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" };
}

[Fact]
public static void Test_ToString_NotNetFramework()
[Theory]
[MemberData(nameof(ToString_TestData_NotNetFramework))]
public static void Test_ToString_NotNetFramework(Half h, string format, IFormatProvider provider, string expected)
{
using (new ThreadCultureChange(CultureInfo.InvariantCulture))
{
foreach (object[] testdata in ToString_TestData_NotNetFramework())
bool isDefaultProvider = provider == null;
if (string.IsNullOrEmpty(format) || format.ToUpperInvariant() == "G")
{
ToStringTest(testdata[0] is float floatData ? (Half)floatData : (Half)testdata[0], (string)testdata[1], (IFormatProvider)testdata[2], (string)testdata[3]);
if (isDefaultProvider)
{
Assert.Equal(expected, h.ToString());
Assert.Equal(expected, h.ToString((IFormatProvider)null));
}
Assert.Equal(expected, h.ToString(provider));
}
}
}

private static void ToStringTest(Half f, string format, IFormatProvider provider, string expected)
{
bool isDefaultProvider = provider == null;
if (string.IsNullOrEmpty(format) || format.ToUpperInvariant() == "G")
{
if (isDefaultProvider)
{
Assert.Equal(expected, f.ToString());
Assert.Equal(expected, f.ToString((IFormatProvider)null));
Assert.Equal(expected, h.ToString(format.ToUpperInvariant()), ignoreCase: true);
Assert.Equal(expected, h.ToString(format.ToLowerInvariant()), ignoreCase: true);
Assert.Equal(expected, h.ToString(format.ToUpperInvariant(), null), ignoreCase: true);
Assert.Equal(expected, h.ToString(format.ToLowerInvariant(), null), ignoreCase: true);
}
Assert.Equal(expected, f.ToString(provider));
}
if (isDefaultProvider)
{
Assert.Equal(expected.Replace('e', 'E'), f.ToString(format.ToUpperInvariant())); // If format is upper case, then exponents are printed in upper case
Assert.Equal(expected.Replace('E', 'e'), f.ToString(format.ToLowerInvariant())); // If format is lower case, then exponents are printed in lower case
Assert.Equal(expected.Replace('e', 'E'), f.ToString(format.ToUpperInvariant(), null));
Assert.Equal(expected.Replace('E', 'e'), f.ToString(format.ToLowerInvariant(), null));
Assert.Equal(expected, h.ToString(format.ToUpperInvariant(), provider), ignoreCase: true);
Assert.Equal(expected, h.ToString(format.ToLowerInvariant(), provider), ignoreCase: true);
}
Assert.Equal(expected.Replace('e', 'E'), f.ToString(format.ToUpperInvariant(), provider));
Assert.Equal(expected.Replace('E', 'e'), f.ToString(format.ToLowerInvariant(), provider));
}

[Fact]
Expand All @@ -1040,7 +1036,7 @@ public static void TryFormat()
{
foreach (object[] testdata in ToString_TestData())
{
float localI = (float)testdata[0];
Half localI = (Half)testdata[0];
string localFormat = (string)testdata[1];
IFormatProvider localProvider = (IFormatProvider)testdata[2];
string localExpected = (string)testdata[3];
Expand Down
44 changes: 20 additions & 24 deletions src/libraries/System.Runtime/tests/System/SingleTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,9 @@ public static IEnumerable<object[]> ToString_TestData()
{
yield return new object[] { -4567.0f, "G", null, "-4567" };
yield return new object[] { -4567.89101f, "G", null, "-4567.891" };
yield return new object[] { -0.001f, "+0.00;-0.00", null, "-0.00" };
yield return new object[] { -0.0f, string.Empty, null, "-0" };
yield return new object[] { -0.0f, "#", null, "" };
yield return new object[] { 0.0f, "G", null, "0" };
yield return new object[] { 4567.0f, "G", null, "4567" };
yield return new object[] { 4567.89101f, "G", null, "4567.891" };
Expand Down Expand Up @@ -777,39 +780,32 @@ public static IEnumerable<object[]> ToString_TestData_NotNetFramework()
yield return new object[] { 32.5f, "N100", invariantFormat, "32.5000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" };
}

[Fact]
public static void Test_ToString_NotNetFramework()
[Theory]
[MemberData(nameof(ToString_TestData_NotNetFramework))]
public static void Test_ToString_NotNetFramework(float f, string format, IFormatProvider provider, string expected)
{
using (new ThreadCultureChange(CultureInfo.InvariantCulture))
{
foreach (object[] testdata in ToString_TestData_NotNetFramework())
bool isDefaultProvider = provider == null;
if (string.IsNullOrEmpty(format) || format.ToUpperInvariant() == "G")
{
ToStringTest((float)testdata[0], (string)testdata[1], (IFormatProvider)testdata[2], (string)testdata[3]);
if (isDefaultProvider)
{
Assert.Equal(expected, f.ToString());
Assert.Equal(expected, f.ToString((IFormatProvider)null));
}
Assert.Equal(expected, f.ToString(provider));
}
}
}

private static void ToStringTest(float f, string format, IFormatProvider provider, string expected)
{
bool isDefaultProvider = provider == null;
if (string.IsNullOrEmpty(format) || format.ToUpperInvariant() == "G")
{
if (isDefaultProvider)
{
Assert.Equal(expected, f.ToString());
Assert.Equal(expected, f.ToString((IFormatProvider)null));
Assert.Equal(expected, f.ToString(format.ToUpperInvariant()), ignoreCase: true);
Assert.Equal(expected, f.ToString(format.ToLowerInvariant()), ignoreCase: true);
Assert.Equal(expected, f.ToString(format.ToUpperInvariant(), null), ignoreCase: true);
Assert.Equal(expected, f.ToString(format.ToLowerInvariant(), null), ignoreCase: true);
}
Assert.Equal(expected, f.ToString(provider));
}
if (isDefaultProvider)
{
Assert.Equal(expected.Replace('e', 'E'), f.ToString(format.ToUpperInvariant())); // If format is upper case, then exponents are printed in upper case
Assert.Equal(expected.Replace('E', 'e'), f.ToString(format.ToLowerInvariant())); // If format is lower case, then exponents are printed in lower case
Assert.Equal(expected.Replace('e', 'E'), f.ToString(format.ToUpperInvariant(), null));
Assert.Equal(expected.Replace('E', 'e'), f.ToString(format.ToLowerInvariant(), null));
Assert.Equal(expected, f.ToString(format.ToUpperInvariant(), provider), ignoreCase: true);
Assert.Equal(expected, f.ToString(format.ToLowerInvariant(), provider), ignoreCase: true);
}
Assert.Equal(expected.Replace('e', 'E'), f.ToString(format.ToUpperInvariant(), provider));
Assert.Equal(expected.Replace('E', 'e'), f.ToString(format.ToLowerInvariant(), provider));
}

[Fact]
Expand Down