Skip to content

Commit

Permalink
fix: do not diagnostic property not found if target is read only (#345)
Browse files Browse the repository at this point in the history
  • Loading branch information
latonz authored Apr 18, 2023
1 parent 80a193a commit 6ec7e80
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,22 +41,25 @@ public static void BuildMappingBody(IMembersContainerBuilderContext<IMemberAssig
continue;
}

if (!MemberPath.TryFind(
if (MemberPath.TryFind(
ctx.Mapping.SourceType,
MemberPathCandidateBuilder.BuildMemberPathCandidates(targetMember.Name),
ctx.IgnoredSourceMemberNames,
memberNameComparer,
out var sourceMemberPath))
{
BuildMemberAssignmentMapping(ctx, sourceMemberPath, new MemberPath(new[] { targetMember }));
continue;
}

if (targetMember.CanSet)
{
ctx.BuilderContext.ReportDiagnostic(
DiagnosticDescriptors.SourceMemberNotFound,
targetMember.Name,
ctx.Mapping.TargetType,
ctx.Mapping.SourceType);
continue;
}

BuildMemberAssignmentMapping(ctx, sourceMemberPath, new MemberPath(new[] { targetMember }));
}

ctx.AddDiagnostics();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public static class EnumerableMappingBuilder
private const string SelectMethodName = nameof(Enumerable.Select);
private const string ToArrayMethodName = nameof(Enumerable.ToArray);
private const string ToListMethodName = nameof(Enumerable.ToList);
private const string AddValueMethodName = nameof(ICollection<object>.Add);
private const string AddMethodName = nameof(ICollection<object>.Add);

private const string ToImmutableArrayMethodName = nameof(ImmutableArray.ToImmutableArray);
private const string ToImmutableListMethodName = nameof(ImmutableList.ToImmutableList);
Expand Down Expand Up @@ -86,8 +86,15 @@ public static class EnumerableMappingBuilder
// 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.HasImplicitInterfaceMethod(ctx.Types.ICollectionT, AddValueMethodName))
return new ForEachAddEnumerableExistingTargetMapping(ctx.Source, ctx.Target, elementMapping, AddValueMethodName);
if (!ctx.Target.IsArrayType() && ctx.Target.HasImplicitInterfaceMethod(ctx.Types.ICollectionT, AddMethodName))
return new ForEachAddEnumerableExistingTargetMapping(ctx.Source, ctx.Target, elementMapping, AddMethodName);

// if a mapping could be created for an immutable collection
// we diagnostic when it is an existing target mapping
if (ResolveImmutableCollectMethod(ctx) != null)
{
ctx.ReportDiagnostic(DiagnosticDescriptors.CannotMapToReadOnlyMember);
}

return null;
}
Expand Down Expand Up @@ -156,8 +163,8 @@ private static LinqConstructorMapping BuildLinqConstructorMapping(
// 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.HasImplicitInterfaceMethod(ctx.Types.ICollectionT, AddValueMethodName))
return new ForEachAddEnumerableMapping(ctx.Source, ctx.Target, elementMapping, objectFactory, AddValueMethodName);
if (!ctx.Target.IsArrayType() && ctx.Target.HasImplicitInterfaceMethod(ctx.Types.ICollectionT, AddMethodName))
return new ForEachAddEnumerableMapping(ctx.Source, ctx.Target, elementMapping, objectFactory, AddMethodName);

return null;
}
Expand Down Expand Up @@ -233,11 +240,9 @@ private static (bool CanMapWithLinq, string? CollectMethod) ResolveCollectMethod

private static IMethodSymbol? ResolveStaticMethod(INamedTypeSymbol namedType, string methodName)
{
var method = namedType.GetMembers(methodName)
.OfType<IMethodSymbol>()
.FirstOrDefault(m => m.IsStatic && m.IsGenericMethod);

return method;
return namedType.GetMembers(methodName)
.OfType<IMethodSymbol>()
.FirstOrDefault(m => m.IsStatic && m.IsGenericMethod);
}

private static ITypeSymbol? GetEnumeratedType(MappingBuilderContext ctx, ITypeSymbol type)
Expand Down
44 changes: 16 additions & 28 deletions test/Riok.Mapperly.Tests/Mapping/EnumerableTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ public void ArrayToArrayOfNullableStringDeepCloning()
}

