Fix 2153 TypeConverterFactory not used

.gitignore

@@ -5,7 +5,7 @@ obj/
 artifacts/
 .tmp/
 cache/
-
+.idea/
 *.user
 *.psess
 *.log

src/CsvHelper/TypeConversion/NotSupportedTypeConverter.cs

@@ -0,0 +1,52 @@
+// Copyright 2009-2024 Josh Close
+// This file is a part of CsvHelper and is dual licensed under MS-PL and Apache 2.0.
+// See LICENSE.txt for details or visit http://www.opensource.org/licenses/ms-pl.html for MS-PL and http://opensource.org/licenses/Apache-2.0 for Apache 2.0.
+// https://github.com/JoshClose/CsvHelper
+using CsvHelper.Configuration;
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace CsvHelper.TypeConversion
+{
+	/// <summary>
+	/// Throws an exception when used. This is here so that it's apparent
+	/// that there is no support for <see cref="Type"/> type conversion. A custom
+	/// converter will need to be created to have a field convert to and
+	/// from <see cref="Type"/>.
+	/// </summary>
+	public class NotSupportedTypeConverter<T> : TypeConverter<T>
+	{
+		/// <summary>
+		/// Throws an exception.
+		/// </summary>
+		/// <param name="text">The string to convert to an object.</param>
+		/// <param name="row">The <see cref="IReaderRow"/> for the current record.</param>
+		/// <param name="memberMapData">The <see cref="MemberMapData"/> for the member being created.</param>
+		/// <returns>The object created from the string.</returns>
+		public override T ConvertFromString(string text, IReaderRow row, MemberMapData memberMapData)
+		{
+			var message = $"Converting " + typeof(T).FullName + " is not supported. " +
+						  "If you want to do this, create your own ITypeConverter and register " +
+						  "it in the TypeConverterFactory by calling AddConverter.";
+			throw new TypeConverterException(this, memberMapData, text ?? string.Empty, row.Context, message);
+		}
+
+		/// <summary>
+		/// Throws an exception.
+		/// </summary>
+		/// <param name="value">The object to convert to a string.</param>
+		/// <param name="row">The <see cref="IWriterRow"/> for the current record.</param>
+		/// <param name="memberMapData">The <see cref="MemberMapData"/> for the member being written.</param>
+		/// <returns>The string representation of the object.</returns>
+		public override string ConvertToString(T value, IWriterRow row, MemberMapData memberMapData)
+		{
+			var message = "Converting " + typeof(T).FullName + " is not supported. " +
+						  "If you want to do this, create your own ITypeConverter and register " +
+						  "it in the TypeConverterFactory by calling AddConverter.";
+			throw new TypeConverterException(this, memberMapData, value, row.Context, message);
+		}
+	}
+}

src/CsvHelper/TypeConversion/TypeConverter.cs

@@ -1,52 +1,36 @@
-// Copyright 2009-2024 Josh Close
-// This file is a part of CsvHelper and is dual licensed under MS-PL and Apache 2.0.
-// See LICENSE.txt for details or visit http://www.opensource.org/licenses/ms-pl.html for MS-PL and http://opensource.org/licenses/Apache-2.0 for Apache 2.0.
-// https://github.com/JoshClose/CsvHelper
 using CsvHelper.Configuration;
