Skip to content

Commit

Permalink
Add new line to be specified for JSON formatting (dotnet#100890)
Browse files Browse the repository at this point in the history
* Add new line to be specified for JSON formatting

Allow the new line string to use for indented JSON to be specified through options.
Resolves dotnet#84117.

* Address review feedback

- Cater for `_newLine` in JsonSerializerOptions caching.
- Lazily initialize field.
- Allow null to reset to default.
- Add assertions.
- Add/update comments.
- Use `nameof()`.
- Remove redundant field.
- Extend tests.

* Update Logging.Console tests

- Update property count to fix assertion.
- Update test to validate `NewLine` can be set/bound.

* Only normalize line endings if needed

Only normalize the line endings if the `JsonWriterOptions` are not using the defaults.

* Address feedback

- Access lazily initialized field through property.
- Update hash code assertion.

* Update exception message

Use similar format string to `Format_InvalidGuidFormatSpecification`/

* Address feedback

Reword comment as suggested.

* Address feedback

- Simplify condition.
- Disallow null for `string NewLine` properties.
  • Loading branch information
martincostello authored and Ruihan-Yin committed May 30, 2024
1 parent d825c65 commit 6a9dd38
Show file tree
Hide file tree
Showing 45 changed files with 357 additions and 141 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public void EnsureConsoleLoggerOptions_ConfigureOptions_SupportsAllProperties()
Assert.Equal(3, typeof(ConsoleFormatterOptions).GetProperties(flags).Length);
Assert.Equal(5, typeof(SimpleConsoleFormatterOptions).GetProperties(flags).Length);
Assert.Equal(4, typeof(JsonConsoleFormatterOptions).GetProperties(flags).Length);
Assert.Equal(6, typeof(JsonWriterOptions).GetProperties(flags).Length);
Assert.Equal(7, typeof(JsonWriterOptions).GetProperties(flags).Length);
}

