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

Allow specifying IndentCharacter and IndentSize when writing JSON #95292

Merged
merged 29 commits into from
Jan 8, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
b0e8003
Add IndentText json option
manandre Nov 25, 2023
d283b53
Add IndentText for json source generator
manandre Nov 25, 2023
eb0d433
Add tests
manandre Nov 25, 2023
6ea532e
IndentText must be non-nullable
manandre Nov 26, 2023
c8995a1
Improve performance
manandre Nov 26, 2023
3f82e03
Add extra tests
manandre Nov 26, 2023
c88aaf3
Cleanup
manandre Nov 27, 2023
86c4de5
Apply suggestions from code review
manandre Nov 28, 2023
976651f
Fixes following code review
manandre Nov 28, 2023
71e77db
Fixes following code review #2
manandre Dec 1, 2023
7c44dda
Add tests for invalid characters
manandre Dec 1, 2023
e42ea75
Handle RawIndent length
manandre Dec 1, 2023
face6cc
Move all to RawIndentation
manandre Dec 1, 2023
29118b4
Update documentation
manandre Dec 1, 2023
6f6eca7
Additional fixes from code review
manandre Dec 4, 2023
037f2fa
Move to the new API
manandre Dec 15, 2023
7c76a35
Extra fixes and enhancements
manandre Dec 15, 2023
cbd6dba
Fixes from code review
manandre Dec 24, 2023
37a305a
Avoid introducing extra fields in JsonWriterOptions
manandre Dec 24, 2023
5b11220
Fix OOM error
manandre Dec 24, 2023
ed7e1b2
Use bitwise logic for IndentedOrNotSkipValidation
manandre Dec 26, 2023
01fc002
Cache indentation options in Utf8JsonWriter
manandre Dec 26, 2023
f00943e
Add missing test around indentation options
manandre Dec 26, 2023
5dc575b
New fixes from code review
manandre Dec 28, 2023
d5e8835
Update src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf…
eiriktsarpalis Dec 29, 2023
2bfbf0c
Add test to check default values of the JsonWriterOptions properties
manandre Dec 29, 2023
00617c2
Fix comment
manandre Jan 2, 2024
4964a53
Merge branch 'main' into json-indent-text
manandre Jan 6, 2024
c14eaf7
Merge branch 'main' into json-indent-text
eiriktsarpalis Jan 8, 2024
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
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(4, typeof(JsonWriterOptions).GetProperties(flags).Length);
Assert.Equal(6, typeof(JsonWriterOptions).GetProperties(flags).Length);
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
}

[Theory]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,7 @@ private static void VerifyHasOnlySimpleProperties(Type type)
// or else NativeAOT would break
Assert.True(prop.PropertyType == typeof(string) ||
prop.PropertyType == typeof(bool) ||
prop.PropertyType == typeof(char) ||
prop.PropertyType == typeof(int) ||
prop.PropertyType.IsEnum, $"ConsoleOptions property '{type.Name}.{prop.Name}' must be a simple type in order for NativeAOT to work");
}
Expand Down
6 changes: 6 additions & 0 deletions src/libraries/System.Text.Json/Common/JsonConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,11 @@ internal static partial class JsonConstants

public const int StackallocByteThreshold = 256;
public const int StackallocCharThreshold = StackallocByteThreshold / 2;

// Two space characters is the default indentation.
public const char DefaultIndentCharacter = ' ';
public const int DefaultIndentSize = 2;
public const int MinimumIndentSize = 0;
public const int MaximumIndentSize = 127;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,16 @@ public JsonSourceGenerationOptionsAttribute(JsonSerializerDefaults defaults)
/// </summary>
public bool WriteIndented { get; set; }

/// <summary>
/// Specifies the default value of <see cref="JsonSerializerOptions.IndentCharacter"/> when set.
/// </summary>
public char IndentCharacter { get; set; }

/// <summary>
/// Specifies the default value of <see cref="JsonSerializerOptions.IndentCharacter"/> when set.
/// </summary>
public int IndentSize { get; set; }

/// <summary>
/// Specifies the default source generation mode for type declarations that don't set a <see cref="JsonSerializableAttribute.GenerationMode"/>.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1168,6 +1168,12 @@ private static void GetLogicForDefaultSerializerOptionsInit(SourceGenerationOpti
if (optionsSpec.WriteIndented is bool writeIndented)
writer.WriteLine($"WriteIndented = {FormatBool(writeIndented)},");

if (optionsSpec.IndentCharacter is char indentCharacter)
writer.WriteLine($"IndentCharacter = {FormatIndentChar(indentCharacter)},");

if (optionsSpec.IndentSize is int indentSize)
writer.WriteLine($"IndentSize = {indentSize},");

writer.Indentation--;
writer.WriteLine("};");

