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.

Sign up for GitHub

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

Jump to bottom

Extend JsonIncludeAttribute and JsonConstructorAttribute support to internal and private members #88452

Merged
merged 2 commits into from
Jul 7, 2023
Merged

Extend JsonIncludeAttribute and JsonConstructorAttribute support to internal and private members #88452

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
96820db
Relax JsonIncludeAttribute to also support private or internal members.
eiriktsarpalis Jul 5, 2023
baee5db
Relax JsonConstructorAttribute to include internal or private constru…
eiriktsarpalis Jul 5, 2023
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
2 changes: 1 addition & 1 deletion docs/project/list-of-diagnostics.md
View file
Open in desktop
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
Open in desktop
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))
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
{
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading