fix: Fixes string interpolation in value string builder (#1339)

This commit is contained in:
Kamron Batman 2023-02-13 23:09:50 -08:00 committed by GitHub
parent b46544d752
commit f24c6a08dd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 87 additions and 242 deletions

View file

@ -1,34 +1,48 @@
using Server.Buffers;
using Xunit;
namespace Server.Tests.Buffers
namespace Server.Tests.Buffers;
public class ValueStringBuilderTests
{
public class ValueStringBuilderTests
[Theory]
[InlineData("Admin Kamron", "Kamron", 0, 6)]
[InlineData("Admin Kamron", "Admin ron", 6, 3)]
[InlineData("Admin Kamron", "Admin", 5, 7)]
public void TestRemove(string original, string removed, int startIndex, int length)
{
[Theory]
[InlineData("Admin Kamron", "Kamron", 0, 6)]
[InlineData("Admin Kamron", "Admin ron", 6, 3)]
[InlineData("Admin Kamron", "Admin", 5, 7)]
public void TestRemove(string original, string removed, int startIndex, int length)
{
using var sb = new ValueStringBuilder(stackalloc char[64]);
sb.Append(original);
sb.Remove(startIndex, length);
using var sb = new ValueStringBuilder(stackalloc char[64]);
sb.Append(original);
sb.Remove(startIndex, length);
Assert.Equal(removed, sb.ToString());
}
Assert.Equal(removed, sb.ToString());
}
[Theory]
[InlineData(8)]
[InlineData(9876)]
[InlineData(-5)]
[InlineData(-130984209)]
public void TestAppendInt32(int value)
{
using var sb = new ValueStringBuilder(stackalloc char[64]);
sb.Append(value);
[Theory]
[InlineData(8)]
[InlineData(9876)]
[InlineData(-5)]
[InlineData(-130984209)]
public void TestAppendInt32(int value)
{
using var sb = new ValueStringBuilder(stackalloc char[64]);
sb.Append(value);
Assert.Equal(value.ToString(), sb.ToString());
}
Assert.Equal(value.ToString(), sb.ToString());
}
[Theory]
[InlineData("Kamron")]
[InlineData("")]
[InlineData(5)]
[InlineData(-30.6)]
public void TestAppendInterpolation(object value)
{
var sb = ValueStringBuilder.Create();
sb.Append( $"Hi, this is {value}.");
sb.Append(" I am a string.");
Assert.Equal($"Hi, this is {value}. I am a string.", sb.ToString());
sb.Dispose();
}
}

View file

@ -208,6 +208,17 @@ public ref struct ValueStringBuilder
}
}
// Compiler generated
public void Append(ref RawInterpolatedStringHandler handler) => Append(handler.Text);
// Compiler generated
public void Append(
IFormatProvider? formatProvider,
[InterpolatedStringHandlerArgument("formatProvider")]
ref RawInterpolatedStringHandler handler
) => Append(handler.Text);
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void Append(string? s)
{
@ -450,165 +461,4 @@ public ref struct ValueStringBuilder
_length -= length;
}
/// <summary>Provides a handler used by the language compiler to append interpolated strings into <see cref="ValueStringBuilder"/> instances.</summary>
[InterpolatedStringHandler]
public ref struct AppendInterpolatedStringHandler
{
// Implementation note:
// As this type is only intended to be targeted by the compiler, public APIs eschew argument validation logic
// in a variety of places, e.g. allowing a null input when one isn't expected to produce a NullReferenceException rather
// than an ArgumentNullException.
/// <summary>The associated StringBuilder to which to append.</summary>
internal ValueStringBuilder _stringBuilder;
/// <summary>Creates a handler used to append an interpolated string into a <see cref="ValueStringBuilder"/>.</summary>
/// <param name="literalLength">The number of constant characters outside of interpolation expressions in the interpolated string.</param>
/// <param name="formattedCount">The number of interpolation expressions in the interpolated string.</param>
/// <param name="stringBuilder">The associated StringBuilder to which to append.</param>
/// <remarks>This is intended to be called only by compiler-generated code. Arguments are not validated as they'd otherwise be for members intended to be used directly.</remarks>
public AppendInterpolatedStringHandler(int literalLength, int formattedCount, ValueStringBuilder stringBuilder)
{
_stringBuilder = stringBuilder;
}
/// <summary>Writes the specified string to the handler.</summary>
/// <param name="value">The string to write.</param>
public void AppendLiteral(string value) => _stringBuilder.Append(value);
// Design note:
// This provides the same set of overloads and semantics as DefaultInterpolatedStringHandler.
/// <summary>Writes the specified value to the handler.</summary>
/// <param name="value">The value to write.</param>
public void AppendFormatted<T>(T value) => _stringBuilder.Append(value);
/// <summary>Writes the specified value to the handler.</summary>
/// <param name="value">The value to write.</param>
/// <param name="format">The format string.</param>
public void AppendFormatted<T>(T value, string? format) => _stringBuilder.Append(value, format);
/// <summary>Writes the specified value to the handler.</summary>
/// <param name="value">The value to write.</param>
/// <param name="alignment">Minimum number of characters that should be written for this value. If the value is negative, it indicates left-aligned and the required minimum is the absolute value.</param>
public void AppendFormatted<T>(T value, int alignment) =>
AppendFormatted(value, alignment, format: null);
/// <summary>Writes the specified value to the handler.</summary>
/// <param name="value">The value to write.</param>
/// <param name="format">The format string.</param>
/// <param name="alignment">Minimum number of characters that should be written for this value. If the value is negative, it indicates left-aligned and the required minimum is the absolute value.</param>
public void AppendFormatted<T>(T value, int alignment, string? format)
{
if (alignment == 0)
{
// This overload is used as a fallback from several disambiguation overloads, so special-case 0.
AppendFormatted(value, format);
}
else if (alignment < 0)
{
// Left aligned: format into the handler, then append any additional padding required.
int start = _stringBuilder.Length;
AppendFormatted(value, format);
int paddingRequired = -alignment - (_stringBuilder.Length - start);
if (paddingRequired > 0)
{
_stringBuilder.Append(' ', paddingRequired);
}
}
else
{
var startingPos = _stringBuilder._length;
AppendFormatted(value, format);
InsertAlignment(startingPos, alignment);
}
}
/// <summary>Writes the specified character span to the handler.</summary>
/// <param name="value">The span to write.</param>
public void AppendFormatted(ReadOnlySpan<char> value) => _stringBuilder.Append(value);
/// <summary>Writes the specified string of chars to the handler.</summary>
/// <param name="value">The span to write.</param>
/// <param name="alignment">Minimum number of characters that should be written for this value. If the value is negative, it indicates left-aligned and the required minimum is the absolute value.</param>
/// <param name="format">The format string.</param>
public void AppendFormatted(ReadOnlySpan<char> value, int alignment = 0, string? format = null)
{
if (alignment == 0)
{
_stringBuilder.Append(value);
}
else
{
bool leftAlign = false;
if (alignment < 0)
{
leftAlign = true;
alignment = -alignment;
}
int paddingRequired = alignment - value.Length;
if (paddingRequired <= 0)
{
_stringBuilder.Append(value);
}
else if (leftAlign)
{
_stringBuilder.Append(value);
_stringBuilder.Append(' ', paddingRequired);
}
else
{
_stringBuilder.Append(' ', paddingRequired);
_stringBuilder.Append(value);
}
}
}
/// <summary>Writes the specified value to the handler.</summary>
/// <param name="value">The value to write.</param>
public void AppendFormatted(string? value) => _stringBuilder.Append(value);
/// <summary>Writes the specified value to the handler.</summary>
/// <param name="value">The value to write.</param>
/// <param name="alignment">Minimum number of characters that should be written for this value. If the value is negative, it indicates left-aligned and the required minimum is the absolute value.</param>
/// <param name="format">The format string.</param>
public void AppendFormatted(string? value, int alignment = 0, string? format = null) =>
// Format is meaningless for strings and doesn't make sense for someone to specify. We have the overload
// simply to disambiguate between ROS<char> and object, just in case someone does specify a format, as
// string is implicitly convertible to both. Just delegate to the T-based implementation.
AppendFormatted<string?>(value, alignment, format);
/// <summary>Writes the specified value to the handler.</summary>
/// <param name="value">The value to write.</param>
/// <param name="alignment">Minimum number of characters that should be written for this value. If the value is negative, it indicates left-aligned and the required minimum is the absolute value.</param>
/// <param name="format">The format string.</param>
public void AppendFormatted(object? value, int alignment = 0, string? format = null) =>
// This overload is expected to be used rarely, only if either a) something strongly typed as object is
// formatted with both an alignment and a format, or b) the compiler is unable to target type to T. It
// exists purely to help make cases from (b) compile. Just delegate to the T-based implementation.
AppendFormatted<object?>(value, alignment, format);
private void InsertAlignment(int startingPos, int alignment)
{
var charsWritten = _stringBuilder._length - startingPos;
var paddingNeeded = alignment - charsWritten;
if (paddingNeeded > 0)
{
var chars = _stringBuilder._chars;
if (chars.Length - _stringBuilder._length < paddingNeeded)
{
_stringBuilder.Grow(paddingNeeded);
}
chars.Slice(startingPos, charsWritten).CopyTo(chars[(startingPos + paddingNeeded)..]);
chars.Slice(startingPos, paddingNeeded).Fill(' ');
_stringBuilder._length += paddingNeeded;
}
}
}
}