-using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Text;
-using System.Threading.Tasks;
 
 namespace CsvHelper.TypeConversion
 {
 	/// <summary>
-	/// Throws an exception when used. This is here so that it's apparent
-	/// that there is no support for <see cref="Type"/> type conversion. A custom
-	/// converter will need to be created to have a field convert to and
-	/// from <see cref="Type"/>.
+	/// Converts values to and from strings.
 	/// </summary>
-	public class TypeConverter : DefaultTypeConverter
+	public abstract class TypeConverter<T> : ITypeConverter
 	{
 		/// <summary>
-		/// Throws an exception.
+		/// Converts the string to a (T) value.
 		/// </summary>
 		/// <param name="text">The string to convert to an object.</param>
 		/// <param name="row">The <see cref="IReaderRow"/> for the current record.</param>
 		/// <param name="memberMapData">The <see cref="MemberMapData"/> for the member being created.</param>
-		/// <returns>The object created from the string.</returns>
-		public override object ConvertFromString(string text, IReaderRow row, MemberMapData memberMapData)
-		{
-			var message = "Converting System.Type is not supported. " +
-						  "If you want to do this, create your own ITypeConverter and register " +
-						  "it in the TypeConverterFactory by calling AddConverter.";
-			throw new TypeConverterException(this, memberMapData, text ?? string.Empty, row.Context, message);
-		}
+		/// <returns>The value created from the string.</returns>
+		public abstract T ConvertFromString(string text, IReaderRow row, MemberMapData memberMapData);
 
 		/// <summary>
-		/// Throws an exception.
+		/// Converts the value to a string.
 		/// </summary>
-		/// <param name="value">The object to convert to a string.</param>
+		/// <param name="value">The value to convert to a string.</param>
 		/// <param name="row">The <see cref="IWriterRow"/> for the current record.</param>
 		/// <param name="memberMapData">The <see cref="MemberMapData"/> for the member being written.</param>
-		/// <returns>The string representation of the object.</returns>
-		public override string ConvertToString(object value, IWriterRow row, MemberMapData memberMapData)
-		{
-			var message = "Converting System.Type is not supported. " +
-						  "If you want to do this, create your own ITypeConverter and register " +
-						  "it in the TypeConverterFactory by calling AddConverter.";
-			throw new TypeConverterException(this, memberMapData, value, row.Context, message);
-		}
+		/// <returns>The string representation of the value.</returns>
+		public abstract string ConvertToString(T value, IWriterRow row, MemberMapData memberMapData);
+
+		object ITypeConverter.ConvertFromString(string text, IReaderRow row, MemberMapData memberMapData) =>
+			ConvertFromString(text, row, memberMapData);
+
+		string ITypeConverter.ConvertToString(object value, IWriterRow row, MemberMapData memberMapData) =>
+			value is T v
+				? ConvertToString(v, row, memberMapData)
+				: throw new System.InvalidCastException();
 	}
 }

src/CsvHelper/TypeConversion/TypeConverterCache.cs

@@ -19,6 +19,7 @@ namespace CsvHelper.TypeConversion
 	public class TypeConverterCache
 	{
 		private readonly Dictionary<Type, ITypeConverter> typeConverters = new Dictionary<Type, ITypeConverter>();
+		private readonly List<ITypeConverterFactory> defaultTypeConverterFactories = new List<ITypeConverterFactory>();
 		private readonly List<ITypeConverterFactory> typeConverterFactories = new List<ITypeConverterFactory>();
 		private readonly Dictionary<Type, ITypeConverterFactory> typeConverterFactoryCache = new Dictionary<Type, ITypeConverterFactory>();
 
@@ -75,6 +76,15 @@ public void AddConverter(Type type, ITypeConverter typeConverter)
 			typeConverters[type] = typeConverter;
 		}
 
+		/// <summary>
+		/// Adds the <see cref="TypeConverter{T}"/> for the given <see cref="System.Type"/>.
+		/// </summary>
+		/// <typeparam name="T">The type the converter converts.</typeparam>
+		/// <param name="typeConverter">The type converter that converts the type.</param>
+		public void AddConverter<T>(TypeConverter<T> typeConverter) =>
+			AddConverter<T>(typeConverter as ITypeConverter);
+
+
 		/// <summary>
 		/// Adds the <see cref="ITypeConverter"/> for the given <see cref="System.Type"/>.
 		/// </summary>
@@ -158,7 +168,7 @@ public ITypeConverter GetConverter(Type type)
 
 			if (!typeConverterFactoryCache.TryGetValue(type, out var factory))
 			{
-				factory = typeConverterFactories.FirstOrDefault(f => f.CanCreate(type));
+				factory = typeConverterFactories.Concat(defaultTypeConverterFactories).FirstOrDefault(f => f.CanCreate(type));
 				if (factory != null)
 				{
 					typeConverterFactoryCache[type] = factory;
@@ -224,7 +234,7 @@ private void CreateDefaultConverters()
 			AddConverter(typeof(sbyte), new SByteConverter());
 			AddConverter(typeof(string), new StringConverter());
 			AddConverter(typeof(TimeSpan), new TimeSpanConverter());
-			AddConverter(typeof(Type), new TypeConverter());
+			AddConverter(new NotSupportedTypeConverter<Type>());
 			AddConverter(typeof(ushort), new UInt16Converter());
 			AddConverter(typeof(uint), new UInt32Converter());
 			AddConverter(typeof(ulong), new UInt64Converter());
@@ -234,9 +244,9 @@ private void CreateDefaultConverters()
 			AddConverter(typeof(TimeOnly), new TimeOnlyConverter());
 #endif
 
-			AddConverterFactory(new EnumConverterFactory());
-			AddConverterFactory(new NullableConverterFactory());
-			AddConverterFactory(new CollectionConverterFactory());
+			defaultTypeConverterFactories.Add(new EnumConverterFactory());
+			defaultTypeConverterFactories.Add(new NullableConverterFactory());
+			defaultTypeConverterFactories.Add(new CollectionConverterFactory());
 		}
 	}
 }

