feat: add [MapperNoInlining] attribute and MapperAttribute.NoInlining property#2240
feat: add [MapperNoInlining] attribute and MapperAttribute.NoInlining property#2240trejjam wants to merge 16 commits into
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.
Make the expression-only scope explicit in the public API name, as requested in PR review feedback: both the attribute and the mapper property only affect expression/queryable projection inlining, not regular mappings. - MapperNoInliningAttribute -> MapperNoExpressionInliningAttribute - MapperAttribute.NoInlining -> NoExpressionInlining - MapperlyNoInlining build property -> MapperlyNoExpressionInlining - Internal IUserMapping.NoInlining and locals renamed accordingly - XML docs clarify the expression-only scope
Address PR review feedback: RMG032 is the root cause when enum projections are not translatable to expressions. Previously it was emitted as a warning and Mapperly fell back to EnumToEnumMappingBuilder, which produced cascading false RMG037/RMG038 diagnostics when the source/target enums had different underlying types (e.g. sbyte vs byte) due to boxed value equality failing across primitive types. - Change RMG032 severity: Warning -> Error - Extend the help message to point users to the new [MapperNoExpressionInlining] attribute and Mapper(NoExpressionInlining) as the supported escape hatch for projection mappings. - When RMG032 applies, bail out (return null) instead of producing a cast-based fallback mapping, eliminating the RMG037/RMG038 cascade. - Also report RMG032 for expression-context enum-to-enum mappings when the underlying types differ, catching the original bug path regardless of the ambient EnumMappingStrategy. - Record the severity change in AnalyzerReleases.Unshipped.md. - Update affected test expectations in QueryableProjectionEnumTest and UseStaticMapperTest.
Add the RMG032 severity change to the 5.0 breaking-change list and a dedicated section explaining the migration path via the new [MapperNoExpressionInlining] attribute and Mapper(NoExpressionInlining) property.
The NoExpressionInlining option only applies to queryable projection mappings, so document it under the queryable projections page where users looking for projection-specific guidance will find it. The queryable-projections page already covers the attribute, the property, per-method and per-mapper opt-outs, and the runtime behavior comparison, so the duplicated section in mapper.mdx can simply be removed.
3d6476e to
df5ea66
Compare
| "The enum mapping strategy ByName, ByValueCheckDefined, explicit enum mappings and ignored enum values cannot be used in projection mappings to map from C to D" | ||
| "The enum mapping strategy ByName, ByValueCheckDefined, explicit enum mappings and ignored enum values cannot be used in projection mappings to map from C to D, consider applying [MapperNoExpressionInlining] to the mapping method or Mapper(NoExpressionInlining = true) to the containing mapper" | ||
| ) | ||
| .HaveDiagnostic(DiagnosticDescriptors.CouldNotMapMember, "Could not map member A.Value of type C to B.Value of type D") |
There was a problem hiding this comment.
I am not sure if this is expected behavior or not.
When I was looking for a similar tests, I found MapPropertyFromSourceUseWithUnsatisfiableParametersShouldDiagnostic. Is this cascade expected, or should we prevent it?
There was a problem hiding this comment.
If I am not mistaken, I saw Mapperly produce an exception for such occasions. That silences the CouldNotMapMember.
| if (ctx.SymbolAccessor.HasAttribute<MapperNoExpressionInliningAttribute>(method)) | ||
| return true; | ||
|
|
||
| if (!isExternal) | ||
| return ctx.Configuration.Mapper.NoExpressionInlining; | ||
|
|
||
| // the configuration of external mappers is not merged into ctx.Configuration, | ||
| // read the MapperAttribute of the containing type instead | ||
| var mapperAttribute = ctx.AttributeAccessor.AccessFirstOrDefault<MapperAttribute>(method.ContainingType); |
There was a problem hiding this comment.
I hope the ctx.SymbolAccessor.HasAttribute is a sufficient solution for the Mapperly usually only reads configuration through ctx.Configuration. From what I found, it's cached, so it should not be a performance issue.
When constructing Mapper -> AnotherMapper -> Method. We need to get info about the AnotherMapper. I did not find better way than ctx.AttributeAccessor.AccessFirstOrDefault. If you want it differently, can you point me in the right way?
df5ea66 to
5853c8e
Compare
feat: add
[MapperNoExpressionInliningAttribute]attribute andMapperAttribute.NoExpressionInliningpropertyDescription
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
MapperNoExpressionInliningAttributemarker attribute (AttributeTargets.Method)NoExpressionInliningproperty onMapperAttribute(default:false)InlineExpressionMappingBuilderContext.TryInlineMapping()before dispatching to inline/rebuildMapperConfigurationandMapperConfigurationMergerChecklist