Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
86 changes: 86 additions & 0 deletions src/FastCloner.Tests/CollectionTests.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,41 @@
using System.Collections;
using System.Collections.Concurrent;
using System.Threading.Tasks;

namespace FastCloner.Tests;

/// <summary>
/// A non-generic class that implements ISet&lt;string&gt; directly.
/// Exercises the code path where type.GetGenericArguments() returns an empty array
/// but IsSetType() matches via the interface.
/// </summary>
public class StringSet : ISet<string>
{
private readonly HashSet<string> _inner = new();

public int Count => _inner.Count;
public bool IsReadOnly => false;

public bool Add(string item) => _inner.Add(item);
public void Clear() => _inner.Clear();
public bool Contains(string item) => _inner.Contains(item);
public void CopyTo(string[] array, int arrayIndex) => _inner.CopyTo(array, arrayIndex);
public void ExceptWith(IEnumerable<string> other) => _inner.ExceptWith(other);
public IEnumerator<string> GetEnumerator() => _inner.GetEnumerator();
public void IntersectWith(IEnumerable<string> other) => _inner.IntersectWith(other);
public bool IsProperSubsetOf(IEnumerable<string> other) => _inner.IsProperSubsetOf(other);
public bool IsProperSupersetOf(IEnumerable<string> other) => _inner.IsProperSupersetOf(other);
public bool IsSubsetOf(IEnumerable<string> other) => _inner.IsSubsetOf(other);
public bool IsSupersetOf(IEnumerable<string> other) => _inner.IsSupersetOf(other);
public bool Overlaps(IEnumerable<string> other) => _inner.Overlaps(other);
public bool Remove(string item) => _inner.Remove(item);
public bool SetEquals(IEnumerable<string> other) => _inner.SetEquals(other);
public void SymmetricExceptWith(IEnumerable<string> other) => _inner.SymmetricExceptWith(other);
public void UnionWith(IEnumerable<string> other) => _inner.UnionWith(other);
void ICollection<string>.Add(string item) => _inner.Add(item);
IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}

public class CollectionTests
{
[Test]
Expand Down Expand Up @@ -176,4 +210,56 @@ public async Task LinkedList_Should_Be_Deep_Cloned_Correctly()
// Original should remain untouched
await Assert.That(original.Count).IsEqualTo(3);
}

[Test]
public async Task NonGenericSetImplementingISet_Should_Be_Deep_Cloned_Correctly()
{
StringSet original = new StringSet();
original.Add("alpha");
original.Add("beta");
original.Add("gamma");

StringSet clone = original.DeepClone();

await Assert.That(clone).IsNotSameReferenceAs(original);
await Assert.That(clone.Count).IsEqualTo(3);
await Assert.That(clone.Contains("alpha")).IsTrue();
await Assert.That(clone.Contains("beta")).IsTrue();
await Assert.That(clone.Contains("gamma")).IsTrue();

// Mutating clone should not affect original
clone.Add("delta");
await Assert.That(clone.Count).IsEqualTo(4);
await Assert.That(original.Count).IsEqualTo(3);
}

[Test]
public async Task NonGenericSetInsideObject_Should_Be_Deep_Cloned_Correctly()
{
ObjectWithNonGenericSet original = new ObjectWithNonGenericSet
{
Name = "test",
Tags = new StringSet()
};
original.Tags.Add("a");
original.Tags.Add("b");

ObjectWithNonGenericSet clone = original.DeepClone();

await Assert.That(clone).IsNotSameReferenceAs(original);
await Assert.That(clone.Tags).IsNotSameReferenceAs(original.Tags);
await Assert.That(clone.Tags.Count).IsEqualTo(2);
await Assert.That(clone.Tags.Contains("a")).IsTrue();
await Assert.That(clone.Tags.Contains("b")).IsTrue();

clone.Tags.Add("c");
await Assert.That(clone.Tags.Count).IsEqualTo(3);
await Assert.That(original.Tags.Count).IsEqualTo(2);
}
}

public class ObjectWithNonGenericSet
{
public string Name { get; set; } = "";
public StringSet Tags { get; set; } = new();
}
14 changes: 12 additions & 2 deletions src/FastCloner/Code/FastClonerExprGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,10 @@ private static bool ShouldDeepCloneStructReadonlyFields(Type type)

internal static object? GenerateProcessMethod(Type realType, bool asObject) => GenerateProcessMethod(realType, asObject && realType.IsValueType(), new ExpressionPosition(0, 0));
public static bool IsListType(Type type) => type.IsGenericType && type.GetGenericTypeDefinition() == typeof(List<>);
public static bool IsSetType(Type type) => type.GetInterfaces().Any(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(ISet<>));
public static bool IsSetType(Type type) => GetSetInterface(type) is not null;

private static Type? GetSetInterface(Type type) =>
type.GetInterfaces().FirstOrDefault(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(ISet<>));

public static bool IsConcurrentBagOrQueue(Type type)
{
Expand Down Expand Up @@ -1862,7 +1865,14 @@ private static object GenerateProcessSetMethod(Type type, ExpressionPosition pos
return Expression.Lambda<Func<object, FastCloneState, object>>(pFrom, pFrom, pState).Compile();
}

Type elementType = type.GenericArguments()[0];
Type? setInterface = GetSetInterface(type);
if (setInterface is null)
return GenerateMemberwiseCloner(type, position);
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

GenerateProcessMethod already branches into GenerateProcessSetMethod only when IsSetType(type) is true, and IsSetType uses GetSetInterface(type). GenerateProcessSetMethod then calls GetSetInterface(type) again and has a setInterface is null fallback that should be unreachable. Consider computing the set interface once (e.g., return it from the predicate or pass it into GenerateProcessSetMethod) to avoid the extra GetInterfaces() scan and dead branch.

Suggested change
Type? setInterface = GetSetInterface(type);
if (setInterface is null)
return GenerateMemberwiseCloner(type, position);
Type setInterface = GetSetInterface(type)!;

Copilot uses AI. Check for mistakes.

Type[] genericArguments = type.GenericArguments();
Type elementType = genericArguments.Length > 0
? genericArguments[0]
: setInterface.GetGenericArguments()[0];
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

elementType is chosen from type.GenericArguments()[0] when the concrete type is generic. That can be incorrect if the set element type comes from the ISet<T> interface but isn’t the first generic parameter of the concrete type (e.g., a MySet<TKey, TValue> : ISet<TValue>). Since setInterface is already computed, derive elementType from setInterface.GetGenericArguments()[0] (or otherwise map the interface’s T) to ensure correctness for all implementations.

Suggested change
Type[] genericArguments = type.GenericArguments();
Type elementType = genericArguments.Length > 0
? genericArguments[0]
: setInterface.GetGenericArguments()[0];
Type elementType = setInterface.GetGenericArguments()[0];

Copilot uses AI. Check for mistakes.

// Fast path check first - avoid creating expressions if we don't need them
bool isImmutable = IsImmutableCollection(type);
Expand Down
Loading