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.

Sign up for GitHub

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: do not diagnostic property not found if target is read only #345

Merged
merged 1 commit into from
Apr 18, 2023
Merged

fix: do not diagnostic property not found if target is read only #345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 7 additions & 4 deletions src/Riok.Mapperly/Descriptors/MappingBodyBuilders/ObjectMemberMappingBodyBuilder.cs
View file
Open in desktop
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
25 changes: 15 additions & 10 deletions src/Riok.Mapperly/Descriptors/MappingBuilders/EnumerableMappingBuilder.cs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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;
""");
}
}