Skip to content

Commit

Permalink
feat: validate usage of MapProperty attributes on enum and queryable …
Browse files Browse the repository at this point in the history
…mapping methods (#1074)
  • Loading branch information
latonz committed Feb 23, 2024
1 parent edc4933 commit fbe7894
Show file tree
Hide file tree
Showing 28 changed files with 202 additions and 116 deletions.
3 changes: 3 additions & 0 deletions src/Riok.Mapperly/AnalyzerReleases.Shipped.md
Expand Up @@ -147,3 +147,6 @@ RMG059 | Mapper | Error | Multiple default user mappings found, only one i
RMG060 | Mapper | Info | Multiple user mappings discovered without specifying an explicit default
RMG061 | Mapper | Error | The referenced mapping was not found
RMG062 | Mapper | Error | The referenced mapping name is ambiguous
RMG063 | Mapper | Error | Cannot configure an enum mapping on a non-enum mapping
RMG064 | Mapper | Error | Cannot configure an object mapping on a non-object mapping
RMG065 | Mapper | Warning | Cannot configure an object mapping on a queryable projection mapping, apply the configurations to an object mapping method instead
7 changes: 5 additions & 2 deletions src/Riok.Mapperly/Configuration/AttributeDataAccessor.cs
Expand Up @@ -20,6 +20,9 @@ public TAttribute AccessSingle<TAttribute>(ISymbol symbol)
where TAttribute : Attribute
where TData : notnull => Access<TAttribute, TData>(symbol).Single();

public TAttribute? AccessFirstOrDefault<TAttribute>(ISymbol symbol)
where TAttribute : Attribute => Access<TAttribute, TAttribute>(symbol).FirstOrDefault();

public TData? AccessFirstOrDefault<TAttribute, TData>(ISymbol symbol)
where TAttribute : Attribute
where TData : notnull => Access<TAttribute, TData>(symbol).FirstOrDefault();
Expand Down Expand Up @@ -158,7 +161,7 @@ is InvocationExpressionSyntax
{
var argMemberPath = argMemberPathStr
.TrimStart(FullNameOfPrefix)
.Split(StringMemberPath.PropertyAccessSeparator)
.Split(StringMemberPath.MemberAccessSeparator)
.Skip(1)
.ToArray();
return new StringMemberPath(argMemberPath);
Expand All @@ -167,7 +170,7 @@ is InvocationExpressionSyntax

if (arg is { Kind: TypedConstantKind.Primitive, Value: string v })
{
return new StringMemberPath(v.Split(StringMemberPath.PropertyAccessSeparator));
return new StringMemberPath(v.Split(StringMemberPath.MemberAccessSeparator));
}

throw new InvalidOperationException($"Cannot create {nameof(StringMemberPath)} from {arg.Kind}");
Expand Down
122 changes: 77 additions & 45 deletions src/Riok.Mapperly/Configuration/MapperConfigurationReader.cs
@@ -1,56 +1,61 @@
using Microsoft.CodeAnalysis;
using Riok.Mapperly.Abstractions;
using Riok.Mapperly.Descriptors;
using Riok.Mapperly.Diagnostics;
using Riok.Mapperly.Helpers;

namespace Riok.Mapperly.Configuration;

public class MapperConfigurationReader
{
private readonly MappingConfiguration _defaultConfiguration;
private readonly AttributeDataAccessor _dataAccessor;
private readonly WellKnownTypes _types;

public MapperConfigurationReader(
AttributeDataAccessor dataAccessor,
WellKnownTypes types,
ISymbol mapperSymbol,
MapperConfiguration defaultMapperConfiguration
)
{
_dataAccessor = dataAccessor;
_types = types;
var mapperConfiguration = _dataAccessor.AccessSingle<MapperAttribute, MapperConfiguration>(mapperSymbol);
Mapper = MapperConfigurationMerger.Merge(mapperConfiguration, defaultMapperConfiguration);
var mapper = MapperConfigurationMerger.Merge(mapperConfiguration, defaultMapperConfiguration);

_defaultConfiguration = new MappingConfiguration(
MapperConfiguration = new MappingConfiguration(
mapper,
new EnumMappingConfiguration(
Mapper.EnumMappingStrategy,
Mapper.EnumMappingIgnoreCase,
mapper.EnumMappingStrategy,
mapper.EnumMappingIgnoreCase,
null,
Array.Empty<IFieldSymbol>(),
Array.Empty<IFieldSymbol>(),
Array.Empty<EnumValueMappingConfiguration>(),
Mapper.RequiredMappingStrategy
mapper.RequiredMappingStrategy
),
new PropertiesMappingConfiguration(
new MembersMappingConfiguration(
Array.Empty<string>(),
Array.Empty<string>(),
Array.Empty<PropertyMappingConfiguration>(),
Mapper.IgnoreObsoleteMembersStrategy,
Mapper.RequiredMappingStrategy
Array.Empty<MemberMappingConfiguration>(),
mapper.IgnoreObsoleteMembersStrategy,
mapper.RequiredMappingStrategy
),
Array.Empty<DerivedTypeMappingConfiguration>()
);
}

public MapperAttribute Mapper { get; }
public MappingConfiguration MapperConfiguration { get; }

public MappingConfiguration BuildFor(MappingConfigurationReference reference)
public MappingConfiguration BuildFor(MappingConfigurationReference reference, DiagnosticCollection diagnostics)
{
if (reference.Method == null)
return _defaultConfiguration;
return MapperConfiguration;

var enumConfig = BuildEnumConfig(reference);
var propertiesConfig = BuildPropertiesConfig(reference.Method);
var enumConfig = BuildEnumConfig(reference, diagnostics);
var membersConfig = BuildMembersConfig(reference, diagnostics);
var derivedTypesConfig = BuildDerivedTypeConfigs(reference.Method);
return new MappingConfiguration(enumConfig, propertiesConfig, derivedTypesConfig);
return new MappingConfiguration(MapperConfiguration.Mapper, enumConfig, membersConfig, derivedTypesConfig);
}

private IReadOnlyCollection<DerivedTypeMappingConfiguration> BuildDerivedTypeConfigs(IMethodSymbol method)
Expand All @@ -61,39 +66,59 @@ private IReadOnlyCollection<DerivedTypeMappingConfiguration> BuildDerivedTypeCon
.ToList();
}

private PropertiesMappingConfiguration BuildPropertiesConfig(IMethodSymbol method)
private MembersMappingConfiguration BuildMembersConfig(MappingConfigurationReference configRef, DiagnosticCollection diagnostics)
{
var ignoredSourceProperties = _dataAccessor
.Access<MapperIgnoreSourceAttribute>(method)
if (configRef.Method == null)
return MapperConfiguration.Members;

var ignoredSourceMembers = _dataAccessor
.Access<MapperIgnoreSourceAttribute>(configRef.Method)
.Select(x => x.Source)
.WhereNotNull()
.ToList();
var ignoredTargetProperties = _dataAccessor
.Access<MapperIgnoreTargetAttribute>(method)
var ignoredTargetMembers = _dataAccessor
.Access<MapperIgnoreTargetAttribute>(configRef.Method)
.Select(x => x.Target)
.WhereNotNull()
.ToList();
var propertyConfigurations = _dataAccessor.Access<MapPropertyAttribute, PropertyMappingConfiguration>(method).ToList();
var ignoreObsolete = _dataAccessor.Access<MapperIgnoreObsoleteMembersAttribute>(method).FirstOrDefault() is not { } methodIgnore
? _defaultConfiguration.Properties.IgnoreObsoleteMembersStrategy
: methodIgnore.IgnoreObsoleteStrategy;
var requiredMapping = _dataAccessor.Access<MapperRequiredMappingAttribute>(method).FirstOrDefault() is not { } methodWarnUnmapped
? _defaultConfiguration.Properties.RequiredMappingStrategy
: methodWarnUnmapped.RequiredMappingStrategy;

return new PropertiesMappingConfiguration(
ignoredSourceProperties,
ignoredTargetProperties,
propertyConfigurations,
ignoreObsolete,
requiredMapping
var memberConfigurations = _dataAccessor.Access<MapPropertyAttribute, MemberMappingConfiguration>(configRef.Method).ToList();
var ignoreObsolete = _dataAccessor
.AccessFirstOrDefault<MapperIgnoreObsoleteMembersAttribute>(configRef.Method)
?.IgnoreObsoleteStrategy;
var requiredMapping = _dataAccessor.AccessFirstOrDefault<MapperRequiredMappingAttribute>(configRef.Method)?.RequiredMappingStrategy;

// ignore the required mapping / ignore obsolete as the same attribute is used for other mapping types
// e.g. enum to enum
var hasMemberConfigs = ignoredSourceMembers.Count > 0 || ignoredTargetMembers.Count > 0 || memberConfigurations.Count > 0;
if (hasMemberConfigs && (configRef.Source.IsEnum() || configRef.Target.IsEnum()))
{
diagnostics.ReportDiagnostic(DiagnosticDescriptors.MemberConfigurationOnNonMemberMapping, configRef.Method);
return MapperConfiguration.Members;
}

if (
hasMemberConfigs
&& configRef.Source.ImplementsGeneric(_types.Get(typeof(IQueryable<>)), out _)
&& configRef.Target.ImplementsGeneric(_types.Get(typeof(IQueryable<>)), out _)
)
{
diagnostics.ReportDiagnostic(DiagnosticDescriptors.MemberConfigurationOnQueryableProjectionMapping, configRef.Method);
return MapperConfiguration.Members;
}

return new MembersMappingConfiguration(
ignoredSourceMembers,
ignoredTargetMembers,
memberConfigurations,
ignoreObsolete ?? MapperConfiguration.Members.IgnoreObsoleteMembersStrategy,
requiredMapping ?? MapperConfiguration.Members.RequiredMappingStrategy
);
}

private EnumMappingConfiguration BuildEnumConfig(MappingConfigurationReference configRef)
private EnumMappingConfiguration BuildEnumConfig(MappingConfigurationReference configRef, DiagnosticCollection diagnostics)
{
if (configRef.Method == null || !configRef.Source.IsEnum() && !configRef.Target.IsEnum())
return _defaultConfiguration.Enum;
if (configRef.Method == null)
return MapperConfiguration.Enum;

var configData = _dataAccessor.AccessFirstOrDefault<MapEnumAttribute, EnumConfiguration>(configRef.Method);
var explicitMappings = _dataAccessor.Access<MapEnumValueAttribute, EnumValueMappingConfiguration>(configRef.Method).ToList();
Expand All @@ -105,18 +130,25 @@ private EnumMappingConfiguration BuildEnumConfig(MappingConfigurationReference c
.Access<MapperIgnoreTargetValueAttribute, MapperIgnoreEnumValueConfiguration>(configRef.Method)
.Select(x => x.Value)
.ToList();
var requiredMapping = _dataAccessor.Access<MapperRequiredMappingAttribute>(configRef.Method).FirstOrDefault()
is not { } methodWarnUnmapped
? _defaultConfiguration.Enum.RequiredMappingStrategy
: methodWarnUnmapped.RequiredMappingStrategy;
var requiredMapping = _dataAccessor.AccessFirstOrDefault<MapperRequiredMappingAttribute>(configRef.Method)?.RequiredMappingStrategy;

// ignore the required mapping as the same attribute is used for other mapping types
// e.g. object to object
var hasEnumConfigs = configData != null || explicitMappings.Count > 0 || ignoredSources.Count > 0 || ignoredTargets.Count > 0;
if (hasEnumConfigs && !configRef.Source.IsEnum() && !configRef.Target.IsEnum())
{
diagnostics.ReportDiagnostic(DiagnosticDescriptors.EnumConfigurationOnNonEnumMapping, configRef.Method);
return MapperConfiguration.Enum;
}

return new EnumMappingConfiguration(
configData?.Strategy ?? _defaultConfiguration.Enum.Strategy,
configData?.IgnoreCase ?? _defaultConfiguration.Enum.IgnoreCase,
configData?.Strategy ?? MapperConfiguration.Enum.Strategy,
configData?.IgnoreCase ?? MapperConfiguration.Enum.IgnoreCase,
configData?.FallbackValue,
ignoredSources,
ignoredTargets,
explicitMappings,
requiredMapping
requiredMapping ?? MapperConfiguration.Enum.RequiredMappingStrategy
);
}
}
5 changes: 4 additions & 1 deletion src/Riok.Mapperly/Configuration/MappingConfiguration.cs
@@ -1,7 +1,10 @@
using Riok.Mapperly.Abstractions;

namespace Riok.Mapperly.Configuration;

public record MappingConfiguration(
MapperAttribute Mapper,
EnumMappingConfiguration Enum,
PropertiesMappingConfiguration Properties,
MembersMappingConfiguration Members,
IReadOnlyCollection<DerivedTypeMappingConfiguration> DerivedTypes
);
Expand Up @@ -2,7 +2,7 @@

namespace Riok.Mapperly.Configuration;

public record PropertyMappingConfiguration(StringMemberPath Source, StringMemberPath Target) : HasSyntaxReference
public record MemberMappingConfiguration(StringMemberPath Source, StringMemberPath Target) : HasSyntaxReference
{
public string? StringFormat { get; set; }

Expand Down
Expand Up @@ -2,10 +2,10 @@

namespace Riok.Mapperly.Configuration;

public record PropertiesMappingConfiguration(
public record MembersMappingConfiguration(
IReadOnlyCollection<string> IgnoredSources,
IReadOnlyCollection<string> IgnoredTargets,
IReadOnlyCollection<PropertyMappingConfiguration> ExplicitMappings,
IReadOnlyCollection<MemberMappingConfiguration> ExplicitMappings,
IgnoreObsoleteMembersStrategy IgnoreObsoleteMembersStrategy,
RequiredMappingStrategy RequiredMappingStrategy
);
6 changes: 3 additions & 3 deletions src/Riok.Mapperly/Configuration/StringMemberPath.cs
Expand Up @@ -2,10 +2,10 @@ namespace Riok.Mapperly.Configuration;

public record StringMemberPath(IReadOnlyCollection<string> Path)
{
public const char PropertyAccessSeparator = '.';
private const string PropertyAccessSeparatorString = ".";
public const char MemberAccessSeparator = '.';
private const string MemberAccessSeparatorString = ".";

public string FullName => string.Join(PropertyAccessSeparatorString, Path);
public string FullName => string.Join(MemberAccessSeparatorString, Path);

public override string ToString() => FullName;
}
14 changes: 9 additions & 5 deletions src/Riok.Mapperly/Descriptors/DescriptorBuilder.cs
Expand Up @@ -28,7 +28,6 @@ public class DescriptorBuilder
private readonly SimpleMappingBuilderContext _builderContext;
private readonly DiagnosticCollection _diagnostics;
private readonly UnsafeAccessorContext _unsafeAccessorContext;
private readonly MapperConfigurationReader _configurationReader;

public DescriptorBuilder(
CompilationContext compilationContext,
Expand All @@ -44,12 +43,17 @@ MapperConfiguration defaultMapperConfiguration
_unsafeAccessorContext = new UnsafeAccessorContext(_methodNameBuilder, symbolAccessor);

var attributeAccessor = new AttributeDataAccessor(symbolAccessor);
_configurationReader = new MapperConfigurationReader(attributeAccessor, mapperDeclaration.Symbol, defaultMapperConfiguration);
var configurationReader = new MapperConfigurationReader(
attributeAccessor,
_types,
mapperDeclaration.Symbol,
defaultMapperConfiguration
);
_diagnostics = new DiagnosticCollection(mapperDeclaration.Syntax.GetLocation());

_builderContext = new SimpleMappingBuilderContext(
compilationContext,
_configurationReader,
configurationReader,
_symbolAccessor,
attributeAccessor,
_unsafeAccessorContext,
Expand Down Expand Up @@ -86,7 +90,7 @@ MapperConfiguration defaultMapperConfiguration
/// </summary>
private void ConfigureMemberVisibility()
{
var includedMembers = _configurationReader.Mapper.IncludedMembers;
var includedMembers = _builderContext.Configuration.Mapper.IncludedMembers;

if (_types.TryGet(UnsafeAccessorName) != null)
{
Expand Down Expand Up @@ -185,7 +189,7 @@ private void BuildMappingMethodNames()

private void BuildReferenceHandlingParameters()
{
if (!_builderContext.MapperConfiguration.UseReferenceHandling)
if (!_builderContext.Configuration.Mapper.UseReferenceHandling)
return;

foreach (var methodMapping in _mappings.MethodMappings)
Expand Down
Expand Up @@ -21,7 +21,7 @@ public interface IMembersBuilderContext<out T>

Dictionary<string, IMappableMember> TargetMembers { get; }

Dictionary<string, List<PropertyMappingConfiguration>> MemberConfigsByRootTargetName { get; }
Dictionary<string, List<MemberMappingConfiguration>> MemberConfigsByRootTargetName { get; }

void AddDiagnostics();

Expand Down
Expand Up @@ -26,7 +26,7 @@ public void AddNullDelegateMemberAssignmentMapping(IMemberAssignmentMapping memb

// set target member to null if null assignments are allowed
// and the source is null
if (BuilderContext.MapperConfiguration.AllowNullPropertyAssignment && memberMapping.TargetPath.Member.Type.IsNullable())
if (BuilderContext.Configuration.Mapper.AllowNullPropertyAssignment && memberMapping.TargetPath.Member.Type.IsNullable())
{
container.AddNullMemberAssignment(SetterMemberPath.Build(BuilderContext, memberMapping.TargetPath));
}
Expand Down Expand Up @@ -95,7 +95,7 @@ private MemberNullDelegateAssignmentMapping GetOrCreateNullDelegateMappingForPat
mapping = new MemberNullDelegateAssignmentMapping(
GetterMemberPath.Build(BuilderContext, nullConditionSourcePath),
parentMapping,
BuilderContext.MapperConfiguration.ThrowOnPropertyMappingNullMismatch,
BuilderContext.Configuration.Mapper.ThrowOnPropertyMappingNullMismatch,
needsNullSafeAccess
);
_nullDelegateMappings[nullConditionSourcePath] = mapping;
Expand Down

0 comments on commit fbe7894

Please sign in to comment.