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

[System.Text.Json] Improve error handling of ref structs, including collection types with ref struct elements. #106083

Merged
merged 4 commits into from
Aug 12, 2024
Merged

[System.Text.Json] Improve error handling of ref structs, including collection types with ref struct elements. #106083

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
104c26a
STJ-SG: tolerate ref struct members with JsonIgnore.
eiriktsarpalis Aug 7, 2024
6662f42
* Skip all ref like members and emit warnings.
eiriktsarpalis Aug 9, 2024
58a696e
Update src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs
eiriktsarpalis Aug 9, 2024
a4542b3
Update src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs
eiriktsarpalis Aug 9, 2024
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 @@ -263,7 +263,7 @@ The diagnostic id values reserved for .NET Libraries analyzer warnings are `SYSL
| __`SYSLIB1222`__ | Constructor annotated with JsonConstructorAttribute is inaccessible. |
| __`SYSLIB1223`__ | Attributes deriving from JsonConverterAttribute are not supported by the source generator. |
| __`SYSLIB1224`__ | Types annotated with JsonSerializableAttribute must be classes deriving from JsonSerializerContext. |
| __`SYSLIB1225`__ | *`SYSLIB1220`-`SYSLIB1229` reserved for System.Text.Json.SourceGeneration.* |
| __`SYSLIB1225`__ | Type includes ref like property, field or constructor parameter. |
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
| __`SYSLIB1226`__ | *`SYSLIB1220`-`SYSLIB1229` reserved for System.Text.Json.SourceGeneration.* |
| __`SYSLIB1227`__ | *`SYSLIB1220`-`SYSLIB1229` reserved for System.Text.Json.SourceGeneration.* |
| __`SYSLIB1228`__ | *`SYSLIB1220`-`SYSLIB1229` reserved for System.Text.Json.SourceGeneration.* |
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 @@ -123,6 +123,14 @@ internal static class DiagnosticDescriptors
category: JsonConstants.SystemTextJsonSourceGenerationName,
defaultSeverity: DiagnosticSeverity.Warning,
isEnabledByDefault: true);

public static DiagnosticDescriptor TypeContainsRefLikeMember { get; } = DiagnosticDescriptorHelper.Create(
id: "SYSLIB1225",
title: new LocalizableResourceString(nameof(SR.TypeContainsRefLikeMemberTitle), SR.ResourceManager, typeof(FxResources.System.Text.Json.SourceGeneration.SR)),
messageFormat: new LocalizableResourceString(nameof(SR.TypeContainsRefLikeMemberFormat), SR.ResourceManager, typeof(FxResources.System.Text.Json.SourceGeneration.SR)),
category: JsonConstants.SystemTextJsonSourceGenerationName,
defaultSeverity: DiagnosticSeverity.Warning,
isEnabledByDefault: true);
}
}
}
107 changes: 65 additions & 42 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 @@ -522,7 +522,7 @@ private TypeGenerationSpec ParseTypeGenerationSpec(in TypeToGenerate typeToGener
out TypeRef? customConverterType,
out bool isPolymorphic);

if (type is INamedTypeSymbol { IsUnboundGenericType: true } or IErrorTypeSymbol)
if (type is { IsRefLikeType: true } or INamedTypeSymbol { IsUnboundGenericType: true } or IErrorTypeSymbol)
{
classType = ClassType.TypeUnsupportedBySourceGen;
}
Expand Down Expand Up @@ -574,6 +574,12 @@ private TypeGenerationSpec ParseTypeGenerationSpec(in TypeToGenerate typeToGener
immutableCollectionFactoryTypeFullName = null;
collectionType = default;
}
else if (valueType.IsRefLikeType || keyType?.IsRefLikeType is true)
{
classType = ClassType.TypeUnsupportedBySourceGen;
immutableCollectionFactoryTypeFullName = null;
collectionType = default;
}
else
{
if (type.CanUseDefaultConstructorForDeserialization(out IMethodSymbol? defaultCtor))
Expand Down Expand Up @@ -1165,6 +1171,17 @@ private bool IsValidDataExtensionPropertyType(ITypeSymbol type)
return null;
}

if (memberType.IsRefLikeType)
{
// Skip all ref-like members and emit a diagnostic unless the property is being explicitly ignored.
if (ignoreCondition is not JsonIgnoreCondition.Always)
{
ReportDiagnostic(DiagnosticDescriptors.TypeContainsRefLikeMember, memberInfo.GetLocation(), declaringType.Name, memberInfo.Name);
}

return null;
}

string effectiveJsonPropertyName = DetermineEffectiveJsonPropertyName(memberInfo.Name, jsonPropertyName, options);
string propertyNameFieldName = DeterminePropertyNameFieldName(effectiveJsonPropertyName);

Expand Down Expand Up @@ -1246,62 +1263,60 @@ private void ProcessMemberCustomAttributes(
switch (attributeType.ToDisplayString())
{
case JsonIgnoreAttributeFullName:
{
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
ImmutableArray<KeyValuePair<string, TypedConstant>> namedArgs = attributeData.NamedArguments;

if (namedArgs.Length == 0)
{
ImmutableArray<KeyValuePair<string, TypedConstant>> namedArgs = attributeData.NamedArguments;

if (namedArgs.Length == 0)
{
ignoreCondition = JsonIgnoreCondition.Always;
}
else if (namedArgs.Length == 1 &&
namedArgs[0].Value.Type?.ToDisplayString() == JsonIgnoreConditionFullName)
{
ignoreCondition = (JsonIgnoreCondition)namedArgs[0].Value.Value!;
}
ignoreCondition = JsonIgnoreCondition.Always;
}
break;
case JsonIncludeAttributeFullName:
else if (namedArgs.Length == 1 &&
namedArgs[0].Value.Type?.ToDisplayString() == JsonIgnoreConditionFullName)
{
hasJsonInclude = true;
ignoreCondition = (JsonIgnoreCondition)namedArgs[0].Value.Value!;
}
break;
}
case JsonIncludeAttributeFullName:
{
hasJsonInclude = true;
break;
}
case JsonNumberHandlingAttributeFullName:
{
ImmutableArray<TypedConstant> ctorArgs = attributeData.ConstructorArguments;
numberHandling = (JsonNumberHandling)ctorArgs[0].Value!;
}
{
ImmutableArray<TypedConstant> ctorArgs = attributeData.ConstructorArguments;
numberHandling = (JsonNumberHandling)ctorArgs[0].Value!;
break;
}
case JsonObjectCreationHandlingAttributeFullName:
{
ImmutableArray<TypedConstant> ctorArgs = attributeData.ConstructorArguments;
objectCreationHandling = (JsonObjectCreationHandling)ctorArgs[0].Value!;
}
{
ImmutableArray<TypedConstant> ctorArgs = attributeData.ConstructorArguments;
objectCreationHandling = (JsonObjectCreationHandling)ctorArgs[0].Value!;
break;
}
case JsonPropertyNameAttributeFullName:
{
ImmutableArray<TypedConstant> ctorArgs = attributeData.ConstructorArguments;
jsonPropertyName = (string)ctorArgs[0].Value!;
// Null check here is done at runtime within JsonSerializer.
}
{
ImmutableArray<TypedConstant> ctorArgs = attributeData.ConstructorArguments;
jsonPropertyName = (string)ctorArgs[0].Value!;
// Null check here is done at runtime within JsonSerializer.
break;
}
case JsonPropertyOrderAttributeFullName:
{
ImmutableArray<TypedConstant> ctorArgs = attributeData.ConstructorArguments;
order = (int)ctorArgs[0].Value!;
}
{
ImmutableArray<TypedConstant> ctorArgs = attributeData.ConstructorArguments;
order = (int)ctorArgs[0].Value!;
break;
}
case JsonExtensionDataAttributeFullName:
{
isExtensionData = true;
}
{
isExtensionData = true;
break;
}
case JsonRequiredAttributeFullName:
{
hasJsonRequiredAttribute = true;
}
break;
default:
{
hasJsonRequiredAttribute = true;
break;
}
}
}
}
Expand Down Expand Up @@ -1433,7 +1448,7 @@ private void ProcessMember(
if (paramCount == 0)
{
constructionStrategy = ObjectConstructionStrategy.ParameterlessConstructor;
constructorParameters = Array.Empty<ParameterGenerationSpec>();
constructorParameters = [];
}
else
{
Expand All @@ -1445,6 +1460,14 @@ private void ProcessMember(
for (int i = 0; i < paramCount; i++)
{
IParameterSymbol parameterInfo = constructor.Parameters[i];

if (parameterInfo.Type.IsRefLikeType)
{
ReportDiagnostic(DiagnosticDescriptors.TypeContainsRefLikeMember, parameterInfo.GetLocation(), type.Name, parameterInfo.Name);
constructionStrategy = ObjectConstructionStrategy.NotApplicable;
continue;
}

TypeRef parameterTypeRef = EnqueueType(parameterInfo.Type, typeToGenerate.Mode);

constructorParameters[i] = new ParameterGenerationSpec
Expand All @@ -1459,7 +1482,7 @@ private void ProcessMember(
}
}

return constructorParameters;
return constructionStrategy is ObjectConstructionStrategy.NotApplicable ? null : constructorParameters;
}

private List<PropertyInitializerGenerationSpec>? ParsePropertyInitializers(
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 @@ -207,4 +207,10 @@
<data name="JsonSerializableAttributeOnNonContextTypeMessageFormat" xml:space="preserve">
<value>The type '{0}' has been annotated with JsonSerializableAttribute but does not derive from JsonSerializerContext. No source code will be generated.</value>
</data>
<data name="TypeContainsRefLikeMemberTitle" xml:space="preserve">
<value>Type includes ref like property, field or constructor parameter.</value>
</data>
<data name="TypeContainsRefLikeMemberFormat" xml:space="preserve">
<value>The type '{0}' includes the ref like property, field or constructor parameter '{1}'. No source code will be generated for the property, field or constructor.</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 @@ -142,6 +142,16 @@
<target state="translated">Typ obsahuje více členů s komentářem JsonExtensionDataAttribute</target>
<note />
</trans-unit>
<trans-unit id="TypeContainsRefLikeMemberFormat">
<source>The type '{0}' includes the ref like property, field or constructor parameter '{1}'. No source code will be generated for the property, field or constructor.</source>
<target state="new">The type '{0}' includes the ref like property, field or constructor parameter '{1}'. No source code will be generated for the property, field or constructor.</target>
<note />
</trans-unit>
<trans-unit id="TypeContainsRefLikeMemberTitle">
<source>Type includes ref like property, field or constructor parameter.</source>
<target state="new">Type includes ref like property, field or constructor parameter.</target>
<note />
</trans-unit>
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
<trans-unit id="TypeNotSupportedMessageFormat">
<source>Did not generate serialization metadata for type '{0}'.</source>
<target state="translated">Nevygenerovala se metadata serializace pro typ {0}.</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 @@ -142,6 +142,16 @@
<target state="translated">Der Typ enthält mehrere Elemente, die mit dem JsonExtensionDataAttribute versehen sind.</target>
<note />
</trans-unit>
<trans-unit id="TypeContainsRefLikeMemberFormat">
<source>The type '{0}' includes the ref like property, field or constructor parameter '{1}'. No source code will be generated for the property, field or constructor.</source>
<target state="new">The type '{0}' includes the ref like property, field or constructor parameter '{1}'. No source code will be generated for the property, field or constructor.</target>
<note />
</trans-unit>
<trans-unit id="TypeContainsRefLikeMemberTitle">
<source>Type includes ref like property, field or constructor parameter.</source>
<target state="new">Type includes ref like property, field or constructor parameter.</target>
<note />
</trans-unit>
<trans-unit id="TypeNotSupportedMessageFormat">
<source>Did not generate serialization metadata for type '{0}'.</source>
<target state="translated">Die Serialisierungsmetadaten für den Typ "{0}" wurden nicht generiert.</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 @@ -142,6 +142,16 @@
<target state="translated">El tipo tiene varios miembros anotados con JsonExtensionDataAttribute.</target>
<note />
</trans-unit>
<trans-unit id="TypeContainsRefLikeMemberFormat">
<source>The type '{0}' includes the ref like property, field or constructor parameter '{1}'. No source code will be generated for the property, field or constructor.</source>
<target state="new">The type '{0}' includes the ref like property, field or constructor parameter '{1}'. No source code will be generated for the property, field or constructor.</target>
<note />
</trans-unit>
<trans-unit id="TypeContainsRefLikeMemberTitle">
<source>Type includes ref like property, field or constructor parameter.</source>
<target state="new">Type includes ref like property, field or constructor parameter.</target>
<note />
</trans-unit>
<trans-unit id="TypeNotSupportedMessageFormat">
<source>Did not generate serialization metadata for type '{0}'.</source>
<target state="translated">No generó metadatos de serialización para el tipo '{0}".</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 @@ -142,6 +142,16 @@
<target state="translated">Le type comporte plusieurs membres annotés avec JsonExtensionDataAttribute.</target>
<note />
</trans-unit>
<trans-unit id="TypeContainsRefLikeMemberFormat">
<source>The type '{0}' includes the ref like property, field or constructor parameter '{1}'. No source code will be generated for the property, field or constructor.</source>
<target state="new">The type '{0}' includes the ref like property, field or constructor parameter '{1}'. No source code will be generated for the property, field or constructor.</target>
<note />
</trans-unit>
<trans-unit id="TypeContainsRefLikeMemberTitle">
<source>Type includes ref like property, field or constructor parameter.</source>
<target state="new">Type includes ref like property, field or constructor parameter.</target>
<note />
</trans-unit>
<trans-unit id="TypeNotSupportedMessageFormat">
<source>Did not generate serialization metadata for type '{0}'.</source>
<target state="translated">Les métadonnées de sérialisation pour le type « {0} » n’ont pas été générées.</target>
Expand Down
10 changes: 10 additions & 0 deletions src/libraries/System.Text.Json/gen/Resources/xlf/Strings.it.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,16 @@
<target state="translated">Nel tipo sono presenti più membri annotati con JsonExtensionDataAttribute.</target>
<note />
</trans-unit>
<trans-unit id="TypeContainsRefLikeMemberFormat">
<source>The type '{0}' includes the ref like property, field or constructor parameter '{1}'. No source code will be generated for the property, field or constructor.</source>
<target state="new">The type '{0}' includes the ref like property, field or constructor parameter '{1}'. No source code will be generated for the property, field or constructor.</target>
<note />
</trans-unit>
<trans-unit id="TypeContainsRefLikeMemberTitle">
<source>Type includes ref like property, field or constructor parameter.</source>
<target state="new">Type includes ref like property, field or constructor parameter.</target>
<note />
</trans-unit>
<trans-unit id="TypeNotSupportedMessageFormat">
<source>Did not generate serialization metadata for type '{0}'.</source>
<target state="translated">Non sono stati generati metadati di serializzazione per il tipo '{0}'.</target>
Expand Down
10 changes: 10 additions & 0 deletions src/libraries/System.Text.Json/gen/Resources/xlf/Strings.ja.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,16 @@
<target state="translated">型には、'JsonExtensionDataAttribute' に注釈が付けられた複数のメンバーが含まれます。</target>
<note />
</trans-unit>
<trans-unit id="TypeContainsRefLikeMemberFormat">
<source>The type '{0}' includes the ref like property, field or constructor parameter '{1}'. No source code will be generated for the property, field or constructor.</source>
<target state="new">The type '{0}' includes the ref like property, field or constructor parameter '{1}'. No source code will be generated for the property, field or constructor.</target>
<note />
</trans-unit>
<trans-unit id="TypeContainsRefLikeMemberTitle">
<source>Type includes ref like property, field or constructor parameter.</source>
<target state="new">Type includes ref like property, field or constructor parameter.</target>
<note />
</trans-unit>
<trans-unit id="TypeNotSupportedMessageFormat">
<source>Did not generate serialization metadata for type '{0}'.</source>
<target state="translated">'{0}'型 のシリアル化メタデータを生成ませんでした。</target>
Expand Down
Loading
Loading