Skip to content

Commit 0bc0c0a

Browse files
[release/6.0] Emit diagnostics & exceptions for sourcegen handling init-only properties & JsonInclude attributes (#59097)
* Emit diagnostic warnings and exceptions when sourcegen is handling init-only inaccessible JsonInclude properties. * add localizations for runtime exception messages * remove exception messages from resource strings * fix alphabetical ordering Co-authored-by: Eirik Tsarpalis <[email protected]>
1 parent a4df6d9 commit 0bc0c0a

25 files changed

+564
-88
lines changed
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
using System.Text;
7+
8+
namespace System.Text.Json.SourceGeneration
9+
{
10+
public sealed partial class JsonSourceGenerator
11+
{
12+
private sealed partial class Emitter
13+
{
14+
/// <summary>
15+
/// Unlike sourcegen warnings, exception messages should not be localized so we keep them in source.
16+
/// </summary>
17+
private static class ExceptionMessages
18+
{
19+
public const string InaccessibleJsonIncludePropertiesNotSupported =
20+
"The member '{0}.{1}' has been annotated with the JsonIncludeAttribute but is not visible to the source generator.";
21+
22+
public const string IncompatibleConverterType =
23+
"The converter '{0}' is not compatible with the type '{1}'.";
24+
25+
public const string InitOnlyPropertyDeserializationNotSupported =
26+
"Deserialization of init-only properties is currently not supported in source generation mode.";
27+
28+
public const string InvalidJsonConverterFactoryOutput =
29+
"The converter '{0}' cannot return null or a JsonConverterFactory instance.";
30+
31+
public const string InvalidSerializablePropertyConfiguration =
32+
"Invalid serializable-property configuration specified for type '{0}'. For more information, see 'JsonSourceGenerationMode.Serialization'.";
33+
};
34+
}
35+
}
36+
}

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

Lines changed: 26 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ private string GenerateForTypeWithUnknownConverter(TypeGenerationSpec typeMetada
324324
}}
325325
else
326326
{{
327-
throw new {InvalidOperationExceptionTypeRef}($""The converter '{{converter.GetType()}}' is not compatible with the type '{{typeToConvert}}'."");
327+
throw new {InvalidOperationExceptionTypeRef}(string.Format(""{ExceptionMessages.IncompatibleConverterType}"", converter.GetType(), typeToConvert));
328328
}}
329329
}}");
330330
}
@@ -333,7 +333,7 @@ private string GenerateForTypeWithUnknownConverter(TypeGenerationSpec typeMetada
333333
metadataInitSource.Append($@"
334334
if (!converter.CanConvert(typeToConvert))
335335
{{
336-
throw new {InvalidOperationExceptionTypeRef}($""The converter '{{converter.GetType()}}' is not compatible with the type '{{typeToConvert}}'."");
336+
throw new {InvalidOperationExceptionTypeRef}(string.Format(""{ExceptionMessages.IncompatibleConverterType}"", converter.GetType(), typeToConvert));
337337
}}");
338338
}
339339

@@ -711,25 +711,28 @@ private string GeneratePropMetadataInitFunc(TypeGenerationSpec typeGenerationSpe
711711
? @$"jsonPropertyName: ""{memberMetadata.JsonPropertyName}"""
712712
: "jsonPropertyName: null";
713713

714-
string getterNamedArg = memberMetadata.CanUseGetter &&
715-
memberMetadata.DefaultIgnoreCondition != JsonIgnoreCondition.Always
716-
? $"getter: static (obj) => (({declaringTypeCompilableName})obj).{clrPropertyName}"
717-
: "getter: null";
718-
719-
string setterNamedArg;
720-
if (memberMetadata.CanUseSetter &&
721-
memberMetadata.DefaultIgnoreCondition != JsonIgnoreCondition.Always)
714+
string getterNamedArg = memberMetadata switch
722715
{
723-
string propMutation = typeGenerationSpec.IsValueType
724-
? @$"{UnsafeTypeRef}.Unbox<{declaringTypeCompilableName}>(obj).{clrPropertyName} = value!"
725-
: $@"(({declaringTypeCompilableName})obj).{clrPropertyName} = value!";
726-
727-
setterNamedArg = $"setter: static (obj, value) => {propMutation}";
728-
}
729-
else
716+
{ DefaultIgnoreCondition: JsonIgnoreCondition.Always } => "getter: null",
717+
{ CanUseGetter: true } => $"getter: static (obj) => (({declaringTypeCompilableName})obj).{clrPropertyName}",
718+
{ CanUseGetter: false, HasJsonInclude: true }
719+
=> @$"getter: static (obj) => throw new {InvalidOperationExceptionTypeRef}(""{string.Format(ExceptionMessages.InaccessibleJsonIncludePropertiesNotSupported, typeGenerationSpec.Type.Name, memberMetadata.ClrName)}"")",
720+
_ => "getter: null"
721+
};
722+
723+
string setterNamedArg = memberMetadata switch
730724
{
731-
setterNamedArg = "setter: null";
732-
}
725+
{ DefaultIgnoreCondition: JsonIgnoreCondition.Always } => "setter: null",
726+
{ CanUseSetter: true, IsInitOnlySetter: true }
727+
=> @$"setter: static (obj, value) => throw new {InvalidOperationExceptionTypeRef}(""{ExceptionMessages.InitOnlyPropertyDeserializationNotSupported}"")",
728+
{ CanUseSetter: true } when typeGenerationSpec.IsValueType
729+
=> $@"setter: static (obj, value) => {UnsafeTypeRef}.Unbox<{declaringTypeCompilableName}>(obj).{clrPropertyName} = value!",
730+
{ CanUseSetter: true }
731+
=> @$"setter: static (obj, value) => (({declaringTypeCompilableName})obj).{clrPropertyName} = value!",
732+
{ CanUseSetter: false, HasJsonInclude: true }
733+
=> @$"setter: static (obj, value) => throw new {InvalidOperationExceptionTypeRef}(""{string.Format(ExceptionMessages.InaccessibleJsonIncludePropertiesNotSupported, typeGenerationSpec.Type.Name, memberMetadata.ClrName)}"")",
734+
_ => "setter: null",
735+
};
733736

