Default value not used when `useOnConversionFailure: true` on nullable properties

[YaroslavKormushyn]

Describe the bug The default value from the class map is not used on conversion failure for nullable properties.

To Reproduce

File:

id,name
73eb2807-9e02-41d7-a140-40411b822,itemName

Class Map:

internal class CustomType {
    public InnerCustomType Item {get;set;}

    [Name("itemName")]
    public string ItemName {get;set;}

    public class InnerCustomType {
        [Name("id")]
        public Guid? Id {get;set;}
    }
}

internal class CustomTypeRequestMap : ClassMap<CustomType>
{
    public CdeAssetImportRequestMap(string[] schemaKeys)
    {
        this.AutoMap(CultureInfo.InvariantCulture);
        this.References<InnerCustomTypeMap>(m => m.Item);
    }

    internal class InnerCustomTypeMap : ClassMap<InnerCustomType>
    {
        public TwinImportDataMap()
        {
            this.Map(m => m.Id).Name("id").Default(null as Guid?, useOnConversionFailure: true);
        }
    }
}

csvReader.GetRecords<CustomType>().ToList();

Expected behavior The default value is used for the invalid id input.

Additional context Seems it's not entering the ConvertFromString in the DefaultTypeConverter at all, where this useOnConversionFailure will be used. So it's throwing an error like usual.

[YaroslavKormushyn]

Just checked, the same applies to non-nullable Guid fields. Seems like the converter will use the useOnConversionFailure from the DefaultTypeConverter only if the input is null. However, the name of the useOnConversionFailure flag suggests it should be global, so that any invalid value results in the default value being applied. https://github.com/JoshClose/CsvHelper/blob/7b3ed4d45af8385e732a42eb161b0d129736edb3/src/CsvHelper/TypeConversion/GuidConverter.cs#L22-L30

[Rob-Hague]

Just checked, the same applies to non-nullable Guid fields.

Indeed, the fix is probably:

diff --git a/src/CsvHelper/TypeConversion/GuidConverter.cs b/src/CsvHelper/TypeConversion/GuidConverter.cs
index be70163c..e0ba7915 100644
--- a/src/CsvHelper/TypeConversion/GuidConverter.cs
+++ b/src/CsvHelper/TypeConversion/GuidConverter.cs
@@ -21,12 +21,9 @@ namespace CsvHelper.TypeConversion
                /// <returns>The object created from the string.</returns>
                public override object? ConvertFromString(string? text, IReaderRow row, MemberMapData memberMapData)
                {
-                       if (text == null)
-                       {
-                               return base.ConvertFromString(text, row, memberMapData);
-                       }
-
-                       return new Guid(text);
+                       return Guid.TryParse(text, out Guid guid)
+                               ? guid
+                               : base.ConvertFromString(text, row, memberMapData);
                }
        }
 }

[twinee]

I have noticed this issue too, the issue comes from this line: if (memberMapData.UseDefaultOnConversionFailure && memberMapData.IsDefaultSet && memberMapData.Member!.MemberType() == memberMapData.Default?.GetType())

memberMapData.Member!.MemberType() will return a Nullable`1 type while memberMapData.Default?.GetType() will return null so the equality comparison fails.

it should be something like this:

if (memberMapData.UseDefaultOnConversionFailure && memberMapData.IsDefaultSet)
{
    var memberType = memberMapData.Member!.MemberType()

    if (memberMapData.Default == null && (!memberType.IsValueType || Nullable.GetUnderlyingType(memberType) != null))
        return memberMapData.Default;

    if (memberType  == memberMapData.Default.GetType())
        return memberMapData.Default;
}

[twinee]

I have noticed this issue too, the problem arises from the type comparison in this line: if (memberMapData.UseDefaultOnConversionFailure && memberMapData.IsDefaultSet && memberMapData.Member!.MemberType() == memberMapData.Default?.GetType())

memberMapData.Member!.MemberType() will return a Nullable`1 type while memberMapData.Default?.GetType() will return null so the equality comparison fails.

it should be something like this:

if (memberMapData.UseDefaultOnConversionFailure && memberMapData.IsDefaultSet)
{
    var memberType = memberMapData.Member!.MemberType();

    if (memberMapData.Default == null && (!memberType.IsValueType || Nullable.GetUnderlyingType(memberType) != null))
        return memberMapData.Default;

    if (memberType == memberMapData.Default.GetType())
        return memberMapData.Default;
}