Skip to content

Commit

Permalink
fix!: replace RMG017 and RMG027 with a new RMG074 (#1334)
Browse files Browse the repository at this point in the history
The refactored member matching works as follows:
* A member matching state and context is created
* For each target member a matching source is looked up
  Now this happens unified for all types (constructor parameter, init property, ...)
* A mapping is created with a MemberMappingInfo containing the information which members are mapped.
* The mapping is added to the container. Based on the MemberMappingInfo the members are marked as mapped.

* refactors and improves the readability of the member matching process
* Introduces a new IgnoredMembersBuilder to build and validate ignored members
* Introduces a new MemberMappingDiagnosticReporter to report all diagnostics after the member mapping build phase
* Introduces a new NestedMappingsContext to handle nested mappings (MapPropertyFromSourceAttribute)
* Introduces a new MemberMappingBuilder to build member (assignment) mappings for all types (objects, existing objects and tuples)
* Introduces a new MembersMappingState holding the state during the member matching process (which members are not yet mapped, which are ignored, ...)
* replace RMG017 and RMG027 with a new RMG074 when a target member path is used where it is not possible
* Introduces a new MemberMappingInfo representing the mapped source and target members. This simplifies marking members as mapped
* Introduces a new ConstructorParameterMember to simplify constructor parameter handling (can be treated as IMappableMember now)
* Introduces a new ISourceValue interface which represents the right side of a member assignment mapping.
  For now only mapped source members and null mapped source members are supported.
  With #631 constant values and method provided values will also be ISourceValues

BREAKING CHANGE: Replaces RMG017 and RMG027 with RMG074
  • Loading branch information
latonz authored Jun 20, 2024
1 parent 9d5e875 commit 833f8a4
Show file tree
Hide file tree
Showing 67 changed files with 1,711 additions and 1,063 deletions.
16 changes: 16 additions & 0 deletions src/Riok.Mapperly/AnalyzerReleases.Shipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -165,3 +165,19 @@ Rule ID | Category | Severity | Notes
RMG071 | Mapper | Warning | Nested properties mapping is not used
RMG072 | Mapper | Warning | The source type of the referenced mapping does not match
RMG073 | Mapper | Warning | The target type of the referenced mapping does not match

## Release 4.0

### New Rules

Rule ID | Category | Severity | Notes
--------|----------|----------|-------
RMG074 | Mapper | Error | Multiple mappings are configured for the same target member

### Removed Rules

Rule ID | Category | Severity | Notes
--------|----------|----------|-------
RMG017 | Mapper | Warning | An init only member can have one configuration at max
RMG027 | Mapper | Warning | A constructor parameter can have one configuration at max
RMG028 | Mapper | Warning | Constructor parameter cannot handle target paths
15 changes: 14 additions & 1 deletion src/Riok.Mapperly/Configuration/MembersMappingConfiguration.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Riok.Mapperly.Abstractions;
using Riok.Mapperly.Symbols;

namespace Riok.Mapperly.Configuration;

Expand All @@ -9,4 +10,16 @@ public record MembersMappingConfiguration(
IReadOnlyCollection<NestedMembersMappingConfiguration> NestedMappings,
IgnoreObsoleteMembersStrategy IgnoreObsoleteMembersStrategy,
RequiredMappingStrategy RequiredMappingStrategy
);
)
{
public IEnumerable<string> GetMembersWithExplicitConfigurations(MappingSourceTarget sourceTarget)
{
var members = sourceTarget switch
{
MappingSourceTarget.Source => ExplicitMappings.Where(x => x.Source.Path.Count > 0).Select(x => x.Source.Path[0]),
MappingSourceTarget.Target => ExplicitMappings.Select(x => x.Target.Path[0]),
_ => throw new ArgumentOutOfRangeException(nameof(sourceTarget), sourceTarget, "Neither source or target"),
};
return members.Distinct();
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
using System.Diagnostics.CodeAnalysis;
using Microsoft.CodeAnalysis;
using Riok.Mapperly.Configuration;
using Riok.Mapperly.Descriptors.Mappings;
using Riok.Mapperly.Descriptors.Mappings.MemberMappings;
using Riok.Mapperly.Symbols;
Expand All @@ -18,22 +15,13 @@ public interface IMembersBuilderContext<out T>

MappingBuilderContext BuilderContext { get; }

IReadOnlyCollection<string> IgnoredSourceMemberNames { get; }
void SetTargetMemberMapped(IMappableMember targetMember);

Dictionary<string, IMappableMember> TargetMembers { get; }
void ConsumeMemberConfig(MemberMappingInfo members);

Dictionary<string, List<MemberMappingConfiguration>> MemberConfigsByRootTargetName { get; }
IEnumerable<IMappableMember> EnumerateUnmappedTargetMembers();

void AddDiagnostics();
IEnumerable<IMappableMember> EnumerateUnmappedOrConfiguredTargetMembers();

/// <summary>
/// Tries to find a (possibly nested) MemberPath on the source type that can be mapped to <paramref name="targetMemberName"/>.
/// </summary>
bool TryFindNestedSourceMembersPath(
string targetMemberName,
[NotNullWhen(true)] out MemberPath? sourceMemberPath,
bool? ignoreCase = null
);

NullMemberMapping BuildNullMemberMapping(MemberPath sourcePath, INewInstanceMapping delegateMapping, ITypeSymbol targetMemberType);
IEnumerable<MemberMappingInfo> MatchMembers(IMappableMember targetMember);
}
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
using System.Diagnostics.CodeAnalysis;
using Microsoft.CodeAnalysis;
using Riok.Mapperly.Descriptors.Mappings;
using Riok.Mapperly.Descriptors.Mappings.MemberMappings;
using Riok.Mapperly.Symbols;

namespace Riok.Mapperly.Descriptors.MappingBodyBuilders.BuilderContext;

/// <summary>
/// A <see cref="IMembersBuilderContext{T}"/> for mappings which create the target object via new().
/// A <see cref="IMembersBuilderContext{T}"/> for mappings which create the target object via new ...().
/// </summary>
/// <typeparam name="T">The mapping type.</typeparam>
public interface INewInstanceBuilderContext<out T> : IMembersBuilderContext<T>
where T : IMapping
{
bool TryMatchParameter(IParameterSymbol parameter, [NotNullWhen(true)] out MemberMappingInfo? memberInfo);

bool TryMatchInitOnlyMember(IMappableMember targetMember, [NotNullWhen(true)] out MemberMappingInfo? memberInfo);

void AddConstructorParameterMapping(ConstructorParameterMapping mapping);

void AddInitMemberMapping(MemberAssignmentMapping mapping);

/// <summary>
/// Maps case insensitive target root member names to their real case sensitive names.
/// For example id => Id. The real name can then be used as key for <see cref="IMembersBuilderContext{T}.MemberConfigsByRootTargetName"/>.
/// This allows resolving case insensitive configuration member names (eg. when mapping to constructor parameters).
/// </summary>
IReadOnlyDictionary<string, string> RootTargetNameCasingMapping { get; }
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using System.Diagnostics.CodeAnalysis;
using Microsoft.CodeAnalysis;
using Riok.Mapperly.Descriptors.Mappings;
using Riok.Mapperly.Descriptors.Mappings.MemberMappings;

Expand All @@ -11,5 +13,7 @@ namespace Riok.Mapperly.Descriptors.MappingBodyBuilders.BuilderContext;
public interface INewValueTupleBuilderContext<out T> : IMembersBuilderContext<T>
where T : IMapping
{
bool TryMatchTupleElement(IFieldSymbol member, [NotNullWhen(true)] out MemberMappingInfo? memberInfo);

void AddTupleConstructorParameterMapping(ValueTupleConstructorParameterMapping mapping);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
using Riok.Mapperly.Abstractions;
using Riok.Mapperly.Configuration;
using Riok.Mapperly.Diagnostics;
using Riok.Mapperly.Symbols;

namespace Riok.Mapperly.Descriptors.MappingBodyBuilders.BuilderContext;

/// <summary>
/// Builds a set of ignored source or target members
/// and reports diagnostics for any invalid configured members.
/// If a member is ignored and configured,
/// the configuration takes precedence and the member is not ignored.
/// </summary>
internal static class IgnoredMembersBuilder
{
internal static HashSet<string> BuildIgnoredMembers(
MappingBuilderContext ctx,
MappingSourceTarget sourceTarget,
IReadOnlyCollection<string> allMembers
)
{
HashSet<string> ignoredMembers =
[
.. sourceTarget == MappingSourceTarget.Source
? ctx.Configuration.Members.IgnoredSources
: ctx.Configuration.Members.IgnoredTargets,
.. GetIgnoredAtMemberMembers(ctx, sourceTarget),
.. GetIgnoredObsoleteMembers(ctx, sourceTarget),
];

RemoveAndReportConfiguredIgnoredMembers(ctx, sourceTarget, ignoredMembers);
ReportUnmatchedIgnoredMembers(ctx, sourceTarget, ignoredMembers, allMembers);
return ignoredMembers;
}

private static void RemoveAndReportConfiguredIgnoredMembers(
MappingBuilderContext ctx,
MappingSourceTarget sourceTarget,
HashSet<string> ignoredMembers
)
{
var isSource = sourceTarget == MappingSourceTarget.Source;
var diagnostic = isSource
? DiagnosticDescriptors.IgnoredSourceMemberExplicitlyMapped
: DiagnosticDescriptors.IgnoredTargetMemberExplicitlyMapped;
var type = isSource ? ctx.Source : ctx.Target;
var configuredMembers = ctx.Configuration.Members.GetMembersWithExplicitConfigurations(sourceTarget);
foreach (var configuredMember in configuredMembers)
{
if (ignoredMembers.Remove(configuredMember))
{
ctx.ReportDiagnostic(diagnostic, configuredMember, type);
}
}
}

private static void ReportUnmatchedIgnoredMembers(
MappingBuilderContext ctx,
MappingSourceTarget sourceTarget,
IEnumerable<string> ignoredMembers,
IEnumerable<string> allMembers
)
{
var isSource = sourceTarget == MappingSourceTarget.Source;
var type = isSource ? ctx.Source : ctx.Target;
var nestedDiagnostic = isSource ? DiagnosticDescriptors.NestedIgnoredSourceMember : DiagnosticDescriptors.NestedIgnoredTargetMember;
var notFoundDiagnostic = isSource
? DiagnosticDescriptors.IgnoredSourceMemberNotFound
: DiagnosticDescriptors.IgnoredTargetMemberNotFound;

var unmatchedMembers = new HashSet<string>(ignoredMembers);
unmatchedMembers.ExceptWith(allMembers);

foreach (var member in unmatchedMembers)
{
if (member.Contains(StringMemberPath.MemberAccessSeparator, StringComparison.Ordinal))
{
ctx.ReportDiagnostic(nestedDiagnostic, member, type);
continue;
}

ctx.ReportDiagnostic(notFoundDiagnostic, member, type);
}
}

private static IEnumerable<string> GetIgnoredAtMemberMembers(MappingBuilderContext ctx, MappingSourceTarget sourceTarget)
{
var type = sourceTarget == MappingSourceTarget.Source ? ctx.Source : ctx.Target;

return ctx
.SymbolAccessor.GetAllAccessibleMappableMembers(type)
.Where(x => ctx.SymbolAccessor.HasAttribute<MapperIgnoreAttribute>(x.MemberSymbol))
.Select(x => x.Name);
}

private static IEnumerable<string> GetIgnoredObsoleteMembers(MappingBuilderContext ctx, MappingSourceTarget sourceTarget)
{
var obsoleteStrategy = ctx.Configuration.Members.IgnoreObsoleteMembersStrategy;
var strategy =
sourceTarget == MappingSourceTarget.Source ? IgnoreObsoleteMembersStrategy.Source : IgnoreObsoleteMembersStrategy.Target;

if (!obsoleteStrategy.HasFlag(strategy))
return Enumerable.Empty<string>();

var type = sourceTarget == MappingSourceTarget.Source ? ctx.Source : ctx.Target;

return ctx
.SymbolAccessor.GetAllAccessibleMappableMembers(type)
.Where(x => ctx.SymbolAccessor.HasAttribute<ObsoleteAttribute>(x.MemberSymbol))
.Select(x => x.Name);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
using Riok.Mapperly.Abstractions;
using Riok.Mapperly.Diagnostics;

namespace Riok.Mapperly.Descriptors.MappingBodyBuilders.BuilderContext;

internal static class MemberMappingDiagnosticReporter
{
public static void ReportDiagnostics(MappingBuilderContext ctx, MembersMappingState state)
{
AddUnusedTargetMembersDiagnostics(ctx, state);
AddUnmappedSourceMembersDiagnostics(ctx, state);
AddUnmappedTargetMembersDiagnostics(ctx, state);
AddNoMemberMappedDiagnostic(ctx, state);
}

private static void AddUnusedTargetMembersDiagnostics(MappingBuilderContext ctx, MembersMappingState state)
{
foreach (var memberConfig in state.UnusedMemberConfigs)
{
ctx.ReportDiagnostic(DiagnosticDescriptors.ConfiguredMappingTargetMemberNotFound, memberConfig.Target.FullName, ctx.Target);
}
}

private static void AddUnmappedSourceMembersDiagnostics(MappingBuilderContext ctx, MembersMappingState state)
{
if (!ctx.Configuration.Members.RequiredMappingStrategy.HasFlag(RequiredMappingStrategy.Source))
return;

foreach (var sourceMemberName in state.UnmappedSourceMemberNames)
{
ctx.ReportDiagnostic(DiagnosticDescriptors.SourceMemberNotMapped, sourceMemberName, ctx.Source, ctx.Target);
}
}

private static void AddUnmappedTargetMembersDiagnostics(MappingBuilderContext ctx, MembersMappingState state)
{
foreach (var targetMember in state.UnmappedTargetMembers)
{
if (targetMember.IsRequired)
{
ctx.ReportDiagnostic(DiagnosticDescriptors.RequiredMemberNotMapped, targetMember.Name, ctx.Target, ctx.Source);
continue;
}

if (targetMember.CanSet && ctx.Configuration.Members.RequiredMappingStrategy.HasFlag(RequiredMappingStrategy.Target))
{
ctx.ReportDiagnostic(DiagnosticDescriptors.SourceMemberNotFound, targetMember.Name, ctx.Target, ctx.Source);
}
}
}

private static void AddNoMemberMappedDiagnostic(MappingBuilderContext ctx, MembersMappingState state)
{
if (!state.HasMemberMapping)
{
ctx.ReportDiagnostic(DiagnosticDescriptors.NoMemberMappings, ctx.Source.ToDisplayString(), ctx.Target.ToDisplayString());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,28 @@ public class MembersContainerBuilderContext<T>(MappingBuilderContext builderCont

public void AddMemberAssignmentMapping(IMemberAssignmentMapping memberMapping) => AddMemberAssignmentMapping(Mapping, memberMapping);

/// <summary>
/// Adds an if-else style block which only executes the <paramref name="memberMapping"/>
/// if the source member is not null.
/// </summary>
/// <param name="memberMapping">The member mapping to be applied if the source member is not null</param>
public void AddNullDelegateMemberAssignmentMapping(IMemberAssignmentMapping memberMapping)
{
var nullConditionSourcePath = MemberPath.Create(
memberMapping.SourceGetter.MemberPath.RootType,
memberMapping.SourceGetter.MemberPath.PathWithoutTrailingNonNullable().ToList()
var nullConditionSourcePath = new NonEmptyMemberPath(
memberMapping.MemberInfo.SourceMember.RootType,
memberMapping.MemberInfo.SourceMember.PathWithoutTrailingNonNullable().ToList()
);
var container = GetOrCreateNullDelegateMappingForPath(nullConditionSourcePath);
AddMemberAssignmentMapping(container, memberMapping);

// set target member to null if null assignments are allowed
// and the source is null
if (BuilderContext.Configuration.Mapper.AllowNullPropertyAssignment && memberMapping.TargetPath.Member.Type.IsNullable())
if (
BuilderContext.Configuration.Mapper.AllowNullPropertyAssignment
&& memberMapping.MemberInfo.TargetMember.Member.Type.IsNullable()
)
{
container.AddNullMemberAssignment(SetterMemberPath.Build(BuilderContext, memberMapping.TargetPath));
container.AddNullMemberAssignment(SetterMemberPath.Build(BuilderContext, memberMapping.MemberInfo.TargetMember));
}
else if (BuilderContext.Configuration.Mapper.ThrowOnPropertyMappingNullMismatch)
{
Expand All @@ -41,9 +49,9 @@ public void AddNullDelegateMemberAssignmentMapping(IMemberAssignmentMapping memb

private void AddMemberAssignmentMapping(IMemberAssignmentMappingContainer container, IMemberAssignmentMapping mapping)
{
SetSourceMemberMapped(mapping.SourceGetter.MemberPath);
AddNullMemberInitializers(container, mapping.TargetPath);
AddNullMemberInitializers(container, mapping.MemberInfo.TargetMember);
container.AddMemberMapping(mapping);
MappingAdded(mapping.MemberInfo);
}

private void AddNullMemberInitializers(IMemberAssignmentMappingContainer container, MemberPath path)
Expand Down
Loading

0 comments on commit 833f8a4

Please sign in to comment.