734737
JsonIgnoreCondition? ignoreCondition = memberMetadata.DefaultIgnoreCondition;
735738
string ignoreConditionNamedArg = ignoreCondition.HasValue
@@ -821,12 +824,12 @@ private string GenerateFastPathFuncForObject(TypeGenerationSpec typeGenSpec)
821824
out Dictionary<string, PropertyGenerationSpec>? serializableProperties,
822825
out bool castingRequiredForProps))
823826
{
824-
string exceptionMessage = @$"""Invalid serializable-property configuration specified for type '{typeRef}'. For more information, see 'JsonSourceGenerationMode.Serialization'.""";
827+
string exceptionMessage = string.Format(ExceptionMessages.InvalidSerializablePropertyConfiguration, typeRef);
825828

826829
return GenerateFastPathFuncForType(
827830
serializeMethodName,
828831
typeRef,
829-
$@"throw new {InvalidOperationExceptionTypeRef}({exceptionMessage});",
832+
$@"throw new {InvalidOperationExceptionTypeRef}(""{exceptionMessage}"");",
830833
canBeNull: false); // Skip null check since we want to throw an exception straightaway.
831834
}
832835

@@ -1202,7 +1205,7 @@ private string GetFetchLogicForRuntimeSpecifiedCustomConverter()
12021205
converter = factory.CreateConverter(type, {OptionsInstanceVariableName});
12031206
if (converter == null || converter is {JsonConverterFactoryTypeRef})
12041207
{{
1205-
throw new {InvalidOperationExceptionTypeRef}($""The converter '{{factory.GetType()}}' cannot return null or a JsonConverterFactory instance."");
1208+
throw new {InvalidOperationExceptionTypeRef}(string.Format(""{ExceptionMessages.InvalidJsonConverterFactoryOutput}"", factory.GetType()));
12061209
}}
12071210
}}
12081211
@@ -1233,7 +1236,7 @@ private string GetFetchLogicForGetCustomConverter_TypesWithFactories()
12331236
{JsonConverterTypeRef}? converter = factory.CreateConverter(type, {Emitter.OptionsInstanceVariableName});
12341237
if (converter == null || converter is {JsonConverterFactoryTypeRef})
12351238
{{
1236-
throw new {InvalidOperationExceptionTypeRef}($""The converter '{{factory.GetType()}}' cannot return null or a JsonConverterFactory instance."");
1239+
throw new {InvalidOperationExceptionTypeRef}(string.Format(""{ExceptionMessages.InvalidJsonConverterFactoryOutput}"", factory.GetType()));
12371240
}}
12381241
12391242
return converter;

src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,22 @@ private sealed class Parser
139139
defaultSeverity: DiagnosticSeverity.Error,
140140
isEnabledByDefault: true);
141141