tests/CsvHelper.Tests/TypeConversion/TypeConverterFactoryTests.cs

@@ -0,0 +1,141 @@
+using System;
+using System.Collections;
+using System.Collections.Generic;
+using System.Globalization;
+using System.IO;
+using System.Linq;
+using CsvHelper.Configuration;
+using CsvHelper.TypeConversion;
+using Xunit;
+
+namespace CsvHelper.Tests.TypeConversion
+{
+	public class TypeConverterFactoryTests
+	{
+		public readonly record struct Option<T> : IEnumerable<T>
+		{
+			public bool IsPresent { get; }
+			private readonly T _value;
+
+			internal Option(T value)
+			{
+				IsPresent = true;
+				_value = value;
+			}
+
+			public IEnumerator<T> GetEnumerator()
+			{
+				if (IsPresent) yield return _value;
+			}
+
+			IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
+		}
+
+		private class MyOptionTypeFactory : ITypeConverterFactory
+		{
+			public bool CanCreate(Type type) =>
+				type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Option<>);
+
+			public bool Create(Type type, TypeConverterCache cache, out ITypeConverter typeConverter)
+			{
+				var wrappedType = type.GetGenericArguments().Single();
+				typeConverter =
+					Activator.CreateInstance(typeof(OptionConverter<>).MakeGenericType(wrappedType)) as
+						ITypeConverter ?? throw new NullReferenceException();
+				return true;
+			}
+
+			internal class OptionConverter<T> : TypeConverter<Option<T>>
+			{
+				public override string ConvertToString(Option<T> value, IWriterRow row,
+					MemberMapData memberMapData)
+				{
+					var wrappedTypeConverter = row.Context.TypeConverterCache.GetConverter<T>();
+					return value.IsPresent
+						? wrappedTypeConverter.ConvertToString(value.Single(), row, memberMapData)
+						: "";
+				}
+
+				public override Option<T> ConvertFromString(string text, IReaderRow row,
+					MemberMapData memberMapData)
+				{
+					var wrappedTypeConverter = row.Context.TypeConverterCache.GetConverter<T>();
+
+					return text == ""
+						? new Option<T>()
+						: new Option<T>((T)wrappedTypeConverter.ConvertFromString(text, row, memberMapData));
+				}
+			}
+		}
+
+		private record RecordWithGenerics(Option<int> MaybeNumber);
+
+		[Fact]
+		void ReadTypeConverterGenericInt()
+		{
+			var input = """
+			            MaybeNumber
+			            23
+
+			            """;
+
+			using var cr = new CsvReader(new StringReader(input), CultureInfo.InvariantCulture);
+			cr.Context.TypeConverterCache.AddConverter(new MyOptionTypeFactory.OptionConverter<int>());
+			var firstRow = cr.GetRecords<RecordWithGenerics>().First();
+			Assert.Equal(new Option<int>(23), firstRow.MaybeNumber);
+		}
+
+		[Fact]
+		void WriteTypeConverterGenericInt()
+		{
+			var expected = """
+			               MaybeNumber
+			               42
+
+			               """;
+
+			var stringWriter = new StringWriter();
+			using var cw = new CsvWriter(stringWriter, CultureInfo.InvariantCulture);
+			cw.Context.TypeConverterCache.AddConverter(new MyOptionTypeFactory.OptionConverter<int>());
+			cw.WriteRecords(new[]
+			{
+				new RecordWithGenerics(new Option<int>(42))
+			});
+			Assert.Equal(expected, stringWriter.ToString());
+		}
+
+		[Fact]
+		void ReadTypeConverterFactory()
+		{
+			var input = """
+			            MaybeNumber
+			            23
+
+			            """;
+
+			using var cr = new CsvReader(new StringReader(input), CultureInfo.InvariantCulture);
+			cr.Context.TypeConverterCache.AddConverterFactory(new MyOptionTypeFactory());
+			var firstRow = cr.GetRecords<RecordWithGenerics>().First();
+			Assert.Equal(new Option<int>(23), firstRow.MaybeNumber);
+		}
+
+		[Fact]
+		void WriteTypeConverterFactory()
+		{
+			var expected = """
+			               MaybeNumber
+			               42
+
+			               """;
+
+			var stringWriter = new StringWriter();
+			using var cw = new CsvWriter(stringWriter, CultureInfo.InvariantCulture);
+			cw.Context.TypeConverterCache.AddConverterFactory(new MyOptionTypeFactory());
+			cw.WriteRecords(new[]
+			{
+				new RecordWithGenerics(new Option<int>(42))
+			});
+			Assert.Equal(expected, stringWriter.ToString());
+		}
+	}
+}