Skip to content

Commit

Permalink
feat: Inline user implemented queryable mapping expressions
Browse files Browse the repository at this point in the history
  • Loading branch information
latonz committed Mar 13, 2024
1 parent 1d27f82 commit 3f9491a
Show file tree
Hide file tree
Showing 53 changed files with 1,036 additions and 147 deletions.
2 changes: 1 addition & 1 deletion docs/docs/configuration/flattening.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ If Mapperly can't resolve the target or source property correctly, it is possibl
by either using the source and target property path names as arrays or using a dot separated property access path string

```csharp
[MapProperty(new[] { nameof(Car.Make), nameof(Car.Make.Id) }, new[] { nameof(CarDto.MakeId) })]
[MapProperty([nameof(Car.Make), nameof(Car.Make.Id)], [nameof(CarDto.MakeId)])]
// Or alternatively
[MapProperty("Make.Id", "MakeId")]
// Or
Expand Down
49 changes: 46 additions & 3 deletions docs/docs/configuration/queryable-projections.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,7 @@ such mappings have several limitations:
- Enum mappings do not support the `ByName` strategy
- Reference handling is not supported
- Nullable reference types are disabled
- User implemented mapping methods need to follow expression tree [limitations](https://learn.microsoft.com/en-us/dotnet/csharp/advanced-topics/expression-trees/#limitations).
:::
:::

## Property configurations

Expand All @@ -76,3 +74,48 @@ public static partial class CarMapper
private static partial CarDto Map(Car car);
}
```

## User-implemented mapping methods

Mapperly tries to inline user-implemented mapping methods.
For this to work, user-implemented mapping methods need satisfy certain limitations:

- Only expression-bodied methods can be inlined.
- The body needs to follow the expression tree [limitations](https://learn.microsoft.com/en-us/dotnet/csharp/advanced-topics/expression-trees/#limitations).
- Nested MethodGroups cannot be inlined.

```csharp
[Mapper]
public static partial class CarMapper
{
public static partial IQueryable<CarDto> ProjectToDto(this IQueryable<Car> q);

// highlight-start
private static string MapCarBrandName(CarBrand brand)
=> band.Name;
// highlight-end
}
```

If Mapperly is unable to inline a user-implemented method, `RMG068` is reported.
Non-inlined method invocations can cause the query to be materialized and more data than necessary could be loaded.

Ordering and aggregating collections can also be implemented with user-implemented mapping methods:

```csharp
[Mapper]
public static partial class CarMapper
{
public static partial IQueryable<CarDto> ProjectToDto(this IQueryable<Car> q);

private static partial CarModelDto MapCarModel(CarModel model);

// highlight-start
private static ICollection<CarModelDto> MapOrderedCarBrandName(ICollection<CarModel> models)
=> models.OrderBy(x => x.Name).Select(x => MapCarModel(x)).ToList(); // note: do not use the method group
// highlight-end
}
```

It is important that the types in the user-implemented mapping method match the types of the objects to be mapped exactly.
Otherwise, Mapperly may not resolve the user-implemented mapping methods.
2 changes: 2 additions & 0 deletions docs/docs/configuration/user-implemented-methods.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ public partial class CarMapper
```

Whenever Mapperly needs a mapping from `TimeSpan` to `int` inside the `CarMapper` implementation, it will use the provided implementation.
The types of the user-implemented mapping method need to match the types to map exactly, including nullability.

If there are multiple user-implemented mapping methods suiting the given type-pair, by default, the first one is used.
This can be customized by using [automatic user-implemented mapping method discovery](#automatic-user-implemented-mapping-method-discovery)
and [default user-implemented mapping method](#default-user-implemented-mapping-method).
Expand Down
7 changes: 7 additions & 0 deletions docs/docs/contributing/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,10 @@ while still supporting older compiler versions.
See `build/package.sh` for details.

To introduce support for a new roslyn version see [common tasks](./common-tasks.md#add-support-for-a-new-roslyn-version).

## C# language features

The Mapperly source generator targets `.netstandard2.0` but uses the latest C# language version
and polyfills generated by [`Meziantou.Polyfill`](https://github.com/meziantou/Meziantou.Polyfill).
Some newer C# language features require new runtime features and therefore cannot be used in Mapperly.
A good C# language features requirements overview is available [here](https://sergeyteplyakov.github.io/Blog/c%23/2024/03/06/CSharp_Language_Features_vs_Target_Frameworks.html).
3 changes: 2 additions & 1 deletion src/Riok.Mapperly/AnalyzerReleases.Shipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -150,5 +150,6 @@ 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
RMG066 | Mapper | Warning | No members are mapped in an object mapping
RMG066 | Mapper | Warning | No members are mapped in an object mapping
RMG067 | Mapper | Error | Invalid usage of the MapPropertyAttribute
RMG068 | Mapper | Info | Cannot inline user implemented queryable expression mapping
10 changes: 8 additions & 2 deletions src/Riok.Mapperly/Descriptors/DescriptorBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ public class DescriptorBuilder
private readonly WellKnownTypes _types;

private readonly MappingCollection _mappings = new();
private readonly InlinedExpressionMappingCollection _inlineMappings = new();

private readonly MethodNameBuilder _methodNameBuilder = new();
private readonly MappingBodyBuilder _mappingBodyBuilder;
private readonly SimpleMappingBuilderContext _builderContext;
Expand Down Expand Up @@ -60,6 +62,7 @@ MapperConfiguration defaultMapperConfiguration
_diagnostics,
new MappingBuilder(_mappings, mapperDeclaration),
new ExistingTargetMappingBuilder(_mappings),
_inlineMappings,
mapperDeclaration.Syntax.GetLocation()
);
}
Expand Down Expand Up @@ -212,8 +215,9 @@ private void AddAccessorsToDescriptor()

private void AddUserMapping(IUserMapping mapping, bool ignoreDuplicates, bool named)
{
var result = _mappings.AddUserMapping(mapping, ignoreDuplicates, named ? mapping.Method.Name : null);
if (result == MappingCollectionAddResult.NotAddedDuplicatedDefault)
var name = named ? mapping.Method.Name : null;
var result = _mappings.AddUserMapping(mapping, name);
if (!ignoreDuplicates && mapping.Default == true && result == MappingCollectionAddResult.NotAddedDuplicated)
{
_diagnostics.ReportDiagnostic(
DiagnosticDescriptors.MultipleDefaultUserMappings,
Expand All @@ -222,6 +226,8 @@ private void AddUserMapping(IUserMapping mapping, bool ignoreDuplicates, bool na
mapping.TargetType.ToDisplayString()
);
}

_inlineMappings.AddUserMapping(mapping, name);
}

private void AddUserMappingDiagnostics()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ public static IEnumerable<IUserMapping> ExtractExternalMappings(SimpleMappingBui
UserMethodMappingExtractor.ExtractUserImplementedMappings(
ctx,
x.MapperType,
x.MapperType.FullyQualifiedIdentifierName(),
true
receiver: x.MapperType.FullyQualifiedIdentifierName(),
isStatic: true,
isExternal: true
)
);

Expand All @@ -44,7 +45,7 @@ private static IEnumerable<IUserMapping> ValidateAndExtractExternalInstanceMappi
return Enumerable.Empty<IUserMapping>();

if (nullableAnnotation != NullableAnnotation.Annotated)
return UserMethodMappingExtractor.ExtractUserImplementedMappings(ctx, type, name, false);
return UserMethodMappingExtractor.ExtractUserImplementedMappings(ctx, type, name, isStatic: false, isExternal: true);

ctx.ReportDiagnostic(DiagnosticDescriptors.ExternalMapperMemberCannotBeNullable, symbol, symbol.ToDisplayString());
return Enumerable.Empty<IUserMapping>();
Expand Down
106 changes: 75 additions & 31 deletions src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
using Microsoft.CodeAnalysis;
using Riok.Mapperly.Abstractions;
using Riok.Mapperly.Descriptors.MappingBuilders;
using Riok.Mapperly.Descriptors.Mappings;
using Riok.Mapperly.Descriptors.Mappings.ExistingTarget;
using Riok.Mapperly.Descriptors.Mappings.UserMappings;
using Riok.Mapperly.Diagnostics;
using Riok.Mapperly.Helpers;

namespace Riok.Mapperly.Descriptors;

Expand All @@ -12,15 +15,8 @@ namespace Riok.Mapperly.Descriptors;
/// </summary>
public class InlineExpressionMappingBuilderContext : MappingBuilderContext
{
private readonly MappingCollection _inlineExpressionMappings;
private readonly MappingBuilderContext _parentContext;

public InlineExpressionMappingBuilderContext(MappingBuilderContext ctx, TypeMappingKey mappingKey)
: base(ctx, (ctx.FindMapping(mappingKey) as IUserMapping)?.Method, null, mappingKey, false)
{
_parentContext = ctx;
_inlineExpressionMappings = new MappingCollection();
}
: base(ctx, (ctx.FindMapping(mappingKey) as IUserMapping)?.Method, null, mappingKey, false) { }

private InlineExpressionMappingBuilderContext(
InlineExpressionMappingBuilderContext ctx,
Expand All @@ -29,11 +25,7 @@ private InlineExpressionMappingBuilderContext(
TypeMappingKey mappingKey,
bool ignoreDerivedTypes
)
: base(ctx, userSymbol, diagnosticLocation, mappingKey, ignoreDerivedTypes)
{
_parentContext = ctx;
_inlineExpressionMappings = ctx._inlineExpressionMappings;
}
: base(ctx, userSymbol, diagnosticLocation, mappingKey, ignoreDerivedTypes) { }

public override bool IsExpression => true;

Expand All @@ -47,40 +39,68 @@ conversionType is not MappingConversionType.EnumToString and not MappingConversi

/// <summary>
/// <inheritdoc cref="MappingBuilderContext.FindNamedMapping"/>
/// Only returns <see cref="INewInstanceUserMapping"/>s.
/// Only inline expression mappings and user implemented mappings are considered.
/// User implemented mappings are tried to be inlined.
/// </summary>
public override INewInstanceMapping? FindNamedMapping(string mappingName)
{
// Only user implemented mappings are taken into account.
// This works as long as the user implemented methods
// follow the expression tree limitations:
// https://learn.microsoft.com/en-us/dotnet/csharp/advanced-topics/expression-trees/#limitations
if (base.FindNamedMapping(mappingName) is INewInstanceUserMapping mapping)
var mapping = InlinedMappings.FindNamed(mappingName, out var ambiguousName, out var isInlined);
if (mapping == null)
{
// resolve named but not yet discovered mappings
mapping = base.FindNamedMapping(mappingName);
isInlined = false;

if (mapping == null)
return null;

Check warning on line 55 in src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs

View check run for this annotation

Codecov / codecov/patch

src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs#L55

Added line #L55 was not covered by tests
}
else if (ambiguousName)
{
ReportDiagnostic(DiagnosticDescriptors.ReferencedMappingAmbiguous, mappingName);
}

Check warning on line 60 in src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs

View check run for this annotation

Codecov / codecov/patch

src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs#L58-L60

Added lines #L58 - L60 were not covered by tests

if (isInlined)
return mapping;

return null;
mapping = TryInlineMapping(mapping);
InlinedMappings.SetInlinedMapping(mappingName, mapping);
return mapping;
}

/// <summary>
/// Tries to find an existing mapping for the provided types + config.
/// The nullable annotation of reference types is ignored and always set to non-nullable.
/// Only inline expression mappings and user implemented mappings are considered.
/// User implemented mappings are tried to be inlined.
/// </summary>
/// <param name="mappingKey">The mapping key.</param>
/// <returns>The <see cref="INewInstanceMapping"/> if a mapping was found or <c>null</c> if none was found.</returns>
public override INewInstanceMapping? FindMapping(TypeMappingKey mappingKey)
{
if (_inlineExpressionMappings.FindNewInstanceMapping(mappingKey) is { } mapping)
var mapping = InlinedMappings.Find(mappingKey, out var isInlined);
if (mapping == null)
return null;

if (isInlined)
return mapping;

// User implemented mappings are also taken into account.
// This works as long as the user implemented methods
// follow the expression tree limitations:
// https://learn.microsoft.com/en-us/dotnet/csharp/advanced-topics/expression-trees/#limitations
if (_parentContext.FindMapping(mappingKey) is UserImplementedMethodMapping userMapping)
return userMapping;
mapping = TryInlineMapping(mapping);
InlinedMappings.SetInlinedMapping(mappingKey, mapping);
return mapping;
}

public INewInstanceMapping? FindNewInstanceMapping(IMethodSymbol method)
{
INewInstanceMapping? mapping = InlinedMappings.FindNewInstanceUserMapping(method, out var isInlined);
if (mapping == null)
return null;

if (isInlined)
return mapping;

Check warning on line 99 in src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs

View check run for this annotation

Codecov / codecov/patch

src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs#L99

Added line #L99 was not covered by tests

return null;
mapping = TryInlineMapping(mapping);
InlinedMappings.SetInlinedMapping(new TypeMappingKey(mapping), mapping);
return mapping;
}

/// <summary>
Expand Down Expand Up @@ -122,13 +142,16 @@ conversionType is not MappingConversionType.EnumToString and not MappingConversi
// unset mark as reusable as an inline expression mapping
// should never be reused by the default mapping builder context,
// only by other inline mapping builder contexts.
var reusable = (options & MappingBuildingOptions.MarkAsReusable) != MappingBuildingOptions.MarkAsReusable;
var reusable = options.HasFlag(MappingBuildingOptions.MarkAsReusable);
options &= ~MappingBuildingOptions.MarkAsReusable;

var mapping = base.BuildMapping(userSymbol, mappingKey, options, diagnosticLocation);
if (reusable && mapping != null)
if (mapping == null)
return null;

Check warning on line 150 in src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs

View check run for this annotation

Codecov / codecov/patch

src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs#L150

Added line #L150 was not covered by tests

if (reusable)
{
_inlineExpressionMappings.AddMapping(mapping, mappingKey.Configuration);
InlinedMappings.AddMapping(mapping, mappingKey.Configuration);
}

return mapping;
Expand Down Expand Up @@ -174,4 +197,25 @@ protected override MappingBuilderContext ContextForMapping(
options.HasFlag(MappingBuildingOptions.IgnoreDerivedTypes)
);
}

private INewInstanceMapping TryInlineMapping(INewInstanceMapping mapping)
{
return mapping switch
{
// inline existing mapping
UserImplementedMethodMapping implementedMapping
=> InlineExpressionMappingBuilder.TryBuildMapping(this, implementedMapping) ?? implementedMapping,

// build an inlined version
IUserMapping userMapping
=> BuildMapping(
userMapping.Method,
new TypeMappingKey(userMapping),
MappingBuildingOptions.Default,
userMapping.Method.GetSyntaxLocation()
) ?? mapping,

_ => mapping,

Check warning on line 218 in src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs

View check run for this annotation

Codecov / codecov/patch

src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs#L218

Added line #L218 was not covered by tests
};
}
}
Loading

0 comments on commit 3f9491a

Please sign in to comment.