From 50251d23b10fb03a67beaf7c32b6e44e2fbf5909 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Kub=C3=AD=C4=8Dek?= Date: Sun, 29 Jan 2023 16:55:55 +0100 Subject: [PATCH 1/3] String.Split() trim last substring if specified --- .../src/System/MemoryExtensions.cs | 6 ++++-- .../src/System/String.Manipulation.cs | 4 ++-- .../tests/System/String.SplitTests.cs | 20 +++++++++++++++++++ 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.cs b/src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.cs index 4baf212be72906..5f9982c8a9a3da 100644 --- a/src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.cs +++ b/src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.cs @@ -3354,7 +3354,8 @@ public static int SplitAny(this ReadOnlySpan source, Span destinati // If the separators list is empty, whitespace is used as separators. In that case, we want to ignore TrimEntries if specified, // since TrimEntries also impacts whitespace. - if (separators.IsEmpty) + // The TrimEntries flag must be left intact if we are constrained by count because we need to process last substring. + if (separators.IsEmpty && destination.Length > source.Length) { options &= ~StringSplitOptions.TrimEntries; } @@ -3395,7 +3396,8 @@ public static int SplitAny(this ReadOnlySpan source, Span destinati // If the separators list is empty, whitespace is used as separators. In that case, we want to ignore TrimEntries if specified, // since TrimEntries also impacts whitespace. - if (separators.IsEmpty) + // The TrimEntries flag must be left intact if we are constrained by count because we need to process last substring. + if (separators.IsEmpty && destination.Length > source.Length) { options &= ~StringSplitOptions.TrimEntries; } diff --git a/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs b/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs index 0255aceb4183cb..2727bbadf1998f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs +++ b/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs @@ -1507,8 +1507,8 @@ private string[] SplitInternal(ReadOnlySpan separators, int count, StringS // But we still need to post-process the results based on the caller-provided flags. return CreateSplitArrayOfThisAsSoleValue(options, count); } - - if (separators.IsEmpty) + // The TrimEntries flag must be left intact if we are constrained by count because we need to process last substring. + if (separators.IsEmpty && count > Length) { // Caller is already splitting on whitespace; no need for separate trim step options &= ~StringSplitOptions.TrimEntries; diff --git a/src/libraries/System.Runtime/tests/System/String.SplitTests.cs b/src/libraries/System.Runtime/tests/System/String.SplitTests.cs index ce7450c72c2db4..b12bd2cb74fc48 100644 --- a/src/libraries/System.Runtime/tests/System/String.SplitTests.cs +++ b/src/libraries/System.Runtime/tests/System/String.SplitTests.cs @@ -491,6 +491,8 @@ public static void SplitNoMatchSingleResult() [InlineData(" ", ',', M, StringSplitOptions.TrimEntries, new[] { "" })] [InlineData(" ", ',', M, StringSplitOptions.RemoveEmptyEntries, new[] { " " })] [InlineData(" ", ',', M, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries, new string[0])] + [InlineData("a b ", ' ', 2, StringSplitOptions.TrimEntries, new[] { "a", "b" })] + [InlineData(" a b ", ' ', 2, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries, new[] { "a", "b" })] public static void SplitCharSeparator(string value, char separator, int count, StringSplitOptions options, string[] expected) { Assert.Equal(expected, value.Split(separator, count, options)); @@ -550,6 +552,12 @@ public static void SplitCharSeparator(string value, char separator, int count, S [InlineData(" ", ",", M, StringSplitOptions.TrimEntries, new[] { "" })] [InlineData(" ", ",", M, StringSplitOptions.RemoveEmptyEntries, new[] { " " })] [InlineData(" ", ",", M, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries, new string[0])] + [InlineData("a b ", null, 2, StringSplitOptions.TrimEntries, new[] { "a b" })] + [InlineData("a b ", "", 2, StringSplitOptions.TrimEntries, new[] { "a b" })] + [InlineData("a b ", " ", 2, StringSplitOptions.TrimEntries, new[] { "a", "b" })] + [InlineData(" a b ", null, 2, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries, new[] { "a b" })] + [InlineData(" a b ", "", 2, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries, new[] { "a b" })] + [InlineData(" a b ", " ", 2, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries, new[] { "a", "b" })] public static void SplitStringSeparator(string value, string separator, int count, StringSplitOptions options, string[] expected) { Assert.Equal(expected, value.Split(separator, count, options)); @@ -610,6 +618,12 @@ public static void SplitNullCharArraySeparator_BindsToCharArrayOverload() [InlineData(" ", new[] { ',', ':' }, M, StringSplitOptions.TrimEntries, new[] { "" })] [InlineData(" ", new[] { ',', ':' }, M, StringSplitOptions.RemoveEmptyEntries, new[] { " " })] [InlineData(" ", new[] { ',', ':' }, M, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries, new string[0])] + [InlineData("a b ", null, 2, StringSplitOptions.TrimEntries, new[] { "a", "b" })] + [InlineData("a b ", new char[0], 2, StringSplitOptions.TrimEntries, new[] { "a", "b" })] + [InlineData("a b ", new char[] { ' ' }, 2, StringSplitOptions.TrimEntries, new[] { "a", "b" })] + [InlineData(" a b ", null, 2, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries, new[] { "a", "b" })] + [InlineData(" a b ", new char[0], 2, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries, new[] { "a", "b" })] + [InlineData(" a b ", new char[] { ' ' }, 2, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries, new[] { "a", "b" })] public static void SplitCharArraySeparator(string value, char[] separators, int count, StringSplitOptions options, string[] expected) { Assert.Equal(expected, value.Split(separators, count, options)); @@ -654,6 +668,12 @@ public static void SplitCharArraySeparator(string value, char[] separators, int [InlineData(" ", new[] { ",", ":" }, M, StringSplitOptions.TrimEntries, new[] { "" })] [InlineData(" ", new[] { ",", ":" }, M, StringSplitOptions.RemoveEmptyEntries, new[] { " " })] [InlineData(" ", new[] { ",", ":" }, M, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries, new string[0])] + [InlineData("a b ", null, 2, StringSplitOptions.TrimEntries, new[] { "a", "b" })] + [InlineData("a b ", new string[0], 2, StringSplitOptions.TrimEntries, new[] { "a", "b" })] + [InlineData("a b ", new string[] { " " }, 2, StringSplitOptions.TrimEntries, new[] { "a", "b" })] + [InlineData(" a b ", null, 2, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries, new[] { "a", "b" })] + [InlineData(" a b ", new string[0], 2, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries, new[] { "a", "b" })] + [InlineData(" a b ", new string[] { " " }, 2, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries, new[] { "a", "b" })] public static void SplitStringArraySeparator(string value, string[] separators, int count, StringSplitOptions options, string[] expected) { Assert.Equal(expected, value.Split(separators, count, options)); From ac5b5515f30102db4e70c7e8a2d7c338a91b3685 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Kub=C3=AD=C4=8Dek?= Date: Mon, 30 Jan 2023 19:08:56 +0100 Subject: [PATCH 2/3] Update src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs Co-authored-by: Stephen Toub --- .../System.Private.CoreLib/src/System/String.Manipulation.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs b/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs index 2727bbadf1998f..7c3236a8dd23ad 100644 --- a/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs +++ b/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs @@ -1507,10 +1507,11 @@ private string[] SplitInternal(ReadOnlySpan separators, int count, StringS // But we still need to post-process the results based on the caller-provided flags. return CreateSplitArrayOfThisAsSoleValue(options, count); } - // The TrimEntries flag must be left intact if we are constrained by count because we need to process last substring. + if (separators.IsEmpty && count > Length) { - // Caller is already splitting on whitespace; no need for separate trim step + // Caller is already splitting on whitespace; no need for separate trim step if the count is sufficient + // to examine the whole input. options &= ~StringSplitOptions.TrimEntries; } From fafa98a7c6fe26a7c5157e43596e89061a4c3cde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Kub=C3=AD=C4=8Dek?= Date: Mon, 30 Jan 2023 19:22:57 +0100 Subject: [PATCH 3/3] doc update --- .../System.Private.CoreLib/src/System/MemoryExtensions.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.cs b/src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.cs index 5f9982c8a9a3da..a718a1461d91e7 100644 --- a/src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.cs +++ b/src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.cs @@ -3353,8 +3353,7 @@ public static int SplitAny(this ReadOnlySpan source, Span destinati string.CheckStringSplitOptions(options); // If the separators list is empty, whitespace is used as separators. In that case, we want to ignore TrimEntries if specified, - // since TrimEntries also impacts whitespace. - // The TrimEntries flag must be left intact if we are constrained by count because we need to process last substring. + // since TrimEntries also impacts whitespace. The TrimEntries flag must be left intact if we are constrained by count because we need to process last substring. if (separators.IsEmpty && destination.Length > source.Length) { options &= ~StringSplitOptions.TrimEntries; @@ -3395,8 +3394,7 @@ public static int SplitAny(this ReadOnlySpan source, Span destinati string.CheckStringSplitOptions(options); // If the separators list is empty, whitespace is used as separators. In that case, we want to ignore TrimEntries if specified, - // since TrimEntries also impacts whitespace. - // The TrimEntries flag must be left intact if we are constrained by count because we need to process last substring. + // since TrimEntries also impacts whitespace. The TrimEntries flag must be left intact if we are constrained by count because we need to process last substring. if (separators.IsEmpty && destination.Length > source.Length) { options &= ~StringSplitOptions.TrimEntries;