142+
private static DiagnosticDescriptor InitOnlyPropertyDeserializationNotSupported { get; } = new DiagnosticDescriptor(
143+
id: "SYSLIB1037",
144+
title: new LocalizableResourceString(nameof(SR.InitOnlyPropertyDeserializationNotSupportedTitle), SR.ResourceManager, typeof(FxResources.System.Text.Json.SourceGeneration.SR)),
145+
messageFormat: new LocalizableResourceString(nameof(SR.InitOnlyPropertyDeserializationNotSupportedFormat), SR.ResourceManager, typeof(FxResources.System.Text.Json.SourceGeneration.SR)),
146+
category: JsonConstants.SystemTextJsonSourceGenerationName,
147+
defaultSeverity: DiagnosticSeverity.Warning,
148+
isEnabledByDefault: true);
149+
150+
private static DiagnosticDescriptor InaccessibleJsonIncludePropertiesNotSupported { get; } = new DiagnosticDescriptor(
151+
id: "SYSLIB1038",
152+
title: new LocalizableResourceString(nameof(SR.InaccessibleJsonIncludePropertiesNotSupportedTitle), SR.ResourceManager, typeof(FxResources.System.Text.Json.SourceGeneration.SR)),
153+
messageFormat: new LocalizableResourceString(nameof(SR.InaccessibleJsonIncludePropertiesNotSupportedFormat), SR.ResourceManager, typeof(FxResources.System.Text.Json.SourceGeneration.SR)),
154+
category: JsonConstants.SystemTextJsonSourceGenerationName,
155+
defaultSeverity: DiagnosticSeverity.Warning,
156+
isEnabledByDefault: true);
157+
142158
public Parser(Compilation compilation, in JsonSourceGenerationContext sourceGenerationContext)
143159
{
144160
_compilation = compilation;
@@ -624,6 +640,7 @@ private TypeGenerationSpec GetOrAddTypeGenerationSpec(Type type, JsonSourceGener
624640
string? converterInstatiationLogic = null;
625641
bool implementsIJsonOnSerialized = false;
626642
bool implementsIJsonOnSerializing = false;
643+
bool hasEncounteredInitOnlyProperties = false;
627644
bool hasTypeFactoryConverter = false;
628645
bool hasPropertyFactoryConverters = false;
629646

@@ -954,6 +971,17 @@ void CacheMemberHelper()
954971
dataExtensionPropGenSpec = GetOrAddTypeGenerationSpec(propType, generationMode);
955972
_implicitlyRegisteredTypes.Add(dataExtensionPropGenSpec);
956973
}
974+
975+
if (!hasEncounteredInitOnlyProperties && spec.CanUseSetter && spec.IsInitOnlySetter)
976+
{
977+
_sourceGenerationContext.ReportDiagnostic(Diagnostic.Create(InitOnlyPropertyDeserializationNotSupported, Location.None, new string[] { type.Name }));
978+
hasEncounteredInitOnlyProperties = true;
979+
}
980+
981+
if (spec.HasJsonInclude && (!spec.CanUseGetter || !spec.CanUseSetter || !spec.IsPublic))
982+
{
983+
_sourceGenerationContext.ReportDiagnostic(Diagnostic.Create(InaccessibleJsonIncludePropertiesNotSupported, Location.None, new string[] { type.Name, spec.ClrName }));
984+
}
957985
}
958986
}
959987

@@ -1079,7 +1107,8 @@ private PropertyGenerationSpec GetPropertyGenerationSpec(
10791107
out bool canUseGetter,
10801108
out bool canUseSetter,
10811109
out bool getterIsVirtual,
1082-
out bool setterIsVirtual);
1110+
out bool setterIsVirtual,
1111+
out bool setterIsInitOnly);
10831112

