Skip to content

Commit

Permalink
fix: use correct user defined mapping configuration for nullable disa…
Browse files Browse the repository at this point in the history
…bled queryable mappings (#1580)
  • Loading branch information
latonz authored Nov 5, 2024
1 parent dd5f9fe commit e7738bf
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ namespace Riok.Mapperly.Descriptors;
/// </summary>
public class InlineExpressionMappingBuilderContext : MappingBuilderContext
{
public InlineExpressionMappingBuilderContext(MappingBuilderContext ctx, TypeMappingKey mappingKey)
: base(ctx, ctx.FindMapping(mappingKey) as IUserMapping, null, mappingKey, false) { }
public InlineExpressionMappingBuilderContext(MappingBuilderContext ctx, IUserMapping? userMapping, TypeMappingKey mappingKey)
: base(ctx, userMapping, null, mappingKey, false) { }

private InlineExpressionMappingBuilderContext(
InlineExpressionMappingBuilderContext ctx,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
using Microsoft.CodeAnalysis;
using Riok.Mapperly.Abstractions;
using Riok.Mapperly.Descriptors.Mappings;
using Riok.Mapperly.Descriptors.Mappings.UserMappings;
using Riok.Mapperly.Diagnostics;
using Riok.Mapperly.Helpers;

Expand All @@ -12,10 +14,18 @@ public static class QueryableMappingBuilder
if (!ctx.IsConversionEnabled(MappingConversionType.Queryable))
return null;

if (!TryBuildMappingKey(ctx, out var mappingKey))
if (!ctx.Source.ImplementsGeneric(ctx.Types.Get(typeof(IQueryable<>)), out var sourceQueryable))
return null;

var inlineCtx = new InlineExpressionMappingBuilderContext(ctx, mappingKey);
if (!ctx.Target.ImplementsGeneric(ctx.Types.Get(typeof(IQueryable<>)), out var targetQueryable))
return null;

var sourceType = sourceQueryable.TypeArguments[0];
var targetType = targetQueryable.TypeArguments[0];

var mappingKey = TryBuildMappingKey(ctx, sourceType, targetType);
var userMapping = ctx.FindMapping(sourceType, targetType) as IUserMapping;
var inlineCtx = new InlineExpressionMappingBuilderContext(ctx, userMapping, mappingKey);
var mapping = inlineCtx.BuildMapping(mappingKey, MappingBuildingOptions.KeepUserSymbol);
if (mapping == null)
return null;
Expand All @@ -28,31 +38,17 @@ public static class QueryableMappingBuilder
return new QueryableProjectionMapping(ctx.Source, ctx.Target, mapping);
}

private static bool TryBuildMappingKey(MappingBuilderContext ctx, out TypeMappingKey mappingKey)
private static TypeMappingKey TryBuildMappingKey(MappingBuilderContext ctx, ITypeSymbol sourceType, ITypeSymbol targetType)
{
mappingKey = default;
if (!ctx.Source.ImplementsGeneric(ctx.Types.Get(typeof(IQueryable<>)), out var sourceQueryable))
return false;

if (!ctx.Target.ImplementsGeneric(ctx.Types.Get(typeof(IQueryable<>)), out var targetQueryable))
return false;

// if nullable reference types are disabled
// and there was no explicit nullable annotation,
// the non-nullable variant is used here.
// Otherwise, this would lead to a select like source.Select(x => x == null ? throw ... : new ...)
// which is not expected in this case.
// see also https://github.com/riok/mapperly/issues/1196
var sourceType = ctx.SymbolAccessor.NonNullableIfNullableReferenceTypesDisabled(
sourceQueryable.TypeArguments[0],
ctx.UserMapping?.SourceType
);
var targetType = ctx.SymbolAccessor.NonNullableIfNullableReferenceTypesDisabled(
targetQueryable.TypeArguments[0],
ctx.UserMapping?.TargetType
);

mappingKey = new TypeMappingKey(sourceType, targetType);
return true;
sourceType = ctx.SymbolAccessor.NonNullableIfNullableReferenceTypesDisabled(sourceType, ctx.UserMapping?.SourceType);
targetType = ctx.SymbolAccessor.NonNullableIfNullableReferenceTypesDisabled(targetType, ctx.UserMapping?.TargetType);

return new TypeMappingKey(sourceType, targetType);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -181,4 +181,38 @@ public Task RecordToClassDisabledNullableContext()

return TestHelper.VerifyGenerator(source, TestHelperOptions.DisabledNullable);
}

[Fact]
public Task RecordToRecordMemberMappingDisabledNullableContext()
{
var source = TestSourceBuilder.MapperWithBodyAndTypes(
"""
public partial System.Linq.IQueryable<B> Map(System.Linq.IQueryable<A> q);

[MapProperty(nameof(A.Value), nameof(B.OtherValue)]
private partial B Map(A source);
""",
"public record A(string Value);",
"public record B(string OtherValue);"
);

return TestHelper.VerifyGenerator(source, TestHelperOptions.DisabledNullable);
}

[Fact]
public Task ClassToClassMemberMappingDisabledNullableContext()
{
var source = TestSourceBuilder.MapperWithBodyAndTypes(
"""
public partial System.Linq.IQueryable<B> Map(System.Linq.IQueryable<A> q);

[MapProperty(nameof(A.Value), nameof(B.OtherValue)]
private partial B Map(A source);
""",
"public class A { public string Value {get; set;} }",
"public class B { public string OtherValue {get; set;} }"
);

return TestHelper.VerifyGenerator(source, TestHelperOptions.DisabledNullable);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//HintName: Mapper.g.cs
// <auto-generated />
#nullable enable
public partial class Mapper
{
[global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")]
public partial global::System.Linq.IQueryable<global::B?>? Map(global::System.Linq.IQueryable<global::A?>? q)
{
if (q == null)
return default;
#nullable disable
return System.Linq.Queryable.Select(
q,
x => new global::B()
{
OtherValue = x.Value,
}
);
#nullable enable
}

[global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")]
private partial global::B? Map(global::A? source)
{
if (source == null)
return default;
var target = new global::B();
target.OtherValue = source.Value;
return target;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
//HintName: Mapper.g.cs
// <auto-generated />
#nullable enable
public partial class Mapper
{
[global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")]
public partial global::System.Linq.IQueryable<global::B?>? Map(global::System.Linq.IQueryable<global::A?>? q)
{
if (q == null)
return default;
#nullable disable
return System.Linq.Queryable.Select(q, x => new global::B(x.Value));
#nullable enable
}

[global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "0.0.1.0")]
private partial global::B? Map(global::A? source)
{
if (source == null)
return default;
var target = new global::B(source.Value);
return target;
}
}

1 comment on commit e7738bf

@github-actions
Copy link

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'SourceGeneratorBenchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 3.

Benchmark suite Current: e7738bf Previous: e9abaf4 Ratio
Riok.Mapperly.Benchmarks.SourceGeneratorBenchmarks.Compile 2103807.791272096 ns (± 151815.69131161875) 120741.05671574519 ns (± 1678.1529391966976) 17.42
Riok.Mapperly.Benchmarks.SourceGeneratorBenchmarks.LargeCompile 45774493.12727273 ns (± 408908.7845536215) 1277208.939174107 ns (± 7926.104374412328) 35.84

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.