Skip to content

Commit

Permalink
Extend JsonIncludeAttribute and JsonConstructorAttribute support …
Browse files Browse the repository at this point in the history 
…to internal and private members (#88452)

* Relax JsonIncludeAttribute to also support private or internal members.

* Relax JsonConstructorAttribute to include internal or private constructors (where applicable).
  • Loading branch information
@eiriktsarpalis
eiriktsarpalis authored Jul 7, 2023
1 parent 2e26fbd commit 24d7f5a
Show file tree
Hide file tree
Showing 32 changed files with 405 additions and 182 deletions.
2 changes: 1 addition & 1 deletion docs/project/list-of-diagnostics.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ The diagnostic id values reserved for .NET Libraries analyzer warnings are `SYSL
| __`SYSLIB1219`__ | *_`SYSLIB1214`-`SYSLIB1219` reserved for Microsoft.Extensions.Options.SourceGeneration.* |
| __`SYSLIB1220`__ | JsonSourceGenerator encountered a [JsonConverterAttribute] with an invalid type argument. |
| __`SYSLIB1221`__ | JsonSourceGenerator does not support this C# language version. |
| __`SYSLIB1222`__ | *`SYSLIB1220`-`SYSLIB229` reserved for System.Text.Json.SourceGeneration.* |
| __`SYSLIB1222`__ | Constructor annotated with JsonConstructorAttribute is inaccessible. |
| __`SYSLIB1223`__ | *`SYSLIB1220`-`SYSLIB229` reserved for System.Text.Json.SourceGeneration.* |
| __`SYSLIB1224`__ | *`SYSLIB1220`-`SYSLIB229` reserved for System.Text.Json.SourceGeneration.* |
| __`SYSLIB1225`__ | *`SYSLIB1220`-`SYSLIB229` reserved for System.Text.Json.SourceGeneration.* |
Expand Down
11 changes: 4 additions & 7 deletions src/libraries/System.Text.Json/Common/ReflectionExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,21 +259,18 @@ public static bool TryGetDeserializationConstructor(
}
}

// For correctness, throw if multiple ctors have [JsonConstructor], even if one or more are non-public.
ConstructorInfo? dummyCtorWithAttribute = ctorWithAttribute;

constructors = type.GetConstructors(BindingFlags.NonPublic | BindingFlags.Instance);
foreach (ConstructorInfo constructor in constructors)
// Search for non-public ctors with [JsonConstructor].
foreach (ConstructorInfo constructor in type.GetConstructors(BindingFlags.NonPublic | BindingFlags.Instance))
{
if (HasJsonConstructorAttribute(constructor))
{
if (dummyCtorWithAttribute != null)
if (ctorWithAttribute != null)
{
deserializationCtor = null;
return false;
}

dummyCtorWithAttribute = constructor;
ctorWithAttribute = constructor;
}
}

Expand Down
8 changes: 8 additions & 0 deletions src/libraries/System.Text.Json/gen/JsonSourceGenerator.DiagnosticDescriptors.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,14 @@ internal static class DiagnosticDescriptors
category: JsonConstants.SystemTextJsonSourceGenerationName,
defaultSeverity: DiagnosticSeverity.Error,
isEnabledByDefault: true);

public static DiagnosticDescriptor JsonConstructorInaccessible { get; } = new DiagnosticDescriptor(
id: "SYSLIB1222",
title: new LocalizableResourceString(nameof(SR.JsonConstructorInaccessibleTitle), SR.ResourceManager, typeof(FxResources.System.Text.Json.SourceGeneration.SR)),
messageFormat: new LocalizableResourceString(nameof(SR.JsonConstructorInaccessibleMessageFormat), SR.ResourceManager, typeof(FxResources.System.Text.Json.SourceGeneration.SR)),
category: JsonConstants.SystemTextJsonSourceGenerationName,
defaultSeverity: DiagnosticSeverity.Warning,
isEnabledByDefault: true);
}
}
}
132 changes: 66 additions & 66 deletions src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -443,20 +443,22 @@ private TypeGenerationSpec ParseTypeGenerationSpec(in TypeToGenerate typeToGener

if (!TryGetDeserializationConstructor(type, useDefaultCtorInAnnotatedStructs, out IMethodSymbol? constructor))
{
classType = ClassType.TypeUnsupportedBySourceGen;
ReportDiagnostic(DiagnosticDescriptors.MultipleJsonConstructorAttribute, typeLocation, type.ToDisplayString());
}
else
else if (constructor != null && !IsSymbolAccessibleWithin(constructor, within: contextType))
{
classType = ClassType.Object;
ReportDiagnostic(DiagnosticDescriptors.JsonConstructorInaccessible, typeLocation, type.ToDisplayString());
constructor = null;
}

implementsIJsonOnSerializing = _knownSymbols.IJsonOnSerializingType.IsAssignableFrom(type);
implementsIJsonOnSerialized = _knownSymbols.IJsonOnSerializedType.IsAssignableFrom(type);
classType = ClassType.Object;

ctorParamSpecs = ParseConstructorParameters(typeToGenerate, constructor, out constructionStrategy, out constructorSetsRequiredMembers);
propertySpecs = ParsePropertyGenerationSpecs(contextType, typeToGenerate, typeLocation, options, out hasExtensionDataProperty);
propertyInitializerSpecs = ParsePropertyInitializers(ctorParamSpecs, propertySpecs, constructorSetsRequiredMembers, ref constructionStrategy);
}
implementsIJsonOnSerializing = _knownSymbols.IJsonOnSerializingType.IsAssignableFrom(type);
implementsIJsonOnSerialized = _knownSymbols.IJsonOnSerializedType.IsAssignableFrom(type);

ctorParamSpecs = ParseConstructorParameters(typeToGenerate, constructor, out constructionStrategy, out constructorSetsRequiredMembers);
propertySpecs = ParsePropertyGenerationSpecs(contextType, typeToGenerate, typeLocation, options, out hasExtensionDataProperty);
propertyInitializerSpecs = ParsePropertyInitializers(ctorParamSpecs, propertySpecs, constructorSetsRequiredMembers, ref constructionStrategy);
}

var typeRef = new TypeRef(type);
Expand Down Expand Up @@ -849,7 +851,7 @@ private bool IsValidDataExtensionPropertyType(ITypeSymbol type)
memberInfo,
hasJsonInclude,
out bool isReadOnly,
out bool isPublic,
out bool isAccessible,
out bool isRequired,
out bool canUseGetter,
out bool canUseSetter,
Expand Down Expand Up @@ -899,7 +901,7 @@ private bool IsValidDataExtensionPropertyType(ITypeSymbol type)
NameSpecifiedInSourceCode = memberInfo.MemberNameNeedsAtSign() ? "@" + memberInfo.Name : memberInfo.Name,
MemberName = memberInfo.Name,
IsProperty = memberInfo is IPropertySymbol,
IsPublic = isPublic,
IsPublic = isAccessible,
IsVirtual = memberInfo.IsVirtual(),
JsonPropertyName = jsonPropertyName,
RuntimePropertyName = runtimePropertyName,
Expand Down Expand Up @@ -1031,14 +1033,14 @@ private void ProcessMember(
ISymbol memberInfo,
bool hasJsonInclude,
out bool isReadOnly,
out bool isPublic,
out bool isAccessible,
out bool isRequired,
out bool canUseGetter,
out bool canUseSetter,
out bool hasJsonIncludeButIsInaccessible,
out bool isSetterInitOnly)
{
isPublic = false;
isAccessible = false;
isReadOnly = false;
isRequired = false;
canUseGetter = false;
Expand All @@ -1049,72 +1051,72 @@ private void ProcessMember(
switch (memberInfo)
{
case IPropertySymbol propertyInfo:
{
IMethodSymbol? getMethod = propertyInfo.GetMethod;
IMethodSymbol? setMethod = propertyInfo.SetMethod;
#if ROSLYN4_4_OR_GREATER
isRequired = propertyInfo.IsRequired;
isRequired = propertyInfo.IsRequired;
#endif

if (getMethod != null)
if (propertyInfo.GetMethod is { } getMethod)
{
if (getMethod.DeclaredAccessibility is Accessibility.Public)
{
if (getMethod.DeclaredAccessibility is Accessibility.Public)
{
isPublic = true;
canUseGetter = true;
}
else if (IsSymbolAccessibleWithin(getMethod, within: contextType))
{
canUseGetter = hasJsonInclude;
}
else
{
hasJsonIncludeButIsInaccessible = hasJsonInclude;
}
isAccessible = true;
canUseGetter = true;
}

if (setMethod != null)
else if (IsSymbolAccessibleWithin(getMethod, within: contextType))
{
isSetterInitOnly = setMethod.IsInitOnly;

if (setMethod.DeclaredAccessibility is Accessibility.Public)
{
isPublic = true;
canUseSetter = true;
}
else if (IsSymbolAccessibleWithin(setMethod, within: contextType))
{
canUseSetter = hasJsonInclude;
}
else
{
hasJsonIncludeButIsInaccessible = hasJsonInclude;
}
isAccessible = true;
canUseGetter = hasJsonInclude;
}
else
{
isReadOnly = true;
hasJsonIncludeButIsInaccessible = hasJsonInclude;
}
}
break;
case IFieldSymbol fieldInfo:

if (propertyInfo.SetMethod is { } setMethod)
{
isReadOnly = fieldInfo.IsReadOnly;
#if ROSLYN4_4_OR_GREATER
isRequired = fieldInfo.IsRequired;
#endif
if (fieldInfo.DeclaredAccessibility is Accessibility.Public)
isSetterInitOnly = setMethod.IsInitOnly;

if (setMethod.DeclaredAccessibility is Accessibility.Public)
{
isPublic = true;
canUseGetter = true;
canUseSetter = !isReadOnly;
isAccessible = true;
canUseSetter = true;
}
else if (IsSymbolAccessibleWithin(setMethod, within: contextType))
{
isAccessible = true;
canUseSetter = hasJsonInclude;
}
else
{
// Unlike properties JsonIncludeAttribute is not supported for internal fields.
hasJsonIncludeButIsInaccessible = hasJsonInclude;
}
}
else
{
isReadOnly = true;
}
break;
case IFieldSymbol fieldInfo:
isReadOnly = fieldInfo.IsReadOnly;
#if ROSLYN4_4_OR_GREATER
isRequired = fieldInfo.IsRequired;
#endif
if (fieldInfo.DeclaredAccessibility is Accessibility.Public)
{
isAccessible = true;
canUseGetter = true;
canUseSetter = !isReadOnly;
}
else if (IsSymbolAccessibleWithin(fieldInfo, within: contextType))
{
isAccessible = true;
canUseGetter = hasJsonInclude;
canUseSetter = hasJsonInclude && !isReadOnly;
}
else
{
hasJsonIncludeButIsInaccessible = hasJsonInclude;
}
break;
default:
Debug.Fail("Method given an invalid symbol type.");
Expand Down Expand Up @@ -1410,20 +1412,18 @@ private bool TryGetDeserializationConstructor(
}
}

// For correctness, throw if multiple ctors have [JsonConstructor], even if one or more are non-public.
IMethodSymbol? dummyCtorWithAttribute = ctorWithAttribute;

// Search for non-public ctors with [JsonConstructor].
foreach (IMethodSymbol constructor in namedType.GetExplicitlyDeclaredInstanceConstructors().Where(ctor => ctor.DeclaredAccessibility is not Accessibility.Public))
{
if (constructor.ContainsAttribute(_knownSymbols.JsonConstructorAttributeType))
{
if (dummyCtorWithAttribute != null)
if (ctorWithAttribute != null)
{
deserializationCtor = null;
return false;
}

dummyCtorWithAttribute = constructor;
ctorWithAttribute = constructor;
}
}

Expand Down
6 changes: 6 additions & 0 deletions src/libraries/System.Text.Json/gen/Resources/Strings.resx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,4 +189,10 @@
<data name="JsonUnsupportedLanguageVersionMessageFormat" xml:space="preserve">
<value>The System.Text.Json source generator is not available in C# '{0}'. Please use language version {1} or greater.</value>
</data>
<data name="JsonConstructorInaccessibleTitle" xml:space="preserve">
<value>Constructor annotated with JsonConstructorAttribute is inaccessible.</value>
</data>
<data name="JsonConstructorInaccessibleMessageFormat" xml:space="preserve">
<value>The constructor on type '{0}' has been annotated with JsonConstructorAttribute but is not accessible by the source generator.</value>
</data>
</root>
10 changes: 10 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 @@ -62,6 +62,16 @@
<target state="translated">Deserializace vlastností pouze pro inicializaci se v současnosti v režimu generování zdroje nepodporuje.</target>
<note />
</trans-unit>
<trans-unit id="JsonConstructorInaccessibleMessageFormat">
<source>The constructor on type '{0}' has been annotated with JsonConstructorAttribute but is not accessible by the source generator.</source>
<target state="new">The constructor on type '{0}' has been annotated with JsonConstructorAttribute but is not accessible by the source generator.</target>
<note />
</trans-unit>
<trans-unit id="JsonConstructorInaccessibleTitle">
<source>Constructor annotated with JsonConstructorAttribute is inaccessible.</source>
<target state="new">Constructor annotated with JsonConstructorAttribute is inaccessible.</target>
<note />
</trans-unit>
<trans-unit id="JsonConverterAttributeInvalidTypeMessageFormat">
<source>The 'JsonConverterAttribute' type '{0}' specified on member '{1}' is not a converter type or does not contain an accessible parameterless constructor.</source>
<target state="translated">Typ JsonConverterAttribute {0} specifikovaný u členu {1} není typem konvertoru nebo neobsahuje přístupný konstruktor bez parametrů.</target>
Expand Down
10 changes: 10 additions & 0 deletions src/libraries/System.Text.Json/gen/Resources/xlf/Strings.de.xlf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,16 @@
<target state="translated">Die Deserialisierung von reinen init-Eigenschaften wird im Quellgenerierungsmodus derzeit nicht unterstützt.</target>
<note />
</trans-unit>
<trans-unit id="JsonConstructorInaccessibleMessageFormat">
<source>The constructor on type '{0}' has been annotated with JsonConstructorAttribute but is not accessible by the source generator.</source>
<target state="new">The constructor on type '{0}' has been annotated with JsonConstructorAttribute but is not accessible by the source generator.</target>
<note />
</trans-unit>
<trans-unit id="JsonConstructorInaccessibleTitle">
<source>Constructor annotated with JsonConstructorAttribute is inaccessible.</source>
<target state="new">Constructor annotated with JsonConstructorAttribute is inaccessible.</target>
<note />
</trans-unit>
<trans-unit id="JsonConverterAttributeInvalidTypeMessageFormat">
<source>The 'JsonConverterAttribute' type '{0}' specified on member '{1}' is not a converter type or does not contain an accessible parameterless constructor.</source>
<target state="translated">Der für den Member "{1}" angegebene JsonConverterAttribute-Typ "{0}" ist kein Konvertertyp oder enthält keinen parameterlosen Konstruktor, auf den zugegriffen werden kann.</target>
Expand Down
10 changes: 10 additions & 0 deletions src/libraries/System.Text.Json/gen/Resources/xlf/Strings.es.xlf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,16 @@
<target state="translated">Actualmente no se admite la deserialización de propiedades de solo inicialización en el modo de generación de origen.</target>
<note />
</trans-unit>
<trans-unit id="JsonConstructorInaccessibleMessageFormat">
<source>The constructor on type '{0}' has been annotated with JsonConstructorAttribute but is not accessible by the source generator.</source>
<target state="new">The constructor on type '{0}' has been annotated with JsonConstructorAttribute but is not accessible by the source generator.</target>
<note />
</trans-unit>
<trans-unit id="JsonConstructorInaccessibleTitle">
<source>Constructor annotated with JsonConstructorAttribute is inaccessible.</source>
<target state="new">Constructor annotated with JsonConstructorAttribute is inaccessible.</target>
<note />
</trans-unit>
<trans-unit id="JsonConverterAttributeInvalidTypeMessageFormat">
<source>The 'JsonConverterAttribute' type '{0}' specified on member '{1}' is not a converter type or does not contain an accessible parameterless constructor.</source>
<target state="translated">El tipo “JsonConverterAttribute” “{0}” especificado en el miembro “{1}” no es un tipo de convertidor o no contiene un constructor sin parámetros accesible.</target>
Expand Down
10 changes: 10 additions & 0 deletions src/libraries/System.Text.Json/gen/Resources/xlf/Strings.fr.xlf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,16 @@
<target state="translated">La désérialisation des propriétés d’initialisation uniquement n’est actuellement pas prise en charge en mode de génération de source.</target>
<note />
</trans-unit>
<trans-unit id="JsonConstructorInaccessibleMessageFormat">
<source>The constructor on type '{0}' has been annotated with JsonConstructorAttribute but is not accessible by the source generator.</source>
<target state="new">The constructor on type '{0}' has been annotated with JsonConstructorAttribute but is not accessible by the source generator.</target>
<note />
</trans-unit>
<trans-unit id="JsonConstructorInaccessibleTitle">
<source>Constructor annotated with JsonConstructorAttribute is inaccessible.</source>
<target state="new">Constructor annotated with JsonConstructorAttribute is inaccessible.</target>
<note />
</trans-unit>
<trans-unit id="JsonConverterAttributeInvalidTypeMessageFormat">
<source>The 'JsonConverterAttribute' type '{0}' specified on member '{1}' is not a converter type or does not contain an accessible parameterless constructor.</source>
<target state="translated">Le type 'JsonConverterAttribute' '{0}' spécifié sur le membre '{1}' n’est pas un type convertisseur ou ne contient pas de constructeur sans paramètre accessible.</target>
Expand Down
Loading

0 comments on commit 24d7f5a

Please sign in to comment.