feat: add [MapperNoInlining] attribute and MapperAttribute.NoInlining property#2240
feat: add [MapperNoInlining] attribute and MapperAttribute.NoInlining property#2240
Conversation
|
|
||
| private bool ShouldSkipInlining(IMethodSymbol method) | ||
| { | ||
| if (SymbolAccessor.HasAttribute<MapperNoInliningAttribute>(method)) |
There was a problem hiding this comment.
Mapperly usually only reads configuration through ctx.Configuration / from the mapping itself. IMO the correct way here would be passing it through the mapping. ctx.Configuration is available in the user mapping extractor.
| TestHelper | ||
| .GenerateMapper(source, TestHelperOptions.AllowDiagnostics) | ||
| .Should() | ||
| .HaveDiagnostics( |
There was a problem hiding this comment.
This should result in RMG032, no? IMO this is somewhat the root cause. We should point to the new NoInliningAttribute in the help of RMG032.
There was a problem hiding this comment.
That's possible. If I understand the flow correctly, since RMG032 is only a warning. It should produce some code. But since the EnumToEnumMappingBuilder has difficulty producing a mapping as a fallback, we end up with Source/TargetNotMapped.
I can change RMG032 to an error -> no fallback to the EnumToEnumMappingBuilder, and propose using NoInlining. Is this ok for you?
There was a problem hiding this comment.
Sgtm. Please note this as a breaking change in the 5.0 migration doc.
| } | ||
| ``` | ||
|
|
||
| ### Inlining |
There was a problem hiding this comment.
I'd move this into the queryable projection documentation page.
| /// will not be inlined or rebuilt in expression context. | ||
| /// Defaults to <c>false</c>. | ||
| /// </summary> | ||
| public bool NoInlining { get; set; } |
There was a problem hiding this comment.
IMO we should include the fact that this only applies to expressions / queryable projection mappings in the name. The same for the separate attribute.
feat: add
[MapperNoInlining]attribute andMapperAttribute.NoInliningpropertyDescription
When a mapper referenced via
[UseStaticMapper]is used from a mapper that has IQueryable projection methods, Mapperly attempts to inline or rebuild the referenced mapping methods into expression trees. For enum mappings configured withEnumMappingStrategy.ByNamewhere the source and target enums have different underlying types, this rebuild in expression context produces falseRMG037/RMG038diagnostics — reporting all enum members as unmapped even though they match by name.I understand the rationale behind the expression-context restrictions for the general case, and the 5.x alpha series has been catching up and tightening these rules further, which makes sense. However, in my project I was relying on
[UseStaticMapper]with enum-by-name mappings in queryable projections, and the stricter inlining behavior broke that usage. We've discussed the projection + enum + inlining interaction before.This PR adds a general opt-out mechanism for expression-tree inlining:
[MapperNoInlining]— method-level attribute to prevent a specific mapping method from being inlined/rebuilt in expression context[Mapper(NoInlining = true)]— mapper-level property to prevent all methods of a mapper from being inlined when referenced via[UseStaticMapper]Both are applied on the callee mapper (the one being referenced), not the consuming mapper.
Example
When inlining is disabled, the method call is preserved as-is in the expression tree. The ORM evaluates it client-side instead of attempting to translate it to SQL.
Changes
MapperNoInliningAttributemarker attribute (AttributeTargets.Method)NoInliningproperty onMapperAttribute(default:false)InlineExpressionMappingBuilderContext.TryInlineMapping()before dispatching to inline/rebuildMapperConfigurationandMapperConfigurationMergerChecklist