Skip to content

Commit

Permalink
Emit diagnostics & exceptions for sourcegen handling init-only proper…
Browse files Browse the repository at this point in the history 
…ties & JsonInclude attributes (#58993)

* Emit diagnostic warnings and exceptions when sourcegen is handling init-only inaccessible JsonInclude properties.

* add localizations for runtime exception messages
  • Loading branch information
@eiriktsarpalis
eiriktsarpalis authored Sep 13, 2021
1 parent 00c38c7 commit e62c4fe
Show file tree
Hide file tree
Showing 23 changed files with 867 additions and 87 deletions.
49 changes: 26 additions & 23 deletions src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ private string GenerateForTypeWithUnknownConverter(TypeGenerationSpec typeMetada
}}
else
{{
throw new {InvalidOperationExceptionTypeRef}($""The converter '{{converter.GetType()}}' is not compatible with the type '{{typeToConvert}}'."");
throw new {InvalidOperationExceptionTypeRef}(string.Format(""{SR.Exception_IncompatibleConverterType}"", converter.GetType(), typeToConvert));
}}
}}");
}
Expand All @@ -333,7 +333,7 @@ private string GenerateForTypeWithUnknownConverter(TypeGenerationSpec typeMetada
metadataInitSource.Append($@"
if (!converter.CanConvert(typeToConvert))
{{
throw new {InvalidOperationExceptionTypeRef}($""The converter '{{converter.GetType()}}' is not compatible with the type '{{typeToConvert}}'."");
throw new {InvalidOperationExceptionTypeRef}(string.Format(""{SR.Exception_IncompatibleConverterType}"", converter.GetType(), typeToConvert));
}}");
}

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

string getterNamedArg = memberMetadata.CanUseGetter &&
memberMetadata.DefaultIgnoreCondition != JsonIgnoreCondition.Always
? $"getter: static (obj) => (({declaringTypeCompilableName})obj).{clrPropertyName}"
: "getter: null";

string setterNamedArg;
if (memberMetadata.CanUseSetter &&
memberMetadata.DefaultIgnoreCondition != JsonIgnoreCondition.Always)
string getterNamedArg = memberMetadata switch
{
string propMutation = typeGenerationSpec.IsValueType
? @$"{UnsafeTypeRef}.Unbox<{declaringTypeCompilableName}>(obj).{clrPropertyName} = value!"
: $@"(({declaringTypeCompilableName})obj).{clrPropertyName} = value!";

setterNamedArg = $"setter: static (obj, value) => {propMutation}";
}
else
{ DefaultIgnoreCondition: JsonIgnoreCondition.Always } => "getter: null",
{ CanUseGetter: true } => $"getter: static (obj) => (({declaringTypeCompilableName})obj).{clrPropertyName}",
{ CanUseGetter: false, HasJsonInclude: true }
=> @$"getter: static (obj) => throw new {InvalidOperationExceptionTypeRef}(""{SR.Format(SR.Exception_InaccessibleJsonIncludePropertiesNotSupported, typeGenerationSpec.Type.Name, memberMetadata.ClrName)}"")",
_ => "getter: null"
};

string setterNamedArg = memberMetadata switch
{
setterNamedArg = "setter: null";
}
{ DefaultIgnoreCondition: JsonIgnoreCondition.Always } => "setter: null",
{ CanUseSetter: true, IsInitOnlySetter: true }
=> @$"setter: static (obj, value) => throw new {InvalidOperationExceptionTypeRef}(""{SR.Exception_InitOnlyPropertyDeserializationNotSupported}"")",
{ CanUseSetter: true } when typeGenerationSpec.IsValueType
=> $@"setter: static (obj, value) => {UnsafeTypeRef}.Unbox<{declaringTypeCompilableName}>(obj).{clrPropertyName} = value!",
{ CanUseSetter: true }
=> @$"setter: static (obj, value) => (({declaringTypeCompilableName})obj).{clrPropertyName} = value!",
{ CanUseSetter: false, HasJsonInclude: true }
=> @$"setter: static (obj, value) => throw new {InvalidOperationExceptionTypeRef}(""{SR.Format(SR.Exception_InaccessibleJsonIncludePropertiesNotSupported, typeGenerationSpec.Type.Name, memberMetadata.ClrName)}"")",
_ => "setter: null",
};

JsonIgnoreCondition? ignoreCondition = memberMetadata.DefaultIgnoreCondition;
string ignoreConditionNamedArg = ignoreCondition.HasValue
Expand Down Expand Up @@ -821,12 +824,12 @@ private string GenerateFastPathFuncForObject(TypeGenerationSpec typeGenSpec)
out Dictionary<string, PropertyGenerationSpec>? serializableProperties,
out bool castingRequiredForProps))
{
string exceptionMessage = @$"""Invalid serializable-property configuration specified for type '{typeRef}'. For more information, see 'JsonSourceGenerationMode.Serialization'.""";
string exceptionMessage = SR.Format(SR.Exception_InvalidSerializablePropertyConfiguration, typeRef);

return GenerateFastPathFuncForType(
serializeMethodName,
typeRef,
$@"throw new {InvalidOperationExceptionTypeRef}({exceptionMessage});",
$@"throw new {InvalidOperationExceptionTypeRef}(""{exceptionMessage}"");",
canBeNull: false); // Skip null check since we want to throw an exception straightaway.
}

Expand Down Expand Up @@ -1202,7 +1205,7 @@ private string GetFetchLogicForRuntimeSpecifiedCustomConverter()
converter = factory.CreateConverter(type, {OptionsInstanceVariableName});
if (converter == null || converter is {JsonConverterFactoryTypeRef})
{{
throw new {InvalidOperationExceptionTypeRef}($""The converter '{{factory.GetType()}}' cannot return null or a JsonConverterFactory instance."");
throw new {InvalidOperationExceptionTypeRef}(string.Format(""{SR.Exception_InvalidJsonConverterFactoryOutput}"", factory.GetType()));
}}
}}
Expand Down Expand Up @@ -1233,7 +1236,7 @@ private string GetFetchLogicForGetCustomConverter_TypesWithFactories()
{JsonConverterTypeRef}? converter = factory.CreateConverter(type, {Emitter.OptionsInstanceVariableName});
if (converter == null || converter is {JsonConverterFactoryTypeRef})
{{
throw new {InvalidOperationExceptionTypeRef}($""The converter '{{factory.GetType()}}' cannot return null or a JsonConverterFactory instance."");
throw new {InvalidOperationExceptionTypeRef}(string.Format(""{SR.Exception_InvalidJsonConverterFactoryOutput}"", factory.GetType()));
}}
return converter;
Expand Down
41 changes: 37 additions & 4 deletions src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,22 @@ private sealed class Parser
defaultSeverity: DiagnosticSeverity.Error,
isEnabledByDefault: true);

private static DiagnosticDescriptor InitOnlyPropertyDeserializationNotSupported { get; } = new DiagnosticDescriptor(
id: "SYSLIB1037",
title: new LocalizableResourceString(nameof(SR.InitOnlyPropertyDeserializationNotSupportedTitle), SR.ResourceManager, typeof(FxResources.System.Text.Json.SourceGeneration.SR)),
messageFormat: new LocalizableResourceString(nameof(SR.InitOnlyPropertyDeserializationNotSupportedFormat), SR.ResourceManager, typeof(FxResources.System.Text.Json.SourceGeneration.SR)),
category: JsonConstants.SystemTextJsonSourceGenerationName,
defaultSeverity: DiagnosticSeverity.Warning,
isEnabledByDefault: true);

private static DiagnosticDescriptor InaccessibleJsonIncludePropertiesNotSupported { get; } = new DiagnosticDescriptor(
id: "SYSLIB1038",
title: new LocalizableResourceString(nameof(SR.InaccessibleJsonIncludePropertiesNotSupportedTitle), SR.ResourceManager, typeof(FxResources.System.Text.Json.SourceGeneration.SR)),
messageFormat: new LocalizableResourceString(nameof(SR.InaccessibleJsonIncludePropertiesNotSupportedFormat), SR.ResourceManager, typeof(FxResources.System.Text.Json.SourceGeneration.SR)),
category: JsonConstants.SystemTextJsonSourceGenerationName,
defaultSeverity: DiagnosticSeverity.Warning,
isEnabledByDefault: true);

