Skip to content

feat: add [MapperNoInlining] attribute and MapperAttribute.NoInlining property#2240

Open
trejjam wants to merge 16 commits into
riok:mainfrom
trejjam:fix/enum-explicit-cast-regression-repro
Open

feat: add [MapperNoInlining] attribute and MapperAttribute.NoInlining property#2240
trejjam wants to merge 16 commits into
riok:mainfrom
trejjam:fix/enum-explicit-cast-regression-repro

Conversation

@trejjam

@trejjam trejjam commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

feat: add [MapperNoExpressionInliningAttribute] attribute and MapperAttribute.NoExpressionInlining property

Description

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 with EnumMappingStrategy.ByName where the source and target enums have different underlying types, this rebuild in expression context produces false RMG037/RMG038 diagnostics — 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

// Per-method opt-out
[Mapper(EnumMappingStrategy = EnumMappingStrategy.ByName)]
public static partial class EnumMapper
{
    [MapperNoInlining]
    public static partial TargetStatus MapToStatus(this SourceStatus source);
}

// Or per-mapper opt-out
[Mapper(EnumMappingStrategy = EnumMappingStrategy.ByName, NoExpressionInlining = true)]
public static partial class EnumMapper
{
    public static partial TargetStatus MapToStatus(this SourceStatus source);
}

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

  • New MapperNoExpressionInliningAttribute marker attribute (AttributeTargets.Method)
  • New NoExpressionInlining property on MapperAttribute (default: false)
  • Check in InlineExpressionMappingBuilderContext.TryInlineMapping() before dispatching to inline/rebuild
  • Configuration pipeline wired through MapperConfiguration and MapperConfigurationMerger
  • Tests covering: bug repro, method-level attribute, mapper-level property, combined, non-projection (no effect)
  • Documentation in queryable projections guide with runtime behavior comparison

Disclosure: This PR was produced with AI assistance. All code has been manually reviewed, verified, and iteratively refined by me to ensure correctness and adherence to project guidelines.

Checklist

  • I did not use AI tools to generate this PR, or I have manually verified that the code is correct, optimal, and follows the project guidelines and architecture
  • I understand that low-quality, AI-generated PRs will be closed immediately without further explanation
  • The existing code style is followed
  • The commit message follows our guidelines
  • Performed a self-review of my code
  • Hard-to-understand areas of my code are commented
  • The documentation is updated (as applicable)
  • Unit tests are added/updated
  • Integration tests are added/updated (as applicable, especially if feature/bug depends on roslyn or framework version in use)

Comment thread src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs Outdated
Comment thread src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs Outdated

private bool ShouldSkipInlining(IMethodSymbol method)
{
if (SymbolAccessor.HasAttribute<MapperNoInliningAttribute>(method))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread test/Riok.Mapperly.Tests/Mapping/UseStaticMapperTest.cs Outdated
@latonz latonz added the enhancement New feature or request label Apr 24, 2026
Comment thread docs/docs/configuration/mapper.mdx Outdated
Comment thread src/Riok.Mapperly.Abstractions/MapperAttribute.cs Outdated
trejjam added 14 commits June 13, 2026 12:20
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.
@trejjam trejjam force-pushed the fix/enum-explicit-cast-regression-repro branch from 3d6476e to df5ea66 Compare June 13, 2026 10:21
"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")

@trejjam trejjam Jun 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I am not mistaken, I saw Mapperly produce an exception for such occasions. That silences the CouldNotMapMember.

Comment on lines +493 to +501
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);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@trejjam trejjam requested a review from latonz June 13, 2026 12:26
@trejjam trejjam force-pushed the fix/enum-explicit-cast-regression-repro branch from df5ea66 to 5853c8e Compare June 13, 2026 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants