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

StringBuilder.Replace with ReadOnlySpan<char> #93938

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -1841,6 +1841,17 @@ private StringBuilder AppendFormat<TArg0, TArg1, TArg2>(IFormatProvider? provide
/// </remarks>
public StringBuilder Replace(string oldValue, string? newValue) => Replace(oldValue, newValue, 0, Length);

/// <summary>
/// Replaces all instances of one read-only character span with another in this builder.
TheMaximum marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
/// <param name="oldValue">The read-only character span to replace.</param>
/// <param name="newValue">The read-only character span to replace <paramref name="oldValue"/> with.</param>
/// <remarks>
/// If <paramref name="newValue"/> is <c>null</c>, instances of <paramref name="oldValue"/>
adamsitnik marked this conversation as resolved.
Show resolved Hide resolved
/// are removed from this builder.
/// </remarks>
public StringBuilder Replace(ReadOnlySpan<char> oldValue, ReadOnlySpan<char> newValue) => Replace(oldValue, newValue, 0, Length);

/// <summary>
/// Determines if the contents of this builder are equal to the contents of another builder.
/// </summary>
Expand Down Expand Up @@ -1950,6 +1961,23 @@ public bool Equals(ReadOnlySpan<char> span)
/// are removed from this builder.
/// </remarks>
public StringBuilder Replace(string oldValue, string? newValue, int startIndex, int count)
{
ArgumentException.ThrowIfNullOrEmpty(oldValue);
Copy link
Member

Choose a reason for hiding this comment

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

This can just be ThrowIfNull. The called method has the check for empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would change the exception message (I think) in case it's an empty string (SR.Arg_EmptySpan instead of SR.Argument_EmptyString), I don't know if that would be an issue? It might be confusing to the user who's using strings to get an exception message indicating an empty span?

return Replace(oldValue.AsSpan(), newValue.AsSpan(), startIndex, count);
}

/// <summary>
/// Replaces all instances of one read-only character span with another in part of this builder.
/// </summary>
/// <param name="oldValue">The read-only character span to replace.</param>
/// <param name="newValue">The read-only character span to replace <paramref name="oldValue"/> with.</param>
/// <param name="startIndex">The index to start in this builder.</param>
/// <param name="count">The number of characters to read in this builder.</param>
/// <remarks>
/// If <paramref name="newValue"/> is <c>null</c>, instances of <paramref name="oldValue"/>
adamsitnik marked this conversation as resolved.
Show resolved Hide resolved
/// are removed from this builder.
/// </remarks>
public StringBuilder Replace(ReadOnlySpan<char> oldValue, ReadOnlySpan<char> newValue, int startIndex, int count)
{
int currentLength = Length;
if ((uint)startIndex > (uint)currentLength)
Expand All @@ -1960,9 +1988,10 @@ public StringBuilder Replace(string oldValue, string? newValue, int startIndex,
{
throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_IndexMustBeLessOrEqual);
}
ArgumentException.ThrowIfNullOrEmpty(oldValue);

newValue ??= string.Empty;
if (oldValue.Length == 0)
{
throw new ArgumentException(SR.Arg_EmptySpan, nameof(oldValue));
}

var replacements = new ValueListBuilder<int>(stackalloc int[128]); // A list of replacement positions in a chunk to apply

Expand Down Expand Up @@ -2225,7 +2254,7 @@ private void Insert(int index, ref char value, int valueCount)
/// <remarks>
/// This routine is very efficient because it does replacements in bulk.
/// </remarks>
private void ReplaceAllInChunk(ReadOnlySpan<int> replacements, StringBuilder sourceChunk, int removeCount, string value)
private void ReplaceAllInChunk(ReadOnlySpan<int> replacements, StringBuilder sourceChunk, int removeCount, ReadOnlySpan<char> value)
{
Debug.Assert(!replacements.IsEmpty);

Expand All @@ -2251,7 +2280,7 @@ private void ReplaceAllInChunk(ReadOnlySpan<int> replacements, StringBuilder sou
while (true)
{
// Copy in the new string for the ith replacement
ReplaceInPlaceAtChunk(ref targetChunk!, ref targetIndexInChunk, ref value.GetRawStringData(), value.Length);
ReplaceInPlaceAtChunk(ref targetChunk!, ref targetIndexInChunk, ref MemoryMarshal.GetReference<char>(value), value.Length);
int gapStart = replacements[i] + removeCount;
i++;
if ((uint)i >= replacements.Length)
Expand Down Expand Up @@ -2289,7 +2318,7 @@ private void ReplaceAllInChunk(ReadOnlySpan<int> replacements, StringBuilder sou
/// <param name="indexInChunk">The index in <paramref name="chunk"/> at which the substring starts.</param>
/// <param name="count">The logical count of the substring.</param>
/// <param name="value">The prefix.</param>
private bool StartsWith(StringBuilder chunk, int indexInChunk, int count, string value)
private bool StartsWith(StringBuilder chunk, int indexInChunk, int count, ReadOnlySpan<char> value)
{
for (int i = 0; i < value.Length; i++)
{
Expand Down
2 changes: 2 additions & 0 deletions src/libraries/System.Runtime/ref/System.Runtime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14925,6 +14925,8 @@ public void CopyTo(int sourceIndex, System.Span<char> destination, int count) {
public System.Text.StringBuilder Replace(char oldChar, char newChar, int startIndex, int count) { throw null; }
public System.Text.StringBuilder Replace(string oldValue, string? newValue) { throw null; }
public System.Text.StringBuilder Replace(string oldValue, string? newValue, int startIndex, int count) { throw null; }
public System.Text.StringBuilder Replace(ReadOnlySpan<char> oldValue, ReadOnlySpan<char> newValue) { throw null; }
public System.Text.StringBuilder Replace(ReadOnlySpan<char> oldValue, ReadOnlySpan<char> newValue, int startIndex, int count) { throw null; }
void System.Runtime.Serialization.ISerializable.GetObjectData(System.Runtime.Serialization.SerializationInfo info, System.Runtime.Serialization.StreamingContext context) { }
public override string ToString() { throw null; }
public string ToString(int startIndex, int length) { throw null; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
Link="System\Text\RuneTests.TestData.cs" />
<Compile Include="..\System\Text\StringBuilderTests.cs"
Link="System\Text\StringBuilderTests.cs" />
<Compile Include="..\System\Text\StringBuilderReplaceTests.cs"
Link="System\Text\StringBuilderReplaceTests.cs" />
<Compile Include="..\System\Uri.CreateStringTests.cs"
Link="System\Uri.CreateStringTests.cs" />
<Compile Include="..\System\Uri.CreateUriTests.cs"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@
<Compile Include="System\StringTests.cs" />
<Compile Include="System\SystemExceptionTests.cs" />
<Compile Include="System\Text\CompositeFormatTests.cs" />
<Compile Include="System\Text\StringBuilderReplaceTests.cs" />
<Compile Include="System\TimeOnlyTests.cs" />
<Compile Include="System\TimeoutExceptionTests.cs" />
<Compile Include="System\TimeSpanTests.cs" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Xunit;

namespace System.Text.Tests
{
public partial class StringBuilderReplaceTests
TheMaximum marked this conversation as resolved.
Show resolved Hide resolved
{
[Theory]
[InlineData("", "a", "!", 0, 0, "")]
[InlineData("aaaabbbbccccdddd", "a", "!", 0, 16, "!!!!bbbbccccdddd")]
[InlineData("aaaabbbbccccdddd", "a", "!", 2, 3, "aa!!bbbbccccdddd")]
[InlineData("aaaabbbbccccdddd", "a", "!", 4, 1, "aaaabbbbccccdddd")]
[InlineData("aaaabbbbccccdddd", "aab", "!", 2, 2, "aaaabbbbccccdddd")]
[InlineData("aaaabbbbccccdddd", "aab", "!", 2, 3, "aa!bbbccccdddd")]
[InlineData("aaaabbbbccccdddd", "aa", "!", 0, 16, "!!bbbbccccdddd")]
[InlineData("aaaabbbbccccdddd", "aa", "$!", 0, 16, "$!$!bbbbccccdddd")]
[InlineData("aaaabbbbccccdddd", "aa", "$!$", 0, 16, "$!$$!$bbbbccccdddd")]
[InlineData("aaaabbbbccccdddd", "aaaa", "!", 0, 16, "!bbbbccccdddd")]
[InlineData("aaaabbbbccccdddd", "aaaa", "$!", 0, 16, "$!bbbbccccdddd")]
[InlineData("aaaabbbbccccdddd", "a", "", 0, 16, "bbbbccccdddd")]
[InlineData("aaaabbbbccccdddd", "b", null, 0, 16, "aaaaccccdddd")]
[InlineData("aaaabbbbccccdddd", "aaaabbbbccccdddd", "", 0, 16, "")]
[InlineData("aaaabbbbccccdddd", "aaaabbbbccccdddd", "", 16, 0, "aaaabbbbccccdddd")]
[InlineData("aaaabbbbccccdddd", "aaaabbbbccccdddde", "", 0, 16, "aaaabbbbccccdddd")]
[InlineData("aaaaaaaaaaaaaaaa", "a", "b", 0, 16, "bbbbbbbbbbbbbbbb")]
public void Replace_StringBuilder(string value, string oldValue, string newValue, int startIndex, int count, string expected)
{
StringBuilder builder;
if (startIndex == 0 && count == value.Length)
{
// Use Replace(string, string) / Replace(ReadOnlySpan<char>, ReadOnlySpan<char>)
builder = new StringBuilder(value);
Replace(builder, oldValue, newValue);
Assert.Equal(expected, builder.ToString());
}

// Use Replace(string, string, int, int) / Replace(ReadOnlySpan<char>, ReadOnlySpan<char>, int, int)
builder = new StringBuilder(value);
Replace(builder, oldValue, newValue, startIndex, count);
Assert.Equal(expected, builder.ToString());
}

[Fact]
public void Replace_StringBuilderWithMultipleChunks()
{
StringBuilder builder = StringBuilderTests.StringBuilderWithMultipleChunks();
Replace(builder, "a", "b", builder.Length - 10, 10);
Assert.Equal(new string('a', builder.Length - 10) + new string('b', 10), builder.ToString());
}

[Fact]
public void Replace_StringBuilderWithMultipleChunks_WholeString()
{
StringBuilder builder = StringBuilderTests.StringBuilderWithMultipleChunks();
Replace(builder, builder.ToString(), "");
Assert.Same(string.Empty, builder.ToString());
}

[Fact]
public void Replace_StringBuilderWithMultipleChunks_LongString()
{
StringBuilder builder = StringBuilderTests.StringBuilderWithMultipleChunks();
Replace(builder, builder.ToString() + "b", "");
Assert.Equal(StringBuilderTests.s_chunkSplitSource, builder.ToString());
}

[Fact]
public void Replace_Invalid()
{
var builder = new StringBuilder(0, 5);
builder.Append("Hello");

AssertExtensions.Throws<ArgumentException>("oldValue", () => Replace(builder, "", "a")); // Old value is empty
AssertExtensions.Throws<ArgumentException>("oldValue", () => Replace(builder, "", "a", 0, 0)); // Old value is empty

AssertExtensions.Throws<ArgumentOutOfRangeException>("requiredLength", () => Replace(builder, "o", "oo")); // New length > builder.MaxCapacity
AssertExtensions.Throws<ArgumentOutOfRangeException>("requiredLength", () => Replace(builder, "o", "oo", 0, 5)); // New length > builder.MaxCapacity

AssertExtensions.Throws<ArgumentOutOfRangeException>("startIndex", () => Replace(builder, "a", "b", -1, 0)); // Start index < 0
AssertExtensions.Throws<ArgumentOutOfRangeException>("count", () => Replace(builder, "a", "b", 0, -1)); // Count < 0

AssertExtensions.Throws<ArgumentOutOfRangeException>("startIndex", () => Replace(builder, "a", "b", 6, 0)); // Count + start index > builder.Length
AssertExtensions.Throws<ArgumentOutOfRangeException>("count", () => Replace(builder, "a", "b", 5, 1)); // Count + start index > builder.Length
AssertExtensions.Throws<ArgumentOutOfRangeException>("count", () => Replace(builder, "a", "b", 4, 2)); // Count + start index > builder.Length
}

protected virtual StringBuilder Replace(StringBuilder builder, string oldValue, string newValue)
=> builder.Replace(oldValue, newValue);

protected virtual StringBuilder Replace(StringBuilder builder, string oldValue, string newValue, int startIndex, int count)
=> builder.Replace(oldValue, newValue, startIndex, count);
}

public partial class StringBuilderReplaceTests_String
TheMaximum marked this conversation as resolved.
Show resolved Hide resolved
{
[Fact]
public void Replace_String_Invalid()
{
var builder = new StringBuilder(0, 5);
builder.Append("Hello");

AssertExtensions.Throws<ArgumentNullException>("oldValue", () => builder.Replace(null, "")); // Old value is null
AssertExtensions.Throws<ArgumentNullException>("oldValue", () => builder.Replace(null, "a", 0, 0)); // Old value is null
}
}

public partial class StringBuilderReplaceTests_Span : StringBuilderReplaceTests
{
protected override StringBuilder Replace(StringBuilder builder, string oldValue, string newValue)
=> builder.Replace(oldValue.AsSpan(), newValue.AsSpan());

protected override StringBuilder Replace(StringBuilder builder, string oldValue, string newValue, int startIndex, int count)
=> builder.Replace(oldValue.AsSpan(), newValue.AsSpan(), startIndex, count);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ namespace System.Text.Tests
{
public partial class StringBuilderTests
{
private static readonly string s_chunkSplitSource = new string('a', 30);
private static readonly string s_noCapacityParamName = "valueCount";

private static StringBuilder StringBuilderWithMultipleChunks() => new StringBuilder(20).Append(s_chunkSplitSource);
internal static readonly string s_chunkSplitSource = new string('a', 30);
internal static StringBuilder StringBuilderWithMultipleChunks() => new StringBuilder(20).Append(s_chunkSplitSource);

[Fact]
public static void Ctor_Empty()
Expand Down Expand Up @@ -1694,87 +1694,6 @@ public static void Replace_Char_Invalid()
AssertExtensions.Throws<ArgumentOutOfRangeException>("count", () => builder.Replace('a', 'b', 4, 2)); // Count + start index > builder.Length
}

[Theory]
[InlineData("", "a", "!", 0, 0, "")]
[InlineData("aaaabbbbccccdddd", "a", "!", 0, 16, "!!!!bbbbccccdddd")]
[InlineData("aaaabbbbccccdddd", "a", "!", 2, 3, "aa!!bbbbccccdddd")]
[InlineData("aaaabbbbccccdddd", "a", "!", 4, 1, "aaaabbbbccccdddd")]
[InlineData("aaaabbbbccccdddd", "aab", "!", 2, 2, "aaaabbbbccccdddd")]
[InlineData("aaaabbbbccccdddd", "aab", "!", 2, 3, "aa!bbbccccdddd")]
[InlineData("aaaabbbbccccdddd", "aa", "!", 0, 16, "!!bbbbccccdddd")]
[InlineData("aaaabbbbccccdddd", "aa", "$!", 0, 16, "$!$!bbbbccccdddd")]
[InlineData("aaaabbbbccccdddd", "aa", "$!$", 0, 16, "$!$$!$bbbbccccdddd")]
[InlineData("aaaabbbbccccdddd", "aaaa", "!", 0, 16, "!bbbbccccdddd")]
[InlineData("aaaabbbbccccdddd", "aaaa", "$!", 0, 16, "$!bbbbccccdddd")]
[InlineData("aaaabbbbccccdddd", "a", "", 0, 16, "bbbbccccdddd")]
[InlineData("aaaabbbbccccdddd", "b", null, 0, 16, "aaaaccccdddd")]
[InlineData("aaaabbbbccccdddd", "aaaabbbbccccdddd", "", 0, 16, "")]
[InlineData("aaaabbbbccccdddd", "aaaabbbbccccdddd", "", 16, 0, "aaaabbbbccccdddd")]
[InlineData("aaaabbbbccccdddd", "aaaabbbbccccdddde", "", 0, 16, "aaaabbbbccccdddd")]
[InlineData("aaaaaaaaaaaaaaaa", "a", "b", 0, 16, "bbbbbbbbbbbbbbbb")]
public static void Replace_String(string value, string oldValue, string newValue, int startIndex, int count, string expected)
{
StringBuilder builder;
if (startIndex == 0 && count == value.Length)
{
// Use Replace(string, string)
builder = new StringBuilder(value);
builder.Replace(oldValue, newValue);
Assert.Equal(expected, builder.ToString());
}
// Use Replace(string, string, int, int)
builder = new StringBuilder(value);
builder.Replace(oldValue, newValue, startIndex, count);
Assert.Equal(expected, builder.ToString());
}

[Fact]
public static void Replace_String_StringBuilderWithMultipleChunks()
{
StringBuilder builder = StringBuilderWithMultipleChunks();
builder.Replace("a", "b", builder.Length - 10, 10);
Assert.Equal(new string('a', builder.Length - 10) + new string('b', 10), builder.ToString());
}

[Fact]
public static void Replace_String_StringBuilderWithMultipleChunks_WholeString()
{
StringBuilder builder = StringBuilderWithMultipleChunks();
builder.Replace(builder.ToString(), "");
Assert.Same(string.Empty, builder.ToString());
}

[Fact]
public static void Replace_String_StringBuilderWithMultipleChunks_LongString()
{
StringBuilder builder = StringBuilderWithMultipleChunks();
builder.Replace(builder.ToString() + "b", "");
Assert.Equal(s_chunkSplitSource, builder.ToString());
}

[Fact]
public static void Replace_String_Invalid()
{
var builder = new StringBuilder(0, 5);
builder.Append("Hello");

AssertExtensions.Throws<ArgumentNullException>("oldValue", () => builder.Replace(null, "")); // Old value is null
AssertExtensions.Throws<ArgumentNullException>("oldValue", () => builder.Replace(null, "a", 0, 0)); // Old value is null

AssertExtensions.Throws<ArgumentException>("oldValue", () => builder.Replace("", "a")); // Old value is empty
AssertExtensions.Throws<ArgumentException>("oldValue", () => builder.Replace("", "a", 0, 0)); // Old value is empty

AssertExtensions.Throws<ArgumentOutOfRangeException>("requiredLength", () => builder.Replace("o", "oo")); // New length > builder.MaxCapacity
AssertExtensions.Throws<ArgumentOutOfRangeException>("requiredLength", () => builder.Replace("o", "oo", 0, 5)); // New length > builder.MaxCapacity

AssertExtensions.Throws<ArgumentOutOfRangeException>("startIndex", () => builder.Replace("a", "b", -1, 0)); // Start index < 0
AssertExtensions.Throws<ArgumentOutOfRangeException>("count", () => builder.Replace("a", "b", 0, -1)); // Count < 0

AssertExtensions.Throws<ArgumentOutOfRangeException>("startIndex", () => builder.Replace("a", "b", 6, 0)); // Count + start index > builder.Length
AssertExtensions.Throws<ArgumentOutOfRangeException>("count", () => builder.Replace("a", "b", 5, 1)); // Count + start index > builder.Length
AssertExtensions.Throws<ArgumentOutOfRangeException>("count", () => builder.Replace("a", "b", 4, 2)); // Count + start index > builder.Length
}

[Theory]
[InlineData("Hello", 0, 5, "Hello")]
[InlineData("Hello", 2, 3, "llo")]
Expand Down
Loading