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.

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

Add parsing error information to composite format string parsing exceptions #85106

Merged
merged 2 commits into from
Apr 25, 2023
Merged

Add parsing error information to composite format string parsing exceptions #85106

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
97eac21
Add parsing error information to composite format string parsing exce…
stephentoub Apr 20, 2023
ae91131
Update src/libraries/System.Private.CoreLib/src/System/Text/Composite…
stephentoub Apr 24, 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
12 changes: 12 additions & 0 deletions src/libraries/System.Private.CoreLib/src/Resources/Strings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2392,6 +2392,18 @@
<data name="Format_InvalidStringWithValue" xml:space="preserve">
<value>The input string '{0}' was not in a correct format.</value>
</data>
<data name="Format_InvalidStringWithOffsetAndReason" xml:space="preserve">
<value>Input string was not in a correct format. Failure to parse near offset {0}. {1}</value>
</data>
<data name="Format_UnexpectedClosingBrace" xml:space="preserve">
<value>Unexpected closing brace without a corresponding opening brace.</value>
</data>
<data name="Format_UnclosedFormatItem" xml:space="preserve">
<value>Format item ends prematurely.</value>
</data>
<data name="Format_ExpectedAsciiDigit" xml:space="preserve">
<value>Expected an ASCII digit.</value>
</data>
<data name="Format_MissingIncompleteDate" xml:space="preserve">
<value>There must be at least a partial date with a year present in the input string '{0}'.</value>
</data>
Expand Down
62 changes: 42 additions & 20 deletions src/libraries/System.Private.CoreLib/src/System/Text/CompositeFormat.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,15 @@ public static CompositeFormat Parse([StringSyntax(StringSyntaxAttribute.Composit
{
ArgumentNullException.ThrowIfNull(format);

if (!TryParse(format, out CompositeFormat? compositeFormat))
var segments = new List<(string? Literal, int ArgIndex, int Alignment, string? Format)>();
int failureOffset = default;
ExceptionResource failureReason = default;
if (!TryParseLiterals(format, segments, ref failureOffset, ref failureReason))
{
ThrowHelper.ThrowFormatInvalidString();
ThrowHelper.ThrowFormatInvalidString(failureOffset, failureReason);
}

return compositeFormat;
return new CompositeFormat(format, segments.ToArray());
}

/// <summary>Try to parse the composite format string <paramref name="format"/>.</summary>
Expand All @@ -91,7 +94,9 @@ public static bool TryParse([StringSyntax(StringSyntaxAttribute.CompositeFormat)
if (format is not null)
{
var segments = new List<(string? Literal, int ArgIndex, int Alignment, string? Format)>();
if (TryParseLiterals(format, segments))
int failureOffset = default;
ExceptionResource failureReason = default;
if (TryParseLiterals(format, segments, ref failureOffset, ref failureReason))
{
compositeFormat = new CompositeFormat(format, segments.ToArray());
return true;
Expand Down Expand Up @@ -119,8 +124,10 @@ internal void ValidateNumberOfArgs(int numArgs)
/// <summary>Parse the composite format string into segments.</summary>
/// <param name="format">The format string.</param>
/// <param name="segments">The list into which to store the segments.</param>
/// <param name="failureOffset">The offset at which a parsing error occured if false is returned.</param>
/// <param name="failureReason">The reason for a parsing failure if false is returned.</param>
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
/// <returns>true if the format string can be parsed successfully; otherwise, false.</returns>
private static bool TryParseLiterals(ReadOnlySpan<char> format, List<(string? Literal, int ArgIndex, int Alignment, string? Format)> segments)
private static bool TryParseLiterals(ReadOnlySpan<char> format, List<(string? Literal, int ArgIndex, int Alignment, string? Format)> segments, ref int failureOffset, ref ExceptionResource failureReason)
{
// This parsing logic is copied from string.Format. It's the same code modified to not format
// as part of parsing and instead store the parsed literals and argument specifiers (alignment
Expand Down Expand Up @@ -161,7 +168,7 @@ private static bool TryParseLiterals(ReadOnlySpan<char> format, List<(string? Li
char brace = format[pos];
if (!TryMoveNext(format, ref pos, out ch))
{
return false;
goto FailureUnclosedFormatItem;
}
if (brace == ch)
{
Expand All @@ -173,7 +180,7 @@ private static bool TryParseLiterals(ReadOnlySpan<char> format, List<(string? Li
// This wasn't an escape, so it must be an opening brace.
if (brace != '{')
{
return false;
goto FailureUnexpectedClosingBrace;
}

// Proceed to parse the hole.
Expand All @@ -197,14 +204,14 @@ private static bool TryParseLiterals(ReadOnlySpan<char> format, List<(string? Li
int index = ch - '0';
if ((uint)index >= 10u)
{
return false;
goto FailureExpectedAsciiDigit;
}

// Common case is a single digit index followed by a closing brace. If it's not a closing brace,
// proceed to finish parsing the full hole format.
if (!TryMoveNext(format, ref pos, out ch))
{
return false;
goto FailureUnclosedFormatItem;
}
if (ch != '}')
{
Expand All @@ -214,7 +221,7 @@ private static bool TryParseLiterals(ReadOnlySpan<char> format, List<(string? Li
index = index * 10 + ch - '0';
if (!TryMoveNext(format, ref pos, out ch))
{
return false;
goto FailureUnclosedFormatItem;
}
}

Expand All @@ -223,7 +230,7 @@ private static bool TryParseLiterals(ReadOnlySpan<char> format, List<(string? Li
{
if (!TryMoveNext(format, ref pos, out ch))
{
return false;
goto FailureUnclosedFormatItem;
}
}

Expand All @@ -240,7 +247,7 @@ private static bool TryParseLiterals(ReadOnlySpan<char> format, List<(string? Li
{
if (!TryMoveNext(format, ref pos, out ch))
{
return false;
goto FailureUnclosedFormatItem;
}
}
while (ch == ' ');
Expand All @@ -252,26 +259,26 @@ private static bool TryParseLiterals(ReadOnlySpan<char> format, List<(string? Li
leftJustify = -1;
if (!TryMoveNext(format, ref pos, out ch))
{
return false;
goto FailureUnclosedFormatItem;
}
}

// Parse alignment digits. The read character must be a digit.
width = ch - '0';
if ((uint)width >= 10u)
{
return false;
goto FailureExpectedAsciiDigit;
}
if (!TryMoveNext(format, ref pos, out ch))
{
return false;
goto FailureUnclosedFormatItem;
}
while (char.IsAsciiDigit(ch))
{
width = width * 10 + ch - '0';
if (!TryMoveNext(format, ref pos, out ch))
{
return false;
goto FailureUnclosedFormatItem;
}
}
width *= leftJustify;
Expand All @@ -281,7 +288,7 @@ private static bool TryParseLiterals(ReadOnlySpan<char> format, List<(string? Li
{
if (!TryMoveNext(format, ref pos, out ch))
{
return false;
goto FailureUnclosedFormatItem;
}
}
}
Expand All @@ -293,7 +300,7 @@ private static bool TryParseLiterals(ReadOnlySpan<char> format, List<(string? Li
if (ch != ':')
{
// Unexpected character
return false;
goto FailureUnclosedFormatItem;
}

// Search for the closing brace; everything in between is the format,
Expand All @@ -303,7 +310,7 @@ private static bool TryParseLiterals(ReadOnlySpan<char> format, List<(string? Li
{
if (!TryMoveNext(format, ref pos, out ch))
{
return false;
goto FailureUnclosedFormatItem;
}

if (ch == '}')
Expand All @@ -315,7 +322,7 @@ private static bool TryParseLiterals(ReadOnlySpan<char> format, List<(string? Li
if (ch == '{')
{
// Braces inside the argument hole are not supported
return false;
goto FailureUnclosedFormatItem;
}
}

Expand All @@ -332,6 +339,21 @@ private static bool TryParseLiterals(ReadOnlySpan<char> format, List<(string? Li
// Continue parsing the rest of the format string.
}

FailureUnexpectedClosingBrace:
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
failureReason = ExceptionResource.Format_UnexpectedClosingBrace;
failureOffset = pos;
return false;

FailureUnclosedFormatItem:
failureReason = ExceptionResource.Format_UnclosedFormatItem;
failureOffset = pos;
return false;

FailureExpectedAsciiDigit:
failureReason = ExceptionResource.Format_ExpectedAsciiDigit;
failureOffset = pos;
return false;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
static bool TryMoveNext(ReadOnlySpan<char> format, ref int pos, out char nextChar)
{
Expand Down
12 changes: 6 additions & 6 deletions src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1439,7 +1439,7 @@ internal StringBuilder AppendFormatHelper(IFormatProvider? provider, string form
// This wasn't an escape, so it must be an opening brace.
if (brace != '{')
{
ThrowHelper.ThrowFormatInvalidString();
ThrowHelper.ThrowFormatInvalidString(pos, ExceptionResource.Format_UnexpectedClosingBrace);
}

// Proceed to parse the hole.
Expand All @@ -1462,7 +1462,7 @@ internal StringBuilder AppendFormatHelper(IFormatProvider? provider, string form
int index = ch - '0';
if ((uint)index >= 10u)
{
ThrowHelper.ThrowFormatInvalidString();
ThrowHelper.ThrowFormatInvalidString(pos, ExceptionResource.Format_ExpectedAsciiDigit);
}

// Common case is a single digit index followed by a closing brace. If it's not a closing brace,
Expand Down Expand Up @@ -1509,7 +1509,7 @@ internal StringBuilder AppendFormatHelper(IFormatProvider? provider, string form
width = ch - '0';
if ((uint)width >= 10u)
{
ThrowHelper.ThrowFormatInvalidString();
ThrowHelper.ThrowFormatInvalidString(pos, ExceptionResource.Format_ExpectedAsciiDigit);
}
ch = MoveNext(format, ref pos);
while (char.IsAsciiDigit(ch) && width < WidthLimit)
Expand All @@ -1532,7 +1532,7 @@ internal StringBuilder AppendFormatHelper(IFormatProvider? provider, string form
if (ch != ':')
{
// Unexpected character
ThrowHelper.ThrowFormatInvalidString();
ThrowHelper.ThrowFormatInvalidString(pos, ExceptionResource.Format_UnclosedFormatItem);
}

// Search for the closing brace; everything in between is the format,
Expand All @@ -1551,7 +1551,7 @@ internal StringBuilder AppendFormatHelper(IFormatProvider? provider, string form
if (ch == '{')
{
// Braces inside the argument hole are not supported
ThrowHelper.ThrowFormatInvalidString();
ThrowHelper.ThrowFormatInvalidString(pos, ExceptionResource.Format_UnclosedFormatItem);
}
}

Expand Down Expand Up @@ -1652,7 +1652,7 @@ static char MoveNext(string format, ref int pos)
pos++;
if ((uint)pos >= (uint)format.Length)
{
ThrowHelper.ThrowFormatInvalidString();
ThrowHelper.ThrowFormatInvalidString(pos, ExceptionResource.Format_UnclosedFormatItem);
}
return format[pos];
}
Expand Down
12 changes: 6 additions & 6 deletions src/libraries/System.Private.CoreLib/src/System/Text/ValueStringBuilder.AppendFormat.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ internal void AppendFormatHelper(IFormatProvider? provider, string format, ReadO
// This wasn't an escape, so it must be an opening brace.
if (brace != '{')
{
ThrowHelper.ThrowFormatInvalidString();
ThrowHelper.ThrowFormatInvalidString(pos, ExceptionResource.Format_UnexpectedClosingBrace);
}

// Proceed to parse the hole.
Expand All @@ -87,7 +87,7 @@ internal void AppendFormatHelper(IFormatProvider? provider, string format, ReadO
int index = ch - '0';
if ((uint)index >= 10u)
{
ThrowHelper.ThrowFormatInvalidString();
ThrowHelper.ThrowFormatInvalidString(pos, ExceptionResource.Format_ExpectedAsciiDigit);
}

// Common case is a single digit index followed by a closing brace. If it's not a closing brace,
Expand Down Expand Up @@ -134,7 +134,7 @@ internal void AppendFormatHelper(IFormatProvider? provider, string format, ReadO
width = ch - '0';
if ((uint)width >= 10u)
{
ThrowHelper.ThrowFormatInvalidString();
ThrowHelper.ThrowFormatInvalidString(pos, ExceptionResource.Format_ExpectedAsciiDigit);
}
ch = MoveNext(format, ref pos);
while (char.IsAsciiDigit(ch) && width < WidthLimit)
Expand All @@ -157,7 +157,7 @@ internal void AppendFormatHelper(IFormatProvider? provider, string format, ReadO
if (ch != ':')
{
// Unexpected character
ThrowHelper.ThrowFormatInvalidString();
ThrowHelper.ThrowFormatInvalidString(pos, ExceptionResource.Format_UnclosedFormatItem);
}

// Search for the closing brace; everything in between is the format,
Expand All @@ -176,7 +176,7 @@ internal void AppendFormatHelper(IFormatProvider? provider, string format, ReadO
if (ch == '{')
{
// Braces inside the argument hole are not supported
ThrowHelper.ThrowFormatInvalidString();
ThrowHelper.ThrowFormatInvalidString(pos, ExceptionResource.Format_UnclosedFormatItem);
}
}

Expand Down Expand Up @@ -269,7 +269,7 @@ static char MoveNext(string format, ref int pos)
pos++;
if ((uint)pos >= (uint)format.Length)
{
ThrowHelper.ThrowFormatInvalidString();
ThrowHelper.ThrowFormatInvalidString(pos, ExceptionResource.Format_UnclosedFormatItem);
}
return format[pos];
}
Expand Down
15 changes: 15 additions & 0 deletions src/libraries/System.Private.CoreLib/src/System/ThrowHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,12 @@ internal static void ThrowFormatInvalidString()
throw new FormatException(SR.Format_InvalidString);
}

[DoesNotReturn]
internal static void ThrowFormatInvalidString(int offset, ExceptionResource resource)
{
throw new FormatException(SR.Format(SR.Format_InvalidStringWithOffsetAndReason, offset, GetResourceString(resource)));
}

[DoesNotReturn]
internal static void ThrowFormatIndexOutOfRange()
{
Expand Down Expand Up @@ -1110,6 +1116,12 @@ private static string GetResourceString(ExceptionResource resource)
return SR.InvalidOperation_TimeProviderNullLocalTimeZone;
case ExceptionResource.InvalidOperation_TimeProviderInvalidTimestampFrequency:
return SR.InvalidOperation_TimeProviderInvalidTimestampFrequency;
case ExceptionResource.Format_UnexpectedClosingBrace:
return SR.Format_UnexpectedClosingBrace;
case ExceptionResource.Format_UnclosedFormatItem:
return SR.Format_UnclosedFormatItem;
case ExceptionResource.Format_ExpectedAsciiDigit:
return SR.Format_ExpectedAsciiDigit;
default:
Debug.Fail("The enum value is not defined, please check the ExceptionResource Enum.");
return "";
Expand Down Expand Up @@ -1303,5 +1315,8 @@ internal enum ExceptionResource
InvalidOperation_SpanOverlappedOperation,
InvalidOperation_TimeProviderNullLocalTimeZone,
InvalidOperation_TimeProviderInvalidTimestampFrequency,
Format_UnexpectedClosingBrace,
Format_UnclosedFormatItem,
Format_ExpectedAsciiDigit,
}
}