Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: map existing target sets correctly #500

Merged
merged 1 commit into from
Jun 14, 2023
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,16 @@ public static class EnumerableMappingBuilder
return CreateForEach(nameof(Queue<object>.Enqueue));

// create a foreach loop with add calls if source is not an array
// and ICollection.Add(T): void is implemented and not explicit
// and void ICollection.Add(T) or bool ISet.Add(T) is implemented and not explicit
// ensures add is not called and immutable types
if (!ctx.Target.IsArrayType() && ctx.Target.HasImplicitGenericImplementation(ctx.Types.Get(typeof(ICollection<>)), AddMethodName))
// ISet.Add(T) is explicitly needed as sets implement the ICollection.Add(T) explicit,
// and override the add method with new
var hasImplicitCollectionAdd = ctx.Target.HasImplicitGenericImplementation(ctx.Types.Get(typeof(ICollection<>)), AddMethodName);
var hasImplicitSetAdd = ctx.Target.HasImplicitGenericImplementation(ctx.Types.Get(typeof(ISet<>)), AddMethodName);
if (!ctx.Target.IsArrayType() && (hasImplicitCollectionAdd || hasImplicitSetAdd))
{
return CreateForEach(AddMethodName);
}

// if a mapping could be created for an immutable collection
// we diagnostic when it is an existing target mapping
Expand Down Expand Up @@ -167,22 +173,22 @@ private static LinqConstructorMapping BuildLinqConstructorMapping(MappingBuilder
}

// create a foreach loop with add calls if source is not an array
// and ICollection.Add(T): void is implemented and not explicit
// ensures add is not called and immutable types
if (!ctx.Target.IsArrayType() && ctx.Target.HasImplicitGenericImplementation(ctx.Types.Get(typeof(ICollection<>)), AddMethodName))
{
var ensureCapacityStatement = EnsureCapacityBuilder.TryBuildEnsureCapacity(ctx.Source, ctx.Target, ctx.Types);
return new ForEachAddEnumerableMapping(
ctx.Source,
ctx.Target,
elementMapping,
objectFactory,
AddMethodName,
ensureCapacityStatement
);
}
// and void ICollection.Add(T) or bool ISet.Add(T) is implemented and not explicit
// ensures .Add() is not called on immutable types
var hasImplicitCollectionAdd = ctx.Target.HasImplicitGenericImplementation(ctx.Types.Get(typeof(ICollection<>)), AddMethodName);
var hasImplicitSetAdd = ctx.Target.HasImplicitGenericImplementation(ctx.Types.Get(typeof(ISet<>)), AddMethodName);
CommonGuy marked this conversation as resolved.
Show resolved Hide resolved
if (ctx.Target.IsArrayType() || (!hasImplicitCollectionAdd && !hasImplicitSetAdd))
return null;

return null;
var ensureCapacityStatement = EnsureCapacityBuilder.TryBuildEnsureCapacity(ctx.Source, ctx.Target, ctx.Types);
return new ForEachAddEnumerableMapping(
ctx.Source,
ctx.Target,
elementMapping,
objectFactory,
AddMethodName,
ensureCapacityStatement
);
}

private static (bool CanMapWithLinq, string? CollectMethod) ResolveCollectMethodName(
Expand Down
15 changes: 9 additions & 6 deletions src/Riok.Mapperly/Helpers/SymbolExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -153,17 +153,20 @@ out bool isExplicit
}

var interfaceSymbol = typedInterface.GetMembers(symbolName).First();

var symbolImplementaton = t.FindImplementationForInterfaceMember(interfaceSymbol);
var symbolImplementation = t.FindImplementationForInterfaceMember(interfaceSymbol);

// if null then the method is unimplemented
// symbol implements genericInterface but has not implemented the corresponding methods
// this can only occur in unit tests
if (symbolImplementaton == null)
throw new NotSupportedException("Symbol implementation cannot be null for objects implementing interface.");
// this is for example the case for arrays (arrays implement several interfaces at runtime)
// and unit tests for which not the full interface is implemented
if (symbolImplementation == null)
{
isExplicit = false;
return false;
}

// check if symbol is explicit
isExplicit = symbolImplementaton switch
isExplicit = symbolImplementation switch
{
IMethodSymbol methodSymbol => methodSymbol.ExplicitInterfaceImplementations.Any(),
IPropertySymbol propertySymbol => propertySymbol.ExplicitInterfaceImplementations.Any(),
Expand Down
59 changes: 59 additions & 0 deletions test/Riok.Mapperly.Tests/Mapping/EnumerableSetTest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
namespace Riok.Mapperly.Tests.Mapping;

public class EnumerableSetTest
{
[Fact]
public void ExistingEnumerableToExistingSet()
{
var source = TestSourceBuilder.Mapping(
"A",
"B",
"class A { public IEnumerable<int> Values { get; } }",
"class B { public ISet<int> Values { get; } }"
);
TestHelper
.GenerateMapper(source)
.Should()
.HaveSingleMethodBody(
"""
var target = new global::B();
foreach (var item in source.Values)
{
target.Values.Add(item);
}

return target;
"""
);
}

[Fact]
public void ExistingEnumerableToExistingHashSet()
{
var source = TestSourceBuilder.Mapping(
"A",
"B",
"class A { public IEnumerable<int> Values { get; } }",
"class B { public HashSet<int> Values { get; } }"
);
TestHelper
.GenerateMapper(source)
.Should()
.HaveSingleMethodBody(
"""
var target = new global::B();
if (global::System.Linq.Enumerable.TryGetNonEnumeratedCount(source.Values, out var sourceCount))
{
target.Values.EnsureCapacity(sourceCount + target.Values.Count);
}

foreach (var item in source.Values)
{
target.Values.Add(item);
}

return target;
"""
);
}
}
6 changes: 3 additions & 3 deletions test/Riok.Mapperly.Tests/Mapping/EnumerableTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -671,9 +671,9 @@ public void EnumerableToReadOnlyArrayPropertyShouldDiagnostic()
.Should()
.HaveSingleMethodBody(
"""
var target = new global::B();
return target;
"""
var target = new global::B();
return target;
"""
)
.HaveDiagnostic(DiagnosticDescriptors.CannotMapToReadOnlyMember);
}
Expand Down