Skip to content

Commit

Permalink
Add parsing error information to composite format string parsing exce…
Browse files Browse the repository at this point in the history 
…ptions (#85106)

* Add parsing error information to composite format string parsing exceptions

* Update src/libraries/System.Private.CoreLib/src/System/Text/CompositeFormat.cs

Co-authored-by: Carlos Sánchez López <1175054+carlossanlop@users.noreply.github.com>

---------

Co-authored-by: Carlos Sánchez López <1175054+carlossanlop@users.noreply.github.com>
  • Loading branch information
@stephentoub @carlossanlop
stephentoub and carlossanlop authored Apr 25, 2023
1 parent a468216 commit 620b0db
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 32 deletions.
12 changes: 12 additions & 0 deletions src/libraries/System.Private.CoreLib/src/Resources/Strings.resx
View file
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
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 <see langword="false"/> is returned.</param>
/// <param name="failureReason">The reason for a parsing failure if <see langword="false"/> is returned.</param>
/// <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:
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
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
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
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,
}
}

0 comments on commit 620b0db

Please sign in to comment.