Skip to content

Fix default value initialization for nullable properties #345

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

Merged
habbes merged 6 commits into from
May 4, 2023
Merged

Fix default value initialization for nullable properties #345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dc113b9
Fix build error in vs2022 csproj
habbes Apr 26, 2023
1209aee
Generate correct C# default value initializations for nullable proper…
habbes Apr 26, 2023
3b170ce
Fix property initialization for VB and refactor
habbes Apr 27, 2023
37cf0cf
Add tests to verify that generated code compiles
habbes Apr 27, 2023
a67e18d
Address review comments
habbes May 2, 2023
791fcb2
Fix build errors
habbes May 2, 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
231 changes: 133 additions & 98 deletions src/Microsoft.OData.CodeGen/Templates/ODataT4CodeGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// ------------------------------------------------------------------------------
// <auto-generated>
// This code was generated by a tool.
// Runtime Version: 16.0.0.0
// Runtime Version: 17.0.0.0
Comment thread
habbes marked this conversation as resolved.
//
// Changes to this file may cause incorrect behavior and will be lost if
// the code is regenerated.
Expand Down Expand Up @@ -33,7 +33,7 @@ namespace Microsoft.OData.CodeGen.Templates
/// <summary>
/// Class to produce the template output
/// </summary>
[global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.VisualStudio.TextTemplating", "16.0.0.0")]
[global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.VisualStudio.TextTemplating", "17.0.0.0")]
public partial class ODataT4CodeGenerator : ODataT4CodeGeneratorBase
{
/// <summary>
Expand Down Expand Up @@ -1745,6 +1745,15 @@ public override int GetHashCode()
protected abstract void WriteRequiredAttribute(string errorMessage);
#endregion Language specific write methods.

/// <summary>
/// Given a nullable type name, e.g. Nullable<T>, returns
/// the underlying type name T. If the type name is not
/// a Nullable<T>, then returns T.
/// </summary>
/// <param name="typeName">The type name.</param>
/// <returns>The underlying type name.</returns>
internal abstract string StripNullableFromTypeName(string typeName);
Copy link
Copy Markdown
Contributor

@ElizabethOkerio ElizabethOkerio Apr 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so the type name here will be in form of a string? Exa Nullable<DateTime> then the method returns DateTime but in string form?

Copy link
Copy Markdown
Contributor

@ElizabethOkerio ElizabethOkerio Apr 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I do not understand but why can't we check if a type is nullable without the conversions to string?

Copy link
Copy Markdown
Contributor Author

@habbes habbes May 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because by the time we get to use this method, the input type name is already in string form. I do agree that it would be more robust to use actual reflection type metadata to detect nullability, but we don't have that information in the method where this method is used. To use types would require refactoring a lot more methods and a lot more work than what I "budgeted" to fix this bug.


internal HashSet<EdmPrimitiveTypeKind> ClrReferenceTypes { get {
if (clrReferenceTypes == null)
{
Expand Down Expand Up @@ -3723,118 +3732,120 @@ internal static string GetPropertyInitializationValue(IEdmProperty property, boo
if (edmCollectionTypeReference == null)
{
IEdmStructuralProperty structuredProperty = property as IEdmStructuralProperty;
if (structuredProperty != null)
if (structuredProperty == null)
{
if (!string.IsNullOrEmpty(structuredProperty.DefaultValueString))
{
string valueClrType = GetClrTypeName(edmTypeReference, useDataServiceCollection, clientTemplate, context);
string defaultValue = structuredProperty.DefaultValueString;
bool isCSharpTemplate = clientTemplate is ODataClientCSharpTemplate;
if (edmTypeReference.Definition.TypeKind == EdmTypeKind.Enum)
{
var enumValues = defaultValue.Split(',');
string fullenumTypeName = GetClrTypeName(edmTypeReference, useDataServiceCollection, clientTemplate, context);
string enumTypeName = GetClrTypeName(edmTypeReference, useDataServiceCollection, clientTemplate, context, false, false);
List<string> customizedEnumValues = new List<string>();
foreach(var enumValue in enumValues)
{
string currentEnumValue = enumValue.Trim();
int indexFirst = currentEnumValue.IndexOf('\'') + 1;
int indexLast = currentEnumValue.LastIndexOf('\'');
if (indexFirst > 0 && indexLast > indexFirst)
{
currentEnumValue = currentEnumValue.Substring(indexFirst, indexLast - indexFirst);
}
// only structured property has default value
return null;
}

var customizedEnumValue = context.EnableNamingAlias ? Customization.CustomizeNaming(currentEnumValue) : currentEnumValue;
if (isCSharpTemplate)
{
currentEnumValue = "(" + fullenumTypeName + ")" + clientTemplate.EnumTypeName + ".Parse(" + clientTemplate.SystemTypeTypeName + ".GetType(\"" + enumTypeName + "\"), \"" + customizedEnumValue + "\")";
}
else
{
currentEnumValue = clientTemplate.EnumTypeName + ".Parse(" + clientTemplate.SystemTypeTypeName + ".GetType(\"" + enumTypeName + "\"), \"" + currentEnumValue + "\")";
}
customizedEnumValues.Add(currentEnumValue);
}
if (isCSharpTemplate)
{
return string.Join(" | ", customizedEnumValues);
}
else
{
return string.Join(" Or ", customizedEnumValues);
}
}
if (string.IsNullOrEmpty(structuredProperty.DefaultValueString))
{
// doesn't have a default value
return null;
}

if (valueClrType.Equals(clientTemplate.StringTypeName))
{
defaultValue = "\"" + defaultValue + "\"";
}
else if (valueClrType.Equals(clientTemplate.BooleanTypeName))
{
// EDMX specifies boolean defaults with capital letter, C# needs this string to be lower case.
if (isCSharpTemplate)
defaultValue = defaultValue.ToLower();
}
else if (valueClrType.Equals(clientTemplate.BinaryTypeName))
{
defaultValue = "System.Text.Encoding.UTF8.GetBytes(\"" + defaultValue + "\")";
}
else if (valueClrType.Equals(clientTemplate.SingleTypeName))
{
if (isCSharpTemplate)
{
defaultValue = defaultValue.EndsWith("f", StringComparison.OrdinalIgnoreCase) ? defaultValue : defaultValue + "f";
}
else
{
defaultValue = defaultValue.EndsWith("f", StringComparison.OrdinalIgnoreCase) ? defaultValue : defaultValue + "F";
}
}
else if (valueClrType.Equals(clientTemplate.DecimalTypeName))
{
if (isCSharpTemplate)
{
// decimal in C# must be initialized with 'm' at the end, like Decimal dec = 3.00m
defaultValue = defaultValue.EndsWith("m", StringComparison.OrdinalIgnoreCase) ? defaultValue : defaultValue + "m";
}
else
{
// decimal in VB must be initialized with 'D' at the end, like Decimal dec = 3.00D
defaultValue = defaultValue.ToLower(CultureInfo.InvariantCulture).Replace("m", "D");
defaultValue = defaultValue.EndsWith("D", StringComparison.OrdinalIgnoreCase) ? defaultValue : defaultValue + "D";
}
}
else if (valueClrType.Equals(clientTemplate.GuidTypeName)
| valueClrType.Equals(clientTemplate.DateTimeOffsetTypeName)
| valueClrType.Equals(clientTemplate.DateTypeName)
| valueClrType.Equals(clientTemplate.TimeOfDayTypeName))
string valueClrType = GetClrTypeName(edmTypeReference, useDataServiceCollection, clientTemplate, context);
Comment thread
habbes marked this conversation as resolved.

// If it's a Nullable<T> type, then we need to use the underlying T to generate initialization code correctly
// The property initializers below do not expect that the specified default to be null. Since the default
// value of a Nullable<T> is null, it's not worth the effort to handle that edge case, Instead we
// assume the user will not specify "null" as the default value of a nullable property since it's redundant.
valueClrType = clientTemplate.StripNullableFromTypeName(valueClrType);
string defaultValue = structuredProperty.DefaultValueString;
bool isCSharpTemplate = clientTemplate is ODataClientCSharpTemplate;
if (edmTypeReference.Definition.TypeKind == EdmTypeKind.Enum)
{
var enumValues = defaultValue.Split(',');
string fullEnumTypeName = GetClrTypeName(edmTypeReference, useDataServiceCollection, clientTemplate, context);
string enumTypeName = GetClrTypeName(edmTypeReference, useDataServiceCollection, clientTemplate, context, false, false);
List<string> customizedEnumValues = new List<string>();
foreach (var enumValue in enumValues)
{
string currentEnumValue = enumValue.Trim();
int indexFirst = currentEnumValue.IndexOf('\'') + 1;
int indexLast = currentEnumValue.LastIndexOf('\'');
if (indexFirst > 0 && indexLast > indexFirst)
{
defaultValue = valueClrType + ".Parse(\"" + defaultValue + "\")";
currentEnumValue = currentEnumValue.Substring(indexFirst, indexLast - indexFirst);
}
else if (valueClrType.Equals(clientTemplate.DurationTypeName))

var customizedEnumValue = context.EnableNamingAlias ? Customization.CustomizeNaming(currentEnumValue) : currentEnumValue;
if (isCSharpTemplate)
{
defaultValue = clientTemplate.XmlConvertClassName + ".ToTimeSpan(\"" + defaultValue + "\")";
currentEnumValue = "(" + fullEnumTypeName + ")" + clientTemplate.EnumTypeName + ".Parse(" + clientTemplate.SystemTypeTypeName + ".GetType(\"" + enumTypeName + "\"), \"" + customizedEnumValue + "\")";
}
else if (valueClrType.Contains("Microsoft.Spatial"))
else
{
defaultValue = string.Format(CultureInfo.InvariantCulture, clientTemplate.GeoTypeInitializePattern, valueClrType, defaultValue);
currentEnumValue = clientTemplate.EnumTypeName + ".Parse(" + clientTemplate.SystemTypeTypeName + ".GetType(\"" + enumTypeName + "\"), \"" + currentEnumValue + "\")";
}
customizedEnumValues.Add(currentEnumValue);
}
if (isCSharpTemplate)
{
return string.Join(" | ", customizedEnumValues);
}
else
{
return string.Join(" Or ", customizedEnumValues);
}
}

return defaultValue;
if (valueClrType.Equals(clientTemplate.StringTypeName))
{
defaultValue = "\"" + defaultValue + "\"";
}
else if (valueClrType.Equals(clientTemplate.BooleanTypeName))
{
// EDMX specifies boolean defaults with capital letter, C# needs this string to be lower case.
if (isCSharpTemplate)
defaultValue = defaultValue.ToLower();
}
else if (valueClrType.Equals(clientTemplate.BinaryTypeName))
{
defaultValue = "System.Text.Encoding.UTF8.GetBytes(\"" + defaultValue + "\")";
}
else if (valueClrType.Equals(clientTemplate.SingleTypeName))
{
if (isCSharpTemplate)
{
defaultValue = defaultValue.EndsWith("f", StringComparison.OrdinalIgnoreCase) ? defaultValue : defaultValue + "f";
}
else
{
// doesn't have a default value
return null;
defaultValue = defaultValue.EndsWith("f", StringComparison.OrdinalIgnoreCase) ? defaultValue : defaultValue + "F";
}
}
else
else if (valueClrType.Equals(clientTemplate.DecimalTypeName))
{
// only structured property has default value
return null;
if (isCSharpTemplate)
{
// decimal in C# must be initialized with 'm' at the end, like Decimal dec = 3.00m
defaultValue = defaultValue.EndsWith("m", StringComparison.OrdinalIgnoreCase) ? defaultValue : defaultValue + "m";
}
else
{
// decimal in VB must be initialized with 'D' at the end, like Decimal dec = 3.00D
defaultValue = defaultValue.ToLower(CultureInfo.InvariantCulture).Replace("m", "D");
defaultValue = defaultValue.EndsWith("D", StringComparison.OrdinalIgnoreCase) ? defaultValue : defaultValue + "D";
}
}
else if (valueClrType.Equals(clientTemplate.GuidTypeName)
| valueClrType.Equals(clientTemplate.DateTimeOffsetTypeName)
| valueClrType.Equals(clientTemplate.DateTypeName)
| valueClrType.Equals(clientTemplate.TimeOfDayTypeName))
{
defaultValue = valueClrType + ".Parse(\"" + defaultValue + "\")";
}
else if (valueClrType.Equals(clientTemplate.DurationTypeName))
{
defaultValue = clientTemplate.XmlConvertClassName + ".ToTimeSpan(\"" + defaultValue + "\")";
}
else if (valueClrType.Contains("Microsoft.Spatial"))
{
defaultValue = string.Format(CultureInfo.InvariantCulture, clientTemplate.GeoTypeInitializePattern, valueClrType, defaultValue);
}

return defaultValue;
}
else
{
Expand Down Expand Up @@ -4121,6 +4132,18 @@ internal override HashSet<string> LanguageKeywords { get {
} }
private HashSet<string> CSharpKeywords;

internal override string StripNullableFromTypeName(string typeName)
{
const string nullablePrefix = "Nullable<";
int indexOfNullablePrefix = typeName.IndexOf(nullablePrefix);
if (indexOfNullablePrefix == -1)
{
return typeName;
}

return typeName.Substring(indexOfNullablePrefix + nullablePrefix.Length).TrimEnd('>');
}

internal override void WriteFileHeader()
{

Expand Down Expand Up @@ -6243,6 +6266,18 @@ internal override HashSet<string> LanguageKeywords { get {
} }
private HashSet<string> VBKeywords;

internal override string StripNullableFromTypeName(string typeName)
{
const string nullablePrefix = "Nullable(Of ";
int indexOfNullablePrefix = typeName.IndexOf(nullablePrefix);
if (indexOfNullablePrefix == -1)
{
return typeName;
}

return typeName.Substring(indexOfNullablePrefix + nullablePrefix.Length).TrimEnd(')');
}

internal override void WriteFileHeader()
{

Expand Down Expand Up @@ -8544,7 +8579,7 @@ internal VSManager(StringBuilder template)
/// <summary>
/// Base class for this transformation
/// </summary>
[global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.VisualStudio.TextTemplating", "16.0.0.0")]
[global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.VisualStudio.TextTemplating", "17.0.0.0")]
public class ODataT4CodeGeneratorBase
{
#region Fields
Expand Down
Loading