public Parser(Compilation compilation, in SourceProductionContext sourceProductionContext)
{
_compilation = compilation;
Expand Down Expand Up @@ -624,6 +640,7 @@ private TypeGenerationSpec GetOrAddTypeGenerationSpec(Type type, JsonSourceGener
string? converterInstatiationLogic = null;
bool implementsIJsonOnSerialized = false;
bool implementsIJsonOnSerializing = false;
bool hasEncounteredInitOnlyProperties = false;
bool hasTypeFactoryConverter = false;
bool hasPropertyFactoryConverters = false;

Expand Down Expand Up @@ -954,6 +971,17 @@ void CacheMemberHelper()
dataExtensionPropGenSpec = GetOrAddTypeGenerationSpec(propType, generationMode);
_implicitlyRegisteredTypes.Add(dataExtensionPropGenSpec);
}

if (!hasEncounteredInitOnlyProperties && spec.CanUseSetter && spec.IsInitOnlySetter)
{
_sourceProductionContext.ReportDiagnostic(Diagnostic.Create(InitOnlyPropertyDeserializationNotSupported, Location.None, new string[] { type.Name }));
hasEncounteredInitOnlyProperties = true;
}

if (spec.HasJsonInclude && (!spec.CanUseGetter || !spec.CanUseSetter || !spec.IsPublic))
{
_sourceProductionContext.ReportDiagnostic(Diagnostic.Create(InaccessibleJsonIncludePropertiesNotSupported, Location.None, new string[] { type.Name, spec.ClrName }));
}
}
}

Expand Down Expand Up @@ -1079,7 +1107,8 @@ private PropertyGenerationSpec GetPropertyGenerationSpec(
out bool canUseGetter,
out bool canUseSetter,
out bool getterIsVirtual,
out bool setterIsVirtual);
out bool setterIsVirtual,
out bool setterIsInitOnly);

