Skip to content

Commit

Permalink
feat: Inline user implemented queryable mapping expressions (#1169)
Browse files Browse the repository at this point in the history
Tries to inline user-implemented methods with an expression body into queryable expression mappings.
This leads to better query mappings by EF Core and should result in less data loaded.
If the inlining fails for some reason (e.g. not a valid c# expression tree), RMG068 is reported.

This works with a syntax rewriter (InlineExpressionRewriter) which
* extends type names to their fully qualified names (since the using statements are not present in the generated code)
* expands extension method invocations (since the using statements are not present in the generated code)
* extracts invocations of other user-defined mappings, to be inlined by the mapping itself
* tries to assume whether the given code can be successfully inlined in a c# expression tree by checking the presence of given syntax declarations

Additionally a new mapping is added (UserImplementedInlinedExpressionMapping) which represents an inlined version of a user implemented mapping.
When building the mapping, it rewrites the source parameter of the user-method to the actual source of the mapping.
Additionally mapping invocations extracted by the InlineExpressionRewriter are inlined.

Fixes #953.
  • Loading branch information
latonz authored Mar 18, 2024
1 parent 3cba05b commit 530066f
Show file tree
Hide file tree
Showing 60 changed files with 1,209 additions and 156 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
50 changes: 47 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,49 @@ 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 to 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 lead to more data being loaded than necessary.

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)
// note: do not use the method group '.Select(MapCarModel)', as that cannot be inlined
=> models.OrderBy(x => x.Name).Select(x => MapCarModel(x)).ToList();
// 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 cannot 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).
1 change: 1 addition & 0 deletions src/Riok.Mapperly/AnalyzerReleases.Shipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,3 +152,4 @@ RMG064 | Mapper | Error | Cannot configure an object mapping on a non-obje
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
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;
}
else if (ambiguousName)
{
ReportDiagnostic(DiagnosticDescriptors.ReferencedMappingAmbiguous, mappingName);
}

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;

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;

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,
};
}
}
Loading

0 comments on commit 530066f

Please sign in to comment.