View file

@ -1,32 +0,0 @@
/*************************************************************************
* ModernUO *
* Copyright 2019-2022 - ModernUO Development Team *
* Email: hi@modernuo.com *
* File: ValueStringBuilderExtensions.cs *
* *
* This program is free software: you can redistribute it and/or modify *
* it under the terms of the GNU General Public License as published by *
* the Free Software Foundation, either version 3 of the License, or *
* (at your option) any later version. *
* *
* You should have received a copy of the GNU General Public License *
* along with this program. If not, see <http://www.gnu.org/licenses/>. *
*************************************************************************/
using System.Runtime.CompilerServices;
namespace Server.Buffers;
public static class ValueStringBuilderExtensions
{
// Compiler generated
public static void Append(
// ReSharper disable once RedundantAssignment
this ref ValueStringBuilder stringBuilder,
[InterpolatedStringHandlerArgument("stringBuilder")]
ref ValueStringBuilder.AppendInterpolatedStringHandler handler)
{
// Reassign since the string builder stored on the interpolated handler is by-value
stringBuilder = handler._stringBuilder;
}
}

View file

@ -213,7 +213,7 @@ public class ClientVersion : IComparable<ClientVersion>, IComparer<ClientVersion
private string ToStringImpl()
{
using var builder = new ValueStringBuilder(stackalloc char[32]);
using var builder = ValueStringBuilder.Create();
if (Major > 5 || Minor > 0 || Revision > 6)
{

View file

@ -50,37 +50,38 @@ public class LocalizationEntry
private static void ParseText(string text, out string[] textSlices, out string stringFormatter)
{
var sb = ValueStringBuilder.Create(256);
using var queue = PooledRefQueue<string>.Create();
bool hasMatch = false;
var prevIndex = 0;
var builder = new ValueStringBuilder(stackalloc char[256]);
using var queue = PooledRefQueue<string>.Create();
foreach (Match match in _textRegex.Matches(text))
{
if (prevIndex < match.Index)
{
var substr = text[prevIndex..match.Index];
builder.Append(substr);
sb.Append(substr);
queue.Enqueue(substr);
}
queue.Enqueue(null);
hasMatch = true;
builder.Append($"{{{int.Parse(match.Groups[1].Value) - 1}}}");
sb.Append($"{{{int.Parse(match.Groups[1].Value) - 1}}}");
prevIndex = match.Index + match.Length;
}
if (prevIndex < text.Length - 1)
{
var substr = prevIndex == 0 ? text : text[prevIndex..];
builder.Append(substr);
sb.Append(substr);
queue.Enqueue(substr);
}
textSlices = queue.ToArray();
stringFormatter = hasMatch ? builder.ToString() : null;
stringFormatter = hasMatch ? sb.ToString() : null;
builder.Dispose();
sb.Dispose();
}
/// <summary>

View file

@ -96,13 +96,22 @@ namespace Server.Compression
new FileInfo(destinationArchiveFileName).EnsureDirectory();
using var builder = ValueStringBuilder.Create();
var sb = ValueStringBuilder.Create();
var i = 0;
foreach (var path in paths)
{
builder.Append($"{(i++ > 0 ? " " : "")}\"{Path.GetRelativePath(relativeTo, path)}\"");
var relativePath = Path.GetRelativePath(relativeTo, path);
if (i++ > 0)
{
sb.Append($" \"{relativePath}\"");
}
else
{
sb.Append($"\"{relativePath}\"");
}
}
var pathsToCompress = builder.ToString();
var pathsToCompress = sb.ToString();
sb.Dispose();
var tarFlags = compressCommand == null ? "-acf" : "-cf";
var useExternalCompression = compressCommand != null ? $"--use-compress-program \"{compressCommand}\" " : "";

View file

@ -939,7 +939,7 @@ public partial class ConditionTeleporter : Teleporter
{
base.GetProperties(list);
using var props = new ValueStringBuilder(stackalloc char[128]);
using var props = ValueStringBuilder.Create(128);
if (GetFlag(ConditionFlag.DenyMounted))
{

View file

@ -83,7 +83,7 @@ namespace Server.Misc
private static void EventSink_ClientVersionReceived(NetState state, ClientVersion version)
{
using var message = ValueStringBuilder.Create();
var sb = ValueStringBuilder.Create();
if (!_enable || state.Mobile?.AccessLevel != AccessLevel.Player)
{
@ -100,47 +100,47 @@ namespace Server.Misc
if (MinRequired != null && version < MinRequired)
{
message.Append($"This server doesn't support clients older than {MinRequired}.");
sb.Append($"This server doesn't support clients older than {MinRequired}.");
shouldKick = strictRequirement;
}
else if (MaxRequired != null && version > MaxRequired)
{
message.Append($"This server doesn't support clients newer than {MaxRequired}.");
sb.Append($"This server doesn't support clients newer than {MaxRequired}.");
shouldKick = strictRequirement;
}
else if (!AllowRegular || !AllowUOTD)
{
if (!AllowRegular && version.Type == ClientType.Regular)
{
message.Append("This server does not allow regular clients to connect.");
sb.Append("This server does not allow regular clients to connect.");
shouldKick = true;
}
else if (!AllowUOTD && state.IsUOTDClient)
{
message.Append("This server does not allow UO:TD clients to connect.");
sb.Append("This server does not allow UO:TD clients to connect.");
shouldKick = true;
}
if (message.Length > 0)
if (sb.Length > 0)
{
if (AllowRegular && AllowUOTD)
{
message.Append(" You can use regular or UO:TD clients.");
sb.Append(" You can use regular or UO:TD clients.");
}
else if (AllowRegular)
{
message.Append(" You can use regular clients.");
sb.Append(" You can use regular clients.");
}
else if (AllowUOTD)
{
message.Append(" You can use UO:TD clients.");
sb.Append(" You can use UO:TD clients.");
}
}
}
if (message.Length > 0)
if (sb.Length > 0)
{
state.Mobile.SendMessage(0x22, message.ToString());
state.Mobile.SendMessage(0x22, sb.ToString());
}
if (shouldKick)
@ -150,7 +150,7 @@ namespace Server.Misc
return;
}
if (message.Length > 0)
if (sb.Length > 0)
{
switch (_invalidClientResponse)
{
@ -170,6 +170,8 @@ namespace Server.Misc
}
}
}
sb.Dispose();
}
private static void OnKick(NetState ns)

View file

@ -71,7 +71,7 @@ namespace Server.SkillHandlers
if (c.Looters.Count > 0)
{
using var sb = new ValueStringBuilder(stackalloc char[128]);
var sb = ValueStringBuilder.Create(128);
int i = 0;
foreach (var looter in c.Looters)
{
@ -93,6 +93,7 @@ namespace Server.SkillHandlers
// This body has been disturbed by ~1_PLAYER_NAMES~
from.SendLocalizedMessage(1042752, sb.ToString());
sb.Dispose();
}
else
{