10841113
string clrName = memberInfo.Name;
10851114
string runtimePropertyName = DetermineRuntimePropName(clrName, jsonPropertyName, _currentContextNamingPolicy);
@@ -1095,6 +1124,7 @@ private PropertyGenerationSpec GetPropertyGenerationSpec(
10951124
RuntimePropertyName = runtimePropertyName,
10961125
PropertyNameVarName = propertyNameVarName,
10971126
IsReadOnly = isReadOnly,
1127+
IsInitOnlySetter = setterIsInitOnly,
10981128
CanUseGetter = canUseGetter,
10991129
CanUseSetter = canUseSetter,
11001130
GetterIsVirtual = getterIsVirtual,
@@ -1227,13 +1257,15 @@ private static void ProcessMember(
12271257
out bool canUseGetter,
12281258
out bool canUseSetter,
12291259
out bool getterIsVirtual,
1230-
out bool setterIsVirtual)
1260+
out bool setterIsVirtual,
1261+
out bool setterIsInitOnly)
12311262
{
12321263
isPublic = false;
12331264
canUseGetter = false;
12341265
canUseSetter = false;
12351266
getterIsVirtual = false;
12361267
setterIsVirtual = false;
1268+
setterIsInitOnly = false;
12371269

12381270
switch (memberInfo)
12391271
{
@@ -1260,15 +1292,16 @@ private static void ProcessMember(
12601292
if (setMethod != null)
12611293
{
12621294
isReadOnly = false;
1295+
setterIsInitOnly = setMethod.IsInitOnly();
12631296

12641297
if (setMethod.IsPublic)
12651298
{
12661299
isPublic = true;
1267-
canUseSetter = !setMethod.IsInitOnly();
1300+
canUseSetter = true;
12681301
}
12691302
else if (setMethod.IsAssembly)
12701303
{
1271-
canUseSetter = hasJsonInclude && !setMethod.IsInitOnly();
1304+
canUseSetter = hasJsonInclude;
12721305
}
12731306

12741307
setterIsVirtual = setMethod.IsVirtual;

src/libraries/System.Text.Json/gen/PropertyGenerationSpec.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ internal sealed class PropertyGenerationSpec
1616
/// </summary>
1717
public bool IsProperty { get; init; }
1818

19+
/// <summary>
20+
/// If representing a property, returns true if either the getter or setter are public.
21+
/// </summary>
1922
public bool IsPublic { get; init; }
2023

2124
public bool IsVirtual { get; init; }
@@ -39,6 +42,11 @@ internal sealed class PropertyGenerationSpec
3942
/// </summary>
4043
public bool IsReadOnly { get; init; }
4144

45+
/// <summary>
46+
/// Whether the property has an init-only set method.
47+
/// </summary>
48+
public bool IsInitOnlySetter { get; init; }
49+
4250
/// <summary>
4351
/// Whether the property has a public or internal (only usable when JsonIncludeAttribute is specified)
4452
/// getter that can be referenced in generated source code.

src/libraries/System.Text.Json/gen/Resources/Strings.resx

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,4 +153,16 @@
153153
<data name="DataExtensionPropertyInvalidTitle" xml:space="preserve">
154154
<value>Data extension property type invalid.</value>
155155
</data>
156-
</root>
156+
<data name="InitOnlyPropertyDeserializationNotSupportedTitle" xml:space="preserve">
157+
<value>Deserialization of init-only properties is currently not supported in source generation mode.</value>
158+
</data>
159+
<data name="InitOnlyPropertyDeserializationNotSupportedFormat" xml:space="preserve">
160+
<value>The type '{0}' defines init-only properties, deserialization of which is currently not supported in source generation mode.</value>
161+
</data>
162+
<data name="InaccessibleJsonIncludePropertiesNotSupportedTitle" xml:space="preserve">
163+
<value>Inaccessible properties annotated with the JsonIncludeAttribute are not supported in source generation mode.</value>
164+
</data>
165+
<data name="InaccessibleJsonIncludePropertiesNotSupportedFormat" xml:space="preserve">
166+
<value>The member '{0}.{1}' has been annotated with the JsonIncludeAttribute but is not visible to the source generator.</value>
167+
</data>
168+
</root>

src/libraries/System.Text.Json/gen/Resources/xlf/Strings.cs.xlf

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,26 @@
3232
<target state="translated">Duplicitní název typu</target>
3333
<note />
3434
</trans-unit>
35+
<trans-unit id="InaccessibleJsonIncludePropertiesNotSupportedFormat">
36+
<source>The member '{0}.{1}' has been annotated with the JsonIncludeAttribute but is not visible to the source generator.</source>
37+
<target state="new">The member '{0}.{1}' has been annotated with the JsonIncludeAttribute but is not visible to the source generator.</target>
38+
<note />
39+
</trans-unit>
40+
<trans-unit id="InaccessibleJsonIncludePropertiesNotSupportedTitle">
41+
<source>Inaccessible properties annotated with the JsonIncludeAttribute are not supported in source generation mode.</source>
42+
<target state="new">Inaccessible properties annotated with the JsonIncludeAttribute are not supported in source generation mode.</target>
43+
<note />
44+
</trans-unit>
45+
<trans-unit id="InitOnlyPropertyDeserializationNotSupportedFormat">
46+
<source>The type '{0}' defines init-only properties, deserialization of which is currently not supported in source generation mode.</source>
47+
<target state="new">The type '{0}' defines init-only properties, deserialization of which is currently not supported in source generation mode.</target>
48+
<note />
49+
</trans-unit>
50+
<trans-unit id="InitOnlyPropertyDeserializationNotSupportedTitle">
51+
<source>Deserialization of init-only properties is currently not supported in source generation mode.</source>
52+
<target state="new">Deserialization of init-only properties is currently not supported in source generation mode.</target>
53+
<note />
54+
</trans-unit>
3555
<trans-unit id="MultipleJsonConstructorAttributeFormat">
3656
<source>Type '{0}' has multiple constructors annotated with 'JsonConstructorAttribute'.</source>
3757
<target state="translated">Typ {0} má více konstruktorů anotovaných s JsonConstructorAttribute. </target>

0 commit comments

Comments
 (0)