Wrong initialization is generated for ReadOnlyCollection<> properties
iam-mholle opened this issue · 2 comments
When generating a builder for a class containing a ReadOnlyCollection<>
property, the generated code initializes the property with a new List<>()
.
Consider the following class:
[AutoGenerateBuilder]
public partial class SomeDTO
{
public ReadOnlyCollection<string> Strings { get; set; }
}
The following builder is being generated, where the _strings
initialization and the WithStrings(() => new List<string>())
call cause the compilation to fail:
public partial class SomeDTOBuilder : Builder<Library.SomeDTO>
{
private bool _stringsIsSet;
private Lazy<System.Collections.ObjectModel.ReadOnlyCollection<string>> _strings = new Lazy<System.Collections.ObjectModel.ReadOnlyCollection<string>>(() => new List<string>());
public SomeDTOBuilder WithStrings(System.Collections.ObjectModel.ReadOnlyCollection<string> value) => WithStrings(() => value);
public SomeDTOBuilder WithStrings(Func<System.Collections.ObjectModel.ReadOnlyCollection<string>> func)
{
_strings = new Lazy<System.Collections.ObjectModel.ReadOnlyCollection<string>>(func);
_stringsIsSet = true;
return this;
}
public SomeDTOBuilder WithStrings(Action<ClassLibrary1.FluentBuilder.IListBuilder<string>> action, bool useObjectInitializer = true) => WithStrings(() =>
{
var builder = new ClassLibrary1.FluentBuilder.IListBuilder<string>();
action(builder);
return (System.Collections.ObjectModel.ReadOnlyCollection<string>) builder.Build(useObjectInitializer);
});
public SomeDTOBuilder WithoutStrings()
{
WithStrings(() => new List<string>());
_stringsIsSet = false;
return this;
}
public override SomeDTO Build(bool useObjectInitializer = true) { /*...*/ }
public static SomeDTO Default() => new SomeDTO();
}
Looking at the FluentBuilder source, the issue seems to be that FluentBuilderGenerator.Extensions.TypeSymbolExtensions.GetDefault
calls FluentBuilderGenerator.Extensions.TypeSymbolExtensions.GetFluentTypeKind
to check for an adequate default initializer, but GetFluentTypeKind
returns interface types found for implementing types, meaning that ReadOnlyCollection : System.Collections.Generic.IList<T> /*, ...*/
causes FluentTypeKind.IList
to be returned from GetFluentTypeKind
which then leads to new List<>()
to be chosen as default initializer.
I think the 4 interface implementation checks in GetFluentTypeKind
should only happen if typeSymbol.TypeKind == TypeKind.Interface
like so:
public static FluentTypeKind GetFluentTypeKind(this ITypeSymbol typeSymbol)
{
if (typeSymbol.SpecialType == SpecialType.System_String)
{
return FluentTypeKind.String;
}
if (typeSymbol.TypeKind == TypeKind.Array)
{
return FluentTypeKind.Array;
}
if (typeSymbol.TypeKind == TypeKind.Interface)
{
if (typeSymbol.ImplementsInterfaceOrBaseClass(typeof(IDictionary<,>)) || typeSymbol.ImplementsInterfaceOrBaseClass(typeof(IDictionary)))
{
return FluentTypeKind.IDictionary;
}
if (typeSymbol.ImplementsInterfaceOrBaseClass(typeof(IList<>)) || typeSymbol.ImplementsInterfaceOrBaseClass(typeof(IList)))
{
return FluentTypeKind.IList;
}
if (typeSymbol.ImplementsInterfaceOrBaseClass(typeof(ICollection<>)) || typeSymbol.ImplementsInterfaceOrBaseClass(typeof(ICollection)))
{
return FluentTypeKind.ICollection;
}
if (typeSymbol.AllInterfaces.Any(i => i.SpecialType == SpecialType.System_Collections_IEnumerable))
{
return FluentTypeKind.IEnumerable;
}
}
return FluentTypeKind.Other;
}
@iam-mholle
This is solved in next release.