From ad3e60cc00354e05e4b33b9ff5aa40fc465074a3 Mon Sep 17 00:00:00 2001 From: Ewout Kramer Date: Sat, 22 Feb 2025 11:44:49 +0100 Subject: [PATCH 1/3] Mmmm.....need to split out nullability generation here --- .../Language/Firely/CSharpFirely2.cs | 114 +++++++++--------- 1 file changed, 60 insertions(+), 54 deletions(-) diff --git a/src/Microsoft.Health.Fhir.CodeGen/Language/Firely/CSharpFirely2.cs b/src/Microsoft.Health.Fhir.CodeGen/Language/Firely/CSharpFirely2.cs index 698394796..e2f0ddf51 100644 --- a/src/Microsoft.Health.Fhir.CodeGen/Language/Firely/CSharpFirely2.cs +++ b/src/Microsoft.Health.Fhir.CodeGen/Language/Firely/CSharpFirely2.cs @@ -1971,7 +1971,7 @@ private void WriteComponent( if (identifierElement.cgIsArray()) _writer.WriteLineIndented("List IIdentifiable>.Identifier { get => Identifier; set => Identifier = value; }"); else - _writer.WriteLineIndented("Identifier IIdentifiable.Identifier { get => Identifier; set => Identifier = value; }"); + _writer.WriteLineIndented("Identifier? IIdentifiable.Identifier { get => Identifier; set => Identifier = value; }"); _writer.WriteLine(string.Empty); } @@ -2089,8 +2089,8 @@ private void WriteDictionaryPairs(string exportName, List ex foreach (WrittenElementInfo info in exportedElements) { string elementProp = $"\"{info.FhirElementName}\""; - _writer.WriteLineIndented($"if ({NullCheck(info.PropertyName, info.PropertyType is ListTypeReference)}) yield return new " + - $"KeyValuePair({elementProp},{info.PropertyName});"); + _writer.WriteLineIndented($"if ({NullCheck("_"+info.PropertyName, info.PropertyType is ListTypeReference)}) yield return new " + + $"KeyValuePair({elementProp},_{info.PropertyName});"); } CloseScope(); @@ -2110,7 +2110,7 @@ private void WriteDictionaryTryGetValue(string exportName, List comparer)"); OpenScope(); - _writer.WriteLineIndented($"var otherT = other as {exportName};"); - _writer.WriteLineIndented("if(otherT == null) return false;"); + _writer.WriteLineIndented($"if(other is not {exportName} otherT) return false;"); _writer.WriteLine(string.Empty); _writer.WriteLineIndented("if(!base.CompareChildren(otherT, comparer)) return false;"); + _writer.WriteIndented( + "#pragma warning disable CS8604 // Possible null reference argument - netstd2.1 has a wrong nullable signature here"); + foreach (WrittenElementInfo info in exportedElements) { if(info.PropertyType is CqlTypeReference) { _writer.WriteLineIndented( - $"if( {info.PropertyName} != otherT.{info.PropertyName} )" + + $"if( _{info.PropertyName} != otherT._{info.PropertyName} )" + $" return false;"); } else if (info.PropertyType is ListTypeReference) { _writer.WriteLineIndented( - $"if(!comparer.ListEquals({info.PropertyName}, otherT.{info.PropertyName}))" + + $"if(!comparer.ListEquals(_{info.PropertyName}, otherT._{info.PropertyName}))" + $" return false;"); } else { _writer.WriteLineIndented( - $"if(!comparer.Equals({info.PropertyName}, otherT.{info.PropertyName}))" + + $"if(!comparer.Equals(_{info.PropertyName}, otherT._{info.PropertyName}))" + $" return false;"); } } + _writer.WriteLine("#pragma warning restore CS8604 // Possible null reference argument."); _writer.WriteLine(string.Empty); if (exportName == "PrimitiveType") @@ -2283,13 +2290,11 @@ private void WriteCopyTo( var specifier = exportName == "Base" ? "virtual" : "override"; _writer.WriteLineIndented($"protected internal {specifier} void CopyToInternal(Base other)"); OpenScope(); - _writer.WriteLineIndented($"var dest = other as {exportName};"); - _writer.WriteLine(string.Empty); - - _writer.WriteLineIndented("if (dest == null)"); - OpenScope(); + _writer.WriteLineIndented($"if(other is not {exportName} dest)"); + _writer.IncreaseIndent(); _writer.WriteLineIndented("throw new ArgumentException(\"Can only copy to an object of the same type\", \"other\");"); - CloseScope(); + _writer.DecreaseIndent(); + _writer.WriteLine(); if (exportName == "Base") { @@ -2313,16 +2318,16 @@ private void WriteCopyTo( if (info.PropertyType is ListTypeReference) { _writer.WriteLineIndented( - $"if({info.PropertyName}.Any())" + - $" dest.{info.PropertyName} = new {info.PropertyType.PropertyTypeString}({info.PropertyName}.DeepCopyInternal());"); + $"if(_{info.PropertyName} is not null)" + + $" dest.{info.PropertyName} = new {info.PropertyType.PropertyTypeString}(_{info.PropertyName}.DeepCopyInternal());"); } else { _writer.WriteLineIndented( - $"if({info.PropertyName} != null) dest.{info.PropertyName} = " + + $"if(_{info.PropertyName} is not null) dest.{info.PropertyName} = " + (info.PropertyType is CqlTypeReference ? - $"{info.PropertyName};" : - $"({info.PropertyType.PropertyTypeString}){info.PropertyName}.DeepCopyInternal();")); + $"_{info.PropertyName};" : + $"({info.PropertyType.PropertyTypeString})_{info.PropertyName}.DeepCopyInternal();")); } } @@ -3032,14 +3037,14 @@ private void WriteElementGettersAndSetters(ElementDefinition element, WrittenEle if (ei.PropertyType is not ListTypeReference) { - _writer.WriteLineIndented($"public {ei.PropertyType.PropertyTypeString} {ei.PropertyName}"); + _writer.WriteLineIndented($"public {ei.PropertyType.PropertyTypeString}? {ei.PropertyName}"); OpenScope(); _writer.WriteLineIndented($"get {{ return _{ei.PropertyName}; }}"); _writer.WriteLineIndented($"set {{ _{ei.PropertyName} = value; OnPropertyChanged(\"{ei.PropertyName}\"); }}"); CloseScope(); - _writer.WriteLineIndented($"private {ei.PropertyType.PropertyTypeString} _{ei.PropertyName};"); + _writer.WriteLineIndented($"private {ei.PropertyType.PropertyTypeString}? _{ei.PropertyName};"); _writer.WriteLine(string.Empty); } else @@ -3047,12 +3052,12 @@ private void WriteElementGettersAndSetters(ElementDefinition element, WrittenEle _writer.WriteLineIndented($"public {ei.PropertyType.PropertyTypeString} {ei.PropertyName}"); OpenScope(); - _writer.WriteLineIndented($"get {{ if(_{ei.PropertyName}==null) _{ei.PropertyName} =" + - $" new {ei.PropertyType.PropertyTypeString}(); return _{ei.PropertyName}; }}"); + _writer.WriteLineIndented($"get => _{ei.PropertyName} ??" + + $" new {ei.PropertyType.PropertyTypeString}();"); _writer.WriteLineIndented($"set {{ _{ei.PropertyName} = value; OnPropertyChanged(\"{ei.PropertyName}\"); }}"); CloseScope(); - _writer.WriteLineIndented($"private {ei.PropertyType.PropertyTypeString} _{ei.PropertyName};"); + _writer.WriteLineIndented($"private {ei.PropertyType.PropertyTypeString}? _{ei.PropertyName};"); _writer.WriteLine(string.Empty); } @@ -3101,40 +3106,31 @@ private void WritePrimitiveHelperProperty(string description, WrittenElementInfo switch (propType) { case PrimitiveTypeReference ptr: - _writer.WriteLineIndented($"public {ptr.ConveniencePropertyTypeString} {helperPropName}"); + var nullableType = ptr.ConveniencePropertyTypeString.EndsWith('?') ? ptr.ConveniencePropertyTypeString : ptr.ConveniencePropertyTypeString + '?'; + _writer.WriteLineIndented($"public {nullableType} {helperPropName}"); OpenScope(); - _writer.WriteIndented($"get {{ return {ei.PropertyName} != null ? "); string propAccess = versionsRemark is not null - ? $"(({MostGeneralValueAccessorType(ptr)}){ei.PropertyName})" - : ei.PropertyName; + ? $"(({MostGeneralValueAccessorType(ptr)}?)_{ei.PropertyName})" + : $"_{ei.PropertyName}"; + _writer.WriteLineIndented($"get => {propAccess}?.Value;"); - _writer.WriteLine($"{propAccess}.Value : null; }}"); _writer.WriteLineIndented("set"); OpenScope(); - - _writer.WriteLineIndented($"if (value == null)"); - - _writer.IncreaseIndent(); - _writer.WriteLineIndented($"{ei.PropertyName} = null;"); - _writer.DecreaseIndent(); - _writer.WriteLineIndented("else"); - _writer.IncreaseIndent(); - _writer.WriteLineIndented($"{ei.PropertyName} = new {ptr.PropertyTypeString}(value);"); - _writer.DecreaseIndent(); + _writer.WriteLineIndented($"{ei.PropertyName} = value is null ? null : new {ptr.PropertyTypeString}(value);"); _writer.WriteLineIndented($"OnPropertyChanged(\"{helperPropName}\");"); CloseScope(suppressNewline: true); CloseScope(); break; case ListTypeReference { Element: PrimitiveTypeReference lptr }: - _writer.WriteLineIndented($"public IEnumerable<{lptr.ConveniencePropertyTypeString}> {helperPropName}"); + _writer.WriteLineIndented($"public IEnumerable<{lptr.ConveniencePropertyTypeString}?>? {helperPropName}"); OpenScope(); - _writer.WriteIndented($"get {{ return {ei.PropertyName} != null ? {ei.PropertyName}"); + _writer.WriteIndented($"get => _{ei.PropertyName}"); if(versionsRemark is not null) - _writer.Write($".Cast<{MostGeneralValueAccessorType(lptr)}>()"); - _writer.WriteLine($".Select(elem => elem.Value) : null; }}"); + _writer.Write($"?.Cast<{MostGeneralValueAccessorType(lptr)}>()"); + _writer.WriteLine($"?.Select(elem => elem.Value);"); _writer.WriteLineIndented("set"); OpenScope(); @@ -3142,7 +3138,7 @@ private void WritePrimitiveHelperProperty(string description, WrittenElementInfo _writer.WriteLineIndented($"if (value == null)"); _writer.IncreaseIndent(); - _writer.WriteLineIndented($"{ei.PropertyName} = null;"); + _writer.WriteLineIndented($"{ei.PropertyName} = null!;"); _writer.DecreaseIndent(); _writer.WriteLineIndented("else"); _writer.IncreaseIndent(); @@ -3442,12 +3438,14 @@ private void WritePrimitiveType( _writer.WriteLine(string.Empty); } - _writer.WriteLineIndented($"public {exportName}({typeName} value)"); + var nullableTypeName = typeName.EndsWith('?') ? typeName : typeName + '?'; + + _writer.WriteLineIndented($"public {exportName}({nullableTypeName} value)"); OpenScope(); _writer.WriteLineIndented("Value = value;"); CloseScope(); - _writer.WriteLineIndented($"public {exportName}(): this(({typeName})null) {{}}"); + _writer.WriteLineIndented($"public {exportName}(): this(({nullableTypeName})null) {{}}"); _writer.WriteLine(string.Empty); // For some primitive pocos, we need a hand-written value propery since we are using @@ -3467,11 +3465,11 @@ private void WritePrimitiveType( _writer.WriteLineIndented("[DataMember]"); - _writer.WriteLineIndented($"public {typeName} Value"); + _writer.WriteLineIndented($"public {nullableTypeName} Value"); OpenScope(); var typeNameInSwitch = typeName.EndsWith("?") ? typeName[..^1] : typeName; - _writer.WriteLineIndented($"get {{ return ObjectValue is {typeNameInSwitch} or null ? ({typeName})ObjectValue : throw COVE.INCORRECT_LITERAL_VALUE_TYPE(null, ObjectValue, this.TypeName); }}"); + _writer.WriteLineIndented($"get {{ return ObjectValue is {typeNameInSwitch} or null ? ({nullableTypeName})ObjectValue : throw COVE.INCORRECT_LITERAL_VALUE_TYPE(null, ObjectValue, this.TypeName); }}"); _writer.WriteLineIndented("set { ObjectValue = value; OnPropertyChanged(\"Value\"); }"); CloseScope(); } @@ -3579,8 +3577,12 @@ private void WriteHeaderComplexDataType() _writer.WriteLineIndented("using Hl7.Fhir.Specification;"); _writer.WriteLineIndented("using Hl7.Fhir.Utility;"); _writer.WriteLineIndented("using Hl7.Fhir.Validation;"); + _writer.WriteLineIndented("using System.Diagnostics.CodeAnalysis;"); _writer.WriteLineIndented("using SystemPrimitive = Hl7.Fhir.ElementModel.Types;"); - _writer.WriteLine(string.Empty); + _writer.WriteLine(); + + _writer.WriteLineIndented("#nullable enable"); + _writer.WriteLine(); WriteCopyright(); @@ -3601,10 +3603,14 @@ private void WriteHeaderPrimitive() _writer.WriteLineIndented("using Hl7.Fhir.Introspection;"); _writer.WriteLineIndented("using Hl7.Fhir.Specification;"); _writer.WriteLineIndented("using Hl7.Fhir.Validation;"); + _writer.WriteLineIndented("using System.Diagnostics.CodeAnalysis;"); _writer.WriteLineIndented("using SystemPrimitive = Hl7.Fhir.ElementModel.Types;"); _writer.WriteLineIndented("using COVE=Hl7.Fhir.Validation.CodedValidationException;"); _writer.WriteLine(string.Empty); + _writer.WriteLineIndented("#nullable enable"); + _writer.WriteLine(); + WriteCopyright(); } From c3d52366c9488d9fab88eaf7cd98b55c311a0b1a Mon Sep 17 00:00:00 2001 From: Ewout Kramer Date: Mon, 24 Feb 2025 15:45:04 +0100 Subject: [PATCH 2/3] Made interface/pattern generation nullable aware. --- .../Language/Firely/CSharpFirely2.cs | 204 +++++++++--------- 1 file changed, 102 insertions(+), 102 deletions(-) diff --git a/src/Microsoft.Health.Fhir.CodeGen/Language/Firely/CSharpFirely2.cs b/src/Microsoft.Health.Fhir.CodeGen/Language/Firely/CSharpFirely2.cs index e2f0ddf51..451bab3e3 100644 --- a/src/Microsoft.Health.Fhir.CodeGen/Language/Firely/CSharpFirely2.cs +++ b/src/Microsoft.Health.Fhir.CodeGen/Language/Firely/CSharpFirely2.cs @@ -1607,7 +1607,7 @@ private void WriteInterfaceComponent( string pn = exportName + "." + interfaceEi.PropertyName; WrittenElementInfo? resourceEi = null; - if (resourceElements.TryGetValue(interfaceEi.FhirElementName ?? string.Empty, out ElementDefinition? resourceEd)) + if (resourceElements.TryGetValue(interfaceEi.FhirElementName, out ElementDefinition? resourceEd)) { resourceEi = BuildElementInfo(resourceExportName, resourceEd); } @@ -1632,67 +1632,27 @@ private void WriteInterfaceElementGettersAndSetters( string interfaceExportName, WrittenElementInfo interfaceEi) { - string pn = interfaceExportName + "." + interfaceEi.PropertyName; - string rt = resourceEi?.PropertyType.PropertyTypeString ?? string.Empty; - string it = interfaceEi.PropertyType.PropertyTypeString; + string pn = interfaceEi.PropertyName; + string it = interfaceEi.PropertyType is ListTypeReference ? interfaceEi.PropertyType.PropertyTypeString : WithNullabilityMarking(interfaceEi.PropertyType.PropertyTypeString); if ((resourceEd == null) || (resourceEi == null)) { - _writer.WriteLineIndented("[IgnoreDataMember]"); - _writer.WriteLineIndented($"{it} {pn}"); - OpenScope(); - _writer.WriteLineIndented($"get {{ return null; }}"); - _writer.WriteLineIndented($"set {{ throw new NotImplementedException(\"Resource {resourceExportName} does not implement {interfaceExportName}.{interfaceEi.FhirElementName}\");}}"); - CloseScope(); + writeEmptyGetterAndSetter(it, pn); } else if (interfaceEi.PropertyType.PropertyTypeString == resourceEi.PropertyType.PropertyTypeString) { - _writer.WriteLineIndented("[IgnoreDataMember]"); - _writer.WriteLineIndented($"{it} {pn}" + - $" {{" + - $" get => {resourceEi.PropertyName};" + - $" set {{ {resourceEi.PropertyName} = value; }}" + - $" }}"); - _writer.WriteLine(); + writeOneOnOneGetterAndSetter(it, pn); } + // a resource is allowed to have a scalar in place of a list - else if ((interfaceEi.PropertyType is ListTypeReference interfaceLTR) && - (interfaceLTR.Element.PropertyTypeString == resourceEi.PropertyType.PropertyTypeString)) + else if ((interfaceEi.PropertyType is ListTypeReference interfaceLtr) && + (interfaceLtr.Element.PropertyTypeString == resourceEi.PropertyType.PropertyTypeString)) { - _writer.WriteLineIndented("[IgnoreDataMember]"); - _writer.WriteLineIndented($"{it} {pn}"); - OpenScope(); - //_writer.WriteLineIndented($"get {{ return new {it}() {{ {resourceEi.PropertyName} }}; }}"); - _writer.WriteLineIndented("get"); - OpenScope(); // getter - _writer.WriteLineIndented($"if ({resourceEi.PropertyName} == null) return new {it}();"); - _writer.WriteLineIndented($"return new {it}() {{ {resourceEi.PropertyName} }};"); - CloseScope(); // getter - - _writer.WriteLineIndented("set"); - OpenScope(); - _writer.WriteLineIndented($"if (value.Count == 0) {{ {resourceEi.PropertyName} = null; }}"); - _writer.WriteLineIndented($"else if (value.Count == 1) {{ {resourceEi.PropertyName} = value.First(); }}"); - _writer.WriteLineIndented($"else {{ throw new NotImplementedException(\"Resource {resourceExportName} can only have a single {pn} value\"); }}"); - CloseScope(); - - CloseScope(); + writeSingleToListGetterAndSetter(it, pn); } else { - WriteIndentedComment( - $"{resourceExportName}.{resourceEi.PropertyName} ({resourceEi.PropertyType.PropertyTypeString}) is incompatible with\n" + - $"{interfaceExportName}.{interfaceEi.FhirElementName} ({interfaceEi.PropertyType.PropertyTypeString})", - isSummary: false, - isRemarks: true); - _writer.WriteLineIndented("[IgnoreDataMember]"); - _writer.WriteLineIndented($"{it} {pn}"); - OpenScope(); - _writer.WriteLineIndented($"get {{ return null; }}"); - _writer.WriteLineIndented($"set {{ throw new NotImplementedException(\"{resourceExportName}.{resourceEi.PropertyName} " + - $"({resourceEi.PropertyType.PropertyTypeString}) is incompatible with" + - $" {interfaceExportName}.{interfaceEi.FhirElementName} ({interfaceEi.PropertyType.PropertyTypeString})\");}}"); - CloseScope(); + writeIncompatibleGetterSetter(it, pn); } if (!TryGetPrimitiveType(interfaceEi.PropertyType, out PrimitiveTypeReference? interfacePtr)) @@ -1700,59 +1660,88 @@ private void WriteInterfaceElementGettersAndSetters( return; } - string ppn = interfaceExportName + "." + interfaceEi.PrimitiveHelperName; - string prt = (resourceEi?.PropertyType is PrimitiveTypeReference rPTR) ? rPTR.ConveniencePropertyTypeString : string.Empty; - string pit = interfacePtr.ConveniencePropertyTypeString; + string ppn = interfaceEi.PrimitiveHelperName!; + string pit = interfaceEi.PropertyType is ListTypeReference ? interfacePtr.ConveniencePropertyTypeString : WithNullabilityMarking(interfacePtr.ConveniencePropertyTypeString); if ((resourceEd == null) || (resourceEi == null)) { - _writer.WriteLineIndented("[IgnoreDataMember]"); - _writer.WriteLineIndented($"{pit} {ppn}"); - OpenScope(); - _writer.WriteLineIndented($"get {{ return null; }}"); - _writer.WriteLineIndented($"set {{ throw new NotImplementedException(\"Resource {resourceExportName}" + - $" does not implement {interfaceExportName}.{interfaceEi.FhirElementName}\");}}"); - CloseScope(); + writeEmptyGetterAndSetter(pit, ppn); } else if (interfaceEi.PropertyType == resourceEi.PropertyType) + { + writeOneOnOneGetterAndSetter(pit, ppn); + } + else + { + writeIncompatibleGetterSetter(pit, ppn); + } + + return; + + void writeOneOnOneGetterAndSetter(string propertyType, string propertyName) { _writer.WriteLineIndented("[IgnoreDataMember]"); - _writer.WriteLineIndented($"{pit} {ppn}" + - $" {{" + - $" get => {resourceEi.PrimitiveHelperName};" + - $" set {{ {resourceEi.PrimitiveHelperName} = value; }}" + - $" }}"); - _writer.WriteLine(); + _writer.WriteLineIndented($"{propertyType} {interfaceExportName}.{propertyName}"); + OpenScope(); + _writer.WriteLineIndented($"get => {propertyName};"); + _writer.WriteLineIndented($"set => {propertyName} = value;"); + CloseScope(); } - // a resource is allowed to have a scalar in place of a list - //else if (interfaceEi.PropertyType == "List<" + resourceEi.PropertyType + ">") - else if (interfaceEi.PropertyType is ListTypeReference) + + void writeSingleToListGetterAndSetter(string propertyType, string propertyName) { _writer.WriteLineIndented("[IgnoreDataMember]"); - _writer.WriteLineIndented($"{pit} {ppn}"); + _writer.WriteLineIndented($"{propertyType} {interfaceExportName}.{propertyName}"); OpenScope(); - _writer.WriteLineIndented($"get {{ return new {pit}() {{ {resourceEi.PropertyType.PropertyTypeString} }}; }}"); + + _writer.WriteLineIndented($"get => {propertyName} is null ? [] : [{propertyName}];"); _writer.WriteLineIndented("set"); OpenScope(); - _writer.WriteLineIndented($"if (value.Count == 1) {{ {resourceEi.PrimitiveHelperName} = value.First(); }}"); - _writer.WriteLineIndented($"else {{ throw new NotImplementedException(\"Resource {resourceExportName} can only have a single {ppn} value\"); }}"); + _writer.WriteLineIndented($"{propertyName} = value switch"); + + OpenScope(); + _writer.WriteLineIndented($"{{ Count: 0 }} => null,"); + _writer.WriteLineIndented($"{{ Count: 1 }} => value.First(),"); + _writer.WriteLineIndented($"_ => throw new NotImplementedException(\"Resource {resourceExportName} can only have a single {propertyName} value\")"); + CloseScope(includeSemicolon: true, suppressNewline: true); + + CloseScope(suppressNewline: true); CloseScope(); + } + void writeIncompatibleGetterSetter(string propertyType, string propertyName) + { + string message = $"{resourceExportName}.{resourceEi.PropertyName} is incompatible with " + + $"{interfaceExportName}.{interfaceEi.FhirElementName}."; + WriteIndentedComment(message, isSummary: false, isRemarks: true); + + _writer.WriteLineIndented("[IgnoreDataMember]"); + _writer.WriteLineIndented($"{propertyType} {interfaceExportName}.{propertyName}"); + + OpenScope(); + _writer.WriteLineIndented($"get => {emptyInterfaceType()};"); + _writer.WriteLineIndented($"set => throw new NotImplementedException(\"{message}\");"); CloseScope(); } - else + + string emptyInterfaceType() => interfaceEi.PropertyType is ListTypeReference ? "[]" : "null"; + + void writeEmptyGetterAndSetter(string propertyType, string propertyName) { - _writer.WriteLineIndented($"// {resourceExportName}.{resourceEi.PropertyName} ({prt}) is incompatible with {interfaceExportName}.{interfaceEi.FhirElementName} ({pit})"); _writer.WriteLineIndented("[IgnoreDataMember]"); - _writer.WriteLineIndented($" {pit} {ppn}"); + _writer.WriteLineIndented($"{propertyType} {interfaceExportName}.{propertyName}"); OpenScope(); - _writer.WriteLineIndented($"get {{ return null; }}"); - _writer.WriteLineIndented($"set {{ throw new NotImplementedException(\"{resourceExportName}.{resourceEi.PropertyName} ({resourceEi.PropertyType.PropertyTypeString}) is incompatible with {interfaceExportName}.{interfaceEi.FhirElementName} ({interfaceEi.PropertyType.PropertyTypeString})\");}}"); + _writer.WriteLineIndented($"get => {emptyInterfaceType()};"); + _writer.WriteLineIndented($"set => throw new NotImplementedException(\"Resource {resourceExportName}" + + $" does not implement {interfaceExportName}.{interfaceEi.FhirElementName}\");"); CloseScope(); } } + private static string WithNullabilityMarking(string type) => type.EndsWith("?") ? type : type + "?"; + + private void WriteInterfaceElements( ComponentDefinition complex, string exportedComplexName, @@ -1779,12 +1768,16 @@ private void WriteInterfaceElements( { WriteIndentedComment(element.Short.Replace("{{title}}", structureName)); _writer.WriteLineIndented($"/// This uses the native .NET datatype, rather than the FHIR equivalent"); - _writer.WriteLineIndented($"{eiPtr.ConveniencePropertyTypeString} {ei.PrimitiveHelperName} {{ get; set; }}"); + + _writer.WriteLineIndented($"{WithNullabilityMarking(eiPtr.ConveniencePropertyTypeString)} {ei.PrimitiveHelperName} {{ get; set; }}"); _writer.WriteLine(); } if (description != null) WriteIndentedComment(description); - _writer.WriteLineIndented($"{ei.PropertyType.PropertyTypeString ?? string.Empty} {ei.PropertyName} {{ get; set; }}"); + var typ = ei.PropertyType is ListTypeReference + ? ei.PropertyType.PropertyTypeString + : WithNullabilityMarking(ei.PropertyType.PropertyTypeString); + _writer.WriteLineIndented($"{typ} {ei.PropertyName} {{ get; set; }}"); _writer.WriteLine(); } } @@ -1888,18 +1881,19 @@ private void WriteComponent( if (identifierElement.cgIsArray()) interfaces.Add("IIdentifiable>"); else - interfaces.Add("IIdentifiable"); + interfaces.Add("IIdentifiable"); } } - var primaryCodeElementInfo = isResource ? getPrimaryCodedElementInfo(complex, exportName) : null; + WrittenElementInfo? primaryCodeElementInfo = isResource ? getPrimaryCodedElementInfo(complex, exportName) : null; if (primaryCodeElementInfo != null) { - interfaces.Add($"ICoded<{primaryCodeElementInfo.PropertyType.PropertyTypeString}>"); + string nullable = primaryCodeElementInfo.PropertyType is ListTypeReference ? "" : "?"; + interfaces.Add($"ICoded<{primaryCodeElementInfo.PropertyType.PropertyTypeString}{nullable}>"); } - var modifierElement = complex.cgGetChild("modifierExtension"); + ElementDefinition? modifierElement = complex.cgGetChild("modifierExtension"); if (modifierElement != null) { if (!modifierElement.cgIsInherited(complex.Structure)) @@ -1971,15 +1965,22 @@ private void WriteComponent( if (identifierElement.cgIsArray()) _writer.WriteLineIndented("List IIdentifiable>.Identifier { get => Identifier; set => Identifier = value; }"); else - _writer.WriteLineIndented("Identifier? IIdentifiable.Identifier { get => Identifier; set => Identifier = value; }"); + _writer.WriteLineIndented("Identifier? IIdentifiable.Identifier { get => Identifier; set => Identifier = value; }"); _writer.WriteLine(string.Empty); } if (primaryCodeElementInfo != null) { - _writer.WriteLineIndented($"{primaryCodeElementInfo.PropertyType.PropertyTypeString} ICoded<{primaryCodeElementInfo.PropertyType.PropertyTypeString}>.Code {{ get => {primaryCodeElementInfo.PropertyName}; set => {primaryCodeElementInfo.PropertyName} = value; }}"); - _writer.WriteLineIndented($"IEnumerable ICoded.ToCodings() => {primaryCodeElementInfo.PropertyName}.ToCodings();"); + var (codedType, bang) = primaryCodeElementInfo.PropertyType switch + { + ListTypeReference { PropertyTypeString: { } n } => (n, string.Empty), + { PropertyTypeString: {} n } => (WithNullabilityMarking(n), "!") + }; + + _writer.WriteLineIndented($"{codedType} ICoded<{codedType}>.Code {{ get => {primaryCodeElementInfo.PropertyName}; " + + $"set => {primaryCodeElementInfo.PropertyName} = value{bang}; }}"); + _writer.WriteLineIndented($"IEnumerable ICoded.ToCodings() => {primaryCodeElementInfo.PropertyName}?.ToCodings() ?? [];"); _writer.WriteLine(string.Empty); } @@ -1989,7 +1990,7 @@ private void WriteComponent( if (birthdayProperty != null) { - _writer.WriteLineIndented($"Hl7.Fhir.Model.Date {Namespace}.IPatient.BirthDate => {birthdayProperty.PropertyName};"); + _writer.WriteLineIndented($"Hl7.Fhir.Model.Date? {Namespace}.IPatient.BirthDate => {birthdayProperty.PropertyName};"); _writer.WriteLine(string.Empty); } } @@ -2982,20 +2983,18 @@ string getTypeNameFromElement() internal static bool TryGetPrimitiveType(TypeReference tr, [NotNullWhen(true)] out PrimitiveTypeReference? ptr) { - if (tr is PrimitiveTypeReference p) + switch (tr) { - ptr = p; - return true; + case PrimitiveTypeReference p: + ptr = p; + return true; + case ListTypeReference { Element: PrimitiveTypeReference pltr }: + ptr = pltr; + return true; + default: + ptr = null; + return false; } - - if (tr is ListTypeReference { Element: PrimitiveTypeReference pltr }) - { - ptr = pltr; - return true; - } - - ptr = null; - return false; } internal WrittenElementInfo BuildElementInfo( @@ -3106,7 +3105,7 @@ private void WritePrimitiveHelperProperty(string description, WrittenElementInfo switch (propType) { case PrimitiveTypeReference ptr: - var nullableType = ptr.ConveniencePropertyTypeString.EndsWith('?') ? ptr.ConveniencePropertyTypeString : ptr.ConveniencePropertyTypeString + '?'; + string nullableType = WithNullabilityMarking(ptr.ConveniencePropertyTypeString); _writer.WriteLineIndented($"public {nullableType} {helperPropName}"); OpenScope(); @@ -3123,7 +3122,8 @@ private void WritePrimitiveHelperProperty(string description, WrittenElementInfo CloseScope(); break; case ListTypeReference { Element: PrimitiveTypeReference lptr }: - _writer.WriteLineIndented($"public IEnumerable<{lptr.ConveniencePropertyTypeString}?>? {helperPropName}"); + string nullableTypeList = WithNullabilityMarking(lptr.ConveniencePropertyTypeString); + _writer.WriteLineIndented($"public IEnumerable<{nullableTypeList}>? {helperPropName}"); OpenScope(); From e00c714d1e6fd275f7ff3566931eeb4a2f436175 Mon Sep 17 00:00:00 2001 From: Ewout Kramer Date: Mon, 24 Feb 2025 21:35:00 +0100 Subject: [PATCH 3/3] Missed the fact that a comment was commenting out an important line --- .../Language/Firely/CSharpFirely2.cs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.Health.Fhir.CodeGen/Language/Firely/CSharpFirely2.cs b/src/Microsoft.Health.Fhir.CodeGen/Language/Firely/CSharpFirely2.cs index 451bab3e3..5d5edce9a 100644 --- a/src/Microsoft.Health.Fhir.CodeGen/Language/Firely/CSharpFirely2.cs +++ b/src/Microsoft.Health.Fhir.CodeGen/Language/Firely/CSharpFirely2.cs @@ -1980,7 +1980,7 @@ private void WriteComponent( _writer.WriteLineIndented($"{codedType} ICoded<{codedType}>.Code {{ get => {primaryCodeElementInfo.PropertyName}; " + $"set => {primaryCodeElementInfo.PropertyName} = value{bang}; }}"); - _writer.WriteLineIndented($"IEnumerable ICoded.ToCodings() => {primaryCodeElementInfo.PropertyName}?.ToCodings() ?? [];"); + _writer.WriteLineIndented($"IReadOnlyCollection ICoded.ToCodings() => {primaryCodeElementInfo.PropertyName}?.ToCodings() ?? [];"); _writer.WriteLine(string.Empty); } @@ -2240,7 +2240,7 @@ private void WriteCompareChildren( _writer.WriteLineIndented("if(!base.CompareChildren(otherT, comparer)) return false;"); - _writer.WriteIndented( + _writer.WriteLineIndented( "#pragma warning disable CS8604 // Possible null reference argument - netstd2.1 has a wrong nullable signature here"); foreach (WrittenElementInfo info in exportedElements) @@ -2265,7 +2265,7 @@ private void WriteCompareChildren( } } - _writer.WriteLine("#pragma warning restore CS8604 // Possible null reference argument."); + _writer.WriteLineIndented("#pragma warning restore CS8604 // Possible null reference argument."); _writer.WriteLine(string.Empty); if (exportName == "PrimitiveType") @@ -3051,8 +3051,7 @@ private void WriteElementGettersAndSetters(ElementDefinition element, WrittenEle _writer.WriteLineIndented($"public {ei.PropertyType.PropertyTypeString} {ei.PropertyName}"); OpenScope(); - _writer.WriteLineIndented($"get => _{ei.PropertyName} ??" + - $" new {ei.PropertyType.PropertyTypeString}();"); + _writer.WriteLineIndented($"get => _{ei.PropertyName} ??= [];"); _writer.WriteLineIndented($"set {{ _{ei.PropertyName} = value; OnPropertyChanged(\"{ei.PropertyName}\"); }}"); CloseScope(); @@ -3069,7 +3068,7 @@ PrimitiveTypeReference or // If the property has had multiple types over time, we need to generate a helper property for each type. if(_elementTypeChanges.TryGetValue(element.Path, out ElementTypeChange[]? changes)) { - var lastChange = changes.Last(); + ElementTypeChange lastChange = changes.Last(); foreach(ElementTypeChange change in changes) { @@ -3123,14 +3122,14 @@ private void WritePrimitiveHelperProperty(string description, WrittenElementInfo break; case ListTypeReference { Element: PrimitiveTypeReference lptr }: string nullableTypeList = WithNullabilityMarking(lptr.ConveniencePropertyTypeString); - _writer.WriteLineIndented($"public IEnumerable<{nullableTypeList}>? {helperPropName}"); + _writer.WriteLineIndented($"public IEnumerable<{nullableTypeList}> {helperPropName}"); OpenScope(); _writer.WriteIndented($"get => _{ei.PropertyName}"); if(versionsRemark is not null) _writer.Write($"?.Cast<{MostGeneralValueAccessorType(lptr)}>()"); - _writer.WriteLine($"?.Select(elem => elem.Value);"); + _writer.WriteLine($"?.Select(elem => elem.Value) ?? [];"); _writer.WriteLineIndented("set"); OpenScope();