[Fact]
public void ArrayToArrayOfReadonlyStruct()
public void ArrayToArrayOfReadOnlyStruct()
{
var source = TestSourceBuilder.Mapping(
"A[]",
Expand All @@ -248,7 +248,7 @@ public void ArrayToArrayOfReadonlyStruct()
}

[Fact]
public void ArrayToArrayOfReadonlyStructDeepCloning()
public void ArrayToArrayOfReadOnlyStructDeepCloning()
{
var source = TestSourceBuilder.Mapping(
"A[]",
Expand Down Expand Up @@ -877,7 +877,7 @@ public Task MapToReadOnlyCollectionProperty()
}

[Fact]
public void EnumerableToReadonlyImmutableListShouldDiagnostic()
public void EnumerableToReadOnlyImmutableListShouldDiagnostic()
{
var source = TestSourceBuilder.Mapping(
"A",
Expand All @@ -887,11 +887,11 @@ public void EnumerableToReadonlyImmutableListShouldDiagnostic()

TestHelper.GenerateMapper(source, TestHelperOptions.AllowInfoDiagnostics)
.Should()
.HaveDiagnostic(new(DiagnosticDescriptors.SourceMemberNotFound));
.HaveDiagnostic(new(DiagnosticDescriptors.CannotMapToReadOnlyMember));
}

[Fact]
public void EnumerableToReadonlyImmutableList()
public void EnumerableToReadOnlyImmutableArrayShouldDiagnostic()
{
var source = TestSourceBuilder.Mapping(
"A",
Expand All @@ -901,37 +901,25 @@ public void EnumerableToReadonlyImmutableList()

TestHelper.GenerateMapper(source, TestHelperOptions.AllowInfoDiagnostics)
.Should()
.HaveDiagnostic(new(DiagnosticDescriptors.SourceMemberNotFound));
.HaveDiagnostic(new(DiagnosticDescriptors.CannotMapToReadOnlyMember));
}

[Fact]
public void EnumerableToReadonlyImmutableArray()
public void EnumerableToReadOnlyImmutableHashSetShouldDiagnostic()
{
var source = TestSourceBuilder.Mapping(
"A",
"B",
"class A { public IEnumerable<int> Value { get; } }",
"class B { public System.Collections.Immutable.ImmutableArray<int> Value { get; } }");
"class A { public IEnumerable<int?> Value { get; } }",
"class B { public System.Collections.Immutable.ImmutableHashSet<int> Value { get; } }");

TestHelper.GenerateMapper(source, TestHelperOptions.AllowInfoDiagnostics)
.Should()
.HaveDiagnostic(new(DiagnosticDescriptors.SourceMemberNotFound));
}

[Fact]
public void EnumerableToReadonlyImmutableHashSet()
{
var source = TestSourceBuilder.Mapping(
"A",
"B",
"class A { public IEnumerable<int? Value { get; } }",
"class B { public System.Collections.Immutable.ImmutableHashSet<int> Value { get; } }");

TestHelper.GenerateMapper(source, TestHelperOptions.AllowInfoDiagnostics).Should().HaveDiagnostic(new(DiagnosticDescriptors.SourceMemberNotFound));
.HaveDiagnostic(new(DiagnosticDescriptors.CannotMapToReadOnlyMember));
}

[Fact]
public void EnumerableToReadonlyImmutableQueue()
public void EnumerableToReadOnlyImmutableQueueShouldDiagnostic()
{
var source = TestSourceBuilder.Mapping(
"A",
Expand All @@ -941,11 +929,11 @@ public void EnumerableToReadonlyImmutableQueue()

TestHelper.GenerateMapper(source, TestHelperOptions.AllowInfoDiagnostics)
.Should()
.HaveDiagnostic(new(DiagnosticDescriptors.SourceMemberNotFound));
.HaveDiagnostic(new(DiagnosticDescriptors.CannotMapToReadOnlyMember));
}

[Fact]
public void EnumerableToReadonlyImmutableStack()
public void EnumerableToReadOnlyImmutableStackShouldDiagnostic()
{
var source = TestSourceBuilder.Mapping(
"A",
Expand All @@ -955,11 +943,11 @@ public void EnumerableToReadonlyImmutableStack()

TestHelper.GenerateMapper(source, TestHelperOptions.AllowInfoDiagnostics)
.Should()
.HaveDiagnostic(new(DiagnosticDescriptors.SourceMemberNotFound));
.HaveDiagnostic(new(DiagnosticDescriptors.CannotMapToReadOnlyMember));
}

[Fact]
public void EnumerableToReadonlyImmutableSortedSet()
public void EnumerableToReadOnlyImmutableSortedSetShouldDiagnostic()
{
var source = TestSourceBuilder.Mapping(
"A",
Expand All @@ -969,7 +957,7 @@ public void EnumerableToReadonlyImmutableSortedSet()

TestHelper.GenerateMapper(source, TestHelperOptions.AllowInfoDiagnostics)
.Should()
.HaveDiagnostic(new(DiagnosticDescriptors.SourceMemberNotFound));
.HaveDiagnostic(new(DiagnosticDescriptors.CannotMapToReadOnlyMember));
}

[Fact]
Expand Down
19 changes: 19 additions & 0 deletions test/Riok.Mapperly.Tests/Mapping/ObjectPropertyTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -349,4 +349,23 @@ public Task WithPrivateSourcePathGetterShouldIgnoreAndDiagnostic()

return TestHelper.VerifyGenerator(source);
}

[Fact]
public void UnmappedReadOnlyTargetPropertyShouldNotDiagnostic()
{
var source = TestSourceBuilder.Mapping(
"A",
"B",
"class A { public string Name { get; } }",
"class B { public string Name { set; } public string FullName { get; } }");

TestHelper.GenerateMapper(source)
.Should()
.HaveSingleMethodBody(
"""
var target = new global::B();
target.Name = source.Name;
return target;
""");
}
}

0 comments on commit 6ec7e80

Please sign in to comment.