[System.Text.Json] Improve error handling of ref structs, including c…

…ollection types with ref struct elements. (#106083) * STJ-SG: tolerate ref struct members with JsonIgnore. * * Skip all ref like members and emit warnings. * Add detection for collections with ref struct elements. * Update src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs Co-authored-by: David Cantú <dacantu@microsoft.com> * Update src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs Co-authored-by: David Cantú <dacantu@microsoft.com> --------- Co-authored-by: David Cantú <dacantu@microsoft.com>
dotnet · Aug 12, 2024 · c1ab6c5 · c1ab6c5
1 parent a38ed97
commit c1ab6c5
Show file tree

Hide file tree

Showing 24 changed files with 482 additions and 44 deletions.
diff --git a/docs/project/list-of-diagnostics.md b/docs/project/list-of-diagnostics.md
@@ -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. |
 |  __`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.* |

diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.DiagnosticDescriptors.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.DiagnosticDescriptors.cs
@@ -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);
         }
     }
 }
diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs
@@ -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;
                 }
@@ -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))
@@ -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);
 
@@ -1246,62 +1263,60 @@ private void ProcessMemberCustomAttributes(
                         switch (attributeType.ToDisplayString())
                         {
                             case JsonIgnoreAttributeFullName:
+                            {
+                                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;
+                            }
                         }
                     }
                 }
@@ -1433,7 +1448,7 @@ private void ProcessMember(
                 if (paramCount == 0)
                 {
                     constructionStrategy = ObjectConstructionStrategy.ParameterlessConstructor;
-                    constructorParameters = Array.Empty<ParameterGenerationSpec>();
+                    constructorParameters = [];
                 }
                 else
                 {
@@ -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
@@ -1459,7 +1482,7 @@ private void ProcessMember(
                     }
                 }
 
-                return constructorParameters;
+                return constructionStrategy is ObjectConstructionStrategy.NotApplicable ? null : constructorParameters;
             }
 
             private List<PropertyInitializerGenerationSpec>? ParsePropertyInitializers(

diff --git a/src/libraries/System.Text.Json/gen/Resources/Strings.resx b/src/libraries/System.Text.Json/gen/Resources/Strings.resx
@@ -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>
diff --git a/src/libraries/System.Text.Json/gen/Resources/xlf/Strings.cs.xlf b/src/libraries/System.Text.Json/gen/Resources/xlf/Strings.cs.xlf
@@ -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>
       <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>

diff --git a/src/libraries/System.Text.Json/gen/Resources/xlf/Strings.de.xlf b/src/libraries/System.Text.Json/gen/Resources/xlf/Strings.de.xlf
@@ -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>

diff --git a/src/libraries/System.Text.Json/gen/Resources/xlf/Strings.es.xlf b/src/libraries/System.Text.Json/gen/Resources/xlf/Strings.es.xlf
@@ -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>

diff --git a/src/libraries/System.Text.Json/gen/Resources/xlf/Strings.fr.xlf b/src/libraries/System.Text.Json/gen/Resources/xlf/Strings.fr.xlf
@@ -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>

diff --git a/src/libraries/System.Text.Json/gen/Resources/xlf/Strings.it.xlf b/src/libraries/System.Text.Json/gen/Resources/xlf/Strings.it.xlf
@@ -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>

diff --git a/src/libraries/System.Text.Json/gen/Resources/xlf/Strings.ja.xlf b/src/libraries/System.Text.Json/gen/Resources/xlf/Strings.ja.xlf
@@ -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>