string clrName = memberInfo.Name;
string runtimePropertyName = DetermineRuntimePropName(clrName, jsonPropertyName, _currentContextNamingPolicy);
Expand All @@ -1095,6 +1124,7 @@ private PropertyGenerationSpec GetPropertyGenerationSpec(
RuntimePropertyName = runtimePropertyName,
PropertyNameVarName = propertyNameVarName,
IsReadOnly = isReadOnly,
IsInitOnlySetter = setterIsInitOnly,
CanUseGetter = canUseGetter,
CanUseSetter = canUseSetter,
GetterIsVirtual = getterIsVirtual,
Expand Down Expand Up @@ -1227,13 +1257,15 @@ private static void ProcessMember(
out bool canUseGetter,
out bool canUseSetter,
out bool getterIsVirtual,
out bool setterIsVirtual)
out bool setterIsVirtual,
out bool setterIsInitOnly)
{
isPublic = false;
canUseGetter = false;
canUseSetter = false;
getterIsVirtual = false;
setterIsVirtual = false;
setterIsInitOnly = false;

switch (memberInfo)
{
Expand All @@ -1260,15 +1292,16 @@ private static void ProcessMember(
if (setMethod != null)
{
isReadOnly = false;
setterIsInitOnly = setMethod.IsInitOnly();

if (setMethod.IsPublic)
{
isPublic = true;
canUseSetter = !setMethod.IsInitOnly();
canUseSetter = true;
}
else if (setMethod.IsAssembly)
{
canUseSetter = hasJsonInclude && !setMethod.IsInitOnly();
canUseSetter = hasJsonInclude;
}

setterIsVirtual = setMethod.IsVirtual;
Expand Down
8 changes: 8 additions & 0 deletions src/libraries/System.Text.Json/gen/PropertyGenerationSpec.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ internal sealed class PropertyGenerationSpec
/// </summary>
public bool IsProperty { get; init; }

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

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

/// <summary>
/// Whether the property has an init-only set method.
/// </summary>
public bool IsInitOnlySetter { get; init; }

/// <summary>
/// Whether the property has a public or internal (only usable when JsonIncludeAttribute is specified)
/// getter that can be referenced in generated source code.
Expand Down
30 changes: 29 additions & 1 deletion src/libraries/System.Text.Json/gen/Resources/Strings.resx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,4 +153,32 @@
<data name="DataExtensionPropertyInvalidTitle" xml:space="preserve">
<value>Data extension property type invalid.</value>
</data>
</root>
<data name="InitOnlyPropertyDeserializationNotSupportedTitle" xml:space="preserve">
<value>Deserialization of init-only properties is currently not supported in source generation mode.</value>
</data>
<data name="InitOnlyPropertyDeserializationNotSupportedFormat" xml:space="preserve">
<value>The type '{0}' defines init-only properties, deserialization of which is currently not supported in source generation mode.</value>
</data>
<data name="InaccessibleJsonIncludePropertiesNotSupportedTitle" xml:space="preserve">
<value>Inaccessible properties annotated with the JsonIncludeAttribute are not supported in source generation mode.</value>
</data>
<data name="InaccessibleJsonIncludePropertiesNotSupportedFormat" xml:space="preserve">
<value>The member '{0}.{1}' has been annotated with the JsonIncludeAttribute but is not visible to the source generator.</value>
</data>
<!-- runtime exception messages -->
<data name="Exception_IncompatibleConverterType" xml:space="preserve">
<value>The converter '{0}' is not compatible with the type '{1}'.</value>
</data>
<data name="Exception_InitOnlyPropertyDeserializationNotSupported" xml:space="preserve">
<value>Deserialization of init-only properties is currently not supported in source generation mode.</value>
</data>
<data name="Exception_InaccessibleJsonIncludePropertiesNotSupported" xml:space="preserve">
<value>The member '{0}.{1}' has been annotated with the JsonIncludeAttribute but is not visible to the source generator.</value>
</data>
<data name="Exception_InvalidSerializablePropertyConfiguration" xml:space="preserve">
<value>Invalid serializable-property configuration specified for type '{0}'. For more information, see 'JsonSourceGenerationMode.Serialization'.</value>
</data>
<data name="Exception_InvalidJsonConverterFactoryOutput" xml:space="preserve">
<value>The converter '{0}' cannot return null or a JsonConverterFactory instance.</value>
</data>
</root>
45 changes: 45 additions & 0 deletions src/libraries/System.Text.Json/gen/Resources/xlf/Strings.cs.xlf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,51 @@
<target state="translated">Duplicitní název typu</target>
<note />
</trans-unit>
<trans-unit id="Exception_InaccessibleJsonIncludePropertiesNotSupported">
<source>The member '{0}.{1}' has been annotated with the JsonIncludeAttribute but is not visible to the source generator.</source>
<target state="new">The member '{0}.{1}' has been annotated with the JsonIncludeAttribute but is not visible to the source generator.</target>
<note />
</trans-unit>
<trans-unit id="Exception_IncompatibleConverterType">
<source>The converter '{0}' is not compatible with the type '{1}'.</source>
<target state="new">The converter '{0}' is not compatible with the type '{1}'.</target>
<note />
</trans-unit>
<trans-unit id="Exception_InitOnlyPropertyDeserializationNotSupported">
<source>Deserialization of init-only properties is currently not supported in source generation mode.</source>
<target state="new">Deserialization of init-only properties is currently not supported in source generation mode.</target>
<note />
</trans-unit>
<trans-unit id="Exception_InvalidJsonConverterFactoryOutput">
<source>The converter '{0}' cannot return null or a JsonConverterFactory instance.</source>
<target state="new">The converter '{0}' cannot return null or a JsonConverterFactory instance.</target>
<note />
</trans-unit>
<trans-unit id="Exception_InvalidSerializablePropertyConfiguration">
<source>Invalid serializable-property configuration specified for type '{0}'. For more information, see 'JsonSourceGenerationMode.Serialization'.</source>
<target state="new">Invalid serializable-property configuration specified for type '{0}'. For more information, see 'JsonSourceGenerationMode.Serialization'.</target>
<note />
</trans-unit>
<trans-unit id="InaccessibleJsonIncludePropertiesNotSupportedFormat">
<source>The member '{0}.{1}' has been annotated with the JsonIncludeAttribute but is not visible to the source generator.</source>
<target state="new">The member '{0}.{1}' has been annotated with the JsonIncludeAttribute but is not visible to the source generator.</target>
<note />
</trans-unit>
<trans-unit id="InaccessibleJsonIncludePropertiesNotSupportedTitle">
<source>Inaccessible properties annotated with the JsonIncludeAttribute are not supported in source generation mode.</source>
<target state="new">Inaccessible properties annotated with the JsonIncludeAttribute are not supported in source generation mode.</target>
<note />
</trans-unit>
<trans-unit id="InitOnlyPropertyDeserializationNotSupportedFormat">
<source>The type '{0}' defines init-only properties, deserialization of which is currently not supported in source generation mode.</source>
<target state="new">The type '{0}' defines init-only properties, deserialization of which is currently not supported in source generation mode.</target>
<note />
</trans-unit>
<trans-unit id="InitOnlyPropertyDeserializationNotSupportedTitle">
<source>Deserialization of init-only properties is currently not supported in source generation mode.</source>
<target state="new">Deserialization of init-only properties is currently not supported in source generation mode.</target>
<note />
</trans-unit>
<trans-unit id="MultipleJsonConstructorAttributeFormat">
<source>Type '{0}' has multiple constructors annotated with 'JsonConstructorAttribute'.</source>
<target state="translated">Typ {0} má více konstruktorů anotovaných s JsonConstructorAttribute. </target>
Expand Down
Loading

0 comments on commit e62c4fe

Please sign in to comment.