Skip to content

Commit

Permalink
Fix string.Split handling of trimming with no replacements (#73234)
Browse files Browse the repository at this point in the history
  • Loading branch information
@stephentoub
stephentoub authored Aug 5, 2022
1 parent 543bcc5 commit b1a309d
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 31 deletions.
57 changes: 26 additions & 31 deletions src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1339,17 +1339,7 @@ private string[] SplitInternal(ReadOnlySpan<char> separators, int count, StringS
{
// Per the method's documentation, we'll short-circuit the search for separators.
// But we still need to post-process the results based on the caller-provided flags.

string candidate = this;
if (((options & StringSplitOptions.TrimEntries) != 0) && (count > 0))
{
candidate = candidate.Trim();
}
if (((options & StringSplitOptions.RemoveEmptyEntries) != 0) && (candidate.Length == 0))
{
count = 0;
}
return (count == 0) ? Array.Empty<string>() : new string[] { candidate };
return CreateSplitArrayOfThisAsSoleValue(options, count);
}

if (separators.IsEmpty)
Expand Down Expand Up @@ -1422,17 +1412,7 @@ private string[] SplitInternal(string? separator, string?[]? separators, int cou
{
// Per the method's documentation, we'll short-circuit the search for separators.
// But we still need to post-process the results based on the caller-provided flags.

string candidate = this;
if (((options & StringSplitOptions.TrimEntries) != 0) && (count > 0))
{
candidate = candidate.Trim();
}
if (((options & StringSplitOptions.RemoveEmptyEntries) != 0) && (candidate.Length == 0))
{
count = 0;
}
return (count == 0) ? Array.Empty<string>() : new string[] { candidate };
return CreateSplitArrayOfThisAsSoleValue(options, count);
}

if (singleSeparator)
Expand All @@ -1458,7 +1438,7 @@ private string[] SplitInternal(string? separator, string?[]? separators, int cou
// Handle the special case of no replaces.
if (sepList.Length == 0)
{
return new string[] { this };
return CreateSplitArrayOfThisAsSoleValue(options, count);
}

string[] result = (options != StringSplitOptions.None)
Expand All @@ -1471,6 +1451,28 @@ private string[] SplitInternal(string? separator, string?[]? separators, int cou
return result;
}

private string[] CreateSplitArrayOfThisAsSoleValue(StringSplitOptions options, int count)
{
Debug.Assert(count >= 0);

if (count != 0)
{
string candidate = this;

if ((options & StringSplitOptions.TrimEntries) != 0)
{
candidate = candidate.Trim();
}

if ((options & StringSplitOptions.RemoveEmptyEntries) == 0 || candidate.Length != 0)
{
return new string[] { candidate };
}
}

return Array.Empty<string>();
}

private string[] SplitInternal(string separator, int count, StringSplitOptions options)
{
var sepListBuilder = new ValueListBuilder<int>(stackalloc int[StackallocIntBufferSizeLimit]);
Expand All @@ -1480,14 +1482,7 @@ private string[] SplitInternal(string separator, int count, StringSplitOptions o
if (sepList.Length == 0)
{
// there are no separators so sepListBuilder did not rent an array from pool and there is no need to dispose it
string candidate = this;
if ((options & StringSplitOptions.TrimEntries) != 0)
{
candidate = candidate.Trim();
}
return ((candidate.Length == 0) && ((options & StringSplitOptions.RemoveEmptyEntries) != 0))
? Array.Empty<string>()
: new string[] { candidate };
return CreateSplitArrayOfThisAsSoleValue(options, count);
}

string[] result = (options != StringSplitOptions.None)
Expand Down
32 changes: 32 additions & 0 deletions src/libraries/System.Runtime/tests/System/String.SplitTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,14 @@ public static void SplitNoMatchSingleResult()
[InlineData(" a,, b, c ", ',', 3, StringSplitOptions.RemoveEmptyEntries, new string[] { " a", " b", " c " })]
[InlineData(" a,, b, c ", ',', 3, StringSplitOptions.TrimEntries, new string[] { "a", "", "b, c" })]
[InlineData(" a,, b, c ", ',', 3, StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries, new string[] { "a", "b", "c" })]
[InlineData(" Monday ", ',', M, StringSplitOptions.None, new[] { " Monday " })]
[InlineData(" Monday ", ',', M, StringSplitOptions.TrimEntries, new[] { "Monday" })]
[InlineData(" Monday ", ',', M, StringSplitOptions.RemoveEmptyEntries, new[] { " Monday " })]
[InlineData(" Monday ", ',', M, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries, new[] { "Monday" })]
[InlineData(" ", ',', M, StringSplitOptions.None, new[] { " " })]
[InlineData(" ", ',', M, StringSplitOptions.TrimEntries, new[] { "" })]
[InlineData(" ", ',', M, StringSplitOptions.RemoveEmptyEntries, new[] { " " })]
[InlineData(" ", ',', M, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries, new string[0])]
public static void SplitCharSeparator(string value, char separator, int count, StringSplitOptions options, string[] expected)
{
Assert.Equal(expected, value.Split(separator, count, options));
Expand Down Expand Up @@ -483,6 +491,14 @@ public static void SplitCharSeparator(string value, char separator, int count, S
[InlineData("Monday, Tuesday,\r, Wednesday,\n, Thursday, Friday", ",", M, StringSplitOptions.TrimEntries, new[] { "Monday", "Tuesday", "", "Wednesday", "", "Thursday", "Friday" })]
[InlineData("Monday, Tuesday,\r, Wednesday,\n, Thursday, Friday", ",", M, StringSplitOptions.RemoveEmptyEntries, new[] { "Monday", " Tuesday", "\r", " Wednesday", "\n", " Thursday", " Friday" })]
[InlineData("Monday, Tuesday,\r, Wednesday,\n, Thursday, Friday", ",", M, StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries, new[] { "Monday", "Tuesday", "Wednesday", "Thursday", "Friday" })]
[InlineData(" Monday ", ",", M, StringSplitOptions.None, new[] { " Monday " })]
[InlineData(" Monday ", ",", M, StringSplitOptions.TrimEntries, new[] { "Monday" })]
[InlineData(" Monday ", ",", M, StringSplitOptions.RemoveEmptyEntries, new[] { " Monday " })]
[InlineData(" Monday ", ",", M, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries, new[] { "Monday" })]
[InlineData(" ", ",", M, StringSplitOptions.None, new[] { " " })]
[InlineData(" ", ",", M, StringSplitOptions.TrimEntries, new[] { "" })]
[InlineData(" ", ",", M, StringSplitOptions.RemoveEmptyEntries, new[] { " " })]
[InlineData(" ", ",", M, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries, new string[0])]
public static void SplitStringSeparator(string value, string separator, int count, StringSplitOptions options, string[] expected)
{
Assert.Equal(expected, value.Split(separator, count, options));
Expand Down Expand Up @@ -531,6 +547,14 @@ public static void SplitNullCharArraySeparator_BindsToCharArrayOverload()
[InlineData("this, is, a, string, with some spaces", new[] { ',', 's', 'a' }, M, StringSplitOptions.TrimEntries, new[] { "thi", "", "i", "", "", "", "", "tring", "with", "ome", "p", "ce", "" })]
[InlineData("this, is, a, string, with some spaces", new[] { ',', 's', 'a' }, M, StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries, new[] { "thi", "i", "tring", "with", "ome", "p", "ce" })]
[InlineData("this, is, a, very long string, with some spaces, commas and more spaces", new[] { ',', 's' }, M, StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries, new[] { "thi", "i", "a", "very long", "tring", "with", "ome", "pace", "comma", "and more", "pace" })]
[InlineData(" Monday ", new[] { ',', ':' }, M, StringSplitOptions.None, new[] { " Monday " })]
[InlineData(" Monday ", new[] { ',', ':' }, M, StringSplitOptions.TrimEntries, new[] { "Monday" })]
[InlineData(" Monday ", new[] { ',', ':' }, M, StringSplitOptions.RemoveEmptyEntries, new[] { " Monday " })]
[InlineData(" Monday ", new[] { ',', ':' }, M, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries, new[] { "Monday" })]
[InlineData(" ", new[] { ',', ':' }, M, StringSplitOptions.None, new[] { " " })]
[InlineData(" ", new[] { ',', ':' }, M, StringSplitOptions.TrimEntries, new[] { "" })]
[InlineData(" ", new[] { ',', ':' }, M, StringSplitOptions.RemoveEmptyEntries, new[] { " " })]
[InlineData(" ", new[] { ',', ':' }, M, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries, new string[0])]
public static void SplitCharArraySeparator(string value, char[] separators, int count, StringSplitOptions options, string[] expected)
{
Assert.Equal(expected, value.Split(separators, count, options));
Expand Down Expand Up @@ -563,6 +587,14 @@ public static void SplitCharArraySeparator(string value, char[] separators, int
[InlineData("this, is, a, string, with some spaces, ", new[] { ",", " s" }, M, StringSplitOptions.TrimEntries, new[] { "this", "is", "a", "", "tring", "with", "ome", "paces", "" })]
[InlineData("this, is, a, string, with some spaces, ", new[] { ",", " s" }, M, StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries, new[] { "this", "is", "a", "tring", "with", "ome", "paces" })]
[InlineData("this, is, a, very long string, with some spaces, commas and more spaces", new[] { ",", " s" }, M, StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries, new[] { "this", "is", "a", "very long", "tring", "with", "ome", "paces", "commas and more", "paces" })]
[InlineData(" Monday ", new[] { ",", ":" }, M, StringSplitOptions.None, new[] { " Monday " })]
[InlineData(" Monday ", new[] { ",", ":" }, M, StringSplitOptions.TrimEntries, new[] { "Monday" })]
[InlineData(" Monday ", new[] { ",", ":" }, M, StringSplitOptions.RemoveEmptyEntries, new[] { " Monday " })]
[InlineData(" Monday ", new[] { ",", ":" }, M, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries, new[] { "Monday" })]
[InlineData(" ", new[] { ",", ":" }, M, StringSplitOptions.None, new[] { " " })]
[InlineData(" ", new[] { ",", ":" }, M, StringSplitOptions.TrimEntries, new[] { "" })]
[InlineData(" ", new[] { ",", ":" }, M, StringSplitOptions.RemoveEmptyEntries, new[] { " " })]
[InlineData(" ", new[] { ",", ":" }, M, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries, new string[0])]
public static void SplitStringArraySeparator(string value, string[] separators, int count, StringSplitOptions options, string[] expected)
{
Assert.Equal(expected, value.Split(separators, count, options));
Expand Down

0 comments on commit b1a309d

Please sign in to comment.