Skip to content

Commit 0986839

Browse files
eiriktsarpalisCopilotjeffhandley
authored
Avoid referencing types of JsonIgnore'd properties. (dotnet#120543)
* Avoid referencing types of JsonIgnore'd properties. * Apply suggestion from @Copilot Co-authored-by: Copilot <[email protected]> * Revert use of Range and Index for down-level support This reverts commit ed3bcc3. * Address feedback. --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Jeff Handley <[email protected]>
1 parent af46988 commit 0986839

6 files changed

Lines changed: 220 additions & 33 deletions

File tree

‎src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -612,7 +612,17 @@ private void GeneratePropMetadataInitFunc(SourceWriter writer, string propInitMe
612612
PropertyGenerationSpec property = properties[i];
613613
string propertyName = property.NameSpecifiedInSourceCode;
614614
string declaringTypeFQN = property.DeclaringType.FullyQualifiedName;
615-
string propertyTypeFQN = property.PropertyType.FullyQualifiedName;
615+
616+
// If the property is ignored and its type is not used anywhere else in the type graph,
617+
// emit a JsonPropertyInfo of type 'object' to avoid unnecessarily referencing the type.
618+
// STJ requires that all ignored properties be included so that it can perform
619+
// necessary run-time validations using configuration not known at compile time
620+
// such as the property naming policy and case sensitivity.
621+
bool isIgnoredPropertyOfUnusedType =
622+
property.DefaultIgnoreCondition is JsonIgnoreCondition.Always &&
623+
!_typeIndex.ContainsKey(property.PropertyType);
624+
625+
string propertyTypeFQN = isIgnoredPropertyOfUnusedType ? "object" : property.PropertyType.FullyQualifiedName;
616626

617627
string getterValue = property switch
618628
{
@@ -653,9 +663,12 @@ private void GeneratePropMetadataInitFunc(SourceWriter writer, string propInitMe
653663
: $"({JsonConverterTypeRef}<{propertyTypeFQN}>){ExpandConverterMethodName}(typeof({propertyTypeFQN}), new {converterFQN}(), {OptionsLocalVariableName})";
654664
}
655665

656-
string attributeProviderFactoryExpr = property.IsProperty
657-
? $"typeof({property.DeclaringType.FullyQualifiedName}).GetProperty({FormatStringLiteral(property.MemberName)}, {InstanceMemberBindingFlagsVariableName}, null, typeof({property.PropertyType.FullyQualifiedName}), {EmptyTypeArray}, null)"
658-
: $"typeof({property.DeclaringType.FullyQualifiedName}).GetField({FormatStringLiteral(property.MemberName)}, {InstanceMemberBindingFlagsVariableName})";
666+
string attributeProviderFactoryExpr = property switch
667+
{
668+
_ when isIgnoredPropertyOfUnusedType => "null",
669+
{ IsProperty: true } => $"typeof({property.DeclaringType.FullyQualifiedName}).GetProperty({FormatStringLiteral(property.MemberName)}, {InstanceMemberBindingFlagsVariableName}, null, typeof({propertyTypeFQN}), {EmptyTypeArray}, null)",
670+
_ => $"typeof({property.DeclaringType.FullyQualifiedName}).GetField({FormatStringLiteral(property.MemberName)}, {InstanceMemberBindingFlagsVariableName})",
671+
};
659672

660673
writer.WriteLine($$"""
661674
var {{InfoVarName}}{{i}} = new {{JsonPropertyInfoValuesTypeRef}}<{{propertyTypeFQN}}>

‎src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/RealWorldContextTests.cs‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1122,6 +1122,7 @@ public void NumberHandlingHonoredOnPoco()
11221122
[InlineData(MemberTypes.Field, nameof(PocoWithMixedVisibilityMembers.FieldWithCustomName), "customField")]
11231123
[InlineData(MemberTypes.Property, nameof(PocoWithMixedVisibilityMembers.BaseProperty))]
11241124
[InlineData(MemberTypes.Property, nameof(PocoWithMixedVisibilityMembers.ShadowProperty))]
1125+
[InlineData(MemberTypes.Property, nameof(PocoWithMixedVisibilityMembers.ExperimentalProperty))]
11251126
public void JsonPropertyInfo_PopulatesAttributeProvider(MemberTypes memberType, string propertyName, string? jsonPropertyName = null)
11261127
{
11271128
if (DefaultContext.JsonSourceGenerationMode is JsonSourceGenerationMode.Serialization)
@@ -1134,7 +1135,17 @@ public void JsonPropertyInfo_PopulatesAttributeProvider(MemberTypes memberType,
11341135
JsonPropertyInfo prop = typeInfo.Properties.FirstOrDefault(prop => prop.Name == name);
11351136
Assert.NotNull(prop);
11361137

1137-
MemberInfo memberInfo = Assert.IsAssignableFrom<MemberInfo>(prop.AttributeProvider);
1138+
MemberInfo memberInfo;
1139+
if (prop.AttributeProvider is null)
1140+
{
1141+
Assert.Equal(typeof(object), prop.PropertyType);
1142+
memberInfo = typeof(PocoWithMixedVisibilityMembers).GetMember(propertyName, memberType, BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance).Single();
1143+
Type actualPropertyType = memberInfo is PropertyInfo pInfo ? pInfo.PropertyType : ((FieldInfo)memberInfo).FieldType;
1144+
Assert.False(typeInfo.Options.TryGetTypeInfo(actualPropertyType, out _));
1145+
return;
1146+
}
1147+
1148+
memberInfo = Assert.IsAssignableFrom<MemberInfo>(prop.AttributeProvider);
11381149
string? actualJsonPropertyName = memberInfo.GetCustomAttribute<JsonPropertyNameAttribute>()?.Name;
11391150

11401151
Assert.True(memberInfo.DeclaringType.IsAssignableFrom(typeInfo.Type));

‎src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/TestClasses.cs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Collections.Generic;
5+
using System.Diagnostics.CodeAnalysis;
56
using System.Text.Json.Serialization;
67
using Microsoft.Extensions.Primitives;
78

@@ -332,8 +333,18 @@ public class PocoWithMixedVisibilityMembers : PocoWithMixedVisibilityMembersBase
332333
public string FieldWithCustomName;
333334

334335
public new int ShadowProperty { get; set; }
336+
337+
#pragma warning disable EXP0001
338+
[JsonIgnore]
339+
public ExperimentalClass ExperimentalProperty { get; set; }
340+
#pragma warning restore EXP0001
335341
}
336342

343+
#if NET
344+
[Experimental("EXP0001")]
345+
#endif
346+
public class ExperimentalClass;
347+
337348
public sealed class ClassWithConflictingIgnoredProperties
338349
{
339350
[JsonIgnore]

‎src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/CompilationHelper.cs‎

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
using Microsoft.CodeAnalysis.CSharp;
1515
using Microsoft.CodeAnalysis.Text;
1616
using Xunit;
17+
using Xunit.Abstractions;
1718

1819
namespace System.Text.Json.SourceGeneration.UnitTests
1920
{
@@ -133,7 +134,10 @@ public static CSharpGeneratorDriver CreateJsonSourceGeneratorDriver(Compilation
133134
#endif
134135
}
135136

136-
public static JsonSourceGeneratorResult RunJsonSourceGenerator(Compilation compilation, bool disableDiagnosticValidation = false)
137+
public static JsonSourceGeneratorResult RunJsonSourceGenerator(
138+
Compilation compilation,
139+
bool disableDiagnosticValidation = false,
140+
ITestOutputHelper? logger = null)
137141
{
138142
var generatedSpecs = ImmutableArray<ContextGenerationSpec>.Empty;
139143
var generator = new JsonSourceGenerator
@@ -144,6 +148,19 @@ public static JsonSourceGeneratorResult RunJsonSourceGenerator(Compilation compi
144148
CSharpGeneratorDriver driver = CreateJsonSourceGeneratorDriver(compilation, generator);
145149
driver.RunGeneratorsAndUpdateCompilation(compilation, out Compilation outCompilation, out ImmutableArray<Diagnostic> diagnostics);
146150

151+
if (logger is not null)
152+
{
153+
foreach (Diagnostic diagnostic in outCompilation.GetDiagnostics().Concat(diagnostics))
154+
{
155+
logger.WriteLine(diagnostic.ToString());
156+
}
157+
158+
foreach (var tree in outCompilation.SyntaxTrees)
159+
{
160+
LogGeneratedCode(tree, logger);
161+
}
162+
}
163+
147164
if (!disableDiagnosticValidation)
148165
{
149166
outCompilation.GetDiagnostics().AssertMaxSeverity(DiagnosticSeverity.Info);
@@ -831,6 +848,59 @@ internal static void AssertMaxSeverity(this IEnumerable<Diagnostic> diagnostics,
831848
{
832849
Assert.DoesNotContain(diagnostics, diagnostic => diagnostic.Severity > maxSeverity);
833850
}
851+
852+
private static void LogGeneratedCode(SyntaxTree tree, ITestOutputHelper logger)
853+
{
854+
logger.WriteLine(FileSeparator);
855+
logger.WriteLine($"{tree.FilePath} content:");
856+
logger.WriteLine(FileSeparator);
857+
using NumberedSourceFileWriter lineWriter = new(logger);
858+
tree.GetRoot().WriteTo(lineWriter);
859+
lineWriter.WriteLine(string.Empty);
860+
}
861+
862+
private static readonly string FileSeparator = new string('=', 140);
863+
864+
private sealed class NumberedSourceFileWriter : TextWriter
865+
{
866+
private readonly ITestOutputHelper _logger;
867+
private readonly StringBuilder _lineBuilder = new StringBuilder();
868+
private int _lineNumber;
869+
870+
internal NumberedSourceFileWriter(ITestOutputHelper logger)
871+
{
872+
_logger = logger;
873+
}
874+
875+
public override Encoding Encoding => Encoding.Unicode;
876+
877+
public override void WriteLine(string? value)
878+
{
879+
_logger.WriteLine($"{++_lineNumber,6}: {_lineBuilder}{value}");
880+
_lineBuilder.Clear();
881+
}
882+
883+
public override void Write(string? value)
884+
{
885+
if (value is null)
886+
{
887+
return;
888+
}
889+
890+
if (value.EndsWith("\r\n", StringComparison.Ordinal))
891+
{
892+
WriteLine(value.Substring(0, value.Length - 2));
893+
}
894+
else if (value.EndsWith("\n", StringComparison.Ordinal))
895+
{
896+
WriteLine(value.Substring(0, value.Length - 1));
897+
}
898+
else
899+
{
900+
_lineBuilder.Append(value);
901+
}
902+
}
903+
}
834904
}
835905

836906
public record struct DiagnosticData(

0 commit comments

Comments
 (0)