[Theory]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,11 +273,13 @@ public void AddSystemdConsole_OutsideConfig_TakesProperty()
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
public void AddJsonConsole_ChangeProperties_IsReadFromLoggingConfiguration()
{
var newLine = Environment.NewLine.Length is 2 ? "\n" : "\r\n";
var configuration = new ConfigurationBuilder().AddInMemoryCollection(new[] {
new KeyValuePair<string, string>("Console:FormatterOptions:TimestampFormat", "HH:mm "),
new KeyValuePair<string, string>("Console:FormatterOptions:UseUtcTimestamp", "true"),
new KeyValuePair<string, string>("Console:FormatterOptions:IncludeScopes", "true"),
new KeyValuePair<string, string>("Console:FormatterOptions:JsonWriterOptions:Indented", "true"),
new KeyValuePair<string, string>("Console:FormatterOptions:JsonWriterOptions:NewLine", newLine),
}).Build();

var loggerProvider = new ServiceCollection()
Expand All @@ -296,11 +298,13 @@ public void AddJsonConsole_ChangeProperties_IsReadFromLoggingConfiguration()
Assert.True(formatter.FormatterOptions.UseUtcTimestamp);
Assert.True(formatter.FormatterOptions.IncludeScopes);
Assert.True(formatter.FormatterOptions.JsonWriterOptions.Indented);
Assert.Equal(newLine, formatter.FormatterOptions.JsonWriterOptions.NewLine);
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
public void AddJsonConsole_OutsideConfig_TakesProperty()
{
var newLine = Environment.NewLine.Length is 2 ? "\n" : "\r\n";
var configuration = new ConfigurationBuilder().AddInMemoryCollection(new[] {
new KeyValuePair<string, string>("Console:FormatterOptions:TimestampFormat", "HH:mm "),
new KeyValuePair<string, string>("Console:FormatterOptions:UseUtcTimestamp", "true"),
Expand All @@ -314,7 +318,8 @@ public void AddJsonConsole_OutsideConfig_TakesProperty()
o.JsonWriterOptions = new JsonWriterOptions()
{
Indented = false,
Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping
Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping,
NewLine = newLine
};
})
)
Expand All @@ -329,6 +334,7 @@ public void AddJsonConsole_OutsideConfig_TakesProperty()
Assert.True(formatter.FormatterOptions.UseUtcTimestamp);
Assert.True(formatter.FormatterOptions.IncludeScopes);
Assert.False(formatter.FormatterOptions.JsonWriterOptions.Indented);
Assert.Equal(newLine, formatter.FormatterOptions.JsonWriterOptions.NewLine);
Assert.Equal(JavaScriptEncoder.UnsafeRelaxedJsonEscaping, formatter.FormatterOptions.JsonWriterOptions.Encoder);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,5 +150,10 @@ public JsonSourceGenerationOptionsAttribute(JsonSerializerDefaults defaults)
/// instead of numeric serialization for all enum types encountered in its type graph.
/// </summary>
public bool UseStringEnumConverter { get; set; }

/// <summary>
/// Specifies the default value of <see cref="JsonSerializerOptions.NewLine"/> when set.
/// </summary>
public string? NewLine { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1147,6 +1147,9 @@ private static void GetLogicForDefaultSerializerOptionsInit(SourceGenerationOpti
if (optionsSpec.MaxDepth is int maxDepth)
writer.WriteLine($"MaxDepth = {maxDepth},");

if (optionsSpec.NewLine is string newLine)
writer.WriteLine($"NewLine = {FormatStringLiteral(newLine)},");

if (optionsSpec.NumberHandling is JsonNumberHandling numberHandling)
writer.WriteLine($"NumberHandling = {FormatNumberHandling(numberHandling)},");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ private SourceGenerationOptionsSpec ParseJsonSourceGenerationOptionsAttribute(IN
bool? ignoreReadOnlyProperties = null;
bool? includeFields = null;
int? maxDepth = null;
string? newLine = null;
JsonNumberHandling? numberHandling = null;
JsonObjectCreationHandling? preferredObjectCreationHandling = null;
bool? propertyNameCaseInsensitive = null;
Expand Down Expand Up @@ -344,6 +345,10 @@ private SourceGenerationOptionsSpec ParseJsonSourceGenerationOptionsAttribute(IN
maxDepth = (int)namedArg.Value.Value!;
break;

case nameof(JsonSourceGenerationOptionsAttribute.NewLine):
newLine = (string)namedArg.Value.Value!;
break;

case nameof(JsonSourceGenerationOptionsAttribute.NumberHandling):
numberHandling = (JsonNumberHandling)namedArg.Value.Value!;
break;
Expand Down Expand Up @@ -411,6 +416,7 @@ private SourceGenerationOptionsSpec ParseJsonSourceGenerationOptionsAttribute(IN
IgnoreReadOnlyProperties = ignoreReadOnlyProperties,
IncludeFields = includeFields,
MaxDepth = maxDepth,
NewLine = newLine,
NumberHandling = numberHandling,
PreferredObjectCreationHandling = preferredObjectCreationHandling,
PropertyNameCaseInsensitive = propertyNameCaseInsensitive,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ public sealed record SourceGenerationOptionsSpec

public required int? MaxDepth { get; init; }

public required string? NewLine { get; init; }

public required JsonNumberHandling? NumberHandling { get; init; }

public required JsonObjectCreationHandling? PreferredObjectCreationHandling { get; init; }
Expand Down
3 changes: 3 additions & 0 deletions src/libraries/System.Text.Json/ref/System.Text.Json.cs
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,7 @@ public JsonSerializerOptions(System.Text.Json.JsonSerializerOptions options) { }
public bool IncludeFields { get { throw null; } set { } }
public bool IsReadOnly { get { throw null; } }
public int MaxDepth { get { throw null; } set { } }
public string NewLine { get { throw null; } set { } }
public System.Text.Json.Serialization.JsonNumberHandling NumberHandling { get { throw null; } set { } }
public System.Text.Json.Serialization.JsonObjectCreationHandling PreferredObjectCreationHandling { get { throw null; } set { } }
public bool PropertyNameCaseInsensitive { get { throw null; } set { } }
Expand Down Expand Up @@ -445,6 +446,7 @@ public partial struct JsonWriterOptions
public bool Indented { get { throw null; } set { } }
public char IndentCharacter { get { throw null; } set { } }
public int IndentSize { get { throw null; } set { } }
public string NewLine { get { throw null; } set { } }
public int MaxDepth { readonly get { throw null; } set { } }
public bool SkipValidation { get { throw null; } set { } }
}
Expand Down Expand Up @@ -1074,6 +1076,7 @@ public JsonSourceGenerationOptionsAttribute(System.Text.Json.JsonSerializerDefau
public bool IgnoreReadOnlyProperties { get { throw null; } set { } }
public bool IncludeFields { get { throw null; } set { } }
public int MaxDepth { get { throw null; } set { } }
public string? NewLine { get { throw null; } set { } }
public System.Text.Json.Serialization.JsonNumberHandling NumberHandling { get { throw null; } set { } }
public System.Text.Json.Serialization.JsonObjectCreationHandling PreferredObjectCreationHandling { get { throw null; } set { } }
public bool PropertyNameCaseInsensitive { get { throw null; } set { } }
Expand Down
3 changes: 3 additions & 0 deletions src/libraries/System.Text.Json/src/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -717,4 +717,7 @@
<data name="InvalidIndentSize" xml:space="preserve">
<value>Indentation size must be between {0} and {1}.</value>
</data>
<data name="InvalidNewLine" xml:space="preserve">
<value>New line can be only "\n" or "\r\n".</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ internal static partial class JsonConstants
public const byte UtcOffsetToken = (byte)'Z';
public const byte TimePrefix = (byte)'T';

public const string NewLineLineFeed = "\n";
public const string NewLineCarriageReturnLineFeed = "\r\n";

// \u2028 and \u2029 are considered respectively line and paragraph separators
// UTF-8 representation for them is E2, 80, A8/A9
public const byte StartingByteOfNonStandardSeparator = 0xE2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,7 @@ public bool Equals(JsonSerializerOptions? left, JsonSerializerOptions? right)
left._unmappedMemberHandling == right._unmappedMemberHandling &&
left._defaultBufferSize == right._defaultBufferSize &&
left._maxDepth == right._maxDepth &&
left.NewLine == right.NewLine && // Read through property due to lazy initialization of the backing field
left._allowOutOfOrderMetadataProperties == right._allowOutOfOrderMetadataProperties &&
left._allowTrailingCommas == right._allowTrailingCommas &&
left._ignoreNullValues == right._ignoreNullValues &&
Expand Down Expand Up @@ -561,6 +562,7 @@ public int GetHashCode(JsonSerializerOptions options)
AddHashCode(ref hc, options._unmappedMemberHandling);
AddHashCode(ref hc, options._defaultBufferSize);
AddHashCode(ref hc, options._maxDepth);
AddHashCode(ref hc, options.NewLine); // Read through property due to lazy initialization of the backing field
AddHashCode(ref hc, options._allowOutOfOrderMetadataProperties);
AddHashCode(ref hc, options._allowTrailingCommas);
AddHashCode(ref hc, options._ignoreNullValues);
Expand Down Expand Up @@ -591,13 +593,13 @@ static void AddListHashCode<TValue>(ref HashCode hc, ConfigurationList<TValue>?

static void AddHashCode<TValue>(ref HashCode hc, TValue? value)
{
if (typeof(TValue).IsValueType)
if (typeof(TValue).IsSealed)
{
hc.Add(value);
}
else
{
Debug.Assert(!typeof(TValue).IsSealed, "Sealed reference types like string should not use this method.");
// Use the built-in hashcode for types that could be overriding GetHashCode().
hc.Add(RuntimeHelpers.GetHashCode(value));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ public static JsonSerializerOptions Web
private bool _ignoreReadOnlyProperties;
private bool _ignoreReadonlyFields;
private bool _includeFields;
private string? _newLine;
private bool _propertyNameCaseInsensitive;
private bool _writeIndented;
private char _indentCharacter = JsonConstants.DefaultIndentCharacter;
Expand Down Expand Up @@ -141,6 +142,7 @@ public JsonSerializerOptions(JsonSerializerOptions options)
_ignoreReadOnlyProperties = options._ignoreReadOnlyProperties;
_ignoreReadonlyFields = options._ignoreReadonlyFields;
_includeFields = options._includeFields;
_newLine = options._newLine;
_propertyNameCaseInsensitive = options._propertyNameCaseInsensitive;
_writeIndented = options._writeIndented;
_indentCharacter = options._indentCharacter;
Expand Down Expand Up @@ -750,6 +752,33 @@ public ReferenceHandler? ReferenceHandler
}
}

/// <summary>
/// Gets or sets the new line string to use when <see cref="WriteIndented"/> is <see langword="true"/>.
/// The default is the value of <see cref="Environment.NewLine"/>.
/// </summary>
/// <exception cref="ArgumentNullException">
/// Thrown when the new line string is <see langword="null"/>.
/// </exception>
/// <exception cref="ArgumentOutOfRangeException">
/// Thrown when the new line string is not <c>\n</c> or <c>\r\n</c>.
/// </exception>
/// <exception cref="InvalidOperationException">
/// Thrown if this property is set after serialization or deserialization has occurred.
/// </exception>
public string NewLine
{
get
{
return _newLine ??= Environment.NewLine;
}
set
{
JsonWriterHelper.ValidateNewLine(value);
VerifyMutable();
_newLine = value;
}
}

/// <summary>
/// Returns true if options uses compatible built-in resolvers or a combination of compatible built-in resolvers.
/// </summary>
Expand Down Expand Up @@ -970,6 +999,7 @@ internal JsonWriterOptions GetWriterOptions()
IndentCharacter = IndentCharacter,
IndentSize = IndentSize,
MaxDepth = EffectiveMaxDepth,
NewLine = NewLine,
#if !DEBUG
SkipValidation = true
#endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ internal static partial class ThrowHelper
// If the exception source is this value, the serializer will re-throw as JsonException.
public const string ExceptionSourceValueToRethrowAsJsonException = "System.Text.Json.Rethrowable";

[DoesNotReturn]
public static void ThrowArgumentOutOfRangeException_NewLine(string parameterName)
{
throw GetArgumentOutOfRangeException(parameterName, SR.InvalidNewLine);
}

[DoesNotReturn]
public static void ThrowArgumentOutOfRangeException_IndentCharacter(string parameterName)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,16 @@ public static void WriteIndentation(Span<byte> buffer, int indent, byte indentBy
}
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static void ValidateNewLine(string value)
{
if (value is null)
ThrowHelper.ThrowArgumentNullException(nameof(value));

if (value is not JsonConstants.NewLineLineFeed and not JsonConstants.NewLineCarriageReturnLineFeed)
ThrowHelper.ThrowArgumentOutOfRangeException_NewLine(nameof(value));
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static void ValidateIndentCharacter(char value)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ namespace System.Text.Json
/// </summary>
public struct JsonWriterOptions
{
private static readonly string s_alternateNewLine = Environment.NewLine.Length == 2 ? JsonConstants.NewLineLineFeed : JsonConstants.NewLineCarriageReturnLineFeed;

internal const int DefaultMaxDepth = 1000;

private int _maxDepth;
Expand Down Expand Up @@ -68,11 +70,11 @@ public char IndentCharacter
/// <exception cref="ArgumentOutOfRangeException"><paramref name="value"/> is out of the allowed range.</exception>
public int IndentSize
{
readonly get => EncodeIndentSize((_optionsMask & IndentSizeMask) >> 3);
readonly get => EncodeIndentSize((_optionsMask & IndentSizeMask) >> OptionsBitCount);
set
{
JsonWriterHelper.ValidateIndentSize(value);
_optionsMask = (_optionsMask & ~IndentSizeMask) | (EncodeIndentSize(value) << 3);
_optionsMask = (_optionsMask & ~IndentSizeMask) | (EncodeIndentSize(value) << OptionsBitCount);
}
}

Expand Down Expand Up @@ -135,11 +137,36 @@ public bool SkipValidation
}
}

/// <summary>
/// Gets or sets the new line string to use when <see cref="Indented"/> is <see langword="true"/>.
/// The default is the value of <see cref="Environment.NewLine"/>.
/// </summary>
/// <exception cref="ArgumentNullException">
/// Thrown when the new line string is <see langword="null"/>.
/// </exception>
/// <exception cref="ArgumentOutOfRangeException">
/// Thrown when the new line string is not <c>\n</c> or <c>\r\n</c>.
/// </exception>
public string NewLine
{
get => (_optionsMask & NewLineBit) != 0 ? s_alternateNewLine : Environment.NewLine;
set
{
JsonWriterHelper.ValidateNewLine(value);
if (value != Environment.NewLine)
_optionsMask |= NewLineBit;
else
_optionsMask &= ~NewLineBit;
}
}

internal bool IndentedOrNotSkipValidation => (_optionsMask & (IndentBit | SkipValidationBit)) != SkipValidationBit; // Equivalent to: Indented || !SkipValidation;

private const int OptionsBitCount = 4;
private const int IndentBit = 1;
private const int SkipValidationBit = 2;
private const int IndentCharacterBit = 4;
private const int IndentSizeMask = JsonConstants.MaximumIndentSize << 3;
private const int NewLineBit = 4;
private const int IndentCharacterBit = 8;
private const int IndentSizeMask = JsonConstants.MaximumIndentSize << OptionsBitCount;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -284,11 +284,11 @@ private void WriteBase64Indented(ReadOnlySpan<char> escapedPropertyName, ReadOnl

int encodedLength = Base64.GetMaxEncodedToUtf8Length(bytes.Length);

Debug.Assert(escapedPropertyName.Length * JsonConstants.MaxExpansionFactorWhileTranscoding < int.MaxValue - indent - encodedLength - 7 - s_newLineLength);
Debug.Assert(escapedPropertyName.Length * JsonConstants.MaxExpansionFactorWhileTranscoding < int.MaxValue - indent - encodedLength - 7 - _newLineLength);

// All ASCII, 2 quotes for property name, 2 quotes to surround the base-64 encoded string value, 1 colon, and 1 space => indent + escapedPropertyName.Length + encodedLength + 6
// Optionally, 1 list separator, 1-2 bytes for new line, and up to 3x growth when transcoding.
int maxRequired = indent + (escapedPropertyName.Length * JsonConstants.MaxExpansionFactorWhileTranscoding) + encodedLength + 7 + s_newLineLength;
int maxRequired = indent + (escapedPropertyName.Length * JsonConstants.MaxExpansionFactorWhileTranscoding) + encodedLength + 7 + _newLineLength;

if (_memory.Length - BytesPending < maxRequired)
{
Expand Down Expand Up @@ -334,11 +334,11 @@ private void WriteBase64Indented(ReadOnlySpan<byte> escapedPropertyName, ReadOnl

int encodedLength = Base64.GetMaxEncodedToUtf8Length(bytes.Length);

Debug.Assert(escapedPropertyName.Length < int.MaxValue - indent - encodedLength - 7 - s_newLineLength);
Debug.Assert(escapedPropertyName.Length < int.MaxValue - indent - encodedLength - 7 - _newLineLength);

// 2 quotes for property name, 2 quotes to surround the base-64 encoded string value, 1 colon, and 1 space => indent + escapedPropertyName.Length + encodedLength + 6
// Optionally, 1 list separator, and 1-2 bytes for new line.
int maxRequired = indent + escapedPropertyName.Length + encodedLength + 7 + s_newLineLength;
int maxRequired = indent + escapedPropertyName.Length + encodedLength + 7 + _newLineLength;

if (_memory.Length - BytesPending < maxRequired)
{
Expand Down
Loading

0 comments on commit 6a9dd38

Please sign in to comment.