Expand Down Expand Up @@ -1344,6 +1350,7 @@ private static string FormatJsonSerializerDefaults(JsonSerializerDefaults defaul

private static string FormatBool(bool value) => value ? "true" : "false";
private static string FormatStringLiteral(string? value) => value is null ? "null" : $"\"{value}\"";
private static string FormatIndentChar(char value) => value is '\t' ? "'\\t'" : $"'{value}'";

/// <summary>
/// Method used to generate JsonTypeInfo given options instance
Expand Down
12 changes: 12 additions & 0 deletions src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,8 @@ private SourceGenerationOptionsSpec ParseJsonSourceGenerationOptionsAttribute(IN
JsonUnmappedMemberHandling? unmappedMemberHandling = null;
bool? useStringEnumConverter = null;
bool? writeIndented = null;
char? indentCharacter = null;
int? indentSize = null;

if (attributeData.ConstructorArguments.Length > 0)
{
Expand Down Expand Up @@ -373,6 +375,14 @@ private SourceGenerationOptionsSpec ParseJsonSourceGenerationOptionsAttribute(IN
writeIndented = (bool)namedArg.Value.Value!;
break;

case nameof(JsonSourceGenerationOptionsAttribute.IndentCharacter):
indentCharacter = (char)namedArg.Value.Value!;
break;

case nameof(JsonSourceGenerationOptionsAttribute.IndentSize):
indentSize = (int)namedArg.Value.Value!;
break;

case nameof(JsonSourceGenerationOptionsAttribute.GenerationMode):
generationMode = (JsonSourceGenerationMode)namedArg.Value.Value!;
break;
Expand Down Expand Up @@ -404,6 +414,8 @@ private SourceGenerationOptionsSpec ParseJsonSourceGenerationOptionsAttribute(IN
UnmappedMemberHandling = unmappedMemberHandling,
UseStringEnumConverter = useStringEnumConverter,
WriteIndented = writeIndented,
IndentCharacter = indentCharacter,
IndentSize = indentSize,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ public sealed record SourceGenerationOptionsSpec

public required bool? WriteIndented { get; init; }

public required char? IndentCharacter { get; init; }

public required int? IndentSize { get; init; }

public JsonKnownNamingPolicy? GetEffectivePropertyNamingPolicy()
=> PropertyNamingPolicy ?? (Defaults is JsonSerializerDefaults.Web ? JsonKnownNamingPolicy.CamelCase : null);
}
Expand Down
6 changes: 6 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 @@ -394,6 +394,8 @@ public JsonSerializerOptions(System.Text.Json.JsonSerializerOptions options) { }
public System.Text.Json.Serialization.JsonUnknownTypeHandling UnknownTypeHandling { get { throw null; } set { } }
public System.Text.Json.Serialization.JsonUnmappedMemberHandling UnmappedMemberHandling { get { throw null; } set { } }
public bool WriteIndented { get { throw null; } set { } }
public char IndentCharacter { get { throw null; } set { } }
public int IndentSize { get { throw null; } set { } }
[System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)]
[System.ObsoleteAttribute("JsonSerializerOptions.AddContext is obsolete. To register a JsonSerializerContext, use either the TypeInfoResolver or TypeInfoResolverChain properties.", DiagnosticId="SYSLIB0049", UrlFormat="https://aka.ms/dotnet-warnings/{0}")]
public void AddContext<TContext>() where TContext : System.Text.Json.Serialization.JsonSerializerContext, new() { }
Expand Down Expand Up @@ -439,6 +441,8 @@ public partial struct JsonWriterOptions
private int _dummyPrimitive;
public System.Text.Encodings.Web.JavaScriptEncoder? Encoder { readonly get { throw null; } set { } }
public bool Indented { get { throw null; } set { } }
public char IndentCharacter { get { throw null; } set { } }
public int IndentSize { 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 +1078,8 @@ public JsonSourceGenerationOptionsAttribute(System.Text.Json.JsonSerializerDefau
public System.Text.Json.Serialization.JsonUnmappedMemberHandling UnmappedMemberHandling { get { throw null; } set { } }
public bool UseStringEnumConverter { get { throw null; } set { } }
public bool WriteIndented { get { throw null; } set { } }
public char IndentCharacter { get { throw null; } set { } }
public int IndentSize { get { throw null; } set { } }
}
[System.Diagnostics.CodeAnalysis.RequiresDynamicCodeAttribute("JsonStringEnumConverter cannot be statically analyzed and requires runtime code generation. Applications should use the generic JsonStringEnumConverter<TEnum> instead.")]
public partial class JsonStringEnumConverter : System.Text.Json.Serialization.JsonConverterFactory
Expand Down
6 changes: 6 additions & 0 deletions src/libraries/System.Text.Json/src/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -708,4 +708,10 @@
<data name="FormatHalf" xml:space="preserve">
<value>Either the JSON value is not in a supported format, or is out of bounds for a Half.</value>
</data>
<data name="InvalidIndentCharacter" xml:space="preserve">
<value>Supported indentation characters are space and horizontal tab.</value>
</data>
<data name="InvalidIndentSize" xml:space="preserve">
<value>Indentation size must be between {0} and {1}.</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ internal static partial class JsonConstants
// Explicitly skipping ReverseSolidus since that is handled separately
public static ReadOnlySpan<byte> EscapableChars => "\"nrt/ubf"u8;

public const int SpacesPerIndent = 2;
public const int RemoveFlagsBitMask = 0x7FFFFFFF;

// In the worst case, an ASCII character represented as a single utf-8 byte could expand 6x when escaped.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,8 @@ public bool Equals(JsonSerializerOptions? left, JsonSerializerOptions? right)
left._includeFields == right._includeFields &&
left._propertyNameCaseInsensitive == right._propertyNameCaseInsensitive &&
left._writeIndented == right._writeIndented &&
left._indentCharacter == right._indentCharacter &&
left._indentSize == right._indentSize &&
left._typeInfoResolver == right._typeInfoResolver &&
CompareLists(left._converters, right._converters);

Expand Down Expand Up @@ -565,6 +567,8 @@ public int GetHashCode(JsonSerializerOptions options)
AddHashCode(ref hc, options._includeFields);
AddHashCode(ref hc, options._propertyNameCaseInsensitive);
AddHashCode(ref hc, options._writeIndented);
AddHashCode(ref hc, options._indentCharacter);
AddHashCode(ref hc, options._indentSize);
AddHashCode(ref hc, options._typeInfoResolver);
AddListHashCode(ref hc, options._converters);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ public static JsonSerializerOptions Default
private bool _includeFields;
private bool _propertyNameCaseInsensitive;
private bool _writeIndented;
private char _indentCharacter = JsonConstants.DefaultIndentCharacter;
private int _indentSize = JsonConstants.DefaultIndentSize;

/// <summary>
/// Constructs a new <see cref="JsonSerializerOptions"/> instance.
Expand Down Expand Up @@ -124,6 +126,8 @@ public JsonSerializerOptions(JsonSerializerOptions options)
_includeFields = options._includeFields;
_propertyNameCaseInsensitive = options._propertyNameCaseInsensitive;
_writeIndented = options._writeIndented;
_indentCharacter = options._indentCharacter;
_indentSize = options._indentSize;
_typeInfoResolver = options._typeInfoResolver;
EffectiveMaxDepth = options.EffectiveMaxDepth;
ReferenceHandlingStrategy = options.ReferenceHandlingStrategy;
Expand Down Expand Up @@ -645,6 +649,50 @@ public bool WriteIndented
}
}

/// <summary>
/// Defines the indentation character being used when <see cref="WriteIndented" /> is enabled. Defaults to the space character.
/// </summary>
/// <remarks>Allowed characters are space and horizontal tab.</remarks>
/// <exception cref="ArgumentOutOfRangeException"><paramref name="value"/> contains an invalid character.</exception>
/// <exception cref="InvalidOperationException">
/// Thrown if this property is set after serialization or deserialization has occurred.
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
/// </exception>
public char IndentCharacter
{
get
{
return _indentCharacter;
}
set
{
JsonWriterHelper.ValidateIndentCharacter(value);
VerifyMutable();
_indentCharacter = value;
}
}

/// <summary>
/// Defines the indentation size being used when <see cref="WriteIndented" /> is enabled. Defaults to two.
/// </summary>
/// <remarks>Allowed values are all integers between 0 and 127.</remarks>
/// <exception cref="ArgumentOutOfRangeException"><paramref name="value"/> is out of the allowed range.</exception>
/// <exception cref="InvalidOperationException">
/// Thrown if this property is set after serialization or deserialization has occurred.
/// </exception>
public int IndentSize
{
get
{
return _indentSize;
}
set
{
JsonWriterHelper.ValidateIndentSize(value);
VerifyMutable();
_indentSize = value;
}
}

/// <summary>
/// Configures how object references are handled when reading and writing JSON.
/// </summary>
Expand Down Expand Up @@ -876,6 +924,8 @@ internal JsonWriterOptions GetWriterOptions()
{
Encoder = Encoder,
Indented = WriteIndented,
IndentCharacter = IndentCharacter,
IndentSize = IndentSize,
MaxDepth = EffectiveMaxDepth,
#if !DEBUG
SkipValidation = true
Expand Down
12 changes: 12 additions & 0 deletions src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,18 @@ 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_IndentCharacter(string parameterName)
{
throw GetArgumentOutOfRangeException(parameterName, SR.InvalidIndentCharacter);
}

[DoesNotReturn]
public static void ThrowArgumentOutOfRangeException_IndentSize(string parameterName, int minimumSize, int maximumSize)
{
throw GetArgumentOutOfRangeException(parameterName, SR.Format(SR.InvalidIndentSize, minimumSize, maximumSize));
}

[DoesNotReturn]
public static void ThrowArgumentOutOfRangeException_MaxDepthMustBePositive(string parameterName)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,28 +10,46 @@ namespace System.Text.Json
{
internal static partial class JsonWriterHelper
{
public static void WriteIndentation(Span<byte> buffer, int indent)
public static void WriteIndentation(Span<byte> buffer, int indent, byte indentByte)
{
Debug.Assert(indent % JsonConstants.SpacesPerIndent == 0);
Debug.Assert(buffer.Length >= indent);

// Based on perf tests, the break-even point where vectorized Fill is faster
// than explicitly writing the space in a loop is 8.
if (indent < 8)
{
int i = 0;
while (i < indent)
while (i + 1 < indent)
{
buffer[i++] = JsonConstants.Space;
buffer[i++] = JsonConstants.Space;
buffer[i++] = indentByte;
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
buffer[i++] = indentByte;
}

if (i < indent)
{
buffer[i] = indentByte;
}
}
else
{
buffer.Slice(0, indent).Fill(JsonConstants.Space);
buffer.Slice(0, indent).Fill(indentByte);
}
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static void ValidateIndentCharacter(char value)
{
if ((byte)value is not JsonConstants.Space and not JsonConstants.Tab)
ThrowHelper.ThrowArgumentOutOfRangeException_IndentCharacter(nameof(value));
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static void ValidateIndentSize(int value)
{
if (value is < JsonConstants.MinimumIndentSize or > JsonConstants.MaximumIndentSize)
ThrowHelper.ThrowArgumentOutOfRangeException_IndentSize(nameof(value), JsonConstants.MinimumIndentSize, JsonConstants.MaximumIndentSize);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static void ValidateProperty(ReadOnlySpan<byte> propertyName)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ public struct JsonWriterOptions
private int _maxDepth;
private int _optionsMask;

internal readonly byte IndentByte => (_optionsMask & IndentCharacterBit) != 0 ? JsonConstants.Tab : JsonConstants.Space;
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// The encoder to use when escaping strings, or <see langword="null" /> to use the default encoder.
/// </summary>
Expand All @@ -43,6 +45,46 @@ public bool Indented
}
}

/// <summary>
/// Defines the indentation character used by <see cref="Utf8JsonWriter"/> when <see cref="Indented"/> is enabled. Defaults to the space character.
/// </summary>
/// <remarks>Allowed characters are space and horizontal tab.</remarks>
/// <exception cref="ArgumentOutOfRangeException"><paramref name="value"/> contains an invalid character.</exception>
public char IndentCharacter
{
readonly get => (char)IndentByte;
set
{
JsonWriterHelper.ValidateIndentCharacter(value);
if (value is not JsonConstants.DefaultIndentCharacter)
_optionsMask |= IndentCharacterBit;
else
_optionsMask &= ~IndentCharacterBit;
}
}

/// <summary>
/// Defines the indentation size used by <see cref="Utf8JsonWriter"/> when <see cref="Indented"/> is enabled. Defaults to two.
/// </summary>
/// <remarks>Allowed values are integers between 0 and 127.</remarks>
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
/// <exception cref="ArgumentOutOfRangeException"><paramref name="value"/> is out of the allowed range.</exception>
public int IndentSize
{
readonly get => HandleDefaultIndentSize((_optionsMask & IndentSizeMask) >> 3);
set
{
JsonWriterHelper.ValidateIndentSize(value);
_optionsMask = (_optionsMask & ~IndentSizeMask) + (HandleDefaultIndentSize(value) << 3);
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
}
}

private static int HandleDefaultIndentSize(int value) => value switch
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
{
0 => JsonConstants.DefaultIndentSize,
JsonConstants.DefaultIndentSize => 0,
_ => value
};

/// <summary>
/// Gets or sets the maximum depth allowed when writing JSON, with the default (i.e. 0) indicating a max depth of 1000.
/// </summary>
Expand Down Expand Up @@ -93,9 +135,11 @@ public bool SkipValidation
}
}

internal bool IndentedOrNotSkipValidation => _optionsMask != SkipValidationBit; // Equivalent to: Indented || !SkipValidation;
internal bool IndentedOrNotSkipValidation => (_optionsMask & (IndentBit + SkipValidationBit)) != SkipValidationBit; // Equivalent to: Indented || !SkipValidation;
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved

private const int IndentBit = 1;
private const int SkipValidationBit = 2;
private const int IndentCharacterBit = 4;
private const int IndentSizeMask = JsonConstants.MaximumIndentSize << 3;
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
}
}
Loading
Loading