From 903efd53a406135d274dc7803bbca64aeda34086 Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Mon, 11 Aug 2025 15:27:45 -0700 Subject: [PATCH 1/3] Explicitly asserting some beliefs we have about particular types of data contracts when working in CodeExporter --- .../Serialization/Schema/CodeExporter.cs | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Runtime.Serialization.Schema/src/System/Runtime/Serialization/Schema/CodeExporter.cs b/src/libraries/System.Runtime.Serialization.Schema/src/System/Runtime/Serialization/Schema/CodeExporter.cs index 28afdac8640528..e65b18f73d13ca 100644 --- a/src/libraries/System.Runtime.Serialization.Schema/src/System/Runtime/Serialization/Schema/CodeExporter.cs +++ b/src/libraries/System.Runtime.Serialization.Schema/src/System/Runtime/Serialization/Schema/CodeExporter.cs @@ -486,7 +486,8 @@ private void GenerateType(DataContract dataContract, ContractCodeDomInfo contrac if (containingContractCodeDomInfo.ReferencedTypeExists) return null; - CodeTypeDeclaration containingType = containingContractCodeDomInfo.TypeDeclaration!; // Nested types by definition have containing types. + Debug.Assert(containingContractCodeDomInfo.TypeDeclaration != null, "Nested types have containing types by definition - types with declaration"); + CodeTypeDeclaration containingType = containingContractCodeDomInfo.TypeDeclaration; if (TypeContainsNestedType(containingType, nestedTypeName)) { for (int i = 1; ; i++) @@ -502,7 +503,8 @@ private void GenerateType(DataContract dataContract, ContractCodeDomInfo contrac CodeTypeDeclaration type = CreateTypeDeclaration(nestedTypeName, dataContract); containingType.Members.Add(type); - contractCodeDomInfo.TypeReference = new CodeTypeReference(containingContractCodeDomInfo.TypeReference!.BaseType + "+" + nestedTypeName); // Again, nested types by definition have containing types. + Debug.Assert(containingContractCodeDomInfo.TypeReference != null, "Nested types have containing types by definition - types with reference"); + contractCodeDomInfo.TypeReference = new CodeTypeReference(containingContractCodeDomInfo.TypeReference.BaseType + "+" + nestedTypeName); if (GenerateInternalTypes) type.TypeAttributes = TypeAttributes.NestedAssembly; @@ -518,8 +520,9 @@ private static CodeTypeDeclaration CreateTypeDeclaration(string typeName, DataCo CodeAttributeDeclaration generatedCodeAttribute = new CodeAttributeDeclaration(typeof(GeneratedCodeAttribute).FullName!); AssemblyName assemblyName = Assembly.GetExecutingAssembly().GetName(); - generatedCodeAttribute.Arguments.Add(new CodeAttributeArgument(new CodePrimitiveExpression(assemblyName.Name!))); - generatedCodeAttribute.Arguments.Add(new CodeAttributeArgument(new CodePrimitiveExpression(assemblyName.Version?.ToString()!))); + Debug.Assert(assemblyName.Name != null, $"Current executing assembly name is not expected to be null in {nameof(CodeExporter)}.{nameof(CreateTypeDeclaration)} scenario"); + generatedCodeAttribute.Arguments.Add(new CodeAttributeArgument(new CodePrimitiveExpression(assemblyName.Name))); + generatedCodeAttribute.Arguments.Add(new CodeAttributeArgument(new CodePrimitiveExpression(assemblyName.Version?.ToString()))); // System.Diagnostics.DebuggerStepThroughAttribute not allowed on enums // ensure that the attribute is only generated on types that are not enums @@ -607,6 +610,7 @@ private static CodeTypeDeclaration CreateTypeDeclaration(string typeName, DataCo if (!TryGetReferencedDictionaryType(collectionContract, out typeReference)) { // ItemContract - aka BaseContract - is never null for CollectionDataContract + Debug.Assert(collectionContract.BaseContract != null, "BaseContract should not be null for CollectionDataContract"); DataContract itemContract = collectionContract.BaseContract!; if (collectionContract.IsDictionaryLike(out _, out _, out _)) { @@ -629,6 +633,7 @@ private static CodeTypeDeclaration CreateTypeDeclaration(string typeName, DataCo private static bool HasDefaultCollectionNames(DataContract collectionContract) { Debug.Assert(collectionContract.Is(DataContractType.CollectionDataContract)); + Debug.Assert(collectionContract.BaseContract != null, "BaseContract should not be null for CollectionDataContract"); // ItemContract - aka BaseContract - is never null for CollectionDataContract DataContract itemContract = collectionContract.BaseContract!; @@ -647,6 +652,7 @@ private static bool HasDefaultCollectionNames(DataContract collectionContract) private bool TryGetReferencedDictionaryType(DataContract collectionContract, [NotNullWhen(true)] out CodeTypeReference? typeReference) { Debug.Assert(collectionContract.Is(DataContractType.CollectionDataContract)); + Debug.Assert(collectionContract.BaseContract != null, "BaseContract should not be null for CollectionDataContract"); // Check if it is a dictionary and use referenced dictionary type if present if (collectionContract.IsDictionaryLike(out _, out _, out _) @@ -819,7 +825,8 @@ private void ExportClassDataContract(DataContract classDataContract, ContractCod { ContractCodeDomInfo baseContractCodeDomInfo = GetContractCodeDomInfo(classDataContract.BaseContract); Debug.Assert(baseContractCodeDomInfo.IsProcessed, "Cannot generate code for type if code for base type has not been generated"); - type.BaseTypes.Add(baseContractCodeDomInfo.TypeReference!); + Debug.Assert(baseContractCodeDomInfo.TypeReference != null, "Class data contracts should have non-null TypeReference"); + type.BaseTypes.Add(baseContractCodeDomInfo.TypeReference); AddBaseMemberNames(baseContractCodeDomInfo, contractCodeDomInfo); if (baseContractCodeDomInfo.ReferencedTypeExists) { @@ -1068,6 +1075,7 @@ private void ExportEnumDataContract(DataContract enumDataContract, ContractCodeD CodeTypeDeclaration type = contractCodeDomInfo.TypeDeclaration; // BaseContract is never null for EnumDataContract + Debug.Assert(enumDataContract.BaseContract != null, "BaseContract should not be null for EnumDataContract"); Type baseType = enumDataContract.BaseContract!.UnderlyingType; type.IsEnum = true; type.BaseTypes.Add(baseType); @@ -1152,7 +1160,9 @@ private void ExportISerializableDataContract(DataContract classDataContract, Con { ContractCodeDomInfo baseContractCodeDomInfo = GetContractCodeDomInfo(classDataContract.BaseContract); GenerateType(classDataContract.BaseContract, baseContractCodeDomInfo); - type.BaseTypes.Add(baseContractCodeDomInfo.TypeReference!); + + Debug.Assert(baseContractCodeDomInfo.TypeReference != null, "Class data contracts should have non-null TypeReference"); + type.BaseTypes.Add(baseContractCodeDomInfo.TypeReference); if (baseContractCodeDomInfo.ReferencedTypeExists) { Type? actualType = (Type?)baseContractCodeDomInfo.TypeReference?.UserData[s_codeUserDataActualTypeKey]; @@ -1202,8 +1212,8 @@ private void ExportCollectionDataContract(DataContract collectionContract, Contr SR.Format(SR.CannotUseGenericTypeAsBase, dataContractName, collectionContract.XmlName.Namespace))); - // ItemContract - aka BaseContract - is never null for CollectionDataContract - DataContract itemContract = collectionContract.BaseContract!; + Debug.Assert(collectionContract.BaseContract != null, "BaseContract should not be null for CollectionDataContract"); + DataContract itemContract = collectionContract.BaseContract; bool isItemTypeNullable = collectionContract.IsItemTypeNullable(); bool isDictionary = collectionContract.IsDictionaryLike(out string? keyName, out string? valueName, out string? itemName); @@ -1235,15 +1245,17 @@ private void ExportCollectionDataContract(DataContract collectionContract, Contr // This is supposed to be set by GenerateType. If it wasn't, there is a problem. Debug.Assert(contractCodeDomInfo.TypeDeclaration != null); + Debug.Assert(baseTypeReference != null, "Base type reference should not be null for Dictionary/List collection data contracts"); CodeTypeDeclaration generatedType = contractCodeDomInfo.TypeDeclaration; - generatedType.BaseTypes.Add(baseTypeReference!); + generatedType.BaseTypes.Add(baseTypeReference); CodeAttributeDeclaration collectionContractAttribute = new CodeAttributeDeclaration(GetClrTypeFullName(typeof(CollectionDataContractAttribute))); collectionContractAttribute.Arguments.Add(new CodeAttributeArgument(ImportGlobals.NameProperty, new CodePrimitiveExpression(dataContractName))); collectionContractAttribute.Arguments.Add(new CodeAttributeArgument(ImportGlobals.NamespaceProperty, new CodePrimitiveExpression(collectionContract.XmlName.Namespace))); if (collectionContract.IsReference != ImportGlobals.DefaultIsReference) collectionContractAttribute.Arguments.Add(new CodeAttributeArgument(ImportGlobals.IsReferenceProperty, new CodePrimitiveExpression(collectionContract.IsReference))); - collectionContractAttribute.Arguments.Add(new CodeAttributeArgument(ImportGlobals.ItemNameProperty, new CodePrimitiveExpression(GetNameForAttribute(itemName!)))); // ItemName is never null for Collection contracts. + Debug.Assert(itemName != null, "ItemName is never null for Collection contracts."); + collectionContractAttribute.Arguments.Add(new CodeAttributeArgument(ImportGlobals.ItemNameProperty, new CodePrimitiveExpression(GetNameForAttribute(itemName)))); if (foundDictionaryBase) { // These are not null if we are working with a dictionary. See CollectionDataContract.IsDictionary From a2c8a97f5a6e9e1444bb22d28b69b65651aa55d9 Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Mon, 27 Apr 2026 11:54:17 -0700 Subject: [PATCH 2/3] Be more consistent when using null-suppressions vs Asserts. Co-authored-by: Copilot --- .../Runtime/Serialization/Schema/CodeExporter.cs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Runtime.Serialization.Schema/src/System/Runtime/Serialization/Schema/CodeExporter.cs b/src/libraries/System.Runtime.Serialization.Schema/src/System/Runtime/Serialization/Schema/CodeExporter.cs index e65b18f73d13ca..eaf1b11f9109a1 100644 --- a/src/libraries/System.Runtime.Serialization.Schema/src/System/Runtime/Serialization/Schema/CodeExporter.cs +++ b/src/libraries/System.Runtime.Serialization.Schema/src/System/Runtime/Serialization/Schema/CodeExporter.cs @@ -312,7 +312,7 @@ internal CodeTypeReference GetCodeTypeReference(DataContract dataContract) // This is supposed to be set by GenerateType. If it wasn't, there is a problem. Debug.Assert(contractCodeDomInfo.TypeReference != null); - return contractCodeDomInfo.TypeReference!; + return contractCodeDomInfo.TypeReference; } private CodeTypeReference GetCodeTypeReference(Type type) @@ -611,7 +611,7 @@ private static CodeTypeDeclaration CreateTypeDeclaration(string typeName, DataCo { // ItemContract - aka BaseContract - is never null for CollectionDataContract Debug.Assert(collectionContract.BaseContract != null, "BaseContract should not be null for CollectionDataContract"); - DataContract itemContract = collectionContract.BaseContract!; + DataContract itemContract = collectionContract.BaseContract; if (collectionContract.IsDictionaryLike(out _, out _, out _)) { GenerateKeyValueType(itemContract.As(DataContractType.ClassDataContract)); @@ -632,11 +632,11 @@ private static CodeTypeDeclaration CreateTypeDeclaration(string typeName, DataCo [RequiresUnreferencedCode(ImportGlobals.SerializerTrimmerWarning)] private static bool HasDefaultCollectionNames(DataContract collectionContract) { + // ItemContract - aka BaseContract - is never null for CollectionDataContract Debug.Assert(collectionContract.Is(DataContractType.CollectionDataContract)); Debug.Assert(collectionContract.BaseContract != null, "BaseContract should not be null for CollectionDataContract"); - // ItemContract - aka BaseContract - is never null for CollectionDataContract - DataContract itemContract = collectionContract.BaseContract!; + DataContract itemContract = collectionContract.BaseContract; bool isDictionary = collectionContract.IsDictionaryLike(out string? keyName, out string? valueName, out string? itemName); if (itemName != itemContract.XmlName.Name) return false; @@ -661,7 +661,7 @@ private bool TryGetReferencedDictionaryType(DataContract collectionContract, [No Type? type = _dataContractSet.GetReferencedType(GenericDictionaryName, GenericDictionaryContract, out DataContract? _, out object[]? _) ?? typeof(Dictionary<,>); // ItemContract - aka BaseContract - is never null for CollectionDataContract - DataContract? itemContract = collectionContract.BaseContract!.As(DataContractType.ClassDataContract); + DataContract? itemContract = collectionContract.BaseContract.As(DataContractType.ClassDataContract); // A dictionary should have a Key/Value item contract that has at least two members: key and value. Debug.Assert(itemContract != null); @@ -692,7 +692,7 @@ private bool TryGetReferencedListType(DataContract itemContract, bool isItemType if (type != null) { typeReference = GetCodeTypeReference(type); - typeReference.TypeArguments.Add(GetElementTypeReference(itemContract, isItemTypeNullable)!); // Lists have an item type + typeReference.TypeArguments.Add(GetElementTypeReference(itemContract, isItemTypeNullable)); // Lists have an item type return true; } } @@ -1076,7 +1076,7 @@ private void ExportEnumDataContract(DataContract enumDataContract, ContractCodeD CodeTypeDeclaration type = contractCodeDomInfo.TypeDeclaration; // BaseContract is never null for EnumDataContract Debug.Assert(enumDataContract.BaseContract != null, "BaseContract should not be null for EnumDataContract"); - Type baseType = enumDataContract.BaseContract!.UnderlyingType; + Type baseType = enumDataContract.BaseContract.UnderlyingType; type.IsEnum = true; type.BaseTypes.Add(baseType); if (baseType.IsDefined(typeof(FlagsAttribute), false)) @@ -1445,6 +1445,7 @@ private static string GetClrIdentifier(string identifier, string defaultIdentifi internal static string GetClrTypeFullName(Type type) { + // Type.FullName returns null for generic type definitions or types with unassigned generic parameters, so we construct the full name manually in those cases. return !type.IsGenericTypeDefinition && type.ContainsGenericParameters ? type.Namespace + "." + type.Name : type.FullName!; } From 10142b9371cfdcc178def34db133f2d6b04653dc Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Mon, 27 Apr 2026 12:14:58 -0700 Subject: [PATCH 3/3] Update src/libraries/System.Runtime.Serialization.Schema/src/System/Runtime/Serialization/Schema/CodeExporter.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../src/System/Runtime/Serialization/Schema/CodeExporter.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Runtime.Serialization.Schema/src/System/Runtime/Serialization/Schema/CodeExporter.cs b/src/libraries/System.Runtime.Serialization.Schema/src/System/Runtime/Serialization/Schema/CodeExporter.cs index eaf1b11f9109a1..8fbb804f2eadbb 100644 --- a/src/libraries/System.Runtime.Serialization.Schema/src/System/Runtime/Serialization/Schema/CodeExporter.cs +++ b/src/libraries/System.Runtime.Serialization.Schema/src/System/Runtime/Serialization/Schema/CodeExporter.cs @@ -1445,8 +1445,9 @@ private static string GetClrIdentifier(string identifier, string defaultIdentifi internal static string GetClrTypeFullName(Type type) { - // Type.FullName returns null for generic type definitions or types with unassigned generic parameters, so we construct the full name manually in those cases. - return !type.IsGenericTypeDefinition && type.ContainsGenericParameters ? type.Namespace + "." + type.Name : type.FullName!; + // Type.FullName can be null for types that contain unassigned generic parameters and for generic type parameters, + // so construct a fallback name only when FullName is unavailable. + return type.FullName ?? (type.Namespace == null ? type.Name : type.Namespace + "." + type.Name); } private static string AppendToValidClrIdentifier(string identifier, string appendString)