Skip to content
Merged

fix #51

Show file tree
Hide file tree
Changes from all commits
Commits
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
31 changes: 18 additions & 13 deletions src/FastCloner.SourceGenerator/CollectionHelperGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -177,13 +177,10 @@ private static void WriteCollectionCloneMethod(CloneGeneratorContext context, Me
else if (isStack) addMethod = "Push";
else if (isLinkedList) addMethod = "AddLast";

// Determine if capacity can be passed to constructor.
// Only standard List, HashSet, Queue, Stack support new T(int capacity).
// For CollectionKind.List, the source must also have .Count (e.g. IEnumerable<T> does not).
bool supportsCapacity = (kind == CollectionKind.List && member.CollectionHasCount) ||
kind == CollectionKind.HashSet ||
kind == CollectionKind.Queue ||
kind == CollectionKind.Stack;
// Capacity pre-allocation requires a VERIFIED .ctor(int capacity) on the concrete type
// (Collection<T>, BindingList<T> and many custom collections have none, issue #50)
// and a Count property on the source (e.g. IEnumerable<T> does not have one).
bool supportsCapacity = member.ConcreteHasCapacityCtor && member.CollectionHasCount;

if (needsState)
{
Expand All @@ -208,9 +205,11 @@ private static void WriteCollectionCloneMethod(CloneGeneratorContext context, Me
sb.AppendLine();
}

// Optimize: if element type is safe, use IEnumerable constructor directly (works for List, HashSet, etc.)
// Note: Queue/Stack/LinkedList also support IEnumerable ctor
if (isSafe && !needsState)
// Optimize: if element type is safe, use the copy constructor directly.
// Requires a ctor VERIFIED to copy (Collection<T>'s IList<T> ctor wraps the source instead)
// and is skipped for stacks: enumerating a stack yields top->bottom, so pushing that
// sequence into a new stack would reverse it.
if (isSafe && !needsState && member.ConcreteHasCopyCtor && !isStack)
{
sb.AppendLine($" return new {concreteType}(source);");
}
Expand Down Expand Up @@ -251,7 +250,9 @@ private static void WriteCollectionCloneMethod(CloneGeneratorContext context, Me
}
else if (kind == CollectionKind.List && member.CollectionHasIndexer)
{
if (context.TargetFramework >= TargetFramework.Net5)
// CollectionsMarshal.SetCount/AsSpan only accept List<T>; other indexable
// collections in the List bucket (Collection<T>, BindingList<T>, ...) use indexed Add.
if (context.TargetFramework >= TargetFramework.Net5 && member.ConcreteIsList)
{
sb.AppendLine(" global::System.Runtime.InteropServices.CollectionsMarshal.SetCount(result, source.Count);");
sb.AppendLine(" var span = global::System.Runtime.InteropServices.CollectionsMarshal.AsSpan(result);");
Expand Down Expand Up @@ -477,7 +478,9 @@ private static void WriteDictionaryCloneMethod(CloneGeneratorContext context, Me

StringBuilder sb = context.Source;
string concreteType = member.ConcreteTypeFullName ?? typeName;
bool supportsCapacity = kind is CollectionKind.Dictionary or CollectionKind.SortedList or CollectionKind.List or CollectionKind.None;
// Capacity pre-allocation requires a VERIFIED .ctor(int capacity) on the concrete type
// (a Dictionary<K,V> subclass doesn't inherit its base's constructors).
bool supportsCapacity = member.ConcreteHasCapacityCtor;
bool isConcurrent = kind == CollectionKind.ConcurrentDictionary;
bool keysAreSafe = member.KeyIsSafe;
bool valuesAreSafe = member.ValueIsSafe;
Expand All @@ -495,7 +498,9 @@ private static void WriteDictionaryCloneMethod(CloneGeneratorContext context, Me
sb.AppendLine(" if (source == null) return null;");
}

if (keysAreSafe && valuesAreSafe && !needsState && kind == CollectionKind.Dictionary)
// The copy-ctor fast path requires a ctor VERIFIED to copy (only the exact BCL Dictionary;
// subclasses don't inherit it and custom dictionary-taking ctors may wrap the source).
if (keysAreSafe && valuesAreSafe && !needsState && kind == CollectionKind.Dictionary && member.ConcreteHasCopyCtor)
{
sb.AppendLine($" return new {concreteType}(source);");
sb.AppendLine(" }");
Expand Down
6 changes: 4 additions & 2 deletions src/FastCloner.SourceGenerator/MemberCloneGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ public static string GetMemberAssignment(CloneGeneratorContext context, MemberMo
case MemberTypeKind.Other:
default:
context.NeedsClonerClass = true;
return $"{memberName} = Cloner<{member.TypeFullName}>.Clone({sourceVar}.{memberName}, {stateVar}){nf}";
// The null-forgiving argument is safe: Cloner<T>.Clone null-guards its input.
return $"{memberName} = Cloner<{member.TypeFullName}>.Clone({sourceVar}.{memberName}!, {stateVar}){nf}";
}
}
}
Expand Down Expand Up @@ -224,7 +225,8 @@ public static void WriteMemberCloning(CloneGeneratorContext context, MemberModel
case MemberTypeKind.Other:
default:
context.NeedsClonerClass = true;
sb.AppendLine($" {resultVar}.{memberName} = Cloner<{member.TypeFullName}>.Clone({sourceVar}.{memberName}, {stateVar}){nf};");
// The null-forgiving argument is safe: Cloner<T>.Clone null-guards its input.
sb.AppendLine($" {resultVar}.{memberName} = Cloner<{member.TypeFullName}>.Clone({sourceVar}.{memberName}!, {stateVar}){nf};");
break;
}

Expand Down
91 changes: 41 additions & 50 deletions src/FastCloner.SourceGenerator/MemberCollector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -199,63 +199,54 @@ private static Accessibility MoreAccessible(Accessibility a, Accessibility b)

private static bool IsPopulatableCollectionType(ITypeSymbol type)
{
switch (type)
{
case IArrayTypeSymbol:
return false;
case INamedTypeSymbol { IsGenericType: true } namedType:
{
string originalDef = namedType.OriginalDefinition.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat);

// List of populatable collection types (can use Clear + Add pattern)
string[] populatableTypes =
[
"global::System.Collections.Generic.List<T>",
"global::System.Collections.Generic.HashSet<T>",
"global::System.Collections.Generic.LinkedList<T>",
"global::System.Collections.Generic.Queue<T>",
"global::System.Collections.Generic.Stack<T>",
"global::System.Collections.Generic.SortedSet<T>",
"global::System.Collections.ObjectModel.Collection<T>",
"global::System.Collections.ObjectModel.ObservableCollection<T>",
"global::System.Collections.Concurrent.ConcurrentBag<T>",
"global::System.Collections.Concurrent.ConcurrentQueue<T>",
"global::System.Collections.Concurrent.ConcurrentStack<T>",
// Dictionaries
"global::System.Collections.Generic.Dictionary<TKey, TValue>",
"global::System.Collections.Generic.SortedDictionary<TKey, TValue>",
"global::System.Collections.Generic.SortedList<TKey, TValue>",
"global::System.Collections.Concurrent.ConcurrentDictionary<TKey, TValue>"
];

foreach (string populatable in populatableTypes)
{
if (originalDef == populatable)
return true;
}
// Getter-only collection members are cloned in place via Clear + Add / indexer writes.
// Verify that API surface on the type instead of consulting a whitelist, so BindingList<T>,
// collection subclasses and custom collections work as well (issue #50).
if (type is IArrayTypeSymbol)
return false;

break;
}
}
// A getter returns a copy of a struct: in-place population would only mutate the copy.
if (type.IsValueType)
return false;

// Also check if the type implements ICollection<T> and has Add method
// This covers custom collection types
foreach (INamedTypeSymbol? iface in type.AllInterfaces)
if (type.TypeKind == TypeKind.Interface)
{
if (iface.IsGenericType)
// Mutable through the interface contract itself: ICollection<T> brings Clear + Add,
// IDictionary<K,V> brings Clear + a settable indexer. Read-only interfaces bring neither.
static bool IsMutableCollectionInterface(INamedTypeSymbol iface) =>
iface.OriginalDefinition.SpecialType == SpecialType.System_Collections_Generic_ICollection_T ||
(iface.MetadataName == "IDictionary`2" && iface.ContainingNamespace.ToDisplayString() == "System.Collections.Generic");

if (type is INamedTypeSymbol named && IsMutableCollectionInterface(named))
return true;

foreach (INamedTypeSymbol iface in type.AllInterfaces)
{
string ifaceDef = iface.OriginalDefinition.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat);
if (ifaceDef == "global::System.Collections.Generic.ICollection<T>")
{
// Has ICollection<T>, check if it's not read-only
// ICollection<T>.IsReadOnly would need runtime check, so we just allow it
// The generated code will handle runtime failures gracefully
if (IsMutableCollectionInterface(iface))
return true;
}
}

return false;
}

return false;

bool isDictionary = TypeAnalyzer.IsDictionaryType(type);
if (!isDictionary && !TypeAnalyzer.IsCollectionType(type))
return false;

CollectionKind kind = TypeAnalyzer.GetCollectionKind(type);

// Read-only wrappers and immutable collections cannot be populated in place (the immutable
// types' public Add/Clear compile but return new instances, silently doing nothing).
if (kind is CollectionKind.ReadOnlyCollection or CollectionKind.ReadOnlyDictionary ||
kind.ToString().StartsWith("Immutable"))
return false;

if (!TypeAnalyzer.HasPublicInstanceMethod(type, "Clear", 0))
return false;

return isDictionary
? TypeAnalyzer.HasPublicSettableIndexer(type)
: TypeAnalyzer.HasPublicInstanceMethod(type, TypeAnalyzer.GetAddMethodName(kind), 1);
}

/// <summary>
Expand Down
Loading
Loading